Skip to content

Commit dcd4bec

Browse files
guillaume-dequenneguillaume-dequenne
committed
SONARPY-2024 Ensure call expressions return types have same type source as their callee (#1890)
1 parent 4ae019f commit dcd4bec

File tree

3 files changed

+158
-16
lines changed

3 files changed

+158
-16
lines changed

‎python-frontend/src/main/java/org/sonar/python/tree/CallExpressionImpl.java

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import java.util.Collection;
2323
import java.util.Collections;
24+
import java.util.HashSet;
2425
import java.util.List;
2526
import java.util.Objects;
2627
import java.util.Optional;
@@ -50,6 +51,7 @@
5051
import org.sonar.python.types.v2.FunctionType;
5152
import org.sonar.python.types.v2.ObjectType;
5253
import org.sonar.python.types.v2.PythonType;
54+
import org.sonar.python.types.v2.TypeSource;
5355
import org.sonar.python.types.v2.UnionType;
5456

5557
import static org.sonar.plugins.python.api.symbols.Symbol.Kind.CLASS;
@@ -187,28 +189,40 @@ public List<Expression> typeDependencies() {
187189

188190
@Override
189191
public PythonType typeV2() {
190-
if (callee().typeV2() instanceof ClassType classType) {
191-
return new ObjectType(classType);
192+
TypeSource typeSource = computeTypeSource();
193+
PythonType pythonType = returnTypeOfCall(callee().typeV2());
194+
return pythonType != PythonType.UNKNOWN ? new ObjectType(pythonType, typeSource) : PythonType.UNKNOWN;
195+
}
196+
197+
static PythonType returnTypeOfCall(PythonType calleeType) {
198+
if (calleeType instanceof ClassType classType) {
199+
return classType;
192200
}
193-
if (callee().typeV2() instanceof FunctionType functionType) {
194-
PythonType returnType = functionType.returnType();
195-
if (returnType.equals(PythonType.UNKNOWN)) {
196-
return PythonType.UNKNOWN;
197-
}
198-
return new ObjectType(returnType);
201+
if (calleeType instanceof FunctionType functionType) {
202+
return functionType.returnType();
199203
}
200-
if (callee().typeV2() instanceof UnionType unionType) {
201-
PythonType result = null;
204+
if (calleeType instanceof UnionType unionType) {
205+
Set<PythonType> types = new HashSet<>();
202206
for (PythonType candidate : unionType.candidates()) {
203-
if (candidate instanceof ClassType classType) {
204-
result = UnionType.or(result, classType);
205-
}
206-
if (candidate instanceof FunctionType functionType) {
207-
result = UnionType.or(result, functionType.returnType());
207+
PythonType typeOfCandidate = returnTypeOfCall(candidate);
208+
if (typeOfCandidate.equals(PythonType.UNKNOWN)) {
209+
return PythonType.UNKNOWN;
208210
}
211+
types.add(typeOfCandidate);
209212
}
210-
return result == null ? PythonType.UNKNOWN : new ObjectType(result);
213+
return UnionType.or(types);
214+
}
215+
if (calleeType instanceof ObjectType objectType) {
216+
Optional<PythonType> pythonType = objectType.resolveMember("__call__");
217+
return pythonType.map(CallExpressionImpl::returnTypeOfCall).orElse(PythonType.UNKNOWN);
211218
}
212219
return PythonType.UNKNOWN;
213220
}
221+
222+
TypeSource computeTypeSource() {
223+
if (callee() instanceof QualifiedExpression qualifiedExpression) {
224+
return qualifiedExpression.qualifier().typeV2().typeSource();
225+
}
226+
return callee().typeV2().typeSource();
227+
}
214228
}

‎python-frontend/src/main/java/org/sonar/python/types/v2/UnionType.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ public boolean isCompatibleWith(PythonType another) {
6666

6767
@Beta
6868
public static PythonType or(Collection<PythonType> candidates) {
69+
if (candidates.isEmpty()) {
70+
return PythonType.UNKNOWN;
71+
}
6972
return candidates
7073
.stream()
7174
.reduce(new UnionType(new HashSet<>()), UnionType::or);

‎python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,73 @@ def foo(param: int):
363363
Assertions.assertThat(lastExpressionStatement.expressions().get(0).typeV2().typeSource()).isEqualTo(TypeSource.TYPE_HINT);
364364
}
365365

366+
@Test
367+
void typeSourceOfCallExpressionResultDependsOnTypeSourceOfQualifier() {
368+
FileInput root = inferTypes("""
369+
def foo(x: int):
370+
y = x.conjugate()
371+
y
372+
z = x.conjugate().conjugate()
373+
z
374+
""");
375+
var functionDef = (FunctionDef) root.statements().statements().get(0);
376+
var yStatement = (ExpressionStatement) functionDef.body().statements().get(1);
377+
PythonType yType = yStatement.expressions().get(0).typeV2();
378+
assertThat(yType).isInstanceOf(ObjectType.class);
379+
assertThat(yType.unwrappedType()).isEqualTo(INT_TYPE);
380+
assertThat(yType.typeSource()).isEqualTo(TypeSource.TYPE_HINT);
381+
382+
var zStatement = (ExpressionStatement) functionDef.body().statements().get(3);
383+
PythonType zType = zStatement.expressions().get(0).typeV2();
384+
assertThat(zType).isInstanceOf(ObjectType.class);
385+
assertThat(zType.unwrappedType()).isEqualTo(INT_TYPE);
386+
assertThat(zType.typeSource()).isEqualTo(TypeSource.TYPE_HINT);
387+
}
388+
389+
@Test
390+
void typeSourceOfCallExpressionResultDependsOnTypeSourceOfName() {
391+
FileInput fileInput = inferTypes("""
392+
from pyasn1.debug import Printer
393+
def foo(p: Printer):
394+
a = p()
395+
a
396+
b = p.__call__()
397+
b
398+
""");
399+
400+
var functionDef = (FunctionDef) fileInput.statements().statements().get(1);
401+
var aStatement = (ExpressionStatement) functionDef.body().statements().get(1);
402+
PythonType aType = aStatement.expressions().get(0).typeV2();
403+
assertThat(aType).isInstanceOf(ObjectType.class);
404+
assertThat(aType.unwrappedType()).isEqualTo(NONE_TYPE);
405+
assertThat(aType.typeSource()).isEqualTo(TypeSource.TYPE_HINT);
406+
407+
var bStatement = (ExpressionStatement) functionDef.body().statements().get(3);
408+
PythonType bType = bStatement.expressions().get(0).typeV2();
409+
assertThat(bType).isInstanceOf(ObjectType.class);
410+
assertThat(bType.unwrappedType()).isEqualTo(NONE_TYPE);
411+
assertThat(bType.typeSource()).isEqualTo(TypeSource.TYPE_HINT);
412+
}
413+
414+
@Test
415+
void typeSourceIsExactByDefault() {
416+
FileInput fileInput = inferTypes("""
417+
random[2]()
418+
""");
419+
CallExpression callExpression = ((CallExpression) ((ExpressionStatement) fileInput.statements().statements().get(0)).expressions().get(0));
420+
421+
CallExpression callExpressionSpy = Mockito.spy(callExpression);
422+
Expression calleeSpy = Mockito.spy(callExpression.callee());
423+
FunctionType functionType = new FunctionType("foo", List.of(), List.of(), INT_TYPE, false, false, false, false, null, null);
424+
Mockito.when(calleeSpy.typeV2()).thenReturn(functionType);
425+
Mockito.when(callExpressionSpy.callee()).thenReturn(calleeSpy);
426+
427+
var resultType = callExpressionSpy.typeV2();
428+
assertThat(resultType.typeSource()).isEqualTo(TypeSource.EXACT);
429+
assertThat(resultType).isInstanceOf(ObjectType.class);
430+
assertThat(resultType.unwrappedType()).isEqualTo(INT_TYPE);
431+
}
432+
366433
@Test
367434
void inferTypesInsideFunction6() {
368435
FileInput root = inferTypes("""
@@ -1791,6 +1858,64 @@ def bar(self): ...
17911858
assertThat(unionType.candidates()).containsExactlyInAnyOrder(classA, classB);
17921859
}
17931860

1861+
@Test
1862+
void return_type_of_call_expression_inconsistent() {
1863+
FileInput fileInput = inferTypes(
1864+
"""
1865+
foo()
1866+
"""
1867+
);
1868+
CallExpression callExpression = ((CallExpression) ((ExpressionStatement) fileInput.statements().statements().get(0)).expressions().get(0));
1869+
CallExpression callExpressionSpy = Mockito.spy(callExpression);
1870+
1871+
// Inconsistent union type, should not happen
1872+
UnionType unionType = new UnionType(Set.of(PythonType.UNKNOWN));
1873+
Name mock = Mockito.mock(Name.class);
1874+
Mockito.when(mock.typeV2()).thenReturn(unionType);
1875+
Mockito.doReturn(mock).when(callExpressionSpy).callee();
1876+
1877+
assertThat(callExpressionSpy.typeV2()).isEqualTo(PythonType.UNKNOWN);
1878+
}
1879+
1880+
@Test
1881+
void return_type_of_call_expression_inconsistent_2() {
1882+
FileInput fileInput = inferTypes(
1883+
"""
1884+
foo()
1885+
"""
1886+
);
1887+
CallExpression callExpression = ((CallExpression) ((ExpressionStatement) fileInput.statements().statements().get(0)).expressions().get(0));
1888+
CallExpression callExpressionSpy = Mockito.spy(callExpression);
1889+
1890+
// Inconsistent union type, should not happen
1891+
UnionType unionType = new UnionType(Set.of());
1892+
Name mock = Mockito.mock(Name.class);
1893+
Mockito.when(mock.typeV2()).thenReturn(unionType);
1894+
Mockito.doReturn(mock).when(callExpressionSpy).callee();
1895+
1896+
assertThat(callExpressionSpy.typeV2()).isEqualTo(PythonType.UNKNOWN);
1897+
}
1898+
1899+
@Test
1900+
void return_type_of_call_expression_inconsistent_3() {
1901+
FileInput fileInput = inferTypes(
1902+
"""
1903+
foo()
1904+
"""
1905+
);
1906+
CallExpression callExpression = ((CallExpression) ((ExpressionStatement) fileInput.statements().statements().get(0)).expressions().get(0));
1907+
CallExpression callExpressionSpy = Mockito.spy(callExpression);
1908+
1909+
// Inconsistent union type, should not happen
1910+
UnionType unionType = new UnionType(Set.of(INT_TYPE));
1911+
Name mock = Mockito.mock(Name.class);
1912+
Mockito.when(mock.typeV2()).thenReturn(unionType);
1913+
Mockito.doReturn(mock).when(callExpressionSpy).callee();
1914+
1915+
assertThat(callExpressionSpy.typeV2().unwrappedType()).isEqualTo(INT_TYPE);
1916+
}
1917+
1918+
17941919
@Test
17951920
void imported_symbol_call_return_type() {
17961921
assertThat(lastExpression(

0 commit comments

Comments
 (0)