Improve class constant static analysis #7154
Conversation
| } elseif ($stmt instanceof PhpParser\Node\Stmt\ClassConst) { | ||
| $member_stmts[] = $stmt; | ||
|
|
||
| foreach ($stmt->consts as $const) { | ||
| $const_id = strtolower($this->fq_class_name) . '::' . $const->name; | ||
|
|
||
| foreach ($codebase->class_constants_to_rename as $original_const_id => $new_const_name) { | ||
| if ($const_id === $original_const_id) { | ||
| $file_manipulations = [ | ||
| new FileManipulation( | ||
| (int) $const->name->getAttribute('startFilePos'), | ||
| (int) $const->name->getAttribute('endFilePos') + 1, | ||
| $new_const_name | ||
| ) | ||
| ]; | ||
|
|
||
| FileManipulationBuffer::add( | ||
| $this->getFilePath(), | ||
| $file_manipulations | ||
| ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Copied this part from ClassAnalyzer. Haven't tested it, but I assume it's probably fine.
AndrolGenhald
force-pushed
the
feature/class-const-improvements
branch
from
71f5b09 to
08e5315
Compare
|
@orklah Can you rerun CI for this now that the int range issue is fixed? I'm not sure if I'm able to do that myself without pushing something. |
|
Done, except for circleCI, I have no dominion there 😄 |
|
Oh, I didn't realize I still had things to fix, I thought I'd handled that already. |
|
I was gonna post the same thing as your edit :p we're both idiots then 😄 |
Add class const covariance support (fixes vimeo#5589). Add check for overriding const from interface in PHP < 8.1 (fixes vimeo#7108). Add check for ambiguous const inheritance.
AndrolGenhald
force-pushed
the
feature/class-const-improvements
branch
from
08e5315 to
0b81f46
Compare
|
I'm not quite sure, but I think there is something related to the order of the tests The failing test expects this exception and message: psalm/tests/Config/ConfigTest.php Line 1534 in 558208e most of the time it works, but sometimes, there is a message coming from here: psalm/src/Psalm/Internal/ErrorHandler.php Line 70 in b924032 Normally, the error handler is installed here: psalm/tests/Config/ConfigTest.php Line 1536 in 558208e but there's also one here: psalm/tests/Config/ConfigTest.php Line 1555 in 558208e What I don't get is that the exception I would expect most of the time is actually RuntimeException, not ConfigException |
|
Regardless, this should be ready for review and I think everything works correctly. Hopefully it's a bit easier to understand than some of the others 😆 |
orklah
added
PR: Need review
release:feature
| public const CONSTANT='bar'; | ||
| } | ||
|
|
||
| interface Baz extends Foo, Bar {} |
There was a problem hiding this comment.
Is this an issue when the type are compatibles?
There was a problem hiding this comment.
It's an issue even when it's the same exact thing: https://3v4l.org/tjYML
There was a problem hiding this comment.
oh, I thought "ambiguous" was too tame a word for something that broke at compile time, I was wrong!
|
Thanks! |
Add class const covariance support (fixes #5589).
Add check for overriding const from interface in PHP < 8.1 (fixes #7108).
Add check for ambiguous const inheritance.