Skip to content

Commit

Permalink
SONARPY-2316 Resolve field members of a ClassType (#2163)
Browse files Browse the repository at this point in the history
  • Loading branch information
maksim-grebeniuk-sonarsource authored Nov 20, 2024
1 parent 8784032 commit 2e55165
Show file tree
Hide file tree
Showing 8 changed files with 157 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from functionReturnTypeImported import ImportedFieldMembersOnlyNamedTuple, ImportedMethodMembersOnlyNamedTuple

def get_imported_field_members_only_named_tuple() -> ImportedFieldMembersOnlyNamedTuple:
return None # FN SONARPY-2316
return None # Noncompliant

def get_imported_method_members_only_named_tuple() -> ImportedMethodMembersOnlyNamedTuple:
return None # Noncompliant
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ def assigned_directly():
my_str_ok: str = 42 # Noncompliant
my_int_ok: int = 42 # OK
my_str_ok: str = "hello" # OK
a : ClassWithFieldOnly = None # FN
a : ClassWithFieldOnly = None # Noncompliant
b : ClassWithMethodOnly = None # Noncompliant

def return_union() -> Union[str, float]:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ public static Symbol symbolFromDescriptor(Descriptor descriptor, ProjectLevelSym
case FUNCTION:
return createFunctionSymbol((FunctionDescriptor) descriptor, projectLevelSymbolTable, createdSymbolsByDescriptor, createdSymbolsByFqn, symbolName);
case VARIABLE:
return new SymbolImpl(symbolName, descriptor.fullyQualifiedName());
var variableDescriptor = (VariableDescriptor) descriptor;
return new SymbolImpl(symbolName, descriptor.fullyQualifiedName(), variableDescriptor.annotatedType());
case AMBIGUOUS:
Set<Symbol> alternatives = new HashSet<>();
AmbiguousSymbolImpl ambiguousSymbol = new AmbiguousSymbolImpl(symbolName, descriptor.fullyQualifiedName(), alternatives);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,13 @@ public class SymbolImpl implements Symbol {
protected Set<String> validForPythonVersions = Collections.emptySet();

public SymbolImpl(String name, @Nullable String fullyQualifiedName) {
this(name, fullyQualifiedName, null);
}

public SymbolImpl(String name, @Nullable String fullyQualifiedName, @Nullable String annotatedTypeName) {
this.name = name;
this.fullyQualifiedName = fullyQualifiedName;
this.annotatedTypeName = annotatedTypeName;
this.kind = Kind.OTHER;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.sonar.python.semantic.v2.UsageV2;
import org.sonar.python.types.v2.ClassType;
import org.sonar.python.types.v2.FunctionType;
import org.sonar.python.types.v2.ObjectType;
import org.sonar.python.types.v2.ParameterV2;
import org.sonar.python.types.v2.PythonType;
import org.sonar.python.types.v2.TypeWrapper;
Expand Down Expand Up @@ -80,9 +81,16 @@ private static Descriptor convert(String moduleFqn, String parentFqn, String sym
if (type instanceof UnknownType.UnresolvedImportType unresolvedImportType) {
return convert(parentFqn, symbolName, unresolvedImportType);
}
if (type instanceof ObjectType objectType && !moduleFqn.equals(parentFqn)) {
return convert(moduleFqn, parentFqn, symbolName, objectType);
}
return new VariableDescriptor(symbolName, symbolFqn(parentFqn, symbolName), null);
}

private static Descriptor convert(String moduleFqn, String parentFqn, String symbolName, ObjectType objectType) {
return new VariableDescriptor(symbolName, symbolFqn(parentFqn, symbolName), typeFqn(moduleFqn, objectType.unwrappedType()));
}

private static Descriptor convert(String moduleFqn, FunctionType type) {

var parameters = type.parameters()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.function.Predicate;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import org.sonar.plugins.python.api.PythonFile;
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.AssignmentStatement;
import org.sonar.plugins.python.api.tree.BaseTreeVisitor;
Expand Down Expand Up @@ -451,10 +453,29 @@ public void visitAssignmentStatement(AssignmentStatement assignmentStatement) {
getFirstAssignmentName(assignmentStatement).ifPresent(lhsName -> {
var assignedValueType = assignmentStatement.assignedValue().typeV2();
lhsName.typeV2(assignedValueType);
addStaticFieldToClass(lhsName);
addStaticFieldToClass(lhsName, PythonType.UNKNOWN);
});
}

@Override
public void visitAnnotatedAssignment(AnnotatedAssignment assignmentStatement) {
super.visitAnnotatedAssignment(assignmentStatement);
Optional.ofNullable(assignmentStatement.variable())
.filter(NameImpl.class::isInstance)
.map(NameImpl.class::cast)
.ifPresent(lhsName -> {
if (currentType() instanceof ClassType) {
Optional.ofNullable(assignmentStatement.annotation())
.map(TypeAnnotation::expression)
.map(Expression::typeV2)
.filter(Predicate.not(PythonType.UNKNOWN::equals))
.map(t -> new ObjectType(t, TypeSource.TYPE_HINT))
.ifPresent(lhsName::typeV2);
addStaticFieldToClass(lhsName, lhsName.typeV2());
}
});
}

private static Optional<NameImpl> getFirstAssignmentName(AssignmentStatement assignmentStatement) {
return Optional.of(assignmentStatement)
.map(AssignmentStatement::lhsExpressions)
Expand All @@ -467,9 +488,12 @@ private static Optional<NameImpl> getFirstAssignmentName(AssignmentStatement ass
.map(NameImpl.class::cast);
}

private void addStaticFieldToClass(Name name) {
private void addStaticFieldToClass(Name name, PythonType type) {
if (currentType() instanceof ClassType ownerClass) {
ownerClass.members().add(new Member(name.name(), PythonType.UNKNOWN));
var memberName = name.name();
var toRemove = ownerClass.members().stream().filter(member -> memberName.equals(member.name())).toList();
ownerClass.members().removeAll(toRemove);
ownerClass.members().add(new Member(memberName, type));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ void inferred_type_after_copy() {
assertThat(copiedSymbol.inferredType()).isEqualTo(InferredTypes.INT);
}

@Test
void annotated_type_name_null_by_default_test() {
var x = new SymbolImpl("x", "module.x");
assertThat(x.annotatedTypeName()).isNull();
}

private Map<String, Symbol> symbols(String... code) {
FileInput fileInput = parse(new SymbolTableBuilder("", PythonTestUtils.pythonFile("foo")), code);
return fileInput.globalVariables().stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -500,24 +500,104 @@ class D(A, C): pass
.isEqualTo("B");
}

@Test
void annotatedClassFieldsInClassDefinition() {
var expression = lastExpression(
"""
class A:
a : str = "hi"
b : int
A
"""
);
assertThat(expression.typeV2()).isInstanceOfSatisfying(ClassType.class, classType -> {
assertThat(classType.members()).hasSize(2);
var aFieldMember = classType.members().stream().filter(m -> "a".equals(m.name())).findFirst().get();
var bFieldMember = classType.members().stream().filter(m -> "b".equals(m.name())).findFirst().get();
assertThat(aFieldMember.name()).isEqualTo("a");
assertThat(aFieldMember.type()).isInstanceOf(ObjectType.class).extracting(PythonType::unwrappedType).isEqualTo(STR_TYPE);
assertThat(bFieldMember.name()).isEqualTo("b");
assertThat(bFieldMember.type()).isInstanceOf(ObjectType.class).extracting(PythonType::unwrappedType).isEqualTo(INT_TYPE);
});
}

@Test
void annotatedClassFieldsInClassDefinitionSymbol() {
var tree = parseWithoutSymbols(
"""
class A:
a : str = "hi"
b : int
"""
);
var projectLevelSymbolTable = new ProjectLevelSymbolTable();
projectLevelSymbolTable.addModule(tree, "", pythonFile("mod.py"));

var symbol = (ClassSymbol) projectLevelSymbolTable.getSymbol("mod.A");
var aFieldSymbol = symbol.resolveMember("a").get();
var bFieldSymbol = symbol.resolveMember("b").get();
assertThat(aFieldSymbol.name()).isEqualTo("a");
assertThat(aFieldSymbol.annotatedTypeName()).isEqualTo("str");
assertThat(bFieldSymbol.name()).isEqualTo("b");
assertThat(bFieldSymbol.annotatedTypeName()).isEqualTo("int");
}

@Test
void annotatedClassFieldOverrideInClassDefinition() {
var expression = lastExpression(
"""
class A:
a : str
a : int
A
"""
);
assertThat(expression.typeV2()).isInstanceOfSatisfying(ClassType.class, classType -> {
assertThat(classType.members()).hasSize(1);
var aFieldMember = classType.members().stream().filter(m -> "a".equals(m.name())).findFirst().get();
assertThat(aFieldMember.name()).isEqualTo("a");
assertThat(aFieldMember.type()).isInstanceOf(ObjectType.class).extracting(PythonType::unwrappedType).isEqualTo(INT_TYPE);
});
}

@Test
void staticFieldsInClassDefinition() {
Expression expr = lastExpression("""
class A:
test = "hi"
A.test
A
""");
assertThat(expr.typeV2())
.isInstanceOf(UnknownType.class);
assertThat(expr.typeV2()).isInstanceOfSatisfying(ClassType.class, classType -> {
assertThat(classType.members()).hasSize(1);
var member = classType.members().iterator().next();
assertThat(member.name()).isEqualTo("test");
assertThat(member.type()).isEqualTo(PythonType.UNKNOWN);
});

Expression expr2 = lastExpression("""
class A:
test = "hi"
test = True
A.test
A
""");
assertThat(expr2.typeV2())
.isEqualTo(PythonType.UNKNOWN);
assertThat(expr2.typeV2()).isInstanceOfSatisfying(ClassType.class, classType -> {
assertThat(classType.members()).hasSize(1);
var member = classType.members().iterator().next();
assertThat(member.name()).isEqualTo("test");
assertThat(member.type()).isEqualTo(PythonType.UNKNOWN);
});

Expression expr3 = lastExpression("""
class A:
test = classmethod(...)
A
""");
assertThat(expr3.typeV2()).isInstanceOfSatisfying(ClassType.class, classType -> {
assertThat(classType.members()).hasSize(1);
var member = classType.members().iterator().next();
assertThat(member.name()).isEqualTo("test");
assertThat(member.type()).isEqualTo(PythonType.UNKNOWN);
});
}

@Test
Expand All @@ -527,10 +607,12 @@ class A:
test = "hi"
class B(A):
pass
B.test
B
""");
assertThat(exprWithInheritance.typeV2())
.isInstanceOf(UnknownType.class);
assertThat(exprWithInheritance.typeV2()).isInstanceOfSatisfying(ClassType.class, classType -> {
var member = classType.resolveMember("test").get();
assertThat(member).isEqualTo(PythonType.UNKNOWN);
});

Expression exprWithInheritance2 = lastExpression("""
class A:
Expand All @@ -539,21 +621,25 @@ class B(A):
test = "hi"
class C(B):
pass
C.test
C
""");
assertThat(exprWithInheritance2.typeV2())
.isInstanceOf(UnknownType.class);
assertThat(exprWithInheritance2.typeV2()).isInstanceOfSatisfying(ClassType.class, classType -> {
var member = classType.resolveMember("test").get();
assertThat(member).isEqualTo(PythonType.UNKNOWN);
});

Expression exprWithMultiInheritance = lastExpression("""
class A:
test = "hi"
class B: pass
class C(A, B):
pass
C.test
C
""");
assertThat(exprWithMultiInheritance.typeV2())
.isInstanceOf(UnknownType.class);
assertThat(exprWithMultiInheritance.typeV2()).isInstanceOfSatisfying(ClassType.class, classType -> {
var member = classType.resolveMember("test").get();
assertThat(member).isEqualTo(PythonType.UNKNOWN);
});

Expression exprWithMultiInheritance2 = lastExpression("""
class A:
Expand All @@ -562,10 +648,13 @@ class B:
test = "hi"
class C(A, B):
pass
C.test
C
""");
assertThat(exprWithMultiInheritance2.typeV2())
.isInstanceOf(UnknownType.class);
assertThat(exprWithMultiInheritance2.typeV2()).isInstanceOfSatisfying(ClassType.class, classType -> {
var member = classType.resolveMember("test").get();
assertThat(member).isEqualTo(PythonType.UNKNOWN);
});

}

@Test
Expand Down

0 comments on commit 2e55165

Please sign in to comment.