-
-
Notifications
You must be signed in to change notification settings - Fork 155
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
Update static analyze tools #1532
Conversation
resolveFromConfigFile="true" | ||
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
xmlns="https://getpsalm.org/schema/config" | ||
xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd" | ||
errorBaseline="psalm-baseline.xml" |
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.
all issues has been fixed
psalm.xml
Outdated
@@ -1,17 +1,16 @@ | |||
<?xml version="1.0"?> | |||
<psalm | |||
errorLevel="8" | |||
errorLevel="7" |
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.
wondered why we use the lowest possible error level in psalm. So bump it up to 7 and fixed couple new violations
> | ||
<projectFiles> | ||
<directory name="src/Mutator" /> |
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.
didn't realize why we scan src/Mutator directory only
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.
We didn't use Psalm for anything else but to guard against @psalm-mutation-free
@@ -68,8 +68,6 @@ public static function getDefinition(): ?Definition | |||
} | |||
|
|||
/** | |||
* @psalm-mutation-free |
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.
all @psalm-mutation-fee
generates
ERROR: ImpureMethodCall - src/Mutator/Arithmetic/MinusEqual.php:76:75 - Cannot call an possibly-mutating method PhpParser\Node::getAttributes from a mutation-free context (see https://psalm.dev/203)
yield new Node\Expr\AssignOp\Plus($node->var, $node->expr, $node->getAttributes());
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.
Let's recall we've added these in #1382 on purpose because we have very annoying and hard to debug bugs.
* | ||
* @return string[] | ||
*/ | ||
public function getMutantCommandLine( | ||
array $tests, | ||
string $mutantFilePath, | ||
array $coverageTests, |
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.
psalm on level 7 generates an error:
ERROR: ParamNameMismatch - src/TestFramework/AbstractTestFrameworkAdapter.php:105:15 - Argument 1 of Infection\TestFramework\AbstractTestFrameworkAdapter::getMutantCommandLine has wrong name $tests, expecting $coverageTests as defined by Infection\AbstractTestFramework\TestFrameworkAdapter::getMutantCommandLine (see https://psalm.dev/230)
array $tests,
that is why property has been renamed. (the next renamed for the same reason)
7f9902b
to
7e66fd1
Compare
composer.json
Outdated
@@ -67,8 +67,7 @@ | |||
"webmozart/path-util": "^2.3" | |||
}, | |||
"conflict": { | |||
"phpunit/php-code-coverage": ">9 <9.1.4", | |||
"symfony/console": "=4.1.5" |
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.
no longer required as we supports symfony/console:^4.1.19
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.
@psalm-mutation-fee
are there for a reason
@@ -68,8 +68,6 @@ public static function getDefinition(): ?Definition | |||
} | |||
|
|||
/** | |||
* @psalm-mutation-free |
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.
Let's recall we've added these in #1382 on purpose because we have very annoying and hard to debug bugs.
7e66fd1
to
80587f3
Compare
@sidz this comment explains the reasons #1380 (comment), this change shows the bug: https://github.com/infection/infection/pull/1382/files#diff-d275481b2f5803179e7bc177f10fd78be286885b52cb0b9922ebe51c125f5137L81 we can add explanation (and probably a link?) to the Mutator interface near infection/src/Mutator/Mutator.php Line 53 in 8c97e07
|
@maks-rafalko added |
aa33fef
to
77d96f1
Compare
.php-cs-fixer.dist.php
Outdated
@@ -63,7 +63,6 @@ | |||
]) | |||
->ignoreDotFiles(false) | |||
->name('*php') | |||
->name('.php_cs.dist') |
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.
This was for PHP CS Fixer 2, can you add ->append([__FILE__])
, so the config is self checked.
77d96f1
to
b93bc5c
Compare
It's time to back to this PR :) so ready for re-review @maks-rafalko, @sanmai |
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.
Tested this by reintroducing the issue fixed in 7d42b2b (that's when we started to use Psalm). It caught it all right.
psalm.xml
Outdated
|
||
<ImpureFunctionCall> | ||
<errorLevel type="suppress"> | ||
<directory name="src/Mutator"/> |
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.
<directory name="src/Mutator"/> | |
<directory name="src/Mutator/Regex"/> |
> | ||
<projectFiles> | ||
<directory name="src/Mutator" /> |
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.
We didn't use Psalm for anything else but to guard against @psalm-mutation-free
b93bc5c
to
c7f975b
Compare
thanks @maks-rafalko, @sanmai |
This PR update psalm and phpstan and fixing some founded violations