From 0a4a5b89f2bfc1f4a41f28df54217c085d7174c6 Mon Sep 17 00:00:00 2001 From: Guillaume Dequenne Date: Thu, 21 Nov 2024 11:03:14 +0100 Subject: [PATCH] SONARPY-1869 Ensure conservative heuristics for generics inheritance --- .../genericTypeWithoutArgumentImported.py | 9 +- .../genericTypeWithoutArgumentImporting.py | 16 +++- .../v2/types/TrivialTypeInferenceVisitor.java | 51 ++++++++++- .../python/index/ClassDescriptorTest.java | 2 +- .../semantic/v2/TypeInferenceV2Test.java | 88 +++++++++++++++++-- 5 files changed, 151 insertions(+), 15 deletions(-) diff --git a/python-checks/src/test/resources/checks/genericTypeWithoutArgumentImported.py b/python-checks/src/test/resources/checks/genericTypeWithoutArgumentImported.py index d410c0d4c2..1c21c5a051 100644 --- a/python-checks/src/test/resources/checks/genericTypeWithoutArgumentImported.py +++ b/python-checks/src/test/resources/checks/genericTypeWithoutArgumentImported.py @@ -1,4 +1,7 @@ -from typing import Generic +from typing import Generic, TypeVar + +T = TypeVar('T') + class SomeGeneric(Generic[T]): ... @@ -7,3 +10,7 @@ class SomeGenericWithTypeParam[T](): ... class MyImportedGenericTypeVarChild(SomeGeneric[T]): ... class MyImportedNonGenericChild(SomeGeneric): ... class MyImportedConcreteChild(SomeGeneric[str]): ... + +# U has not been defined +class SomeGenericIncorrectlyDefined(Generic[U]): + ... diff --git a/python-checks/src/test/resources/checks/genericTypeWithoutArgumentImporting.py b/python-checks/src/test/resources/checks/genericTypeWithoutArgumentImporting.py index c9c1745335..833eeada7c 100644 --- a/python-checks/src/test/resources/checks/genericTypeWithoutArgumentImporting.py +++ b/python-checks/src/test/resources/checks/genericTypeWithoutArgumentImporting.py @@ -1,5 +1,11 @@ -# mod.py -from genericTypeWithoutArgumentImported import SomeGeneric, SomeGenericWithTypeParam, MyImportedGenericTypeVarChild, MyImportedNonGenericChild, MyImportedConcreteChild +from genericTypeWithoutArgumentImported import ( + SomeGeneric, + SomeGenericWithTypeParam, + MyImportedGenericTypeVarChild, + MyImportedNonGenericChild, + MyImportedConcreteChild, + SomeGenericIncorrectlyDefined +) def local_generic(): from typing import Generic @@ -16,8 +22,7 @@ def bar() -> SomeGenericWithTypeParam: # Noncompliant def returning_imported_child() -> MyImportedGenericTypeVarChild: ... # Noncompliant def returning_imported_non_generic_child() -> MyImportedNonGenericChild: ... # OK -# FP SONARPY-2356: MyImportedConcreteChild is not actually generic (specialized class) -def returning_imported_concrete_child() -> MyImportedConcreteChild: ... # Noncompliant +def returning_imported_concrete_child() -> MyImportedConcreteChild: ... # OK class MyChild(SomeGeneric[T]): ... def returning_my_child() -> MyChild: # FN @@ -30,3 +35,6 @@ def returning_my_non_generic_child() -> MyNonGenericChild: # OK class MyConcreteChild(SomeGeneric[str]): ... def returning_my_concrete_chil3() -> MyConcreteChild: # OK ... + +def returning_incorrect_generic() -> SomeGenericIncorrectlyDefined: + ... diff --git a/python-frontend/src/main/java/org/sonar/python/semantic/v2/types/TrivialTypeInferenceVisitor.java b/python-frontend/src/main/java/org/sonar/python/semantic/v2/types/TrivialTypeInferenceVisitor.java index c81eb4502f..cbea40a724 100644 --- a/python-frontend/src/main/java/org/sonar/python/semantic/v2/types/TrivialTypeInferenceVisitor.java +++ b/python-frontend/src/main/java/org/sonar/python/semantic/v2/types/TrivialTypeInferenceVisitor.java @@ -37,9 +37,11 @@ import org.sonar.plugins.python.api.tree.AliasedName; import org.sonar.plugins.python.api.tree.AnnotatedAssignment; import org.sonar.plugins.python.api.tree.ArgList; +import org.sonar.plugins.python.api.tree.AssignmentExpression; import org.sonar.plugins.python.api.tree.AssignmentStatement; import org.sonar.plugins.python.api.tree.BaseTreeVisitor; import org.sonar.plugins.python.api.tree.BinaryExpression; +import org.sonar.plugins.python.api.tree.CallExpression; import org.sonar.plugins.python.api.tree.ClassDef; import org.sonar.plugins.python.api.tree.ComprehensionExpression; import org.sonar.plugins.python.api.tree.DictCompExpression; @@ -82,6 +84,7 @@ import org.sonar.python.tree.SetLiteralImpl; import org.sonar.python.tree.StringLiteralImpl; import org.sonar.python.tree.SubscriptionExpressionImpl; +import org.sonar.python.tree.TreeUtils; import org.sonar.python.tree.TupleImpl; import org.sonar.python.tree.UnaryExpressionImpl; import org.sonar.python.types.v2.ClassType; @@ -113,6 +116,7 @@ public class TrivialTypeInferenceVisitor extends BaseTreeVisitor { private final Deque typeStack = new ArrayDeque<>(); private final Set importedModulesFQN = new HashSet<>(); private final TypeChecker typeChecker; + private final List typeVarNames = new ArrayList<>(); private final Map wildcardImportedTypes = new HashMap<>(); @@ -320,12 +324,17 @@ private void addParentClass(ClassTypeBuilder classTypeBuilder, RegularArgument r } PythonType argumentType = getTypeV2FromArgument(regularArgument); classTypeBuilder.addSuperClass(argumentType); - if (typeChecker.typeCheckBuilder().isGeneric().check(argumentType) == TriBool.TRUE && regularArgument.expression().is(Tree.Kind.SUBSCRIPTION)) { - // SONARPY-2356: checking that we have a subscription only is too naive (e.g. specialized classes) + if (isParentAGenericClass(regularArgument, argumentType)) { classTypeBuilder.withIsGeneric(true); } } + private boolean isParentAGenericClass(RegularArgument regularArgument, PythonType argumentType) { + return typeChecker.typeCheckBuilder().isGeneric().check(argumentType) == TriBool.TRUE + && regularArgument.expression() instanceof SubscriptionExpression subscriptionExpression + && subscriptionExpression.subscripts().expressions().stream().anyMatch(expression -> expression instanceof Name name && typeVarNames.contains(name.name())); + } + private static PythonType getTypeV2FromArgument(RegularArgument regularArgument) { Expression expression = regularArgument.expression(); // Ensure we support correctly typing symbols like "List[str] / list[str]" @@ -503,6 +512,44 @@ public void visitAnnotatedAssignment(AnnotatedAssignment assignmentStatement) { }); } + @Override + public void visitCallExpression(CallExpression callExpression) { + super.visitCallExpression(callExpression); + assignPossibleTypeVar(callExpression); + } + + private void assignPossibleTypeVar(CallExpression callExpression) { + PythonType pythonType = callExpression.callee().typeV2(); + TriBool check = typeChecker.typeCheckBuilder().isTypeWithName("typing.TypeVar").check(pythonType); + if (check == TriBool.TRUE) { + Tree parent = TreeUtils.firstAncestor(callExpression, t -> t.is(Tree.Kind.ASSIGNMENT_STMT, Tree.Kind.ANNOTATED_ASSIGNMENT, Tree.Kind.ASSIGNMENT_EXPRESSION)); + Optional assignedName = Optional.empty(); + if (parent instanceof AssignmentStatement assignmentStatement) { + assignedName = extractAssignedName(assignmentStatement); + } + if (parent instanceof AnnotatedAssignment annotatedAssignment) { + assignedName = extractAssignedName(annotatedAssignment); + } + if (parent instanceof AssignmentExpression assignmentExpression) { + assignedName = extractAssignedName(assignmentExpression); + } + assignedName.ifPresent(name -> typeVarNames.add(name.name())); + } + } + + private static Optional extractAssignedName(AssignmentExpression assignmentExpression) { + return Optional.of(assignmentExpression.lhsName()).map(Name.class::cast); + } + + private static Optional extractAssignedName(AnnotatedAssignment annotatedAssignment) { + return Optional.of(annotatedAssignment.variable()).filter(v -> v.is(Tree.Kind.NAME)).map(Name.class::cast); + } + + private static Optional extractAssignedName(AssignmentStatement assignmentStatement) { + return assignmentStatement.lhsExpressions().stream().findFirst() + .map(ExpressionList::expressions).stream().flatMap(List::stream).findFirst().filter(v -> v.is(Tree.Kind.NAME)).map(Name.class::cast); + } + private static Optional getFirstAssignmentName(AssignmentStatement assignmentStatement) { return Optional.of(assignmentStatement) .map(AssignmentStatement::lhsExpressions) diff --git a/python-frontend/src/test/java/org/sonar/python/index/ClassDescriptorTest.java b/python-frontend/src/test/java/org/sonar/python/index/ClassDescriptorTest.java index 6590301c03..8bc4b10f42 100644 --- a/python-frontend/src/test/java/org/sonar/python/index/ClassDescriptorTest.java +++ b/python-frontend/src/test/java/org/sonar/python/index/ClassDescriptorTest.java @@ -109,7 +109,7 @@ void classDescriptorGenerics() { ClassDescriptor classDescriptor = lastClassDescriptor( "from typing import Generic", "class A(Generic[str]): ..."); - assertThat(classDescriptor.supportsGenerics()).isTrue(); + assertThat(classDescriptor.supportsGenerics()).isFalse(); assertDescriptorToProtobuf(classDescriptor); } diff --git a/python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java b/python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java index 0b82950be9..f649d5ba25 100644 --- a/python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java +++ b/python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java @@ -156,14 +156,15 @@ void builtinGenericType() { void userDefinedGenericType() { FileInput fileInput = inferTypes( """ - from typing import Generic + from typing import Generic, TypeVar + T = TypeVar('T') class MyClass(Generic[T]): ... x = MyClass[str]() x """ ); - PythonType classType = ((ClassDef) fileInput.statements().statements().get(1)).name().typeV2(); - ObjectType xType = (ObjectType) ((ExpressionStatement) fileInput.statements().statements().get(3)).expressions().get(0).typeV2(); + PythonType classType = ((ClassDef) fileInput.statements().statements().get(2)).name().typeV2(); + ObjectType xType = (ObjectType) ((ExpressionStatement) fileInput.statements().statements().get(4)).expressions().get(0).typeV2(); assertThat(xType.unwrappedType()).isEqualTo(classType); // SONARPY-2356: Instantiation of specialized classes assertThat(xType.attributes()).isEmpty(); @@ -171,6 +172,78 @@ class MyClass(Generic[T]): ... @Test void inheritedGenericType() { + FileInput fileInput = inferTypes( + """ + from typing import Generic, TypeVar + T = TypeVar('T') + class MyClass(Generic[T]): ... + class MyOtherClass(MyClass[T]): ... + x = MyOtherClass[str]() + x + """ + ); + ClassType myOtherClassType = (ClassType) ((ClassDef) fileInput.statements().statements().get(3)).name().typeV2(); + assertThat(myOtherClassType.isGeneric()).isTrue(); + PythonType xType = ((ExpressionStatement) fileInput.statements().statements().get(5)).expressions().get(0).typeV2(); + assertThat(xType.unwrappedType()).isEqualTo(myOtherClassType); + } + + @Test + void inheritedGenericTypeUnsupportedExpression() { + FileInput fileInput = inferTypes( + """ + from typing import Generic, TypeVar + T = TypeVar('T') + class MyClass(Generic[T()]): ... + class MyOtherClass(MyClass[T]): ... + x = MyOtherClass[str]() + x + """ + ); + ClassType myOtherClassType = (ClassType) ((ClassDef) fileInput.statements().statements().get(3)).name().typeV2(); + assertThat(myOtherClassType.isGeneric()).isFalse(); + PythonType xType = ((ExpressionStatement) fileInput.statements().statements().get(5)).expressions().get(0).typeV2(); + assertThat(xType.unwrappedType()).isInstanceOf(UnknownType.class); + } + + @Test + void inheritedGenericTypeVarAnnotatedAssignment() { + FileInput fileInput = inferTypes( + """ + from typing import Generic, TypeVar + T: TypeVar = TypeVar('T') + class MyClass(Generic[T]): ... + class MyOtherClass(MyClass[T]): ... + x = MyOtherClass[str]() + x + """ + ); + ClassType myOtherClassType = (ClassType) ((ClassDef) fileInput.statements().statements().get(3)).name().typeV2(); + assertThat(myOtherClassType.isGeneric()).isTrue(); + PythonType xType = ((ExpressionStatement) fileInput.statements().statements().get(5)).expressions().get(0).typeV2(); + assertThat(xType.unwrappedType()).isEqualTo(myOtherClassType); + } + + @Test + void inheritedGenericTypeVarAssignmentExpression() { + FileInput fileInput = inferTypes( + """ + from typing import Generic, TypeVar + foo(T:=TypeVar('T')) + class MyClass(Generic[T]): ... + class MyOtherClass(MyClass[T]): ... + x = MyOtherClass[str]() + x + """ + ); + ClassType myOtherClassType = (ClassType) ((ClassDef) fileInput.statements().statements().get(3)).name().typeV2(); + assertThat(myOtherClassType.isGeneric()).isTrue(); + PythonType xType = ((ExpressionStatement) fileInput.statements().statements().get(5)).expressions().get(0).typeV2(); + assertThat(xType.unwrappedType()).isEqualTo(myOtherClassType); + } + + @Test + void inheritedGenericTypeUndefinedTypeVar() { FileInput fileInput = inferTypes( """ from typing import Generic @@ -181,9 +254,10 @@ class MyOtherClass(MyClass[T]): ... """ ); ClassType myOtherClassType = (ClassType) ((ClassDef) fileInput.statements().statements().get(2)).name().typeV2(); - assertThat(myOtherClassType.isGeneric()).isTrue(); + // TypeVar is undefined: not a proper generic + assertThat(myOtherClassType.isGeneric()).isFalse(); PythonType xType = ((ExpressionStatement) fileInput.statements().statements().get(4)).expressions().get(0).typeV2(); - assertThat(xType.unwrappedType()).isEqualTo(myOtherClassType); + assertThat(xType.unwrappedType()).isInstanceOf(UnknownType.class); } @Test @@ -219,9 +293,9 @@ class MyOtherClass(MyClass[str]): ... ); ClassType myOtherClassType = (ClassType) ((ClassDef) fileInput.statements().statements().get(2)).name().typeV2(); // SONARPY-2356: MyOtherClass can no longer be considered generic (specialized version of MyClass) - assertThat(myOtherClassType.isGeneric()).isTrue(); + assertThat(myOtherClassType.isGeneric()).isFalse(); PythonType xType = ((ExpressionStatement) fileInput.statements().statements().get(4)).expressions().get(0).typeV2(); - assertThat(xType.unwrappedType()).isEqualTo(myOtherClassType); + assertThat(xType.unwrappedType()).isInstanceOf(UnknownType.class); } @Test