Inconsistent baseline behavior running psalm with paths with non-autoloaded statements

[gready-hub]

Issue

Inconsistent baseline behaviour between ./psalm and ./psalm ./some/file.php when the file contains a require or include statement which is not autoloaded.

1. Pass: $ vendor/bin/psalm
2. Fail: $ vendor/bin/psalm src/Service/EpicService.php

Introduced with release of 5.16.0 with commit https://github.com/vimeo/psalm/commit/670bd6afce56d8c5f828e8237bfe4003706c89bd

References:

• https://actd.nl/2021/01/06/understanding-psalm-autoloader/

Pre-requisites

1. composer.json a. autoload src/ b. does not autoload legacy/global.php

2. psalm.xml exists a. Includes ./src and ./legacy

3. psalm-baseline.xml exists a. populated by psalm set-baseline=psalm-baseline.xml b. has baseline entries for legacy/global.php

4. Structure

   composer.json
   composer.lock
   
   psalm.xml
   psalm-baseline.xml
   
   vendor/
   legacy/
     global.php
   src/
     Service/
       EpicService.php
   

5. Files global.php

   /**
    * @return 'not_oops'
    */
   function naughty_function(): string {
     return 'oops';
   }
   

   EpicService.php

   namespace IncredibleApp\Service;
   
   // By merely including a file with violations it will be reported by Psalm 
   // and the baseline will not be respected when psalm is called with paths
   require_once __DIR__.'/../../legacy/global.php';
   
   class EpicService {
     public function beEpic(): int {
       return 1;
     }
   }
   

Example

$ vendor/bin/psalm set-baseline=psalm-baseline.xml
# Success

$ vendor/bin/psalm
# No issues reported

$ vendor/bin/psalm src/Service/EpicService.php
ERROR: InvalidReturnType
at legacy/global.php:4:12
The declared return type ''not_oops'' for naughty_function is incorrect, got ''oops'' (see https://psalm.dev/011)
 * @return 'not_oops'


ERROR: InvalidReturnStatement
at legacy/global.php:7:12
The inferred type ''oops'' does not match the declared return type ''not_oops'' for naughty_function (see https://psalm.dev/128)
    return 'oops';

[psalm-github-bot[bot]]

Hey @gready-hub, can you reproduce the issue on https://psalm.dev? These will be used as phpunit tests when implementing the feature or fixing this bug.

[gready-hub]

My hunch is that this is probably caused by the inferred paths from the magically loaded require_once are just slightly different from those stored in the baseline issues map, causing them not to be located.

[gready-hub]

My hunch is that this is probably caused by the inferred paths from the magically loaded require_once are just slightly different from those stored in the baseline issues map, causing them not to be located.

This is incorrect. Since we didn't specify the require_once file in the paths Psalm wont try to filter out the errors from it.

Question: Is this intentional behaviour?

        if ($paths_to_check !== null) {
            $filtered_issue_baseline = [];
            foreach ($paths_to_check as $path_to_check) {
                // +1 to remove the initial slash from $path_to_check
                var_dump($path_to_check, $config->base_dir, substr($path_to_check, strlen($config->base_dir) + 1));
                $path_to_check = substr($path_to_check, strlen($config->base_dir) + 1);
                if (isset($issue_baseline[$path_to_check])) {
                    $filtered_issue_baseline[$path_to_check] = $issue_baseline[$path_to_check];
                }
            }
            $issue_baseline = $filtered_issue_baseline;
        }

        var_dump($filtered_issue_baseline);

string(58) "/home/gread/freelancer-dev/fl-gaf/src/Auth/EpicService.php"
string(33) "/home/gread/freelancer-dev/fl-gaf"
string(24) "src/Auth/EpicService.php"
array(1) {
  ["src/Auth/EpicService.php"]=>
  array(1) {
    ["UnusedClass"]=>
    array(2) {
      ["o"]=>
      int(1)
      ["s"]=>
      array(1) {
        [0]=>
        string(11) "EpicService"
      }
    }
  }
}

[gready-hub]

Ah it's because of the introduction of this https://github.com/vimeo/psalm/commit/670bd6af

[gready-hub]

Running this patch successfully at the minute. Could possibly simplify the interface and refactor $is_full to simply be null or non-empty-list of $paths_to_check.

• works for single file with includes, included file is not reported on
• works for single file which regresses the baseline, new issue reported
• works for full scan, no issues reported
• works for full scan which regresses the baseline, new issue reported

Branch 5.21.1

diff --git a/src/Psalm/Internal/Cli/Psalm.php b/src/Psalm/Internal/Cli/Psalm.php
index c354cf21a..711fee641 100644
--- a/src/Psalm/Internal/Cli/Psalm.php
+++ b/src/Psalm/Internal/Cli/Psalm.php
@@ -396,6 +396,7 @@ final class Psalm
                 $start_time,
                 isset($options['stats']),
                 self::initBaseline($options, $config, $current_dir, $path_to_config, $paths_to_check),
+                $paths_to_check,
             );
         } else {
             self::autoGenerateConfig($project_analyzer, $current_dir, $init_source_dir, $vendor_dir);
diff --git a/src/Psalm/IssueBuffer.php b/src/Psalm/IssueBuffer.php
index 33fc16bc8..7433e47c4 100644
--- a/src/Psalm/IssueBuffer.php
+++ b/src/Psalm/IssueBuffer.php
@@ -542,7 +542,8 @@ final class IssueBuffer
         bool $is_full,
         float $start_time,
         bool $add_stats = false,
-        array $issue_baseline = []
+        array $issue_baseline = [],
+        array $paths = null,
     ): void {
         if (!$project_analyzer->stdout_report_options) {
             throw new UnexpectedValueException('Cannot finish without stdout report options');
@@ -570,7 +571,14 @@ final class IssueBuffer
 
             ksort(self::$issues_data);
 
+            $pathSet = !$is_full && !empty($paths) ? array_flip($paths) : null;
             foreach (self::$issues_data as $file_path => $file_issues) {
+                // If we're not doing a full run and the file isn't in the list of files to report, skip it
+                if ($pathSet !== null && !isset($pathSet[$file_path])) {
+                    unset(self::$issues_data[$file_path]);
+                    continue;
+                }
+
                 usort(
                     $file_issues,
                     static fn(IssueData $d1, IssueData $d2): int => [$d1->file_path, $d1->line_from, $d1->column_from]

[weirdan]

cc: @edsrzf