Skip to content

Deprecate using "_" as a class name #15360

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

Merged
merged 1 commit into from
Aug 12, 2024

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Aug 12, 2024

@Girgias Girgias merged commit 0a23b06 into php:master Aug 12, 2024
10 checks passed
@Girgias Girgias deleted the class-underscore-deprecate branch August 12, 2024 15:10
@jrfnl
Copy link
Contributor

jrfnl commented Aug 12, 2024

@Girgias Quick question as I can't test this yet what with 3v4l being behind: is this deprecation strictly about class names ? Or about all OO names, i.e. class, interface, trait and enum ?

I presume the latter, but would like to be sure when writing the sniff for PHPCompatibility.

If the latter - would it be prudent to have some tests with the other OO structures too ?

@Girgias
Copy link
Member Author

Girgias commented Aug 13, 2024

Yes, internally a class, interface, trait, and enum are all the same thing.

Added some tests: 5622def

@jrfnl
Copy link
Contributor

jrfnl commented Aug 13, 2024

Thanks for confirming Gina!

@SerafimArts
Copy link
Contributor

SerafimArts commented Nov 11, 2024

Sorry it took so long, I just saw the PR. It seems we forgot about constants (like const _ = new Placeholder()) and defines (like define('_', fopen(__FILE__, 'rb'))).

As I understand, constants were simply forgotten? Or was it done intentionally and constants will be deprecated in 8.5?

@iluuu1994
Copy link
Member

In terms of the originally stated use-case, i.e. pattern matching, there's no need for it. Identifiers in the global context of pattern matching always refer to class names rather than constants. TBH I don't know why this deprecation was added at all, given that 1. we have already decided to switch away to * exactly to prevent it, and 2. we have realized that a wildcard may not be necessary at all, given PHP has a mixed type. Anyway, hopefully the impact is minimal.

@SerafimArts
Copy link
Contributor

Identifiers in the global context of pattern matching always refer to class names rather than constants

What about:

const _ = Some::class;
// or
define('_', Some::class);

I admit that this will also work now, given the code in the PR. But I haven't checked.

class_alias(Some::class, '_');

@iluuu1994
Copy link
Member

As mentioned, consts are not relevant. class_alias() seems to correctly warn. https://3v4l.org/QEcZs/rfc#vgit.master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants