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

Psalm 4.0 ignores baseline when resolveFromConfigFile is implicitly "true" #4410

Closed
sebastianbergmann opened this issue Oct 25, 2020 · 2 comments
Labels

Comments

@sebastianbergmann
Copy link
Contributor

I have set up Psalm in my projects, for instance for PHPUnit, like so:

$ ls -lh .psalm                      
total 88K
-rw-r--r--. 1 sb sb  76K Oct 25 06:14 baseline.xml
drwxr-xr-x. 3 sb sb 4.0K Oct 25 06:14 cache
-rw-r--r--. 1 sb sb  591 Oct 25 06:14 config.xml
-rw-r--r--. 1 sb sb 1.8K Jul 26 12:20 static-analysis.xml

.psalm/config.xml

<?xml version="1.0"?>
<psalm
    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"
    cacheDirectory=".psalm/cache"
    totallyTyped="false"
    checkForThrowsDocblock="true"
    findUnusedVariablesAndParams="true"
    findUnusedCode="true"
    errorBaseline=".psalm/baseline.xml"
>
    <projectFiles>
        <directory name="src" />
        <ignoreFiles>
            <directory name="vendor" />
        </ignoreFiles>
    </projectFiles>
</psalm>

Psalm 3.18

$ ./tools/psalm --version                 
Psalm 3.18.2@19aa905f7c3c7350569999a93c40ae91ae4e1626
$ ./tools/psalm --config=.psalm/config.xml
Scanning files...
Analyzing files...

░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░  60 / 322 (18%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 120 / 322 (37%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 180 / 322 (55%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 240 / 322 (74%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 300 / 322 (93%)
░░░░░░░░░░░░░░░░░░░░░░

------------------------------
No errors found!
------------------------------
1085 other issues found.
You can display them with --show-info=true
------------------------------
Psalm can automatically fix 122 of these issues.
Run Psalm again with 
--alter --issues=MissingReturnType,UnusedVariable,InvalidReturnType,MissingParamType,PossiblyUnusedMethod,UnusedMethod --dry-run
to see what it can fix.
------------------------------

Checks took 3.19 seconds and used 229.881MB of memory
Psalm was able to infer types for 93.8080% of the codebase

As you can see, this works fine with Psalm 3.18.

Psalm 4.0

$ ./tools/psalm --version                 
Psalm 4.0.1@b1e2e30026936ef8d5bf6a354d1c3959b6231f44

The first problem I run into when upgrading from Psalm 3 to Psalm 4 is that the handling of paths has changed in Psalm's XML configuration file:

$ ./tools/psalm --config=.psalm/config.xml
Problem parsing /usr/local/src/phpunit/.psalm/config.xml:
  Could not resolve config path to /usr/local/src/phpunit/.psalm//src

With the changes shown below I am able to run Psalm 4:

diff --git a/.psalm/config.xml b/.psalm/config.xml
index 8821e7f0b..2ce131438 100644
--- a/.psalm/config.xml
+++ b/.psalm/config.xml
@@ -3,17 +3,17 @@
     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"
-    cacheDirectory=".psalm/cache"
+    cacheDirectory="cache"
     totallyTyped="false"
     checkForThrowsDocblock="true"
     findUnusedVariablesAndParams="true"
     findUnusedCode="true"
-    errorBaseline=".psalm/baseline.xml"
+    errorBaseline="baseline.xml"
 >
     <projectFiles>
-        <directory name="src" />
+        <directory name="../src" />
         <ignoreFiles>
-            <directory name="vendor" />
+            <directory name="../vendor" />
$ ./tools/psalm --config=.psalm/config.xml
Scanning files...
Analyzing files...

░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░  60 / 322 (18%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 120 / 322 (37%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 180 / 322 (55%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 240 / 322 (74%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 300 / 322 (93%)
░░░░░░░░░░░░░░░░░░░░░░

ERROR: DocblockTypeContradiction - /usr/local/src/phpunit/src/Framework/Assert.php:110:15 - Docblock-defined type string for $key is always string (see https://psalm.dev/155)
        if (!(is_int($key) || is_string($key))) {
.
.
.
------------------------------
1089 errors found
------------------------------
3 other issues found.
You can display them with --show-info=true
------------------------------
Psalm can automatically fix 120 of these issues.
Run Psalm again with 
--alter --issues=MissingReturnType,UnusedVariable,InvalidReturnType,MissingParamType,PossiblyUnusedMethod,UnusedMethod --dry-run
to see what it can fix.
------------------------------

Checks took 5.32 seconds and used 155.797MB of memory
Psalm was able to infer types for 92.7308% of the codebase

However, the baseline is ignored.

When I make the changes show below then the baseline is not ignored:

diff --git a/.psalm/config.xml b/.psalm/config.xml
index 8821e7f0b..6989ee238 100644
--- a/.psalm/config.xml
+++ b/.psalm/config.xml
@@ -3,6 +3,7 @@
     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"
+    resolveFromConfigFile="false"
$ ./tools/psalm --config=.psalm/config.xml
Scanning files...
Analyzing files...

░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░  60 / 322 (18%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 120 / 322 (37%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 180 / 322 (55%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 240 / 322 (74%)
░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 300 / 322 (93%)
░░░░░░░░░░░░░░░░░░░░░░

ERROR: ArgumentTypeCoercion - src/Framework/TestResult.php:683:40 - Argument 1 of SebastianBergmann\CodeCoverage\CodeCoverage::start expects PHPUnit\Framework\TestCase|PHPUnit\Runner\PhptTestCase|string, parent type PHPUnit\Framework\Test provided (see https://psalm.dev/193)
            $this->codeCoverage->start($test);
.
.
.
------------------------------
17 errors found
------------------------------
1075 other issues found.
You can display them with --show-info=true
------------------------------
Psalm can automatically fix 120 of these issues.
Run Psalm again with 
--alter --issues=MissingReturnType,UnusedVariable,InvalidReturnType,MissingParamType,PossiblyUnusedMethod,UnusedMethod --dry-run
to see what it can fix.
------------------------------

Checks took 5.54 seconds and used 155.598MB of memory
Psalm was able to infer types for 92.7308% of the codebase

As far as I understand it, the two sets of changes to the XML configuration file should be equivalent. Of course, the one which works for me (which is also explained in the release notes for Psalm 4.0.0) is likely the correct one. Still I wonder whether the other should also work and whether that fact that it does not means that there is a bug somewhere.

@psalm-github-bot
Copy link

Hey @sebastianbergmann, can you reproduce the issue on https://psalm.dev ?

@muglug muglug added the bug label Oct 25, 2020
@muglug muglug changed the title Psalm 4.0 ignores baseline Psalm 4.0 ignores baseline when resolveFromConfigFile="true" Oct 25, 2020
@muglug muglug changed the title Psalm 4.0 ignores baseline when resolveFromConfigFile="true" Psalm 4.0 ignores baseline when resolveFromConfigFile is implicitly "true" Oct 25, 2020
@muglug
Copy link
Collaborator

muglug commented Oct 25, 2020

Still I wonder whether the other should also work

Yes, I only tested the happy path (just switching the config) for PHPUnit, not the unhappy one. Clearly a bug.

@muglug muglug closed this as completed in b26983c Oct 25, 2020
danog pushed a commit to danog/psalm that referenced this issue Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants