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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion platforms/core-configuration/model-core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ description = "Implementation of configuration model types and annotation metada
errorprone {
disabledChecks.addAll(
"AnnotateFormatMethod", // 1 occurrence, needs errorprone annotations
"GetClassOnEnum", // 4 occurrences
"ImmutableEnumChecker", // 1 occurrences
"ReferenceEquality", // 3 occurrences
"UndefinedEquals", // 2 occurrences
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@
import org.gradle.internal.snapshot.ValueSnapshot;
import org.gradle.internal.snapshot.ValueSnapshotter;

import javax.annotation.Nullable;

public class EnumValueSnapshot implements ValueSnapshot {
private final String className;
private final String name;

public EnumValueSnapshot(Enum<?> value) {
// Don't retain the value, to allow ClassLoader to be collected
this.className = value.getClass().getName();
this.className = value.getDeclaringClass().getName();
this.name = value.name();
}

Expand All @@ -44,19 +46,17 @@ public String getName() {
}

@Override
public ValueSnapshot snapshot(Object value, ValueSnapshotter snapshotter) {
public ValueSnapshot snapshot(@Nullable Object value, ValueSnapshotter snapshotter) {
if (isEqualEnum(value)) {
return this;
}
return snapshotter.snapshot(value);
}

private boolean isEqualEnum(Object value) {
if (value instanceof Enum) {
private boolean isEqualEnum(@Nullable Object value) {
if (value instanceof Enum<?>) {
Enum<?> enumValue = (Enum<?>) value;
if (enumValue.name().equals(name) && enumValue.getClass().getName().equals(className)) {
return true;
}
return enumValue.name().equals(name) && enumValue.getDeclaringClass().getName().equals(className);
}
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
/**
* Isolates an Enum value and is a snapshot for that value.
*/
public class IsolatedEnumValueSnapshot extends EnumValueSnapshot implements Isolatable<Enum> {
public class IsolatedEnumValueSnapshot extends EnumValueSnapshot implements Isolatable<Enum<?>> {
private final Enum<?> value;

public IsolatedEnumValueSnapshot(Enum<?> value) {
Expand All @@ -43,17 +43,17 @@ public ValueSnapshot asSnapshot() {
}

@Override
public Enum isolate() {
public Enum<?> isolate() {
return value;
}

@Nullable
@Override
public <S> S coerce(Class<S> type) {
if (type.isAssignableFrom(value.getClass())) {
if (type.isInstance(value)) {
return type.cast(value);
}
if (type.isEnum() && type.getName().equals(value.getClass().getName())) {
if (type.isEnum() && type.getName().equals(value.getDeclaringClass().getName())) {
return type.cast(Enum.valueOf(Cast.uncheckedNonnullCast(type.asSubclass(Enum.class)), value.name()));
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@
package org.gradle.internal.snapshot.impl

enum Type1 {
ONE, TWO
ONE {}, TWO {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ All of them match the consumer attributes:
com {
acme {
'Flavor.groovy'('package com.acme; enum Flavor { free, paid }')
'BuildType.groovy'('package com.acme; enum BuildType { debug, release }')
'BuildType.groovy'('package com.acme; enum BuildType { debug {}, release {} }')
'TypedAttributesPlugin.groovy'('''package com.acme

import org.gradle.api.Plugin
Expand Down