Skip to content

Don't crash on constants that reference themselves#2984

Merged
dantleech merged 2 commits intophpactor:masterfrom
mamazu:self-referencing-const
Dec 20, 2025
Merged

Don't crash on constants that reference themselves#2984
dantleech merged 2 commits intophpactor:masterfrom
mamazu:self-referencing-const

Conversation

@mamazu
Copy link
Contributor

@mamazu mamazu commented Dec 18, 2025

Fixes #2982

Implementation details

This is currently only solved for the case of a class constant. Maybe we could unifiy this with how variables work in phpactor because $a = $a doesn't crash.
If a constant assigns itself, then we assume mixed type (any guess is fine).

Remaining problems

The code doesn't work, maybe we should also add a diagnostic that disallows this kind of code. Even if you try to declare a const in a subclass of the abstract class, php crashes with "can't override a constant"

You can still run the NodeContextResolver into an infinite loop, maybe we could have some kind of protection against that.

@mamazu mamazu force-pushed the self-referencing-const branch 2 times, most recently from 5841394 to 92e8af7 Compare December 18, 2025 19:06
@mamazu mamazu force-pushed the self-referencing-const branch from 92e8af7 to 92fa96e Compare December 18, 2025 19:11
assertion: function (Diagnostics $diagnostics): void {
Assert::assertCount(0, $diagnostics);
}
);
Copy link
Collaborator

@dantleech dantleech Dec 18, 2025

Choose a reason for hiding this comment

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

why is this here - i.e. there is no diagnostic for this (and I don't think it makes sense to have one)? if fr testing can it be a file in lib/WorseReflection/Tests/Inference/reflection/self-referencing-cnstant.test instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know where to put it, the class that caused the infinite loop in the example was the diagnostic provider. But yeah just checking for any reflection errors should be enough.

@dantleech dantleech merged commit 56f98e4 into phpactor:master Dec 20, 2025
11 checks passed
@dantleech
Copy link
Collaborator

ta

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.

Crash on self-referential 'abstract' declaration

2 participants