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 authored Nov 22, 2024
1 parent 0a4a5b8 commit 3be15cc
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 8 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 @@ -52,7 +52,21 @@ public boolean isIdentityComparableWith(InferredType other) {
if (other instanceof UnionType) {
return other.isIdentityComparableWith(this);
}
return this.equals(other);
return isComparingTypeWithMetaclass(other) || this.equals(other);
}

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

private static boolean hasMetaclassInHierarchy(ClassSymbol classSymbol) {
return classSymbol.hasMetaClass() || classSymbol.superClasses().stream().filter(ClassSymbol.class::isInstance).anyMatch(c -> hasMetaclassInHierarchy((ClassSymbol) c));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,32 @@ void isIdentityComparableWith() {
assertThat(aType.isIdentityComparableWith(new DeclaredType(b))).isTrue();
}

@Test
void isIdentityComparableWithMetaclass() {
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);

UnknownClassType unknownClassType = new UnknownClassType(metaclassSymbol);

assertThat(InferredTypes.TYPE.isIdentityComparableWith(InferredTypes.TYPE)).isTrue();
assertThat(InferredTypes.TYPE.isIdentityComparableWith(metaClassType)).isTrue();
assertThat(InferredTypes.TYPE.isIdentityComparableWith(superMetaClassType)).isTrue();
assertThat(InferredTypes.TYPE.isIdentityComparableWith(unknownClassType)).isFalse();

assertThat(metaClassType.isIdentityComparableWith(InferredTypes.TYPE)).isTrue();
assertThat(metaClassType.isIdentityComparableWith(metaClassType)).isTrue();
assertThat(metaClassType.isIdentityComparableWith(superMetaClassType)).isFalse();

assertThat(superMetaClassType.isIdentityComparableWith(InferredTypes.TYPE)).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 3be15cc

Please sign in to comment.