From f5729926aa5141990d54ee492d8145d41fbbe7f9 Mon Sep 17 00:00:00 2001 From: David Kunzmann Date: Fri, 1 Dec 2023 13:21:04 +0100 Subject: [PATCH] SONARPY-1561: Infer types of variables used as decorators. (#1668) --- .../nonCallableCalled.py | 14 ++++++- .../resources/checks/nonCallableCalled.py | 20 +++++++++ .../python/cfg/ControlFlowGraphBuilder.java | 20 +++++++-- .../python/api/cfg/ControlFlowGraphTest.java | 29 +++++++++++++ .../sonar/python/types/TypeInferenceTest.java | 41 ++++++++++++++++++- 5 files changed, 117 insertions(+), 7 deletions(-) diff --git a/python-checks/src/test/resources/checks/confusingTypeChecking/nonCallableCalled.py b/python-checks/src/test/resources/checks/confusingTypeChecking/nonCallableCalled.py index 138c78d5bf..44fc9abd46 100644 --- a/python-checks/src/test/resources/checks/confusingTypeChecking/nonCallableCalled.py +++ b/python-checks/src/test/resources/checks/confusingTypeChecking/nonCallableCalled.py @@ -1,4 +1,4 @@ -from typing import Set, FrozenSet, Union, Coroutine +from typing import Set, FrozenSet, Union, Coroutine, Callable import asyncio def empty_union(x: Union['A', 'B']): @@ -37,3 +37,15 @@ async def bar(func: Coroutine): await asyncio.gather( func() ) + + +def decorators(decorator: Callable, non_callable: str): + + @non_callable() # Noncompliant + def foo(): + ... + + @decorator() + def bar(): + ... + diff --git a/python-checks/src/test/resources/checks/nonCallableCalled.py b/python-checks/src/test/resources/checks/nonCallableCalled.py index 9dee8c4c2a..1f9c4ce7bb 100644 --- a/python-checks/src/test/resources/checks/nonCallableCalled.py +++ b/python-checks/src/test/resources/checks/nonCallableCalled.py @@ -80,6 +80,13 @@ class A(Base): ... a = A() a() # OK + +def decorators(): + x = 42 + @x() # Noncompliant + def foo(): + ... + ####################################### # Valid case: Calling a callable object ####################################### @@ -97,6 +104,19 @@ def call_function(): func() max() +############################# +# Valid case: Call a decorator +############################# + +def decorators(): + class Dec: + def __call__(self, *args): + ... + + @Dec("foo") + def foo(): + ... + ############################# # Valid case: Call a method ############################# diff --git a/python-frontend/src/main/java/org/sonar/python/cfg/ControlFlowGraphBuilder.java b/python-frontend/src/main/java/org/sonar/python/cfg/ControlFlowGraphBuilder.java index f297895338..60b4113f26 100644 --- a/python-frontend/src/main/java/org/sonar/python/cfg/ControlFlowGraphBuilder.java +++ b/python-frontend/src/main/java/org/sonar/python/cfg/ControlFlowGraphBuilder.java @@ -164,10 +164,7 @@ private PythonCfgBlock build(Statement statement, PythonCfgBlock currentBlock) { case WITH_STMT: return buildWithStatement((WithStatement) statement, currentBlock); case CLASSDEF: - ClassDef classDef = (ClassDef) statement; - PythonCfgBlock block = build(classDef.body().statements(), currentBlock); - block.addElement(classDef.name()); // represents binding to class name - return block; + return buildClassDefStatement((ClassDef) statement, currentBlock); case RETURN_STMT: return buildReturnStatement((ReturnStatement) statement, currentBlock); case RAISE_STMT: @@ -186,6 +183,8 @@ private PythonCfgBlock build(Statement statement, PythonCfgBlock currentBlock) { return buildBreakStatement((BreakStatement) statement, currentBlock); case MATCH_STMT: return buildMatchStatement((MatchStatement) statement, currentBlock); + case FUNCDEF: + return buildFuncDefStatement((FunctionDef) statement, currentBlock); default: currentBlock.addElement(statement); } @@ -193,6 +192,19 @@ private PythonCfgBlock build(Statement statement, PythonCfgBlock currentBlock) { return currentBlock; } + private PythonCfgBlock buildClassDefStatement(ClassDef classDef, PythonCfgBlock currentBlock) { + PythonCfgBlock block = build(classDef.body().statements(), currentBlock); + block.addElement(classDef.name()); // represents binding to class name + classDef.decorators().stream().forEach(currentBlock::addElement); + return block; + } + + private static PythonCfgBlock buildFuncDefStatement(FunctionDef functionDef, PythonCfgBlock currentBlock) { + currentBlock.addElement(functionDef); + functionDef.decorators().stream().forEach(currentBlock::addElement); + return currentBlock; + } + private PythonCfgBlock buildMatchStatement(MatchStatement statement, PythonCfgBlock successor) { List caseBlocks = statement.caseBlocks(); PythonCfgBlock matchingBlock = null; diff --git a/python-frontend/src/test/java/org/sonar/plugins/python/api/cfg/ControlFlowGraphTest.java b/python-frontend/src/test/java/org/sonar/plugins/python/api/cfg/ControlFlowGraphTest.java index 2acec7198b..b9f37f7c4b 100644 --- a/python-frontend/src/test/java/org/sonar/plugins/python/api/cfg/ControlFlowGraphTest.java +++ b/python-frontend/src/test/java/org/sonar/plugins/python/api/cfg/ControlFlowGraphTest.java @@ -20,6 +20,7 @@ package org.sonar.plugins.python.api.cfg; import java.util.Arrays; +import java.util.List; import java.util.stream.Collectors; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -706,6 +707,34 @@ void if_stmt_test_predecessors() { "after_if(succ = [END], pred = [before, if_body])"); } + @Test + void decorators() { + ControlFlowGraph cfg = cfg( + "class Dec:", + " def a():", + " ...", + "dec = Dec()", + "@dec.a()", + "def foo():", + " ..." + ); + List elements = cfg.start().elements(); + assertThat(elements).hasSize(5); + assertThat(elements).extracting(Tree::getKind).containsExactly(Kind.NAME, Kind.FUNCDEF, Kind.ASSIGNMENT_STMT, Kind.DECORATOR, Kind.FUNCDEF); + + cfg = cfg( + "class Dec:", + " def __call__(self):", + " ...", + "@Dec", + "class OtherClass:", + " ..." + ); + elements = cfg.start().elements(); + assertThat(elements).hasSize(5); + assertThat(elements).extracting(Tree::getKind).containsExactly(Kind.NAME, Kind.FUNCDEF, Kind.DECORATOR, Kind.NAME, Kind.EXPRESSION_STMT); + } + @Test void parameters() { FileInput fileInput = PythonTestUtils.parse("def f(p1, p2): pass"); diff --git a/python-frontend/src/test/java/org/sonar/python/types/TypeInferenceTest.java b/python-frontend/src/test/java/org/sonar/python/types/TypeInferenceTest.java index 1d5bcb2181..a64d72bb59 100644 --- a/python-frontend/src/test/java/org/sonar/python/types/TypeInferenceTest.java +++ b/python-frontend/src/test/java/org/sonar/python/types/TypeInferenceTest.java @@ -24,9 +24,12 @@ import org.sonar.plugins.python.api.symbols.ClassSymbol; import org.sonar.plugins.python.api.tree.AssignmentStatement; import org.sonar.plugins.python.api.tree.CallExpression; +import org.sonar.plugins.python.api.tree.Decorator; import org.sonar.plugins.python.api.tree.Expression; import org.sonar.plugins.python.api.tree.ExpressionStatement; import org.sonar.plugins.python.api.tree.FileInput; +import org.sonar.plugins.python.api.tree.Name; +import org.sonar.plugins.python.api.tree.QualifiedExpression; import org.sonar.plugins.python.api.tree.RegularArgument; import org.sonar.plugins.python.api.tree.ReturnStatement; import org.sonar.plugins.python.api.tree.Tree; @@ -34,16 +37,18 @@ import org.sonar.plugins.python.api.types.InferredType; import org.sonar.python.PythonTestUtils; import org.sonar.python.semantic.SymbolImpl; +import org.sonar.python.semantic.SymbolTableBuilder; import static org.assertj.core.api.Assertions.assertThat; import static org.sonar.python.PythonTestUtils.getLastDescendant; import static org.sonar.python.PythonTestUtils.lastExpression; import static org.sonar.python.PythonTestUtils.lastExpressionInFunction; -import static org.sonar.python.PythonTestUtils.lastStatement;import static org.sonar.python.PythonTestUtils.parse; +import static org.sonar.python.PythonTestUtils.lastStatement; +import static org.sonar.python.PythonTestUtils.parse; +import static org.sonar.python.PythonTestUtils.pythonFile; import static org.sonar.python.types.InferredTypes.BOOL; import static org.sonar.python.types.InferredTypes.COMPLEX; import static org.sonar.python.types.InferredTypes.DECL_INT; -import static org.sonar.python.types.InferredTypes.DECL_LIST; import static org.sonar.python.types.InferredTypes.DECL_STR; import static org.sonar.python.types.InferredTypes.DICT; import static org.sonar.python.types.InferredTypes.FLOAT; @@ -759,4 +764,36 @@ void user_defined_attributes_reassigned() { ).type()).isEqualTo(DECL_INT); } + @Test + void decorators() { + Decorator decorator = lastDecorator( + "class A:", + " def dec_method():", + " ...", + "my_dec = A()", + "@my_dec.dec_method()", + "def a_function():", + " ..."); + CallExpression ce = (CallExpression) decorator.expression(); + QualifiedExpression qe = (QualifiedExpression) ce.callee(); + assertThat(typeName(qe.qualifier().type())).isEqualTo("A"); + assertThat(qe.name().symbol().fullyQualifiedName()).isEqualTo("some_package.some_module.A.dec_method"); + assertThat(ce.calleeSymbol().fullyQualifiedName()).isEqualTo("some_package.some_module.A.dec_method"); + + decorator = lastDecorator( + "class A:", + " def __call__():", + " ...", + "@A", + "class OtherClass:", + " ..."); + Name name = (Name) decorator.expression(); + assertThat(name.type()).isEqualTo(anyType()); + assertThat(name.symbol().fullyQualifiedName()).isEqualTo("some_package.some_module.A"); + } + + private static Decorator lastDecorator(String... code) { + FileInput fileInput = parse(new SymbolTableBuilder("some_package", pythonFile("some_module")), code); + return PythonTestUtils.getLastDescendant(fileInput, t -> t.is(Tree.Kind.DECORATOR)); + } }