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

Fix getClass on enum classes #28985

Merged
merged 2 commits into from May 8, 2024
Merged

Fix getClass on enum classes #28985

merged 2 commits into from May 8, 2024

Conversation

mlopatkin
Copy link
Member

Enum constants may be implemented as subclasses, so getClass may be
unreliable, e.g. a subclass returns false from isEnum. This commit
fixes a bunch of places where the invalid class was exposed.

Reviewing cheatsheet

Before merging the PR, comments starting with

  • ❌ ❓must be fixed
  • 🤔 💅 should be fixed
  • 💭 may be fixed
  • 🎉 celebrate happy things

@mlopatkin mlopatkin requested review from a team as code owners April 26, 2024 18:34
@mlopatkin mlopatkin requested review from bamboo and abstratt and removed request for a team April 26, 2024 18:34
@mlopatkin
Copy link
Member Author

@bot-gradle test this

@bot-gradle
Copy link
Collaborator

I've triggered the following builds for you. Click here to see all build failures.

@mlopatkin mlopatkin added this to the 8.9 RC1 milestone Apr 26, 2024
@mlopatkin mlopatkin added the a:chore Minor issue without significant impact label Apr 26, 2024
Copy link
Member

@bamboo bamboo left a comment

Choose a reason for hiding this comment

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

Nice!

@@ -367,6 +367,6 @@ class IsolatableSerializerRegistryTest extends Specification {
}

enum EnumType {
FOO, BAR
FOO {}, BAR
Copy link
Member

Choose a reason for hiding this comment

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

❓ Is that enough to force a subtype? Perhaps adding a toString override might make the intention clearer

@bamboo bamboo added this pull request to the merge queue May 8, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch May 8, 2024
Enum constants may be implemented as subclasses, so getClass may be
unreliable, e.g. a subclass returns `false` from `isEnum`. This commit
fixes a bunch of places where the invalid class was exposed.
@bamboo
Copy link
Member

bamboo commented May 8, 2024

@bot-gradle merge

@bot-gradle bot-gradle added this pull request to the merge queue May 8, 2024
Merged via the queue into master with commit 2a768fa May 8, 2024
23 checks passed
@bot-gradle bot-gradle deleted the ml/fix-getclass-on-enum branch May 8, 2024 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 min review a:chore Minor issue without significant impact build-script-change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants