Skip to content

Commit 9de54d6

Browse files
Seppli11sonartech
authored andcommitted
SONARPY-3752 Serialize attributes of FunctionType params and return type (#838)
GitOrigin-RevId: e0334ce302d9cf2606c5044d6cac076290d1c45a
1 parent 20d1e5a commit 9de54d6

14 files changed

Lines changed: 367 additions & 84 deletions

File tree

python-checks/src/test/resources/checks/nonCallableCalled.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,3 +411,12 @@ def object_typevar():
411411
def uncallable_inside_lambda_are_non_compliant():
412412
lambda_call = (lambda : print("something"))() # OK
413413
uncallable_inside_lambda = lambda:(print("something"))() # Noncompliant
414+
415+
416+
def fp_loop_of_class_list():
417+
class MyClass: ...
418+
419+
list_of_classes = [MyClass]
420+
421+
for clazz in list_of_classes:
422+
clazz() # Noncompliant

python-frontend/src/main/java/org/sonar/plugins/python/api/types/v2/ObjectType.java

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,11 @@
2727
@Beta
2828
public final class ObjectType implements PythonType {
2929
private final TypeWrapper typeWrapper;
30-
private final List<PythonType> attributes;
30+
private final List<TypeWrapper> attributes;
3131
private final List<Member> members;
3232
private final TypeSource typeSource;
3333

34-
private ObjectType(TypeWrapper typeWrapper, List<PythonType> attributes, List<Member> members, TypeSource typeSource) {
34+
private ObjectType(TypeWrapper typeWrapper, List<TypeWrapper> attributes, List<Member> members, TypeSource typeSource) {
3535
this.typeWrapper = typeWrapper;
3636
this.attributes = attributes;
3737
this.members = members;
@@ -86,16 +86,13 @@ public boolean equals(Object o) {
8686
ObjectType that = (ObjectType) o;
8787
List<String> membersNames = members.stream().map(Member::name).toList();
8888
List<String> otherMembersNames = that.members.stream().map(Member::name).toList();
89-
List<String> attributesNames = attributes.stream().map(PythonType::key).toList();
90-
List<String> otherAttributesNames = that.attributes.stream().map(PythonType::key).toList();
91-
return Objects.equals(typeWrapper, that.typeWrapper) && Objects.equals(membersNames, otherMembersNames) && Objects.equals(attributesNames, otherAttributesNames);
89+
return Objects.equals(typeWrapper, that.typeWrapper) && Objects.equals(membersNames, otherMembersNames) && Objects.equals(attributes, that.attributes);
9290
}
9391

9492
@Override
9593
public int hashCode() {
9694
List<String> membersNames = members.stream().map(Member::name).toList();
97-
List<String> attributesNames = attributes.stream().map(PythonType::key).toList();
98-
return Objects.hash(typeWrapper, attributesNames, membersNames);
95+
return Objects.hash(typeWrapper, attributes, membersNames);
9996
}
10097

10198
public PythonType type() {
@@ -107,7 +104,7 @@ public TypeWrapper typeWrapper() {
107104
}
108105

109106
public List<PythonType> attributes() {
110-
return attributes;
107+
return attributes.stream().map(TypeWrapper::type).toList();
111108
}
112109

113110
public List<Member> members() {
@@ -130,7 +127,7 @@ public String toString() {
130127

131128
public static class Builder implements TypeBuilder<ObjectType> {
132129
private TypeWrapper typeWrapper;
133-
private List<PythonType> attributes = List.of();
130+
private List<TypeWrapper> attributes = List.of();
134131
private List<Member> members = List.of();
135132
private TypeSource typeSource = TypeSource.EXACT;
136133

@@ -145,7 +142,7 @@ public static Builder fromTypeWrapper(TypeWrapper typeWrapper) {
145142
public static Builder fromType(PythonType type) {
146143
if (type instanceof ObjectType objectType) {
147144
return new Builder(objectType.typeWrapper())
148-
.withAttributes(objectType.attributes())
145+
.withTypeWrapperAttributes(objectType.attributes)
149146
.withMembers(objectType.members())
150147
.withTypeSource(objectType.typeSource());
151148
}
@@ -168,6 +165,11 @@ public Builder withType(PythonType type) {
168165
}
169166

170167
public Builder withAttributes(List<PythonType> attributes) {
168+
this.attributes = attributes.stream().map(TypeWrapper::of).toList();
169+
return this;
170+
}
171+
172+
public Builder withTypeWrapperAttributes(List<TypeWrapper> attributes) {
171173
this.attributes = attributes;
172174
return this;
173175
}

python-frontend/src/main/java/org/sonar/python/index/DescriptorsToProtobuf.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@
2121
import java.util.Collection;
2222
import java.util.HashSet;
2323
import java.util.List;
24+
import java.util.Objects;
2425
import java.util.Set;
2526
import javax.annotation.CheckForNull;
26-
2727
import org.slf4j.Logger;
2828
import org.slf4j.LoggerFactory;
2929
import org.sonar.plugins.python.api.LocationInFile;
@@ -316,8 +316,11 @@ private static TypeAnnotationDescriptor fromProtobuf(SymbolsProtos.Type typeProt
316316
if (kind == null) {
317317
return null;
318318
}
319-
List<TypeAnnotationDescriptor> args = new ArrayList<>();
320-
typeProto.getArgsList().forEach(proto -> args.add(fromProtobuf(proto)));
319+
List<TypeAnnotationDescriptor> args = typeProto.getArgsList().stream()
320+
.map(DescriptorsToProtobuf::fromProtobuf)
321+
.filter(Objects::nonNull)
322+
.toList();
323+
321324
return new TypeAnnotationDescriptor(
322325
typeProto.getPrettyPrintedName(),
323326
kind,

python-frontend/src/main/java/org/sonar/python/semantic/v2/converter/PythonTypeToDescriptorConverter.java

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,8 @@ private static Descriptor convert(FunctionType type) {
139139
.toList();
140140

141141
var returnType = type.returnType();
142-
var unwrappedReturnType = returnType.unwrappedType();
143-
var isSelfReturnType = containsSelfType(returnType);
144-
var annotatedReturnTypeName = FullyQualifiedNameHelper.getFullyQualifiedName(unwrappedReturnType).orElse(null);
145-
var returnTypeAnnotationDescriptor = createTypeAnnotationDescriptor(unwrappedReturnType, isSelfReturnType);
142+
var annotatedReturnTypeName = FullyQualifiedNameHelper.getFullyQualifiedName(returnType.unwrappedType()).orElse(null);
143+
var returnTypeAnnotationDescriptor = createTypeAnnotationDescriptor(returnType);
146144

147145
// Using FunctionType#name and FunctionType#fullyQualifiedName instead of symbol is only accurate if the function has not been reassigned
148146
// This logic should be revisited when tackling SONARPY-2285
@@ -158,10 +156,6 @@ private static Descriptor convert(FunctionType type) {
158156
returnTypeAnnotationDescriptor);
159157
}
160158

161-
private static boolean containsSelfType(PythonType returnType){
162-
return returnType instanceof SelfType || returnType.unwrappedType() instanceof SelfType;
163-
}
164-
165159
@VisibleForTesting
166160
static Descriptor convert(String moduleFqn, String parentFqn, String symbolName, SelfType selfType) {
167161
var innerType = selfType.innerType();
@@ -238,10 +232,9 @@ private static Descriptor convert(String parentFqn, String symbolName, UnknownTy
238232

239233
private static FunctionDescriptor.Parameter convert(ParameterV2 parameter) {
240234
var type = parameter.declaredType().type();
241-
var isSelf = containsSelfType(type);
242235
var unwrappedType = type.unwrappedType();
243236
var annotatedType = FullyQualifiedNameHelper.getFullyQualifiedName(unwrappedType).orElse(null);
244-
var typeAnnotationDescriptor = createTypeAnnotationDescriptor(unwrappedType, isSelf);
237+
var typeAnnotationDescriptor = createTypeAnnotationDescriptor(type);
245238

246239
return new FunctionDescriptor.Parameter(parameter.name(),
247240
annotatedType,
@@ -263,17 +256,29 @@ private static boolean usagesContainAssignment(List<UsageV2> symbolUsages) {
263256
}
264257

265258
@CheckForNull
266-
private static TypeAnnotationDescriptor createTypeAnnotationDescriptor(PythonType type, boolean isSelf) {
267-
if (type instanceof SelfType selfType) {
268-
return createTypeAnnotationDescriptor(selfType.innerType(), isSelf);
259+
private static TypeAnnotationDescriptor createTypeAnnotationDescriptor(PythonType type) {
260+
return createTypeAnnotationDescriptor(type, false, List.of());
261+
}
262+
263+
@CheckForNull
264+
private static TypeAnnotationDescriptor createTypeAnnotationDescriptor(PythonType type, boolean isSelf, List<TypeAnnotationDescriptor> args) {
265+
if (type instanceof ObjectType objectType) {
266+
List<TypeAnnotationDescriptor> objArgs = objectType.attributes().stream()
267+
.map(PythonTypeToDescriptorConverter::createTypeAnnotationDescriptor)
268+
.filter(Objects::nonNull)
269+
.toList();
270+
return createTypeAnnotationDescriptor(objectType.unwrappedType(), isSelf, objArgs);
271+
} else if (type instanceof SelfType selfType) {
272+
return createTypeAnnotationDescriptor(selfType.innerType(), true, args);
269273
} else if (type instanceof ClassType classType) {
270-
return new TypeAnnotationDescriptor(classType.name(), TypeAnnotationDescriptor.TypeKind.INSTANCE, List.of(), classType.fullyQualifiedName(), isSelf);
274+
return new TypeAnnotationDescriptor(classType.name(), TypeAnnotationDescriptor.TypeKind.INSTANCE, args, classType.fullyQualifiedName(), isSelf);
271275
} else if (type instanceof FunctionType functionType) {
272-
return new TypeAnnotationDescriptor(functionType.name(), TypeAnnotationDescriptor.TypeKind.CALLABLE, List.of(), functionType.fullyQualifiedName(), false);
276+
return new TypeAnnotationDescriptor(functionType.name(), TypeAnnotationDescriptor.TypeKind.CALLABLE, args, functionType.fullyQualifiedName(), false);
273277
} else if (type instanceof UnknownType.UnresolvedImportType importType) {
274-
return new TypeAnnotationDescriptor(importType.importPath(), TypeAnnotationDescriptor.TypeKind.INSTANCE, List.of(),
275-
FullyQualifiedNameHelper.getFullyQualifiedName(importType).orElse(null), false);
278+
return new TypeAnnotationDescriptor(importType.importPath(), TypeAnnotationDescriptor.TypeKind.INSTANCE, args,
279+
FullyQualifiedNameHelper.getFullyQualifiedName(importType).orElse(null), false);
276280
}
277281
return null;
278282
}
283+
279284
}

python-frontend/src/main/java/org/sonar/python/semantic/v2/converter/TypeAnnotationToPythonTypeConverter.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,12 @@
1616
*/
1717
package org.sonar.python.semantic.v2.converter;
1818

19+
import java.util.List;
1920
import java.util.Optional;
2021
import java.util.Set;
2122
import java.util.function.Predicate;
2223
import java.util.stream.Collectors;
24+
import org.sonar.plugins.python.api.types.v2.ObjectType;
2325
import org.sonar.plugins.python.api.types.v2.PythonType;
2426
import org.sonar.plugins.python.api.types.v2.SelfType;
2527
import org.sonar.plugins.python.api.types.v2.TypeWrapper;
@@ -35,9 +37,13 @@ public PythonType convert(ConversionContext context, TypeAnnotationDescriptor ty
3537
if (fullyQualifiedName == null) {
3638
return PythonType.UNKNOWN;
3739
}
38-
if (Boolean.TRUE.equals(type.isSelf())) {
40+
List<PythonType> attributes = type.args().stream().map(t -> convert(context, t)).toList();
3941

40-
return SelfType.fromTypeWrapper(TypeWrapper.of(typeFromFQN(fullyQualifiedName, context)));
42+
if (Boolean.TRUE.equals(type.isSelf())) {
43+
var selfType = SelfType.fromTypeWrapper(TypeWrapper.of(typeFromFQN(fullyQualifiedName, context)));
44+
return ObjectType.Builder.fromType(selfType)
45+
.withAttributes(attributes)
46+
.build();
4147
}
4248
// _SpecialForm is the type used for some special types, like Callable, Union, TypeVar, ...
4349
// It comes from CPython impl: https://github.com/python/cpython/blob/e39ae6bef2c357a88e232dcab2e4b4c0f367544b/Lib/typing.py#L439
@@ -46,7 +52,9 @@ public PythonType convert(ConversionContext context, TypeAnnotationDescriptor ty
4652
if ("typing._SpecialForm".equals(fullyQualifiedName)) {
4753
return PythonType.UNKNOWN;
4854
}
49-
return typeFromFQN(fullyQualifiedName, context);
55+
return ObjectType.Builder.fromType(typeFromFQN(fullyQualifiedName, context))
56+
.withAttributes(attributes)
57+
.build();
5058
case TYPE:
5159
return context.lazyTypesContext().getOrCreateLazyType("type");
5260
case TYPE_ALIAS:

python-frontend/src/main/java/org/sonar/python/semantic/v2/types/LoopAssignment.java

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,10 @@
1616
*/
1717
package org.sonar.python.semantic.v2.types;
1818

19-
import java.util.Collection;
20-
import java.util.Optional;
21-
import java.util.stream.Stream;
22-
import org.sonar.plugins.python.api.TriBool;
2319
import org.sonar.plugins.python.api.symbols.v2.SymbolV2;
2420
import org.sonar.plugins.python.api.tree.Expression;
2521
import org.sonar.plugins.python.api.tree.Name;
22+
import org.sonar.plugins.python.api.types.v2.ClassType;
2623
import org.sonar.plugins.python.api.types.v2.ObjectType;
2724
import org.sonar.plugins.python.api.types.v2.PythonType;
2825

@@ -33,13 +30,25 @@ public LoopAssignment(SymbolV2 lhsSymbol, Name lhsName, Expression rhs) {
3330

3431
@Override
3532
public PythonType rhsType() {
36-
return Optional.of(super.rhsType())
37-
.filter(ObjectType.class::isInstance)
38-
.map(ObjectType.class::cast)
39-
.filter(t -> t.hasMember("__iter__") == TriBool.TRUE)
40-
.map(ObjectType::attributes)
41-
.map(Collection::stream)
42-
.flatMap(Stream::findFirst)
43-
.orElse(PythonType.UNKNOWN);
33+
PythonType rhsType = super.rhsType();
34+
if (rhsType instanceof ObjectType objectType && objectType.hasMember("__iter__").isTrue()) {
35+
// depending on the origin (e.g. typeshed/user-defined), the attribute type may be wrapped in an ObjectType or not,
36+
// independent of if the type is actually an instance or a class. As such, the logic in the map below assumes
37+
// the rhsType contains instances of the attribute type, and forces the loop variable type to be an ObjectType.
38+
var loopVarType = objectType.attributes().stream()
39+
.findFirst()
40+
.orElse(PythonType.UNKNOWN);
41+
42+
if (shouldBeWrappedInObjectType(loopVarType)) {
43+
return ObjectType.fromType(loopVarType);
44+
}
45+
46+
return loopVarType;
47+
}
48+
return PythonType.UNKNOWN;
49+
}
50+
51+
private static boolean shouldBeWrappedInObjectType(PythonType type) {
52+
return type instanceof ClassType classType && !"typing.Callable".equals(classType.fullyQualifiedName());
4453
}
4554
}

python-frontend/src/main/java/org/sonar/python/semantic/v2/types/typecalculator/CallReturnTypeCalculator.java

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,14 @@ private static PythonType collapseSelfTypeIfNeeded(CallExpression callExpr, Pyth
9797
}
9898

9999
private static boolean containsSelfType(PythonType type, TypePredicateContext typePredicateContext) {
100-
return TypeInferenceMatcher.of(TypeInferenceMatchers.isObjectSatisfying(TypeInferenceMatchers.isSelf()))
101-
.evaluate(type, typePredicateContext).isTrue();
100+
if (type instanceof SelfType || type.unwrappedType() instanceof SelfType) {
101+
return true;
102+
}
103+
if (type instanceof ObjectType objectType) {
104+
return objectType.attributes().stream().anyMatch(t -> containsSelfType(t, typePredicateContext));
105+
}
106+
107+
return false;
102108
}
103109

104110
private static boolean isInstanceOrClassMethodCall(CallExpression callExpr, TypePredicateContext typePredicateContext) {
@@ -133,10 +139,16 @@ private static PythonType getReceiverType(CallExpression callExpr) {
133139
}
134140

135141
private static PythonType collapseSelfType(PythonType returnType, PythonType receiverType) {
136-
if (returnType instanceof ObjectType objectType && objectType.type() instanceof SelfType) {
137-
return ObjectType.Builder.fromType(objectType)
138-
.withType(receiverType)
142+
if (returnType instanceof ObjectType objectType) {
143+
var objectBuilder = ObjectType.Builder.fromType(objectType);
144+
if (objectType.type() instanceof SelfType) {
145+
objectBuilder.withType(receiverType);
146+
}
147+
return objectBuilder
148+
.withAttributes(objectType.attributes().stream().map(t -> collapseSelfType(t, receiverType)).toList())
139149
.build();
150+
} else if (returnType instanceof SelfType) {
151+
return receiverType;
140152
}
141153
return PythonType.UNKNOWN;
142154
}

python-frontend/src/test/java/org/sonar/python/index/FunctionDescriptorTest.java

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,25 @@ void parameterWithDefaultValue() {
6969
assertDescriptorToProtobuf(functionDescriptor);
7070
}
7171

72+
@ParameterizedTest
73+
@MethodSource("parameterWithTypeTestCases")
74+
void parameterWithType(String code, String expectedAnnotatedType) {
75+
FunctionDescriptor functionDescriptor = lastFunctionDescriptor(code);
76+
FunctionDescriptor.Parameter parameter = functionDescriptor.parameters().get(0);
77+
assertThat(parameter.annotatedType()).isEqualTo(expectedAnnotatedType);
78+
assertDescriptorToProtobuf(functionDescriptor);
79+
}
80+
81+
static Stream<Arguments> parameterWithTypeTestCases() {
82+
return Stream.of(
83+
Arguments.of("def foo(x: str): ...", "str"),
84+
Arguments.of("def foo(x: list[something]): ...", "list"),
85+
Arguments.of("def foo(x: str): ...", "str")
86+
);
87+
}
88+
7289
@Test
73-
void parameterWithType() {
90+
void parameterWithPositional() {
7491
FunctionDescriptor functionDescriptor = lastFunctionDescriptor("def foo(x: str): ...");
7592
FunctionDescriptor.Parameter parameter = functionDescriptor.parameters().get(0);
7693
assertThat(parameter.annotatedType()).isEqualTo("str");
@@ -93,14 +110,6 @@ void parameterWithKeywordOnly() {
93110
assertDescriptorToProtobuf(functionDescriptor);
94111
}
95112

96-
@Test
97-
void parameterWithPositional() {
98-
FunctionDescriptor functionDescriptor = lastFunctionDescriptor("def foo(x: str): ...");
99-
FunctionDescriptor.Parameter parameter = functionDescriptor.parameters().get(0);
100-
assertThat(parameter.annotatedType()).isEqualTo("str");
101-
assertDescriptorToProtobuf(functionDescriptor);
102-
}
103-
104113
@Test
105114
void variadicParameter() {
106115
FunctionDescriptor functionDescriptor = lastFunctionDescriptor("def foo(*x, **y): ...");

0 commit comments

Comments
 (0)