Skip to content

Commit

Permalink
SONARPY-2316 Resolve field members of a ClassType
Browse files Browse the repository at this point in the history
  • Loading branch information
maksim-grebeniuk-sonarsource committed Nov 19, 2024
1 parent e7df9ae commit c91153d
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 27 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 @@ -70,6 +70,13 @@ public SymbolImpl(String name, @Nullable String fullyQualifiedName) {
this.kind = Kind.OTHER;
}

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

public SymbolImpl(SymbolsProtos.VarSymbol varSymbol, String moduleName, boolean isFromClass) {
this.name = varSymbol.getName();
this.fullyQualifiedName = TypeShed.normalizedFqn(varSymbol.getFullyQualifiedName(), moduleName, name);
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) {
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,32 @@ 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) {
scan(assignmentStatement.assignedValue());
scan(assignmentStatement.annotation());
scan(assignmentStatement.variable());

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 +491,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 @@ -477,14 +477,13 @@ class D(C, A):
@Test
void multiInheritanceConflicts() {
Expression exprWithMultiInheritance1 = lastExpression("""
class A: B = "hi"
class A: B = 10
class C:
class B: pass
class D(A, C): pass
D.B
""");
assertThat(exprWithMultiInheritance1.typeV2())
.isInstanceOf(UnknownType.class);
assertThat(exprWithMultiInheritance1.typeV2()).isEqualTo(PythonType.UNKNOWN);

Expression exprWithMultiInheritance2 = lastExpression("""
class A:
Expand All @@ -500,24 +499,87 @@ 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 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 +589,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 +603,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 +630,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 c91153d

Please sign in to comment.