Skip to content

[Config] Fix support for attributes on class constants and enum cases #61079

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
Jul 16, 2025

Conversation

ruudk
Copy link
Contributor

@ruudk ruudk commented Jul 9, 2025

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues
License MIT

I'm using ReflectionClassResource in a project to track changes of files. I noticed that attributes on enum cases were not tracked properly. The cases were visible, because the enum cases are available as class constants.

But the attributes were missing.

This solves the problem.

Not sure if we should see this as a bug or feature. I think it's a bug.

@carsonbot carsonbot added this to the 7.4 milestone Jul 9, 2025
@ruudk ruudk force-pushed the ReflectionClassResource-enum branch 2 times, most recently from 230a888 to b3f6c68 Compare July 9, 2025 12:03
@OskarStark OskarStark changed the title [Config] Add support for enums in ReflectionClassResource [Config] Add support for enums in ReflectionClassResource Jul 9, 2025
Copy link
Member

@GromNaN GromNaN left a comment

Choose a reason for hiding this comment

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

I am curious about your use case for enums with configuration.

@ruudk
Copy link
Contributor Author

ruudk commented Jul 9, 2025

I am curious about your use case for enums with configuration.

I have a compiler that creates PHP code based on some classes and enums I declare in my application. I use the resources to quickly check if there are changes and if I should recompile.

To be specific: it's creating Webonyx GraphQL type files, which are annoying to create by hand (arrays etc). Therefore we have our own abstraction with attributes.

@ruudk
Copy link
Contributor Author

ruudk commented Jul 9, 2025

@GromNaN that is a valid use case for these classes, right?

@nicolas-grekas
Copy link
Member

Works for me as a bugfix on 6.4

The cases were visible, because the enum cases are available as class constants.

Doesn't this mean that the more generic fix is to fix support for attributes on class constants?

@nicolas-grekas nicolas-grekas modified the milestones: 7.4, 6.4 Jul 15, 2025
@ruudk
Copy link
Contributor Author

ruudk commented Jul 15, 2025

Works for me as a bugfix on 6.4

Thanks. I'll rebase this PR later.

Doesn't this mean that the more generic fix is to fix support for attributes on class constants?

Sure, but that's a PHP 8.5 feature: https://wiki.php.net/rfc/attributes-on-constants

Maybe we should also add support for that already?

@nicolas-grekas
Copy link
Member

That RFC is for constants. Here we need class-constants, which do support attributes in PHP 8.1:
https://www.php.net/manual/fr/reflectionclassconstant.getattributes.php

@ruudk ruudk changed the title [Config] Add support for enums in ReflectionClassResource [Config] Add support for class constants and enums in ReflectionClassResource Jul 16, 2025
@ruudk ruudk changed the base branch from 7.4 to 6.4 July 16, 2025 12:32
@ruudk ruudk force-pushed the ReflectionClassResource-enum branch 2 times, most recently from 1134b52 to c8844f4 Compare July 16, 2025 12:35
@ruudk
Copy link
Contributor Author

ruudk commented Jul 16, 2025

@nicolas-grekas Implemented it for class constants. I think this solution is much better :) Thanks!

@ruudk ruudk requested review from OskarStark and GromNaN July 16, 2025 12:43
@ruudk ruudk force-pushed the ReflectionClassResource-enum branch from c8844f4 to e39e189 Compare July 16, 2025 12:52
@nicolas-grekas nicolas-grekas changed the title [Config] Add support for class constants and enums in ReflectionClassResource [Config] Add support for attributes on class constants and enum cases Jul 16, 2025
@nicolas-grekas nicolas-grekas changed the title [Config] Add support for attributes on class constants and enum cases [Config] Fix support for attributes on class constants and enum cases Jul 16, 2025
@ruudk ruudk force-pushed the ReflectionClassResource-enum branch from 440bf0a to 3de864d Compare July 16, 2025 13:04
@nicolas-grekas nicolas-grekas force-pushed the ReflectionClassResource-enum branch from 1cc544b to 51b2c78 Compare July 16, 2025 13:23
@nicolas-grekas
Copy link
Member

Thank you @ruudk.

@nicolas-grekas nicolas-grekas merged commit 1ce1a15 into symfony:6.4 Jul 16, 2025
11 checks passed
This was referenced Jul 31, 2025
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