Skip to content

Commit d0e8a7f

Browse files
SONARPY-930 Generate CFG for match statement (#995)
1 parent 32bf0f0 commit d0e8a7f

File tree

10 files changed

+189
-1
lines changed

10 files changed

+189
-1
lines changed

python-checks/src/main/java/org/sonar/python/checks/InvariantReturnCheck.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.sonar.plugins.python.api.tree.FunctionDef;
3838
import org.sonar.plugins.python.api.tree.Name;
3939
import org.sonar.plugins.python.api.tree.ParenthesizedExpression;
40+
import org.sonar.plugins.python.api.tree.Pattern;
4041
import org.sonar.plugins.python.api.tree.ReturnStatement;
4142
import org.sonar.plugins.python.api.tree.Tree;
4243
import org.sonar.plugins.python.api.tree.Tree.Kind;
@@ -98,7 +99,7 @@ private static void collectBranchingBlock(List<LatestExecutedBlock> collectedBlo
9899
if (!TreeUtils.hasDescendant(tryStatement.body(), t -> t.is(Kind.RETURN_STMT))) {
99100
collectedBlocks.add(new LatestExecutedBlock(branchingBlock));
100101
}
101-
} else if (branchingTree.is(Kind.IF_STMT)) {
102+
} else if (branchingTree.is(Kind.IF_STMT) || branchingTree instanceof Pattern) {
102103
collectedBlocks.add(new LatestExecutedBlock(branchingBlock));
103104
} else {
104105
collectBlocksHavingReturnBeforeExceptOrFinallyBlock(collectedBlocks, branchingBlock);

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,3 +206,17 @@ def try_block_without_jump_statements():
206206
print("except")
207207
return 42
208208
print("dead code") # Noncompliant
209+
210+
211+
def match_statement_no_fp(value):
212+
match value:
213+
case "1": return
214+
case "2": return
215+
print("reachable")
216+
217+
218+
def match_statement_fn(value):
219+
match value:
220+
case "1": return
221+
case x: return
222+
print("unreachable") # FN, "case x" will match anything

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,3 +301,19 @@ def assignment_expression_fn():
301301
def assignment_expression_no_fp():
302302
a = 3 # No fp: a will be read before it's written into
303303
dict = {'b' : a, 'c' : (a:=3)} # FN (final assignment expression is unused)
304+
305+
def match_statement_fn(value):
306+
a = 42
307+
match value:
308+
case 1: ...
309+
case a: ... # FN
310+
311+
312+
class MyClass:
313+
CONST = 99
314+
315+
def match_statement_no_fp(value):
316+
a = MyClass()
317+
match value:
318+
case 1: ...
319+
case a.CONST: a = 42 # OK, read before write

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,15 @@ def assignment_expression_fn(a): # FN (first dict key computation overwrites "a"
5050

5151
def assignment_expression_no_fp(a):
5252
dict = {'b' : a, 'c' : (a:=3)} # OK, read before write
53+
54+
def match_statement_fp(value, param): # Noncompliant
55+
match value:
56+
case param.CONST: param = 42 # FP here: b.CONST should be a reading usage of b
57+
case "other": ...
58+
value = 42 # OK
59+
60+
def match_statement_fn(value, param):
61+
match value:
62+
case 1: ...
63+
case param: ... # FN, c is overridden without having been read
64+
value = 42 # OK

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,3 +344,16 @@ async def asyn6(i): # Noncompliant
344344

345345
class A:
346346
def func(*, x, y): ...
347+
348+
349+
def match_statement_no_fp(value):
350+
match value:
351+
case 42:
352+
return "hello"
353+
return no_fp_with_match(value + 1)
354+
355+
356+
def match_statement_fn(value): # FN, "case x" will always match
357+
match value:
358+
case x:
359+
return match_statement_fn(value + 1)

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -498,3 +498,25 @@ def unreachable_implicit_return_none(cond): # Noncompliant
498498
except Exception:
499499
return 42
500500
# Implicit "None" return is unreachable
501+
502+
def basic_match_statement_noncompliant(value): # Noncompliant
503+
match value:
504+
case 1:
505+
return 42
506+
return 42
507+
508+
509+
def match_statement_no_fp_with_implicit_return(value): # OK, implicit "None" return is reachable
510+
match value:
511+
case 1:
512+
return 42
513+
case 2:
514+
return 42
515+
516+
517+
def match_statement_fn_with_capture(value): # FN, implicit "None" return is unreachable
518+
match value:
519+
case 1:
520+
return 42
521+
case x:
522+
return 42

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,3 +326,15 @@ class Foo:
326326

327327
def single_return():
328328
return # OK
329+
330+
331+
def match_statement(value):
332+
match value:
333+
case "42":
334+
foo()
335+
return # Noncompliant
336+
337+
def match_statement_single_return(value):
338+
match value:
339+
case "42":
340+
return # OK

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,3 +179,14 @@ def assignment_expression():
179179
def assignment_expression_fn():
180180
b = 41
181181
dict = {k: (k := b + 1)} # FN, key is evaluated first and value second
182+
183+
def match_statement_no_fp(value):
184+
match value:
185+
case x:
186+
...
187+
x = 42
188+
189+
def match_statement_no_fp_reassignment(value):
190+
match value:
191+
case x: # OK, though should be raised by S1854 (dead store)
192+
x = 42

python-frontend/src/main/java/org/sonar/python/cfg/ControlFlowGraphBuilder.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.sonar.plugins.python.api.cfg.ControlFlowGraph;
3535
import org.sonar.plugins.python.api.tree.AnyParameter;
3636
import org.sonar.plugins.python.api.tree.BreakStatement;
37+
import org.sonar.plugins.python.api.tree.CaseBlock;
3738
import org.sonar.plugins.python.api.tree.ClassDef;
3839
import org.sonar.plugins.python.api.tree.ContinueStatement;
3940
import org.sonar.plugins.python.api.tree.ElseClause;
@@ -42,8 +43,11 @@
4243
import org.sonar.plugins.python.api.tree.FinallyClause;
4344
import org.sonar.plugins.python.api.tree.ForStatement;
4445
import org.sonar.plugins.python.api.tree.FunctionDef;
46+
import org.sonar.plugins.python.api.tree.Guard;
4547
import org.sonar.plugins.python.api.tree.IfStatement;
48+
import org.sonar.plugins.python.api.tree.MatchStatement;
4649
import org.sonar.plugins.python.api.tree.ParameterList;
50+
import org.sonar.plugins.python.api.tree.Pattern;
4751
import org.sonar.plugins.python.api.tree.RaiseStatement;
4852
import org.sonar.plugins.python.api.tree.ReturnStatement;
4953
import org.sonar.plugins.python.api.tree.Statement;
@@ -182,13 +186,37 @@ private PythonCfgBlock build(Statement statement, PythonCfgBlock currentBlock) {
182186
return tryStatement(((TryStatement) statement), currentBlock);
183187
case BREAK_STMT:
184188
return buildBreakStatement((BreakStatement) statement, currentBlock);
189+
case MATCH_STMT:
190+
return buildMatchStatement((MatchStatement) statement, currentBlock);
185191
default:
186192
currentBlock.addElement(statement);
187193
}
188194

189195
return currentBlock;
190196
}
191197

198+
private PythonCfgBlock buildMatchStatement(MatchStatement statement, PythonCfgBlock successor) {
199+
List<CaseBlock> caseBlocks = statement.caseBlocks();
200+
PythonCfgBlock matchingBlock = null;
201+
PythonCfgBlock falseSuccessor = successor;
202+
for (int i = caseBlocks.size() - 1; i >= 0; i--) {
203+
PythonCfgBlock caseBodyBlock = createSimpleBlock(successor);
204+
CaseBlock caseBlock = caseBlocks.get(i);
205+
Pattern pattern = caseBlock.pattern();
206+
Guard guard = caseBlock.guard();
207+
caseBodyBlock = build(caseBlock.body().statements(), caseBodyBlock);
208+
matchingBlock = createBranchingBlock(pattern, caseBodyBlock, falseSuccessor);
209+
if (guard != null) {
210+
matchingBlock.addElement(guard.condition());
211+
}
212+
matchingBlock.addElement(pattern);
213+
matchingBlock.addElement(statement.subjectExpression());
214+
blocks.add(matchingBlock);
215+
falseSuccessor = matchingBlock;
216+
}
217+
return matchingBlock;
218+
}
219+
192220
private PythonCfgBlock buildWithStatement(WithStatement withStatement, PythonCfgBlock successor) {
193221
PythonCfgBlock withBodyBlock = build(withStatement.statements().statements(), createSimpleBlock(successor));
194222
// exceptions may be raised inside with block and be caught by context manager

python-frontend/src/test/java/org/sonar/plugins/python/api/cfg/ControlFlowGraphTest.java

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,13 @@
2020
package org.sonar.plugins.python.api.cfg;
2121

2222
import java.util.Arrays;
23+
import java.util.List;
2324
import java.util.stream.Collectors;
2425
import org.junit.Test;
2526
import org.mockito.Mockito;
2627
import org.sonar.plugins.python.api.PythonFile;
28+
import org.sonar.plugins.python.api.tree.Expression;
29+
import org.sonar.plugins.python.api.tree.ExpressionStatement;
2730
import org.sonar.plugins.python.api.tree.FileInput;
2831
import org.sonar.plugins.python.api.tree.FunctionDef;
2932
import org.sonar.plugins.python.api.tree.Parameter;
@@ -596,6 +599,43 @@ public void with_statement() {
596599
);
597600
}
598601

602+
@Test
603+
public void match_statement() {
604+
ControlFlowGraph cfg = cfg("" +
605+
"foo() ",
606+
"match x:",
607+
" case \"1\": y()",
608+
" case a.b if w(): z()",
609+
"bar()"
610+
);
611+
612+
assertThat(cfg.blocks()).hasSize(6);
613+
614+
PythonCfgBranchingBlock start = (PythonCfgBranchingBlock) cfg.start();
615+
PythonCfgEndBlock end = (PythonCfgEndBlock) cfg.end();
616+
assertThat(end.predecessors()).hasSize(1);
617+
CfgBlock barBlock = end.predecessors().stream().findFirst().get();
618+
619+
assertThat(barBlock.elements()).extracting(Tree::getKind).containsExactly(Kind.EXPRESSION_STMT);
620+
ExpressionStatement expressionStatement = (ExpressionStatement) barBlock.elements().get(0);
621+
assertThat(expressionStatement.expressions()).extracting(Tree::getKind).containsExactly(Kind.CALL_EXPR);
622+
623+
assertThat(start.branchingTree().getKind()).isEqualTo(Kind.STRING_LITERAL_PATTERN);
624+
assertThat(start.elements()).extracting(Tree::getKind).containsExactly(Kind.EXPRESSION_STMT, Kind.NAME, Kind.STRING_LITERAL_PATTERN);
625+
626+
PythonCfgSimpleBlock firstCaseBody = (PythonCfgSimpleBlock) start.trueSuccessor();
627+
assertThat(firstCaseBody.elements()).extracting(Tree::getKind).containsExactly(Kind.EXPRESSION_STMT);
628+
assertThat(firstCaseBody.successors()).containsExactly(barBlock);
629+
630+
PythonCfgBranchingBlock secondCaseEvaluation = (PythonCfgBranchingBlock) start.falseSuccessor();
631+
assertThat(secondCaseEvaluation.elements()).extracting(Tree::getKind).containsExactly(Kind.NAME, Kind.VALUE_PATTERN, Kind.CALL_EXPR);
632+
633+
PythonCfgSimpleBlock secondCaseTrueSuccessor = (PythonCfgSimpleBlock) secondCaseEvaluation.trueSuccessor();
634+
assertThat(secondCaseTrueSuccessor.successors()).containsExactly(barBlock);
635+
PythonCfgSimpleBlock secondCaseFalseSuccessor = (PythonCfgSimpleBlock) secondCaseEvaluation.falseSuccessor();
636+
assertThat(secondCaseFalseSuccessor).isEqualTo(barBlock);
637+
}
638+
599639
@Test
600640
public void CFGBlock_toString() {
601641
PythonCfgEndBlock endBlock = new PythonCfgEndBlock();
@@ -609,6 +649,25 @@ public void CFGBlock_toString() {
609649
assertThat(cfg.start().toString()).isEqualTo("2:2:PASS_STMT;ASSERT_STMT");
610650
}
611651

652+
@Test
653+
public void CFG_matchStatement() {
654+
ControlFlowGraph cfg = cfg("" +
655+
"foo() ",
656+
"match x:",
657+
" case \"1\": y()",
658+
" case a.b: z()",
659+
"bar()"
660+
);
661+
assertThat(cfg).hasToString("" +
662+
"0[label=\"4:14:EXPRESSION_STMT\"];" +
663+
"1[label=\"4:9:EXPRESSION_STMT;NAME;STRING_LITERAL_PATTERN\"];" +
664+
"2[label=\"5:14:EXPRESSION_STMT\"];" +
665+
"3[label=\"5:9:NAME;VALUE_PATTERN\"];" +
666+
"4[label=\"6:2:EXPRESSION_STMT\"];" +
667+
"5[label=\"END\"];" +
668+
"0->4;1->0;1->3;2->4;3->2;3->4;4->5;");
669+
}
670+
612671
@Test
613672
public void CFG_toString() {
614673
ControlFlowGraph cfg = cfg("" +

0 commit comments

Comments
 (0)