Skip to content

Commit 65840c9

Browse files
guillaume-dequennesonartech
authored andcommitted
SONARPY-3997 Compute ReachingDefinitionsAnalysis at module-level (#1018)
GitOrigin-RevId: 372eff5a6e5c40910ec716dfdd87024e9a830082
1 parent 1562462 commit 65840c9

File tree

4 files changed

+115
-5
lines changed

4 files changed

+115
-5
lines changed

python-checks/src/test/resources/checks/datetime_constructor_pytz_timezone.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ def timezone_in_a_variable():
1313
return dt1
1414

1515
some_timezone_1 = pytz.timezone('US/Eastern')
16-
dt1 = datetime.datetime(2022, 1, 1, tzinfo=some_timezone_1) # FN because of ReachingDefinitionsAnalysis limitations : it only runs in functions
16+
#^^^^^^^^^^^^^^^^^^^^^^^^^^^> 1 {{The pytz.timezone is created here.}}
17+
dt1 = datetime.datetime(2022, 1, 1, tzinfo=some_timezone_1) # Noncompliant {{Don't pass a "pytz.timezone" to the "datetime.datetime" constructor.}}
18+
#^^^^^^^^^^^^^^^^^^^^^^
1719

1820
def compliant():
1921
other_timezone = datetime.timezone(datetime.timedelta(hours=2))

python-frontend/src/main/java/org/sonar/python/cfg/fixpoint/ReachingDefinitionsAnalysis.java

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,14 @@
3737
import org.sonar.plugins.python.api.tree.AssignmentStatement;
3838
import org.sonar.plugins.python.api.tree.BaseTreeVisitor;
3939
import org.sonar.plugins.python.api.tree.Expression;
40+
import org.sonar.plugins.python.api.tree.FileInput;
4041
import org.sonar.plugins.python.api.tree.FunctionDef;
4142
import org.sonar.plugins.python.api.tree.Name;
4243
import org.sonar.plugins.python.api.tree.Tree;
4344
import org.sonar.python.tree.TreeUtils;
4445

4546
import static org.sonar.plugins.python.api.tree.Tree.Kind.ASSIGNMENT_STMT;
47+
import static org.sonar.plugins.python.api.tree.Tree.Kind.CLASSDEF;
4648
import static org.sonar.plugins.python.api.tree.Tree.Kind.FUNCDEF;
4749
import static org.sonar.plugins.python.api.tree.Tree.Kind.TRY_STMT;
4850

@@ -74,17 +76,38 @@ public Set<Expression> valuesAtLocation(Name variable) {
7476
return assignedExpressions;
7577
}
7678
FunctionDef enclosingFunction = (FunctionDef) TreeUtils.firstAncestorOfKind(variable, FUNCDEF);
77-
if (enclosingFunction == null || TreeUtils.hasDescendant(enclosingFunction, t -> t.is(TRY_STMT))) {
79+
if (enclosingFunction != null) {
80+
return valuesInFunctionScope(variable, enclosingFunction);
81+
}
82+
83+
FileInput fileInput = TreeUtils.firstAncestorOfClass(variable, FileInput.class);
84+
if (fileInput == null) {
85+
return Collections.emptySet();
86+
}
87+
return analyzeScope(variable, fileInput, () -> ControlFlowGraph.build(fileInput, pythonFile), fileInput.globalVariables());
88+
}
89+
90+
private Set<Expression> valuesInFunctionScope(Name variable, FunctionDef enclosingFunction) {
91+
return analyzeScope(variable, enclosingFunction, () -> ControlFlowGraph.build(enclosingFunction, pythonFile), enclosingFunction.localVariables());
92+
}
93+
94+
private Set<Expression> analyzeScope(Name variable, Tree scopeTree, Supplier<ControlFlowGraph> cfgSupplier, Set<Symbol> variables) {
95+
if (hasTryStatementInScope(scopeTree)) {
7896
return Collections.emptySet();
7997
}
80-
ControlFlowGraph cfg = ControlFlowGraph.build(enclosingFunction, pythonFile);
98+
ControlFlowGraph cfg = cfgSupplier.get();
8199
if (cfg == null) {
82100
return Collections.emptySet();
83101
}
84-
compute(cfg, enclosingFunction.localVariables());
102+
compute(cfg, variables);
85103
return assignedExpressionByName.getOrDefault(variable, Collections.emptySet());
86104
}
87105

106+
private static boolean hasTryStatementInScope(Tree tree) {
107+
return tree.children().stream().anyMatch(child ->
108+
child.is(TRY_STMT) || (!child.is(FUNCDEF, CLASSDEF) && hasTryStatementInScope(child)));
109+
}
110+
88111
private Set<Expression> getAssignedExpressions(Name variable, ProgramStateAtElement programStateAtElement) {
89112
Symbol symbol = variable.symbol();
90113
if (symbol == null) {

python-frontend/src/test/java/org/sonar/python/SubscriptionVisitorTest.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,4 +215,30 @@ public void initialize(Context context) {
215215
.withFailMessage("valuesAtLocation was not called by both checks")
216216
.isZero();
217217
}
218+
219+
@Test
220+
void reachingDefinitionAnalysis_at_module_level() {
221+
var latch = new CountDownLatch(1);
222+
223+
var check = new PythonSubscriptionCheck() {
224+
@Override
225+
public void initialize(Context context) {
226+
context.registerSyntaxNodeConsumer(Tree.Kind.EXPRESSION_STMT, ctx -> {
227+
var exprStmt = (ExpressionStatement) ctx.syntaxNode();
228+
var expr = exprStmt.expressions().get(0);
229+
if (expr instanceof Name name && "x".equals(name.name())) {
230+
assertThat(ctx.valuesAtLocation(name)).extracting(e -> ((NumericLiteral) e).valueAsString()).containsExactly("42");
231+
latch.countDown();
232+
}
233+
});
234+
}
235+
};
236+
237+
var fileInput = PythonTestUtils.parse("x = 42\nx");
238+
var context = new Builder(fileInput, PythonTestUtils.pythonFile("file")).build();
239+
SubscriptionVisitor.analyze(List.of(check), context);
240+
assertThat(latch.getCount())
241+
.withFailMessage("valuesAtLocation was not called at module level")
242+
.isZero();
243+
}
218244
}

python-frontend/src/test/java/org/sonar/python/cfg/fixpoint/ReachingDefinitionsAnalysisTest.java

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,37 @@ void valuesAtLocation_branches() {
8282
@Test
8383
void valuesAtLocation_outside_function() {
8484
Name x = (Name) lastExpression("x = 42; x");
85-
assertThat(analysis.valuesAtLocation(x)).isEmpty();
85+
assertThat(analysis.valuesAtLocation(x)).extracting(ReachingDefinitionsAnalysisTest::getValueAsString).containsExactly("42");
8686
}
8787

8888
@Test
8989
void valuesAtLocation_outside_function_annotated() {
9090
Name x = (Name) lastExpression("x: int = 42; x");
91+
assertThat(analysis.valuesAtLocation(x)).extracting(ReachingDefinitionsAnalysisTest::getValueAsString).containsExactly("42");
92+
}
93+
94+
@Test
95+
void valuesAtLocation_outside_function_branches() {
96+
Name x = (Name) lastExpression(
97+
"if p:",
98+
" x = 1",
99+
"else:",
100+
" x = 2",
101+
"x"
102+
);
103+
assertThat(analysis.valuesAtLocation(x)).extracting(ReachingDefinitionsAnalysisTest::getValueAsString).containsExactlyInAnyOrder("1", "2");
104+
}
105+
106+
@Test
107+
void valuesAtLocation_outside_function_try_stmt() {
108+
Name x = (Name) lastExpression(
109+
"x = 1",
110+
"try:",
111+
" x = 2",
112+
"except:",
113+
" pass",
114+
"x"
115+
);
91116
assertThat(analysis.valuesAtLocation(x)).isEmpty();
92117
}
93118

@@ -97,6 +122,12 @@ void valuesAtLocation_invalid_cfg() {
97122
assertThat(analysis.valuesAtLocation(x)).isEmpty();
98123
}
99124

125+
@Test
126+
void valuesAtLocation_outside_function_invalid_cfg() {
127+
Name x = (Name) lastExpression("x = 42", "break", "x");
128+
assertThat(analysis.valuesAtLocation(x)).isEmpty();
129+
}
130+
100131
@Test
101132
void valuesAtLocation_no_name_assignment() {
102133
Name x = (Name) lastExpressionInFunction("x.foo = 42", "x");
@@ -158,6 +189,34 @@ void compound_assignments() {
158189
assertThat(analysis.valuesAtLocation(x)).isEmpty();
159190
}
160191

192+
@Test
193+
void try_stmt_in_nested_function() {
194+
Name x = (Name) lastExpressionInFunction(
195+
"x = 1",
196+
"def inner():",
197+
" try:",
198+
" pass",
199+
" except:",
200+
" pass",
201+
"x"
202+
);
203+
assertThat(analysis.valuesAtLocation(x)).extracting(ReachingDefinitionsAnalysisTest::getValueAsString).containsExactly("1");
204+
}
205+
206+
@Test
207+
void valuesAtLocation_outside_function_try_stmt_in_nested_function() {
208+
Name x = (Name) lastExpression(
209+
"x = 1",
210+
"def f():",
211+
" try:",
212+
" pass",
213+
" except:",
214+
" pass",
215+
"x"
216+
);
217+
assertThat(analysis.valuesAtLocation(x)).extracting(ReachingDefinitionsAnalysisTest::getValueAsString).containsExactly("1");
218+
}
219+
161220
@Test
162221
void try_stmt() {
163222
Name x = (Name) lastExpressionInFunction(

0 commit comments

Comments
 (0)