[Console] Fix docblock Command->addOption #44935
Closed
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 @np4me2k Commits ------- 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 | no This 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/a Had 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 | no Use `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 #38323 The 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 | Todo
Given:
```php
namespace App\Model;
enum Suit: string
{
case Hearts = 'H';
case Diamonds = 'D';
case Clubs = 'C';
case Spades = 'S';
}
```
and the controller:
```php
class 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 :  after  Commits ------- 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 `
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution 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 |
|
Please report the issue to phpstorm, these annotations should be accepted. |
|
@nicolas-grekas phpstan does not accept them either |
stof
reviewed
Jan 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) | ||
| * @param string|array|null $shortcut The shortcuts, can be null, a string of shortcuts delimited by | or an array of shortcuts |
There was a problem hiding this comment.
should be string|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) | ||
| * @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 InputOption::VALUE_* constants |
There was a problem hiding this comment.
our description is about wrong here. It is a bitmask of those constants.
There was a problem hiding this comment.
Well the InputOption->__construct docblock is the same / more different that is listed here.
symfony/src/Symfony/Component/Console/Input/InputOption.php
Lines 55 to 62 in 137aa3b
|
That's a bug in phpstan then, the spec makes the type optional: |
|
Well... changing the base of the PR was not that smart... sorry about that. |
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 free
to 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