Skip to content

[Console] Fix OutputInterface options int-mask for PHPStan #48384

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 30, 2022

Conversation

ogizanagi
Copy link
Contributor

@ogizanagi ogizanagi commented Nov 29, 2022

Q A
Branch? 6.2
Bug fix? yes
New feature? no
Deprecations? no
Tickets N/A
License MIT
Doc PR N/A

When upgrading an application to 6.2, I encountered this issue with PHPStan:

 ------ -------------------------------------------------------------------------------------------------------
  Line   Pilot/Runner/Output/MultiplexingOutput.php
 ------ -------------------------------------------------------------------------------------------------------
  79     Default value of the parameter #3 $options (0) of method
         App\Pilot\Runner\Output\MultiplexingOutput::write() is incompatible with type 1|2|4|16|32|64|128|256.
 ------ -------------------------------------------------------------------------------------------------------

The MultiplexingOutput implements the OutputInterface and defined 0 as the default value for $options:

    public function write(string|iterable $messages, bool $newline = false, int $options = 0): void

as defined by the interface:

public function write(string|iterable $messages, bool $newline = false, int $options = 0);

When trying to use self::OUTPUT_NORMAL | self::VERBOSITY_NORMAL as default value:

 ------ -------------------------------------------------------------------------------------------------------
  Line   Pilot/Runner/Output/MultiplexingOutput.php
 ------ -------------------------------------------------------------------------------------------------------
  79     Default value of the parameter #3 $options (33) of method
         App\Pilot\Runner\Output\MultiplexingOutput::write() is incompatible with type 1|2|4|16|32|64|128|256.
 ------ -------------------------------------------------------------------------------------------------------

Or simply using Output::write() with specific options:

 ------ -------------------------------------------------------------------------------------------------------
  Line   Pilot/Runner/Output/MultiplexingOutput.php
 ------ -------------------------------------------------------------------------------------------------------
  81     Parameter #3 $options of method Symfony\Component\Console\Output\Output::write() expects
         1|2|4|16|32|64|128|256, 33 given.
 ------ -------------------------------------------------------------------------------------------------------

Using PHPStan's int-mask-of is the solution for this, and allows by nature the 0 value.
It was even used at some point, but reverted to use self::VERBOSITY_*|self::OUTPUT_*. However, it does not behave as expected for masks.

So, either we use int-mask-of, or we revert to int?

* @param self::VERBOSITY_*|self::OUTPUT_* $options A bitmask of options (one of the OUTPUT or VERBOSITY constants),
* 0 is considered the same as self::OUTPUT_NORMAL | self::VERBOSITY_NORMAL
* @param bool $newline Whether to add a newline
* @param int-mask-of<self::VERBOSITY_*,self::OUTPUT_*> $options A bitmask of options (one of the OUTPUT or VERBOSITY constants),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Psalm doesn't work correctly when combining globs together here. Let's go for either int-mask-of<self::*> or int.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While it works for both Psalm/PhpStan, it seems to break the docblock parsing of PhpStorm:

SCR-20221129-ojk

Let's simply revert these changes to <6.2.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I'm not a PhpStorm user myself, does this also create issues when using this method in PhpStorm? (e.g. issues with completion or adding false-positive inspection errors)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not to my knowledge nor experience actually ; the int typehint is still in use for the quick documentation, autocomplete or inspections, but I don't know if it can cause quirks with some plugins.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the EAP version of PHPStorm has full support for parsing constant wildcards (the stable release supports the case with a name prefix but not the full wildcard on the class IIRC). But indeed, it will fallback to int in such case.

@OskarStark OskarStark changed the title [Console] Fix OutputInterface options int-mask for PHPStan [Console] Fix OutputInterface options int-mask for PHPStan Nov 30, 2022
@chalasr
Copy link
Member

chalasr commented Nov 30, 2022

Thank you @ogizanagi.

@chalasr chalasr merged commit 258fb73 into symfony:6.2 Nov 30, 2022
@ogizanagi ogizanagi deleted the output-phpstan branch November 30, 2022 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants