{"action":"created","issue":{"url":"https://api.github.com/repos/symfony/symfony/issues/27520","repository_url":"https://api.github.com/repos/symfony/symfony","labels_url":"https://api.github.com/repos/symfony/symfony/issues/27520/labels{/name}","comments_url":"https://api.github.com/repos/symfony/symfony/issues/27520/comments","events_url":"https://api.github.com/repos/symfony/symfony/issues/27520/events","html_url":"https://github.com/symfony/symfony/issues/27520","id":329829482,"node_id":"MDU6SXNzdWUzMjk4Mjk0ODI=","number":27520,"title":"[RFC] Allow for an integer $type on ControllerTrait::addFlash() and FlashBagInterface::add()","user":{"login":"Aerendir","id":1968622,"node_id":"MDQ6VXNlcjE5Njg2MjI=","avatar_url":"https://avatars3.githubusercontent.com/u/1968622?v=4","gravatar_id":"","url":"https://api.github.com/users/Aerendir","html_url":"https://github.com/Aerendir","followers_url":"https://api.github.com/users/Aerendir/followers","following_url":"https://api.github.com/users/Aerendir/following{/other_user}","gists_url":"https://api.github.com/users/Aerendir/gists{/gist_id}","starred_url":"https://api.github.com/users/Aerendir/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Aerendir/subscriptions","organizations_url":"https://api.github.com/users/Aerendir/orgs","repos_url":"https://api.github.com/users/Aerendir/repos","events_url":"https://api.github.com/users/Aerendir/events{/privacy}","received_events_url":"https://api.github.com/users/Aerendir/received_events","type":"User","site_admin":false},"labels":[{"id":125366,"node_id":"MDU6TGFiZWwxMjUzNjY=","url":"https://api.github.com/repos/symfony/symfony/labels/HttpFoundation","name":"HttpFoundation","color":"DDDDDD","default":false},{"id":83934034,"node_id":"MDU6TGFiZWw4MzkzNDAzNA==","url":"https://api.github.com/repos/symfony/symfony/labels/RFC","name":"RFC","color":"fbca04","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":22,"created_at":"2018-06-06T11:35:38Z","updated_at":"2018-06-09T17:46:42Z","closed_at":null,"author_association":"NONE","body":"Currently both `ControllerTrait::addFlash()` and `FlashBagInterface::add()` have a `string` type hint.\r\n\r\nThis is not useful to set messages.\r\n\r\nI found myself using Monolog `Logger` constants to set the type of messages:\r\n\r\n```php\r\nuse Monolog\\Logger;\r\n\r\n$this->addFlash(Logger::ERROR, 'An error happened');\r\n```\r\n\r\nBut using `declare(strict_types=1)` cause this to be considered an error (correctly).\r\n\r\nI suggest remove the method type hint and add a check inside the `ControllerTrait::addFlash()` method:\r\n\r\n```php\r\n    /**\r\n     * Adds a flash message to the current session for type.\r\n     *\r\n     * @throws \\LogicException\r\n     *\r\n     * @final\r\n     */\r\n    protected function addFlash($type, string $message)\r\n    {\r\n+        if (!is_string($type) && !is_int($type)) {\r\n+            throw new \\InvalidArgumentException('Only strings and ints are accepted');\r\n+        }\r\n        \r\n        if (!$this->container->has('session')) {\r\n            throw new \\LogicException('You can not use the addFlash method if sessions are disabled. Enable them in \"config/packages/framework.yaml\".');\r\n        }\r\n\r\n        $this->container->get('session')->getFlashBag()->add($type, $message);\r\n    }\r\n```\r\n\r\nAccording to this, also the `FlashBagInterface::add()` DocComment has to be updated:\r\n\r\n```php\r\n    /**\r\n     * Adds a flash message for type.\r\n     *\r\n-     * @param string $type\r\n+     * @param string|int $type\r\n     * @param string $message\r\n     */\r\n    public function add($type, $message);\r\n```\r\n\r\nThe classes that implement `FlashBagInterface` don't have any method type hint and inherit the DocBlock from `FlashBagInterface`, so no changes are required.\r\n\r\n**CONCRETE PROBLEM**\r\n\r\nThis type hint causes concrete problems.\r\n\r\nIn fact I could bypass the type hinting forcing using the typecasting of the constants provided by `Logger`:\r\n\r\n```php\r\nuse Monolog\\Logger;\r\n\r\n- $this->addFlash(Logger::ERROR, 'An error happened');\r\n+ $this->addFlash((string) Logger::ERROR, 'An error happened');\r\n```\r\n\r\nBut this cause problems in Twig:\r\n\r\n```\r\n<div class=\"Notifications\">\r\n    {% for notification in notifications %}\r\n        {# INFO #}\r\n        {% if 200 === notification.level %}\r\n            {% set level = 'info' %}\r\n...\r\n```"},"comment":{"url":"https://api.github.com/repos/symfony/symfony/issues/comments/395986940","html_url":"https://github.com/symfony/symfony/issues/27520#issuecomment-395986940","issue_url":"https://api.github.com/repos/symfony/symfony/issues/27520","id":395986940,"node_id":"MDEyOklzc3VlQ29tbWVudDM5NTk4Njk0MA==","user":{"login":"Aerendir","id":1968622,"node_id":"MDQ6VXNlcjE5Njg2MjI=","avatar_url":"https://avatars3.githubusercontent.com/u/1968622?v=4","gravatar_id":"","url":"https://api.github.com/users/Aerendir","html_url":"https://github.com/Aerendir","followers_url":"https://api.github.com/users/Aerendir/followers","following_url":"https://api.github.com/users/Aerendir/following{/other_user}","gists_url":"https://api.github.com/users/Aerendir/gists{/gist_id}","starred_url":"https://api.github.com/users/Aerendir/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Aerendir/subscriptions","organizations_url":"https://api.github.com/users/Aerendir/orgs","repos_url":"https://api.github.com/users/Aerendir/repos","events_url":"https://api.github.com/users/Aerendir/events{/privacy}","received_events_url":"https://api.github.com/users/Aerendir/received_events","type":"User","site_admin":false},"created_at":"2018-06-09T17:46:42Z","updated_at":"2018-06-09T17:46:42Z","author_association":"NONE","body":"Ok @nicolas-grekas , then this issue can be closed if reputed not relevant.\r\n\r\nI solved my specific use case, so it is all about Symfony: if you think this is something should be fixed or not...\r\n\r\nOr if the type hinting has to be added to the `SessionBag` class too, or not...\r\n\r\nI made you aware that there are methods type hinted and methods that aren't: the choice is yours.\r\n\r\nJust for reference:\r\n\r\n[`FlashBagInterface`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Session/Flash/FlashBagInterface.php#L23-L29) (_not_ type hinted):\r\n\r\n```php\r\n    /**\r\n     * Adds a flash message for type.\r\n     *\r\n     * @param string $type\r\n     * @param string $message\r\n     */\r\n    public function add($type, $message);\r\n```\r\n\r\n[`FlashBag`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Session/Flash/FlashBag.php#L54-L60) (_not_ type hinted)\r\n\r\n```php\r\n    /**\r\n     * {@inheritdoc}\r\n     */\r\n    public function add($type, $message)\r\n    {\r\n        $this->flashes[$type][] = $message;\r\n    }\r\n```\r\n\r\n[`AutoExpireFlashBag`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Session/Flash/AutoExpireFlashBag.php#L60-L66) (_not_ type hinted)\r\n\r\n```php\r\n    /**\r\n     * {@inheritdoc}\r\n     */\r\n    public function add($type, $message)\r\n    {\r\n        $this->flashes[$type][] = $message;\r\n    }\r\n```\r\n\r\n[`ControllerTrait`](https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Controller/ControllerTrait.php#L145-L159) (**type hinted**)\r\n\r\n```php\r\n    /**\r\n     * Adds a flash message to the current session for type.\r\n     *\r\n     * @throws \\LogicException\r\n     *\r\n     * @final\r\n     */\r\n    protected function addFlash(string $type, string $message)\r\n    {\r\n        if (!$this->container->has('session')) {\r\n            throw new \\LogicException('You can not use the addFlash method if sessions are disabled. Enable them in \"config/packages/framework.yaml\".');\r\n        }\r\n        $this->container->get('session')->getFlashBag()->add($type, $message);\r\n    }\r\n```"}}
{"actor":{"display_login":"Aerendir"}}