Skip to content

Commit 1cf4466

Browse files
Seppli11sonartech
authored andcommitted
SONARPY-3634 Resolve type when imported twice (#826)
GitOrigin-RevId: 5baae177e4f15587baef0991bdbb7f0fc8178c45
1 parent 5dec4db commit 1cf4466

3 files changed

Lines changed: 102 additions & 21 deletions

File tree

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@
4444
import org.sonar.python.index.VariableDescriptor;
4545

4646
public class PythonTypeToDescriptorConverter {
47-
4847
public Descriptor convert(String moduleFqn, SymbolV2 symbol, Set<PythonType> types) {
4948
var candidates = types.stream()
5049
.map(type -> convert(moduleFqn, moduleFqn, symbol.name(), type, symbol.usages()))
@@ -227,7 +226,7 @@ private static boolean usagesContainAssignment(List<UsageV2> symbolUsages) {
227226
private static TypeAnnotationDescriptor createTypeAnnotationDescriptor(PythonType type, boolean isSelf) {
228227
if (type instanceof SelfType selfType) {
229228
return createTypeAnnotationDescriptor(selfType.innerType(), isSelf);
230-
}else if (type instanceof ClassType classType) {
229+
} else if (type instanceof ClassType classType) {
231230
return new TypeAnnotationDescriptor(classType.name(), TypeAnnotationDescriptor.TypeKind.INSTANCE, List.of(), classType.fullyQualifiedName(), isSelf);
232231
} else if (type instanceof FunctionType functionType) {
233232
return new TypeAnnotationDescriptor(functionType.name(), TypeAnnotationDescriptor.TypeKind.CALLABLE, List.of(), functionType.fullyQualifiedName(), false);

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

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -865,32 +865,30 @@ public void visitName(Name name) {
865865
return;
866866
}
867867

868-
var bindingUsages = new ArrayList<UsageV2>();
869-
for (var usage : symbolV2.usages()) {
870-
if (usage.kind().equals(UsageV2.Kind.GLOBAL_DECLARATION)) {
871-
// Don't infer type for global variables
872-
return;
873-
}
874-
if (usage.isBindingUsage()) {
875-
bindingUsages.add(usage);
876-
}
877-
if (bindingUsages.size() > 1) {
878-
// no need to iterate over usages if there is more than one binding usage
879-
return;
880-
}
868+
var bindingUsages = symbolV2.usages().stream()
869+
.filter(UsageV2::isBindingUsage)
870+
.toList();
871+
872+
boolean multipleBindingUsages = bindingUsages.size() > 1;
873+
boolean onlyImportUsages = bindingUsages.stream().allMatch(usage -> UsageV2.Kind.IMPORT.equals(usage.kind()));
874+
boolean hasGlobalStatementUsage = symbolV2.usages().stream().anyMatch(usage -> UsageV2.Kind.GLOBAL_DECLARATION.equals(usage.kind()));
875+
876+
if (hasGlobalStatementUsage || (multipleBindingUsages && !onlyImportUsages)) {
877+
return;
881878
}
882879

883-
bindingUsages.stream()
884-
.findFirst()
885-
.filter(UsageV2::isBindingUsage)
880+
var types = bindingUsages.stream()
886881
.map(UsageV2::tree)
887-
.filter(Expression.class::isInstance)
888-
.map(Expression.class::cast)
882+
.flatMap(TreeUtils.toStreamInstanceOfMapper(Expression.class))
889883
.map(Expression::typeV2)
890884
// TODO: classes (SONARPY-1829) and functions should be propagated like other types
891885
.filter(t -> shouldTypeBeEagerlyPropagated(t, name))
892-
.ifPresent(type -> setTypeToName(name, type));
886+
.distinct()
887+
.toList();
893888

889+
if (types.size() == 1) {
890+
setTypeToName(name, types.get(0));
891+
}
894892
}
895893

896894
private boolean shouldTypeBeEagerlyPropagated(PythonType t, Name name) {

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1361,6 +1361,20 @@ def nested():
13611361
assertThat(symbolV2.usages()).extracting(UsageV2::kind).containsExactlyInAnyOrder(UsageV2.Kind.ASSIGNMENT_LHS, UsageV2.Kind.GLOBAL_DECLARATION, UsageV2.Kind.OTHER);
13621362
}
13631363

1364+
@Test
1365+
void global_variable_not_eagerly_propagated() {
1366+
FileInput fileInput = inferTypes("""
1367+
a = object()
1368+
def foo():
1369+
global a
1370+
1371+
a
1372+
""");
1373+
FunctionDef foo = PythonTestUtils.getFirstDescendant(fileInput, FunctionDef.class::isInstance);
1374+
Name aName = PythonTestUtils.getLastDescendant(foo.body(), Name.class::isInstance);
1375+
assertThat(aName.typeV2()).isEqualTo(PythonType.UNKNOWN);
1376+
}
1377+
13641378
@Test
13651379
void nonlocal_variable_in_nested_function() {
13661380
FileInput fileInput = inferTypes("""
@@ -3345,6 +3359,76 @@ void import_unknown_httpx_client() {
33453359
assertThat(statement.typeV2()).isEqualTo(PythonType.UNKNOWN);
33463360
}
33473361

3362+
@Test
3363+
void duplicate_imports_in_from_import() {
3364+
var requestExpr = lastExpression("""
3365+
from flask import request, request
3366+
def test():
3367+
request
3368+
""");
3369+
3370+
PythonType requestType = PROJECT_LEVEL_TYPE_TABLE.getType("flask.request");
3371+
assertThat(requestType).isNotEqualTo(PythonType.UNKNOWN);
3372+
3373+
assertThat(requestExpr.typeV2()).isEqualTo(requestType);
3374+
}
3375+
3376+
static Stream<Arguments> duplicateImportsNotUnknownSource() {
3377+
return Stream.of(
3378+
Arguments.argumentSet("Duplicate from-import",
3379+
"""
3380+
from flask import request
3381+
from flask import request
3382+
def test():
3383+
request
3384+
""", PROJECT_LEVEL_TYPE_TABLE.getType("flask.request")),
3385+
Arguments.argumentSet("Duplicate import-statement", """
3386+
import flask
3387+
import flask
3388+
def test():
3389+
flask
3390+
""", PROJECT_LEVEL_TYPE_TABLE.getType("flask")));
3391+
}
3392+
3393+
@ParameterizedTest()
3394+
@MethodSource("duplicateImportsNotUnknownSource")
3395+
void duplicate_imports_not_unknown(String code, PythonType expectedType) {
3396+
assertThat(expectedType).isNotEqualTo(PythonType.UNKNOWN);
3397+
assertThat(lastExpression(code).typeV2()).isEqualTo(expectedType);
3398+
}
3399+
3400+
static Stream<Arguments> duplicateImportsUnknownSource() {
3401+
return Stream.of(
3402+
Arguments.argumentSet("Duplicate from-import with different types",
3403+
"""
3404+
if PY3:
3405+
from io import StringIO
3406+
else:
3407+
from cStringIO import StringIO
3408+
def test():
3409+
StringIO
3410+
"""),
3411+
Arguments.argumentSet("Duplicate from-import with additional unrelated binding",
3412+
"""
3413+
from flask import request, request
3414+
request = 1
3415+
def test():
3416+
request
3417+
"""),
3418+
Arguments.argumentSet("Duplicate import-statement with additional unrelated binding", """
3419+
import flask
3420+
flask = 1
3421+
def test():
3422+
flask
3423+
"""));
3424+
}
3425+
3426+
@ParameterizedTest
3427+
@MethodSource("duplicateImportsUnknownSource")
3428+
void duplicate_imports_unknown(String code) {
3429+
assertThat(lastExpression(code).typeV2()).isEqualTo(PythonType.UNKNOWN);
3430+
}
3431+
33483432
@Test
33493433
void returnTypeOfTypeshedSymbol() {
33503434
FileInput fileInput = inferTypes("""

0 commit comments

Comments
 (0)