Skip to content

Commit

Permalink
SONARPY-2332 Fix FP on S5845 when comparing enum objects with types
Browse files Browse the repository at this point in the history
  • Loading branch information
thomas-serre-sonarsource committed Nov 22, 2024
1 parent 2e55165 commit aab7085
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,8 @@ class ComparingTypeAndEnum(unittest.TestCase):
def test_type_of_enum_and_enum_class(self):
EnumType = Enum("EnumType", names=["one", "two"])
enum_member = EnumType(1)
# FP SONARPY-2332: EnumType is a proper "type" object since the Enum constructor with argument creates a new Enum type
self.assertIs(type(enum_member), EnumType) # Noncompliant
# EnumType is a proper "type" object since the Enum constructor with argument creates a new Enum type
self.assertIs(type(enum_member), EnumType)

class DerivedEnumWithMembers(Enum):
ONE = 1
Expand All @@ -208,7 +208,7 @@ class DerivedEnumWithMembers(Enum):
class ComparingTypeAndDerivedEnumWithMembers(unittest.TestCase):
def test_type_of_derived_enum_and_derived_enum_class(self):
derived_enum_member = DerivedEnumWithMembers(1)
self.assertIs(type(enum_member), EnumType)
self.assertIs(type(derived_enum_member), EnumType)

class DerivedEnumWithoutMembers(Enum):
...
Expand All @@ -218,12 +218,12 @@ class ComparingTypeAndDerivedEnumWithoutMembers(unittest.TestCase):
def test_type_of_derived_enum_and_derived_enum_class(self):
DerivedEnumType = DerivedEnumWithoutMembers("DerivedEnumType", names=["one", "two"])
derived_enum_member = DerivedEnumType(1)
# FP SONARPY-2332: DerivedEnumType is a proper "type" object because `DerivedEnumWithoutMembers(str, list)` will create a new enum,
# DerivedEnumType is a proper "type" object because `DerivedEnumWithoutMembers(str, list)` will create a new enum,
# just like `Enum(str, list)` would
self.assertIs(type(derived_enum_member), DerivedEnumType) # Noncompliant
self.assertIs(type(derived_enum_member), DerivedEnumType)

DerivedEnumTypeFromDict = DerivedEnumWithoutMembers("DerivedEnumType", OrderedDict([("one", 1), ("two", 2)]))
derived_enum_member_from_dict = DerivedEnumTypeFromDict(1)
# FP SONARPY-2332: DerivedEnumTypeFromDict is a proper "type" object because `DerivedEnumWithoutMembers(str, list)` will create a new enum,
# DerivedEnumTypeFromDict is a proper "type" object because `DerivedEnumWithoutMembers(str, list)` will create a new enum,
# just like `Enum(str, list)`
self.assertIs(type(derived_enum_member_from_dict), DerivedEnumTypeFromDict) # Noncompliant
self.assertIs(type(derived_enum_member_from_dict), DerivedEnumTypeFromDict)
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@

public class RuntimeType implements InferredType {

// Name of the type returned by the type() function
private static String TYPE_CLASS_NAME = "type";

private ClassSymbol typeClass;
private String builtinFullyQualifiedName;
private Set<String> typeClassSuperClassesFQN = null;
Expand All @@ -52,9 +55,27 @@ public boolean isIdentityComparableWith(InferredType other) {
if (other instanceof UnionType) {
return other.isIdentityComparableWith(this);
}
if (isComparingTypeWithMetaclass(other)) {
return true;
}
return this.equals(other);
}

private boolean isComparingTypeWithMetaclass(InferredType other) {
if (other instanceof RuntimeType otherRuntimeType) {
boolean hasOtherMetaClass = hasMetaclassInHierarchy(otherRuntimeType);
boolean hasThisMetaClass = hasMetaclassInHierarchy(this);
return (TYPE_CLASS_NAME.equals(getTypeClass().name()) && hasOtherMetaClass)
|| (hasThisMetaClass && TYPE_CLASS_NAME.equals(otherRuntimeType.getTypeClass().name()));
}
return false;
}

private static boolean hasMetaclassInHierarchy(RuntimeType runtimeType) {
return runtimeType.getTypeClass().hasMetaClass()
|| runtimeType.getTypeClass().superClasses().stream().filter(c -> c instanceof ClassSymbol).anyMatch(c -> ((ClassSymbol) c).hasMetaClass());
}

@Override
public boolean canHaveMember(String memberName) {
if (MOCK_FQNS.stream().anyMatch(this::mustBeOrExtend)){
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,31 @@ void isIdentityComparableWith() {
assertThat(aType.isIdentityComparableWith(new DeclaredType(b))).isTrue();
}

@Test
void isIdentityComparableWithMetaclass() {
RuntimeType typeType = new RuntimeType(new ClassSymbolImpl("type", "type"));

ClassSymbolImpl metaclassSymbol = new ClassSymbolImpl("Meta", "Meta");
metaclassSymbol.setHasMetaClass();
RuntimeType metaClassType = new RuntimeType(metaclassSymbol);

ClassSymbolImpl classSymbolWithSuperMetaClass = new ClassSymbolImpl("SuperMeta", "SuperMeta");
classSymbolWithSuperMetaClass.addSuperClass(metaclassSymbol);
RuntimeType superMetaClassType = new RuntimeType(classSymbolWithSuperMetaClass);

assertThat(typeType.isIdentityComparableWith(typeType)).isTrue();
assertThat(typeType.isIdentityComparableWith(metaClassType)).isTrue();
assertThat(typeType.isIdentityComparableWith(superMetaClassType)).isTrue();

assertThat(metaClassType.isIdentityComparableWith(typeType)).isTrue();
assertThat(metaClassType.isIdentityComparableWith(metaClassType)).isTrue();
assertThat(metaClassType.isIdentityComparableWith(superMetaClassType)).isFalse();

assertThat(superMetaClassType.isIdentityComparableWith(typeType)).isTrue();
assertThat(superMetaClassType.isIdentityComparableWith(metaClassType)).isFalse();
assertThat(superMetaClassType.isIdentityComparableWith(superMetaClassType)).isTrue();
}

@Test
void member() {
ClassSymbolImpl x = new ClassSymbolImpl("x", "x");
Expand Down

0 comments on commit aab7085

Please sign in to comment.