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
maksim-grebeniuk-sonarsource committed Nov 5, 2024
1 parent 95e8e9b commit 8db40ed
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 13 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 @@ -129,3 +129,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 @@ -20,7 +20,6 @@
package org.sonar.python.semantic.v2;

import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.sonar.plugins.python.api.PythonFile;
import org.sonar.plugins.python.api.tree.ExpressionStatement;
Expand Down Expand Up @@ -267,7 +266,6 @@ void resolveStubsWithImportedModuleVariableDescriptor() {
}

@Test
@Disabled("SONARPY-2290")
void importFunctionWithDecorators() {
var projectLevelSymbolTable = new ProjectLevelSymbolTable();
var libTree = parseWithoutSymbols(
Expand All @@ -294,7 +292,6 @@ def foo(): ...
}

@Test
@Disabled("SONARPY-2290")
void importFunctionWithImportedDecorators() {
var projectLevelSymbolTable = new ProjectLevelSymbolTable();
var libTree = parseWithoutSymbols(
Expand All @@ -305,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 @@ -322,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 @@ -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 8db40ed

Please sign in to comment.