Skip to content

Fix error reported on the wrong line #4991

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.

Sign up for GitHub

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

Merged
VincentLanglet merged 11 commits into from
Feb 19, 2026
Merged

Fix error reported on the wrong line #4991

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a5cece4
Fix error reported on the wrong line
VincentLanglet Feb 18, 2026
c720728
Refactor
VincentLanglet Feb 18, 2026
42bf0a3
Rework
VincentLanglet Feb 18, 2026
6adc439
Fix
VincentLanglet Feb 18, 2026
c3da32a
Handle more cases
VincentLanglet Feb 18, 2026
154a83e
More
VincentLanglet Feb 18, 2026
af0f984
Another strategy
VincentLanglet Feb 19, 2026
13fdcd9
Fix
VincentLanglet Feb 19, 2026
23963be
Revert for binary
VincentLanglet Feb 19, 2026
ebd0f63
More
VincentLanglet Feb 19, 2026
37eeade
Fix
VincentLanglet Feb 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion src/Analyser/RuleErrorTransformer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use PHPStan\Fixable\PhpPrinter;
use PHPStan\Fixable\PhpPrinterIndentationDetectorVisitor;
use PHPStan\Fixable\ReplacingNodeVisitor;
use PHPStan\Node\PropertyAssignNode;
use PHPStan\Node\VirtualNode;
use PHPStan\Rules\FileRuleError;
use PHPStan\Rules\FixableNodeRuleError;
Expand Down Expand Up @@ -55,7 +56,6 @@ public function transform(
Node $node,
): Error
{
$line = $node->getStartLine();
$canBeIgnored = true;
$fileName = $scope->getFileDescription();
$filePath = $scope->getFile();
Expand All @@ -75,6 +75,8 @@ public function transform(
&& $ruleError->getLine() !== -1
) {
$line = $ruleError->getLine();
} else {
$line = self::getStartLineFromNode($node);
Copy link
Copy Markdown
Contributor

@staabm staabm Feb 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I just had a look thru the codebase.

we have already PhpDocLineHelper::detectLine() which is called like line(PhpDocLineHelper::detectLine($node, $phpDocTag)) .

this makes me think we should not have this special fallback case here, but call something like NodeChainLineHelper::detectLine($node) right at the ErrorBuilder call-sites where ->line() is invoked.

Copy link
Copy Markdown
Contributor Author

@VincentLanglet VincentLanglet Feb 19, 2026

Choose a reason for hiding this comment

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

this makes me think we should not have this special fallback case here, but call something like NodeChainLineHelper::detectLine($node) right at the ErrorBuilder call-sites where ->line() is invoked.

Lot of existing rules are not calling line() and rely on the defaut fallback which was already implemented in RuleErrortransformer $node->getStartLine().

There is tons of rule to fix, so it would be easier to modify the fallback.

Even binary operator are wrong
https://phpstan.org/r/090b54c3-0255-4027-9934-491d8ab962b7

And yes PipeOperator has the same issue too https://phpstan.org/r/7ab46b15-18c9-4d5b-bfc9-27bc78efe3d4

But there is so much to fix, I feel like we should first:

  • validate a strategy with ondrej
  • Fix/Implement it, step by step

I start to feel like we should fix rules one by one...

Copy link
Copy Markdown
Contributor

@staabm staabm Feb 19, 2026

Choose a reason for hiding this comment

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

agree - thanks for looking into it

Copy link
Copy Markdown
Contributor Author

@VincentLanglet VincentLanglet Feb 19, 2026

Choose a reason for hiding this comment

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

I tried another strategy with af0f984

I feel like it's safer to avoid a general strategy in RuleTransformer and instead to fix rules one by one. WDYT ?

Copy link
Copy Markdown
Contributor

@staabm staabm Feb 19, 2026

Choose a reason for hiding this comment

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

yes, thats what I had in mind with my comment

}
if (
$ruleError instanceof FileRuleError
Expand Down Expand Up @@ -166,4 +168,24 @@ public function transform(
);
}

public static function getStartLineFromNode(Node $node): int
{
if (
$node instanceof Node\Expr\PropertyFetch
|| $node instanceof Node\Expr\StaticPropertyFetch
|| $node instanceof Node\Expr\NullsafePropertyFetch
|| $node instanceof Node\Expr\MethodCall
|| $node instanceof Node\Expr\NullsafeMethodCall
|| $node instanceof Node\Expr\StaticCall
) {
$line = $node->name->getStartLine();
} elseif ($node instanceof PropertyAssignNode) {
$line = self::getStartLineFromNode($node->getPropertyFetch());
} else {
return $node->getStartLine();
}

return $line !== -1 ? $line : $node->getStartLine();
}

}
7 changes: 2 additions & 5 deletions src/Rules/FunctionCallParametersCheck.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use PhpParser\Node;
use PhpParser\Node\Expr;
use PHPStan\Analyser\MutatingScope;
use PHPStan\Analyser\RuleErrorTransformer;
use PHPStan\Analyser\Scope;
use PHPStan\DependencyInjection\AutowiredParameter;
use PHPStan\DependencyInjection\AutowiredService;
Expand Down Expand Up @@ -89,11 +90,7 @@ public function check(
string $namedArgumentMessage,
): array
{
if ($funcCall instanceof Node\Expr\MethodCall || $funcCall instanceof Node\Expr\StaticCall || $funcCall instanceof Node\Expr\FuncCall) {
$funcCallLine = $funcCall->name->getStartLine();
} else {
$funcCallLine = $funcCall->getStartLine();
}
$funcCallLine = RuleErrorTransformer::getStartLineFromNode($funcCall);
Copy link
Copy Markdown
Contributor

@staabm staabm Feb 18, 2026

Choose a reason for hiding this comment

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

Do we still need this line?

Copy link
Copy Markdown
Contributor Author

@VincentLanglet VincentLanglet Feb 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, because

  • I compute the line from the node only if it's not set and this Check set the line.
  • If I remove the ->line(...) calls, since the AttributesCheck calls the functionCallParametersCheck with another Node than the one from the ClassAttributesRule it makes a test from ClassAttributesRuleTest to fail.


$functionParametersMinCount = 0;
$functionParametersMaxCount = 0;
Expand Down
13 changes: 13 additions & 0 deletions tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3844,6 +3844,19 @@ public function testBug12875(): void
$this->analyse([__DIR__ . '/data/bug-12875.php'], []);
}

public function testBug14150(): void
{
$this->checkThisOnly = false;
$this->checkNullables = true;
$this->checkUnionTypes = true;
$this->analyse([__DIR__ . '/data/bug-14150.php'], [
[
'Call to an undefined method Bug14150Method\HelloWorld::y().',
21,
],
]);
}

#[RequiresPhp('>= 8.1')]
public function testBug13805(): void
{
Expand Down
13 changes: 13 additions & 0 deletions tests/PHPStan/Rules/Methods/CallStaticMethodsRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -909,6 +909,19 @@ public function testRestrictedInternalClassNameUsage(): void
]);
}

public function testBug14150(): void
{
$this->checkThisOnly = false;
$this->checkExplicitMixed = true;
$this->checkImplicitMixed = true;
$this->analyse([__DIR__ . '/data/bug-14150-static.php'], [
[
'Call to an undefined static method Bug14150MethodStatic\HelloWorld::y().',
21,
],
]);
}

public function testBug13556(): void
{
$this->checkThisOnly = false;
Expand Down
10 changes: 10 additions & 0 deletions tests/PHPStan/Rules/Methods/NullsafeMethodCallRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ public function testBug8664(): void
$this->analyse([__DIR__ . '/../../Analyser/data/bug-8664.php'], []);
}

public function testBug14150(): void
{
$this->analyse([__DIR__ . '/data/bug-14150-nullsafe.php'], [
[
'Using nullsafe method call on non-nullable type $this(Bug14150NullsafeMethod\HelloWorld). Use -> instead.',
21,
],
]);
}

#[RequiresPhp('>= 8.0')]
public function testBug9293(): void
{
Expand Down
23 changes: 23 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-14150-nullsafe.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php // lint >= 8.0

namespace Bug14150NullsafeMethod;

class HelloWorld
{
public int $x = 5;

/**
* @return $this
*/
public function x()
{
return $this;
}

public function testUnknownMethod(): void
{
$this
->x()
?->y();
}
}
23 changes: 23 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-14150-static.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

namespace Bug14150MethodStatic;

final class HelloWorld
{
public int $x = 5;

/**
* @return $this
*/
public static function x()
{
return new self();
}

public function testUnknownMethod(): void
{
(new HelloWorld())
::x()
::y();
}
}
23 changes: 23 additions & 0 deletions tests/PHPStan/Rules/Methods/data/bug-14150.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php declare(strict_types = 1);

namespace Bug14150Method;

class HelloWorld
{
public int $x = 5;

/**
* @return $this
*/
public function x()
{
return $this;
}

public function testUnknownMethod(): void
{
$this
->x()
->y();
}
}
14 changes: 14 additions & 0 deletions tests/PHPStan/Rules/Properties/NullsafePropertyFetchRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,20 @@ public function testBug8517(): void
$this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-8517.php'], []);
}

public function testBug14150(): void
{
$this->analyse([__DIR__ . '/data/bug-14150-nullsafe.php'], [
[
'Using nullsafe property access on non-nullable type $this(Bug14150NullsafeProperty\HelloWorld). Use -> instead.',
20,
],
[
'Using nullsafe property access on non-nullable type $this(Bug14150NullsafeProperty\HelloWorld). Use -> instead.',
27,
],
]);
}

#[RequiresPhp('>= 8.0')]
public function testBug9105(): void
{
Expand Down
16 changes: 16 additions & 0 deletions tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,22 @@ public function testBug13654(): void
$this->analyse([__DIR__ . '/data/bug-13654.php'], []);
}

public function testBug14150(): void
{
$this->checkExplicitMixed = true;
$this->checkImplicitMixed = true;
$this->analyse([__DIR__ . '/data/bug-14150.php'], [
[
'Property Bug14150Properties\HelloWorld::$x (int) does not accept null.',
20,
],
[
'Property Bug14150Properties\HelloWorld::$x (int) does not accept null.',
27,
],
]);
}

#[RequiresPhp('>= 8.5')]
public function testCloneWith(): void
{
Expand Down
29 changes: 29 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-14150-nullsafe.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php // lint >= 8.0

namespace Bug14150NullsafeProperty;

class HelloWorld
{
public int $x = 5;

/**
* @return $this
*/
public function x()
{
return $this;
}

public function test3(): void
{
$this
?->x;
}

public function test4(): void
{
$this
->x()
?->x;
}
}
29 changes: 29 additions & 0 deletions tests/PHPStan/Rules/Properties/data/bug-14150.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php declare(strict_types = 1);

namespace Bug14150Properties;

class HelloWorld
{
public int $x = 5;

/**
* @return $this
*/
public function x()
{
return $this;
}

public function test3(): void
{
$this
Comment thread
VincentLanglet marked this conversation as resolved.
->x = null;
}

public function test4(): void
{
$this
->x()
->x = null;
Comment thread
VincentLanglet marked this conversation as resolved.
}
}
Loading