Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Console] Fix docblock Command->addOption#44935
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Closed
Closed
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
This PR was merged into the 6.1 branch.Discussion----------List Basecom.de as sponsor of Symfony 6.1| Q | A| ------------- | ---| Branch? | 6.1| Bug fix? | no| New feature? | no| Deprecations? | no| Tickets | -| License | MIT| Doc PR | -Thanks to them! 🙏/cc@np4me2kCommits-------fe7db59 List Basecom.de as sponsor of Symfony 6.1
This PR was merged into the 6.1 branch.Discussion----------[Contracts] update branch-alias| Q | A| ------------- | ---| Branch? | 6.1| Bug fix? | no| New feature? | no| Deprecations? | no| Tickets | -| License | MIT| Doc PR | -Commits-------106c7c2 [Contracts] update branch-alias
Signed-off-by: Johan M. von Behren <johan@vonbehren.eu>
This PR was merged into the 6.1 branch.Discussion----------Fix typo in sponsor name| Q | A| ------------- | ---| Branch? | 6.1| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets | no| License | MIT| Doc PR | noThis fixes a typo in the name of basecom (sponsor for 6.1).Commits-------933a3fc Fix typo in sponsor name
* 6.0: Avoid duplicated session listener registration in tests [HttpFoundation] fix SessionHandlerFactory using connections [gha] swap the php versions we use in jobs [DoctrineBridge] fix calling get_class on non-object [Serializer] Remove some type hints for API Platform compatibility Update PR template ResponseListener needs only 2 parameters [Lock] create lock table if it does not exist [HttpClient] Fix handling error info in MockResponse [SecurityBundle] Fix invalid reference with `always_authenticate_before_granting` Don't mention psr/container v3, it doesn't exist Bump Symfony version to 6.0.1 Update VERSION for 6.0.0 Update CHANGELOG for 6.0.0 Bump Symfony version to 5.4.1 Update VERSION for 5.4.0 Update CHANGELOG for 5.4.0
* 6.0: [HttpFoundation] fix tests Fix low-deps
* 6.0: Add missing files from 5.3 Remove unneeded files Tweak bug report template Fix using FileLinkFormatter after serialization [Serializer] Remove DecoderInterface type hint for API Platform compatibility Prevent infinite nesting of lazy `ObjectManager` instances when `ObjectManager` is reset [DependencyInjection] Skip parameter attribute configurators in AttributeAutoconfigurationPass if we can't get the constructor reflector [HttpKernel] fix sending Vary: Accept-Language when appropriate
* 6.0: Use external links instead of fake issue templates Fix description that does not work well Document deprecations in Security Tokens
* 6.0: Don't rely on deprecated strategy constants
* 6.0: Fix TranslationTrait for multiple domains Fix parameter types for ProcessHelper::mustRun() Fix: Wording [SecurityBundle] Fix ambiguous deprecation message on missing provider Remove dead code in tests Remove return void PHPDoc in test Fix merge Fix compatibility with symfony/security-core 6.x
* 6.0: Fix KernelBrowser::loginUser() causing deprecation use $sessionId instead of $sessionCookiePath on SessionUtils::popSessionCookie call [Translation][Loco] Make http requests synchronous when reading the Loco API
…e to webprofilerbundle (chr-hertel)This PR was merged into the 6.1 branch.Discussion----------[HttpKernel][WebProfilerBundle] adding xdebug_info page to webprofilerbundle| Q | A| ------------- | ---| Branch? | 6.1| Bug fix? | no| New feature? | yes| Deprecations? | no| Tickets | n/a| License | MIT| Doc PR | n/aHad a pretty annoying day and thought this should be a thing ...Commits-------845f35f adding xdebug_info page to webprofilerbundle
This PR was squashed before being merged into the 6.1 branch.Discussion----------Use str_ends_with() and str_starts_with()| Q | A| ------------- | ---| Branch? | 6.1| Bug fix? | no| New feature? | no| Deprecations? | no| Tickets | no| License | MIT| Doc PR | noUse `str_ends_with()` and `str_starts_with()` instead of `substr()`.(replaces#44482 and#44476 🥲)Commits-------28d4d6b Use str_ends_with() and str_starts_with()
* 6.0: Don't access uninitialized typed property ChromePhpHandler::$response Remove FQCN type hints on properties Remove UPGRADE-5.4.md
* 6.0: Fix polyfill-php73 requirement [CI] Fix package-tests workflow checks for contracts do not call preg_match() on null Fix UPGRADE files for framework.messenger.reset_on_message option [Messenger] [DI] Add auto-registration for BatchHandlerInterface Remove duplicated entry in UPGRADE-6.0.md Handle alias in completion script
* 6.0: [HttpClient] Fix handling thrown \Exception in \Generator in MockResponse [DebugBundle] Add missing README [Lock] Fix missing argument in PostgreSqlStore::putOffExpiration with DBAL connection Bump Symfony version to 6.0.2 Update VERSION for 6.0.1 Update CHANGELOG for 6.0.1 Bump Symfony version to 5.4.2 Update VERSION for 5.4.1 Update CHANGELOG for 5.4.1 [String] Fix requiring wcswitch table several times [HttpClient] Fix response id property check in MockResponse
…erjason)This PR was merged into the 6.1 branch.Discussion----------[HttpFoundation] Update cookie date time format| Q | A| ------------- | ---| Branch? | 6.1| Bug fix? | yes/no| New feature? | yes/no| Deprecations? | yes/no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tickets |Fix#41606| License | MIT| Doc PR | symfony/symfony-docs#... <!-- required for new features -->Wasn't sure if it will be a bug, feature and/or deprecation according to the issue. 😄Commits-------e2ff187 Update cookie date time format
…gunApiTransport (starred-gijs)This PR was merged into the 6.1 branch.Discussion----------[Mailer] [Mailgun] Allow multiple TagHeaders with MailgunApiTransport| Q | A| ------------- | ---| Branch? | 6.1| Bug fix? | no| New feature? | yes| Deprecations? | no| Tickets | -| License | MIT| Doc PR | -Mailgun allows you to set [3 tags per message](https://documentation.mailgun.com/en/latest/user_manual.html#tagging) but the current implementation in MailgunApiTransport, any consecutive TagHeader will override the previous one. Because it uses FormDataPart to build the payload, by using a integer key, it allows to set multiple parts with the same name#38323The MailgunHttpTransport already supports multiple TagHeaders and I added a test to prove that.Commits-------fa65173 [Mailer][Mailgun] Allow multiple tags with MailgunApiTransport
…ains() (fancyweb)This PR was merged into the 6.1 branch.Discussion----------Leverage str_starts_with(), str_ends_with() and str_contains()| Q | A| ------------- | ---| Branch? | 6.1| Bug fix? | no| New feature? | no| Deprecations? | no| Tickets | -| License | MIT| Doc PR | -Continuation of the previous ones. Let's see if the CI is green first.Commits-------bbe96c7 Leverage str_starts_with(), str_ends_with() and str_contains()
… local dev configuration. (GromNaN)This PR was merged into the 6.1 branch.Discussion----------[Assets] Accept empty `base_url`, in order to simplify local dev configuration.| Q | A| ------------- | ---| Branch? | 6.1| Bug fix? | no| New feature? | yes| Deprecations? | no| Tickets |Fix#28391| License | MIT| Doc PR | -Commits-------e3ac469 [Assets] Allows empty base url for dev
…cked enums (ogizanagi)This PR was squashed before being merged into the 6.1 branch.Discussion----------[HttpKernel] Add a controller argument resolver for backed enums| Q | A| ------------- | ---| Branch? | 6.1| Bug fix? | no| New feature? | yes <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tickets | N/A| License | MIT| Doc PR | TodoGiven:```phpnamespace App\Model;enum Suit: string{ case Hearts = 'H'; case Diamonds = 'D'; case Clubs = 'C'; case Spades = 'S';}```and the controller:```phpclass CardController{ #[Route('/cards/{suit}')] public function list(Suit $suit): Response { // [...] }}```A request to `/cards/H` would inject the `Suit::Hearts` enum case into the controller `$suit` argument.This core resolver aims to resolve backed enum from route path parameters, so we assume a 404 Not Found should be thrown on an invalid backing value provided.Commits-------8466cfe [HttpKernel] Add a controller argument resolver for backed enums…correctly used (lyrixx)This PR was merged into the 6.1 branch.Discussion----------[Serializer] Give more hints when an attribute is not correctly used| Q | A| ------------- | ---| Branch? | 6.1| Bug fix? | no| New feature? | kind of| Deprecations? | no| Tickets || License | MIT| Doc PR |before :afterCommits-------78bb078 [Serializer] Give more hints when an attribute is not correctly used
* 6.0: (22 commits) Bump license year Bump license year [Validator] Error using CssColor with doctrine annotations [HttpClient] Turn negative timeout to a very long timeout Features goes to 6.x branch [Translation] [LocoProvider] Fix use of asset ids fix lowest required PropertyInfo component release [Validator] throw when Constraint::_construct() has not been called Bump Symfony version to 6.0.3 Update VERSION for 6.0.2 Update CHANGELOG for 6.0.2 Bump Symfony version to 5.4.3 Update VERSION for 5.4.2 Update CHANGELOG for 5.4.2 Bump Symfony version to 5.3.14 Update VERSION for 5.3.13 Update CHANGELOG for 5.3.13 Bump Symfony version to 4.4.37 Update VERSION for 4.4.36 Update CONTRIBUTORS for 4.4.36 ...
* 6.0: Bump license year Bump license year
* 6.0: [CI] If package is not bridge, it should be in replace section Update ExprBuilder.php
PHPStorm is parsing docblocks for codehinting. Because `$default` is a mixed field, it parses the codeblock`Expected parameter of type '\Symfony\Component\Console\Command\mixed|\Symfony\Component\Console\Command\The|void', 'string' provided `
carsonbot commentedJan 6, 2022
Hey! I see that this is your first PR. That is great! Welcome! Symfony has acontribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
Member
nicolas-grekas commentedJan 6, 2022
Please report the issue to phpstorm, these annotations should be accepted. |
Member
stof commentedJan 6, 2022
@nicolas-grekas phpstan does not accept them either |
stof reviewedJan 6, 2022
| * @param $shortcut The shortcuts, can be null, a string of shortcuts delimited by | or an array of shortcuts | ||
| * @param $mode The option mode: One of the InputOption::VALUE_* constants | ||
| * @param $default The default value (must be null for InputOption::VALUE_NONE) | ||
| * @paramstring|array|null$shortcut The shortcuts, can be null, a string of shortcuts delimited by | or an array of shortcuts |
Member
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
should bestring|string[]|null in phpdoc, to also document the type of array items
| * @param $mode The option mode: One of the InputOption::VALUE_* constants | ||
| * @param $default The default value (must be null for InputOption::VALUE_NONE) | ||
| * @paramstring|array|null$shortcut The shortcuts, can be null, a string of shortcuts delimited by | or an array of shortcuts | ||
| * @paramint|null$mode The option mode: One of the InputOption::VALUE_* constants |
Member
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
our description is about wrong here. It is a bitmask of those constants.
Author
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Well theInputOption->__construct docblock is the same / more different that is listed here.
symfony/src/Symfony/Component/Console/Input/InputOption.php
Lines 55 to 62 in137aa3b
| /** | |
| * @param string|array|null $shortcut The shortcuts, can be null, a string of shortcuts delimited by | or an array of shortcuts | |
| * @param int|null $mode The option mode: One of the VALUE_* constants | |
| * @param string|bool|int|float|array|null $default The default value (must be null for self::VALUE_NONE) | |
| * | |
| * @throws InvalidArgumentException If option mode is invalid or incompatible | |
| */ | |
| publicfunction__construct(string$name,string|array$shortcut =null,int$mode =null,string$description ='',string|bool|int|float|array$default =null) |
Member
nicolas-grekas commentedJan 6, 2022
That's a bug in phpstan then, the spec makes the type optional: |
Author
MarcHagen commentedJan 6, 2022
Well... changing the base of the PR was not that smart... sorry about that. |
Author
MarcHagen commentedJan 6, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Yes quick search revealed that the issue from 2018, which is not all that good either because it assumes the type is mandatory |
Merged
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PHPStorm is parsing docblocks for codehinting. Because
$defaultis a mixed field, it parses the codeblock.Becuase there are no type for each
@paramit assumes theTheis a type/class that can be used.Hence the
\Symfony\Component\Console\Command\ThesuggestionI've seached in the git history why these are removed, and came out on#41912