Skip to content

Fix file path of trait errors reported directly in the trait#5780

Open
janedbal wants to merge 4 commits into
phpstan:2.2.xfrom
janedbal:fix-trait-error-file-path
Open

Fix file path of trait errors reported directly in the trait#5780
janedbal wants to merge 4 commits into
phpstan:2.2.xfrom
janedbal:fix-trait-error-file-path

Conversation

@janedbal
Copy link
Copy Markdown
Contributor

Fixes phpstan/phpstan#14718.

Problem

Since 2.2.0, an error originating inside a trait that is collapsed into a single error and reported once directly in the trait (via the new ConstantConditionInTraitRuleError::removeTraitContext() path) ends up with an inconsistent file pair:

  • getFile() → the trait file (correct)
  • getFilePath() → the using class's file (stale)
  • getTraitFilePath()null

removeTraitContext() moved file onto the trait file but left filePath pointing at the using-class file. Two things break as a result:

  1. Inline @phpstan-ignore no longer suppresses the error (the regression reported in #14718). AnalyserResultFinalizer looks up line-ignores by getFilePath(), which points at the using-class file, so the ignore registered for the trait never matches.
  2. The editorUrl link points at the wrong file. TableErrorFormatter builds it from getTraitFilePath() ?? getFilePath()null ?? <using-class file>, i.e. the using-class file glued to the trait's line number.

Both worked correctly in 2.1.x.

Fix

Make filePath follow file onto the trait file in removeTraitContext(), the same way changeFilePath() keeps file and filePath in sync. One line.

Verification

Minimal reproduction: https://github.com/janedbal/phpstan-bug-14718-repro

Before (2.2.0): error reported despite the inline ignore, editor link → using-class file.
After: getFile() === getFilePath() === <trait file>, getTraitFilePath() === null.

Added a unit test in ErrorTest covering removeTraitContext().

Co-Authored-By: Claude Code

When an error inside a trait is reported once directly in the trait
(Error::removeTraitContext()), the displayed file is moved to the trait
file but filePath was left pointing at the using-class file. This broke
inline @PHPStan-Ignore matching (looked up by filePath) and the editorUrl
(getTraitFilePath() ?? getFilePath()), both resolving to the wrong file.

Make filePath follow the file onto the trait, matching changeFilePath().

Fixes phpstan/phpstan#14718

Co-Authored-By: Claude Code
@ondrejmirtes
Copy link
Copy Markdown
Member

Would be nice to have the repro repo here in e2e/ and e2e-tests, showing all the behaviour described in the issue - non-working ignore comment, impossible baseline generation, problem disappearing with primed result cache etc.

Covers the behaviours from the issue against the fix:
- inline @PHPStan-Ignore on a trait-deduplicated error suppresses it,
  consistently with an empty and a primed result cache
- with the ignore removed, the error is reported in the trait file and a
  generated baseline records the trait file path, not the using-class file

Co-Authored-By: Claude Code
@janedbal
Copy link
Copy Markdown
Contributor Author

janedbal commented May 29, 2026

Added e2e/bug-14718 + an e2e-tests job. The fixture is a trait used by two classes, with the deduplicated identical.alwaysFalse error reported directly in the trait. The job asserts, on the fixed build:

  • an inline // @phpstan-ignore on that error suppresses it (exit 0) — and stays suppressed with a primed result cache (the run is repeated without clearing);
  • with the ignore removed, the error is reported in the trait file, and --generate-baseline writes path: src/FooTrait.php (the trait file), not the using-class file.

For comparison, reverting the one-line fix flips all of these: the ignore is skipped, the editor URL / baseline path point at the using-class file (src/Foo.php).

One honest caveat: I could not reproduce the "error disappears on the second (primed-cache) run" facet in this minimal two-file fixture — it's deterministic across cache states here, so I didn't encode an assertion for it. That facet looks like it depends on the ConstantConditionInTraitRule dedup count() being sensitive to which files' collected data is restored from cache, which may need a larger setup (or be a separate concern). Happy to dig further or restructure the fixture if you have a setup in mind that triggers it.

Standalone repro repo (same case): https://github.com/janedbal/phpstan-bug-14718-repro


Note: (Claude anwering)

@ondrejmirtes
Copy link
Copy Markdown
Member

Superb, please fix the build and mark it as ready.

janedbal added 2 commits May 29, 2026 15:19
The phrase tripped PHPStan's own ignore.parseError rule during
self-analysis. Reword the comment.

Co-Authored-By: Claude Code
The identical.alwaysFalse error in MbFunctionsReturnTypeExtensionTrait
(used by StrSplitFunctionReturnTypeExtension) is now attributed to the
trait file instead of the using class, so move its baseline entry.

Co-Authored-By: Claude Code
@janedbal janedbal marked this pull request as ready for review May 29, 2026 13:47
@phpstan-bot
Copy link
Copy Markdown
Collaborator

This pull request has been marked as ready for review.

Comment thread build/baseline-8.0.neon
identifier: identical.alwaysFalse
count: 1
path: ../src/Type/Php/StrSplitFunctionReturnTypeExtension.php
path: ../src/Type/Php/MbFunctionsReturnTypeExtensionTrait.php
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't this a BC break? The ignore work correctly before. It's a bit awkward, but for errors reported in traits, I think on current stable version, both the trait or the class path can be used in path in ignoreErrors.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is achieved here:

$filePath = $this->fileHelper->normalizePath($error->getFilePath());
if (isset($this->ignoreErrorsByFile[$filePath])) {
foreach ($this->ignoreErrorsByFile[$filePath] as $ignoreError) {
$i = $ignoreError['index'];
$ignore = $ignoreError['ignoreError'];
$result = $processIgnoreError($error, $i, $ignore);
if (!$result) {
unset($errors[$errorIndex]);
$ignoredErrors[] = [$error, $ignore];
continue 2;
}
}
}
$traitFilePath = $error->getTraitFilePath();
if ($traitFilePath !== null) {
$normalizedTraitFilePath = $this->fileHelper->normalizePath($traitFilePath);
if (isset($this->ignoreErrorsByFile[$normalizedTraitFilePath])) {
foreach ($this->ignoreErrorsByFile[$normalizedTraitFilePath] as $ignoreError) {
$i = $ignoreError['index'];
$ignore = $ignoreError['ignoreError'];
$result = $processIgnoreError($error, $i, $ignore);
if (!$result) {
unset($errors[$errorIndex]);
$ignoredErrors[] = [$error, $ignore];
continue 2;
}
}
}
}

I suspect, wouldn't be a better place to fix this in FileAnalyser where @phpstan-ignore are processed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Some @phpstan-ignore are ignored in traits since v2.2.0

3 participants