Skip to content

Implement MagicClassConstantRule#2741

Merged
ondrejmirtes merged 18 commits intophpstan:1.10.xfrom
staabm:magicclass
Nov 17, 2023
Merged

Implement MagicClassConstantRule#2741
ondrejmirtes merged 18 commits intophpstan:1.10.xfrom
staabm:magicclass

Conversation

@staabm
Copy link
Contributor

@staabm staabm commented Nov 15, 2023 •
edited
Loading

ondrejmirtes
ondrejmirtes requested changes Nov 15, 2023
View reviewed changes
conf/config.level0.neon Outdated
Copy link
Member

@ondrejmirtes ondrejmirtes Nov 15, 2023

Choose a reason for hiding this comment

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

This should be enabled only with bleeding edge. The code does not error, it just returns an empty string AFAIK.

Copy link
Contributor Author

@staabm staabm Nov 15, 2023

Choose a reason for hiding this comment

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

I have used a feature flag with a more general name, so I can use it for similar rules for __FUNCTION__ and __METHOD__

ondrejmirtes
ondrejmirtes requested changes Nov 15, 2023
View reviewed changes
src/Rules/Constants/MagicClassConstantRule.php Outdated
Copy link
Member

@ondrejmirtes ondrejmirtes Nov 15, 2023

Choose a reason for hiding this comment

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

The message should be different. It just returns an empty string.

@ondrejmirtes
Copy link
Member

ondrejmirtes commented Nov 15, 2023

Can you also please go through other magic constants and report them when used in a wrong way? Maybe it can all be done inside this single rule.

@clxmstaab clxmstaab force-pushed the magicclass branch 2 times, most recently from 5ef5af0 to 92c94d4 Compare November 15, 2023 15:22
@staabm
Copy link
Contributor Author

staabm commented Nov 15, 2023 •
edited
Loading

there is one interessting leftover case:

// test.php
function doFooParams(
	string $file = __FILE__,
	int $line = __LINE__,
	string $class = __CLASS__,
	string $dir = __DIR__,
	string $namespace = __NAMESPACE__,
	string $method = __METHOD__,
	string $function = __FUNCTION__,
	string $trait = __TRAIT__
): void
{
}

since used in the parameter-list and not the function body it false-positives with

https://3v4l.org/BbZWn

$ php bin/phpstan analyze test.php --debug
Note: Using configuration file C:\dvl\Workspace\phpstan-src\phpstan.neon.dist.
C:\dvl\Workspace\phpstan-src\test.php
 ------ -----------------------------------------------------------------------------
  Line   test.php
 ------ -----------------------------------------------------------------------------
  6      Magic constant __CLASS__ is always empty when used outside a class.
  8      Magic constant __NAMESPACE__ is always empty when used in global namespace.
  9      Magic constant __METHOD__ is always empty when used outside a function.
  10     Magic constant __FUNCTION__ is always empty when used outside a function.
 ------ -----------------------------------------------------------------------------

@staabm
Copy link
Contributor Author

staabm commented Nov 15, 2023

we could fix the above case by moving enterFunction before the processParamNode call in

phpstan-src/src/Analyser/NodeScopeResolver.php

Lines 451 to 459 in 231ed3e

foreach ($stmt->params as $param) {
$this->processParamNode($param, $scope, $nodeCallback);
}
if ($stmt->returnType !== null) {
$nodeCallback($stmt->returnType, $scope);
}
$functionScope = $scope->enterFunction(

wdyt?

staabm
staabm commented Nov 16, 2023
View reviewed changes
Copy link
Contributor Author

@staabm staabm left a comment

Choose a reason for hiding this comment

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

Instead of changing NodeScopeResolver I went with adding a dedicated new NodeVisitor to prevent the false positives

ondrejmirtes
ondrejmirtes requested changes Nov 17, 2023
View reviewed changes
src/Rules/Constants/MagicConstantContextRule.php Outdated
Copy link
Member

@ondrejmirtes ondrejmirtes Nov 17, 2023

Choose a reason for hiding this comment

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

Please deleted "when used" from all messages. They'll still make sense.

src/Rules/Constants/MagicConstantContextRule.php Outdated
Copy link
Member

@ondrejmirtes ondrejmirtes Nov 17, 2023

Choose a reason for hiding this comment

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

"outside a trait"

@clxmstaab clxmstaab force-pushed the magicclass branch 2 times, most recently from c1172f6 to 9191a31 Compare November 17, 2023 09:23
@staabm
Copy link
Contributor Author

staabm commented Nov 17, 2023 •
edited
Loading

(btw: interessting that issuebot did not realize that the snippet from issue phpstan/phpstan#10099 gets a new error with this PR - locally it works as expected though)

ondrejmirtes
ondrejmirtes requested changes Nov 17, 2023
View reviewed changes
src/Rules/Constants/MagicConstantContextRule.php Outdated
Copy link
Member

@ondrejmirtes ondrejmirtes Nov 17, 2023

Choose a reason for hiding this comment

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

This doesn't seem right. What about private $foo = __METHOD__;?

Also a false positive here - you can have an anonymous function outside a class/function and it's going to report '{closure}'.

@ondrejmirtes ondrejmirtes merged commit 918c47d into phpstan:1.10.x Nov 17, 2023
@ondrejmirtes ondrejmirtes mentioned this pull request Nov 17, 2023
Closed
@ondrejmirtes
Copy link
Member

ondrejmirtes commented Nov 17, 2023

Thank you.

@staabm staabm deleted the magicclass branch November 17, 2023 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ondrejmirtes ondrejmirtes ondrejmirtes requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@staabm @ondrejmirtes @clxmstaab