Skip to content
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.

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

Suppress MissingClassConstType if the class is final #10951

Closed
ThomasLandauer opened this issue May 4, 2024 · 1 comment · Fixed by #10953
Closed

Suppress MissingClassConstType if the class is final #10951

ThomasLandauer opened this issue May 4, 2024 · 1 comment · Fixed by #10953

Comments

@ThomasLandauer
Copy link
Contributor

MissingClassConstType was added in #10828 by @jack-worman

The RFC kinda mentions that typed constants are only useful with regard to child classes - see https://wiki.php.net/rfc/typed_class_constants

By default, child classes can override class constants of their parents, so sometimes it is not easy to assume what the value and the type of a class constant really is, unless either their defining class or the constant itself is final

So when a class is final (=best practice for most classes today), there's no need to to type something like const foo = 5;, since it's obvious that it's an int.

So I'm suggesting to restrict MissingClassConstType to cases where neither the class nor the const itself are final.

Copy link

Hey @ThomasLandauer, can you reproduce the issue on https://psalm.dev? These will be used as phpunit tests when implementing the feature or fixing this bug.

jack-worman added a commit to jack-worman/psalm that referenced this issue May 4, 2024
jack-worman added a commit to jack-worman/psalm that referenced this issue May 4, 2024
jack-worman added a commit to jack-worman/psalm that referenced this issue May 4, 2024
jack-worman added a commit to jack-worman/psalm that referenced this issue May 4, 2024
@weirdan weirdan closed this as completed May 5, 2024
@weirdan weirdan linked a pull request May 5, 2024 that will close this issue
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 a pull request may close this issue.

2 participants