Skip to content
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

Raise PHPStan level to 8 #1409

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

derrabus
Copy link
Member

@derrabus derrabus commented Mar 6, 2024

Q A
Type improvement
BC Break no
Fixed issues N/A

Summary

Let's make our static analysis a little bit stricter.

@derrabus derrabus added this to the 3.8.0 milestone Mar 6, 2024
@derrabus derrabus force-pushed the improvement/phpstan-level-8 branch from de7e03b to 77fe85e Compare March 6, 2024 13:55
@@ -145,7 +145,7 @@ public function getConfiguration(): Configuration
{
if ($this->configuration === null) {
$this->configuration = $this->configurationLoader->getConfiguration();
$this->freeze();
$this->frozen = true;
Copy link
Member

Choose a reason for hiding this comment

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

Why these changes where the frozen() method gets replaced with the direct set to true?

Copy link
Member Author

Choose a reason for hiding this comment

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

PHPStan treats the method calls as impure and complains that $this->configuration might be null when we return it.

Copy link
Member

Choose a reason for hiding this comment

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

I was playing around with annotations and other configurations, but it seems like PHPStan can't be convinced that $this->freeze() does nothing evil with other property states.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, and this is fine imho. freeze() is impure, so PHPStan's behavior is correct.

@derrabus derrabus merged commit 6458e87 into doctrine:3.8.x Mar 11, 2024
11 checks passed
@derrabus derrabus deleted the improvement/phpstan-level-8 branch March 11, 2024 22:30
derrabus added a commit to derrabus/migrations that referenced this pull request Mar 11, 2024
* 3.8.x:
  Raise PHPStan level to 8 (doctrine#1409)
  Remove unused paramaters from DiffCommand and VersionCommand (doctrine#1386)
  Simplify InlineParameterFormatterTest (doctrine#1411)
  fix: Allow enum param types: ArrayParameterType and ParameterType (doctrine#1408)
derrabus added a commit to derrabus/migrations that referenced this pull request Mar 11, 2024
* 3.8.x:
  Raise PHPStan level to 8 (doctrine#1409)
  Remove unused paramaters from DiffCommand and VersionCommand (doctrine#1386)
  Simplify InlineParameterFormatterTest (doctrine#1411)
  fix: Allow enum param types: ArrayParameterType and ParameterType (doctrine#1408)
derrabus added a commit to derrabus/migrations that referenced this pull request Mar 11, 2024
* 3.8.x:
  Fix CS
  Raise PHPStan level to 8 (doctrine#1409)
  Remove unused paramaters from DiffCommand and VersionCommand (doctrine#1386)
  Simplify InlineParameterFormatterTest (doctrine#1411)
  fix: Allow enum param types: ArrayParameterType and ParameterType (doctrine#1408)
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.

2 participants