Skip to content

Commit

Permalink
SONARPY-2290 Support decorators for FunctionType to descriptors conve…
Browse files Browse the repository at this point in the history
…rter (#2125)
  • Loading branch information
1 parent 8ae11f2 commit 5dfa012
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
public class ChangeMethodContractCheck extends PythonSubscriptionCheck {

private static final Set<String> IGNORING_DECORATORS = Set.of(
"abc.abstractmethod",
"abstractmethod",
"overload"
);
Expand Down
19 changes: 19 additions & 0 deletions python-checks/src/test/resources/checks/changeMethodContract.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,3 +115,22 @@ def tzname(self): # Noncompliant
class MyDictionary(dict):
def get(self, key):
...

from abc import abstractmethod
import abc

class AbstractSuperclass:
@abstractmethod
def foo(self, a: int):
...

@abc.abstractmethod
def bar(self, a: int):
...

class InheritedFromAbstractSuperclass(AbstractSuperclass):
def foo(self): # Noncompliant
...

def bar(self): # Noncompliant
...
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ public PythonType convert(ConversionContext ctx, FunctionDescriptor from) {

var decorators = from.decorators()
.stream()
.map(decoratorName -> Stream.of(ctx.moduleFqn(), decoratorName)
.filter(Predicate.not(String::isEmpty))
.collect(Collectors.joining(".")))
.map(ctx.lazyTypesContext()::getOrCreateLazyType)
.map(TypeWrapper::of)
.toList();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand All @@ -36,6 +37,7 @@
import org.sonar.python.types.v2.FunctionType;
import org.sonar.python.types.v2.ParameterV2;
import org.sonar.python.types.v2.PythonType;
import org.sonar.python.types.v2.TypeWrapper;
import org.sonar.python.types.v2.UnionType;
import org.sonar.python.types.v2.UnknownType;

Expand Down Expand Up @@ -87,13 +89,20 @@ private static Descriptor convert(String moduleFqn, FunctionType type) {
.map(parameter -> convert(moduleFqn, parameter))
.toList();

var decorators = type.decorators()
.stream()
.map(TypeWrapper::type)
.map(decorator -> typeFqn(moduleFqn, decorator))
.filter(Objects::nonNull)
.toList();

// Using FunctionType#name and FunctionType#fullyQualifiedName instead of symbol is only accurate if the function has not been reassigned
// This logic should be revisited when tackling SONARPY-2285
return new FunctionDescriptor(type.name(), type.fullyQualifiedName(),
parameters,
type.isAsynchronous(),
type.isInstanceMethod(),
List.of(),
decorators,
type.hasDecorators(),
type.definitionLocation().orElse(null),
null,
Expand Down Expand Up @@ -167,8 +176,10 @@ private static FunctionDescriptor.Parameter convert(String moduleFqn, ParameterV
private static String typeFqn(String moduleFqn, PythonType type) {
if (type instanceof UnknownType.UnresolvedImportType importType) {
return importType.importPath();
} else if (type instanceof ClassType classType) {
return moduleFqn + "." + classType.name();
} else if (type instanceof ClassType) {
return moduleFqn + "." + type.name();
} else if (type instanceof FunctionType functionType) {
return functionType.fullyQualifiedName();
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,9 @@ def lib_decorator(): ...
projectLevelSymbolTable.addModule(libTree, "", pythonFile("lib.py"));
var lib2Tree = parseWithoutSymbols(
"""
import lib
import lib as l
@lib.lib_decorator
@l.lib_decorator
def foo(): ...
"""
);
Expand All @@ -319,7 +319,39 @@ def foo(): ...
);
var fooType = (FunctionType) ((ExpressionStatement) fileInput.statements().statements().get(1)).expressions().get(0).typeV2();
var typeWrapper = (LazyTypeWrapper) fooType.decorators().get(0);
assertThat(typeWrapper.hasImportPath("lib2.lib.lib_decorator")).isTrue();
assertThat(typeWrapper.hasImportPath("lib.lib_decorator")).isTrue();
}

@Test
void importedFunctionDecoratorNamesTest() {
var projectLevelSymbolTable = new ProjectLevelSymbolTable();
var libTree = parseWithoutSymbols(
"""
from abc import abstractmethod
class A:
@abstractmethod
def foo(self): ...
"""
);
projectLevelSymbolTable.addModule(libTree, "", pythonFile("lib.py"));

var projectLevelTypeTable = new ProjectLevelTypeTable(projectLevelSymbolTable);
var mainFile = pythonFile("main.py");
var fileInput = parseAndInferTypes(projectLevelTypeTable, mainFile, """
from lib import A
from datetime import tzinfo
A.foo
tzinfo.tzname
"""
);
var fooType = (FunctionType) ((ExpressionStatement) fileInput.statements().statements().get(2)).expressions().get(0).typeV2();
var typeWrapper = (LazyTypeWrapper) fooType.decorators().get(0);
assertThat(typeWrapper.hasImportPath("abc.abstractmethod")).isTrue();
var tznameType = (FunctionType) ((ExpressionStatement) fileInput.statements().statements().get(3)).expressions().get(0).typeV2();
typeWrapper = (LazyTypeWrapper) tznameType.decorators().get(0);
// SONARPY-2300 - need to fix serializer to use fully qualified names
assertThat(typeWrapper.hasImportPath("abstractmethod")).isTrue();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,19 @@ class PythonTypeToDescriptorConverterTest {
@Test
void testConvertFunctionTypeWithoutDecorator() {
ParameterV2 parameterV2 = new ParameterV2("param", TypeWrapper.of(intTypeWrapper.type()), false, true, false, false, false, location);
FunctionType functionType = new FunctionType("functionType", "my_package.foo.functionType", List.of(new ModuleType("bar")), List.of(parameterV2), List.of(), floatTypeWrapper, TypeOrigin.LOCAL, true, false, true, false, null, location);
FunctionType functionType = new FunctionType("functionType",
"my_package.foo.functionType",
List.of(new ModuleType("bar")),
List.of(parameterV2),
List.of(TypeWrapper.of(new UnknownType.UnresolvedImportType("abc.abstractmethod"))),
floatTypeWrapper,
TypeOrigin.LOCAL,
true,
true,
true,
false,
null,
location);
Descriptor descriptor = converter.convert("foo", new SymbolV2("myFunction"), Set.of(functionType));

assertThat(descriptor).isInstanceOf(FunctionDescriptor.class);
Expand All @@ -79,9 +91,8 @@ void testConvertFunctionTypeWithoutDecorator() {
// TODO SONARPY-2223 support for type annotation is missing in FunctionType
assertThat(functionDescriptor.typeAnnotationDescriptor()).isNull();

// TODO SONARPY-2223 support for decorators is missing in FunctionType
assertThat(functionDescriptor.hasDecorators()).isFalse();
assertThat(functionDescriptor.decorators()).isEmpty();
assertThat(functionDescriptor.hasDecorators()).isTrue();
assertThat(functionDescriptor.decorators()).isNotEmpty().containsOnly("abc.abstractmethod");
assertThat(functionDescriptor.definitionLocation()).isEqualTo(location);

assertThat(functionDescriptor.parameters()).hasSize(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ void declared_return_type() {

@Test
void decorators() {
// TODO: SONARPY-1772 Handle decorators
FunctionType functionType = functionType("@something\ndef fn(p1, *args): pass");
assertThat(functionType.hasDecorators()).isTrue();

Expand Down

0 comments on commit 5dfa012

Please sign in to comment.