Skip to content

Commit dce39f8

Browse files
thomas-serre-sonarsourcesonartech
thomas-serre-sonarsource
authored and
sonartech
committed
SONARPY-3851 Cache LiveVariableAnalysis (#1026)
GitOrigin-RevId: 45ba3c2e62e9b64beebfcefeed4b9e8f26505bf8
1 parent e4658ec commit dce39f8

File tree

6 files changed

+112
-47
lines changed

6 files changed

+112
-47
lines changed

‎python-checks/src/main/java/org/sonar/python/checks/DeadStoreCheck.java‎

Lines changed: 46 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -55,55 +55,11 @@
5555
@Rule(key = "S1854")
5656
public class DeadStoreCheck extends PythonSubscriptionCheck {
5757

58+
public static final String QUICK_FIX_MESSAGE = "Remove the unused assignment";
5859
private static final String MESSAGE_TEMPLATE = "Remove this assignment to local variable '%s'; the value is never used.";
59-
6060
private static final String SECONDARY_MESSAGE_TEMPLATE = "'%s' is reassigned here.";
61-
public static final String QUICK_FIX_MESSAGE = "Remove the unused assignment";
62-
6361
private boolean isTemplateVariablesAccessEnabled = false;
6462

65-
@Override
66-
public void initialize(Context context) {
67-
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, this::checkTemplateVariablesAccessEnabled);
68-
context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, ctx -> {
69-
FunctionDef functionDef = (FunctionDef) ctx.syntaxNode();
70-
if (TreeUtils.hasDescendant(functionDef, tree -> tree.is(Tree.Kind.TRY_STMT))) {
71-
return;
72-
}
73-
ControlFlowGraph cfg = ctx.cfg(functionDef);
74-
if (cfg == null) {
75-
return;
76-
}
77-
LiveVariablesAnalysis lva = LiveVariablesAnalysis.analyze(cfg);
78-
cfg.blocks().forEach(block -> verifyBlock(ctx, block, lva.getLiveVariables(block), lva.getReadSymbols(), functionDef));
79-
});
80-
}
81-
82-
private void checkTemplateVariablesAccessEnabled(SubscriptionContext ctx) {
83-
var importedNamesCollector = new ImportedNamesCollector();
84-
importedNamesCollector.collect(ctx.syntaxNode());
85-
isTemplateVariablesAccessEnabled = importedNamesCollector.anyMatches("pandas"::equals);
86-
}
87-
88-
/**
89-
* Bottom-up approach, keeping track of which variables will be read by successor elements.
90-
*/
91-
private void verifyBlock(SubscriptionContext ctx, CfgBlock block, LiveVariablesAnalysis.LiveVariables blockLiveVariables,
92-
Set<Symbol> readSymbols, FunctionDef functionDef) {
93-
94-
var stringLiteralValuesCollector = new StringLiteralValuesCollector();
95-
if (isTemplateVariablesAccessEnabled) {
96-
stringLiteralValuesCollector.collect(functionDef);
97-
}
98-
DeadStoreUtils.findUnnecessaryAssignments(block, blockLiveVariables, functionDef)
99-
.stream()
100-
// symbols should have at least one read usage (otherwise will be reported by S1481)
101-
.filter(unnecessaryAssignment -> readSymbols.contains(unnecessaryAssignment.symbol))
102-
.filter((unnecessaryAssignment -> !isException(unnecessaryAssignment.symbol, unnecessaryAssignment.element, functionDef,
103-
stringLiteralValuesCollector)))
104-
.forEach(unnecessaryAssignment -> raiseIssue(ctx, unnecessaryAssignment));
105-
}
106-
10763
private static void raiseIssue(SubscriptionContext ctx, DeadStoreUtils.UnnecessaryAssignment unnecessaryAssignment) {
10864
Tree element;
10965
if (unnecessaryAssignment.element instanceof ClassDef classDefElement) {
@@ -247,7 +203,6 @@ private static boolean isFunctionDeclarationSymbol(Symbol symbol) {
247203
return symbol.usages().stream().anyMatch(u -> u.kind() == Usage.Kind.FUNC_DECLARATION);
248204
}
249205

250-
251206
private static boolean isExceptionForQuickFix(Statement tree) {
252207
switch (tree.getKind()) {
253208
// foo:str = bar
@@ -265,6 +220,51 @@ private static boolean isExceptionForQuickFix(Statement tree) {
265220
}
266221
}
267222

223+
@Override
224+
public void initialize(Context context) {
225+
context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, this::checkTemplateVariablesAccessEnabled);
226+
context.registerSyntaxNodeConsumer(Tree.Kind.FUNCDEF, ctx -> {
227+
FunctionDef functionDef = (FunctionDef) ctx.syntaxNode();
228+
if (TreeUtils.hasDescendant(functionDef, tree -> tree.is(Tree.Kind.TRY_STMT))) {
229+
return;
230+
}
231+
ControlFlowGraph cfg = ctx.cfg(functionDef);
232+
if (cfg == null) {
233+
return;
234+
}
235+
LiveVariablesAnalysis lva = ctx.lva(functionDef);
236+
if (lva == null) {
237+
return;
238+
}
239+
cfg.blocks().forEach(block -> verifyBlock(ctx, block, lva.getLiveVariables(block), lva.getReadSymbols(), functionDef));
240+
});
241+
}
242+
243+
private void checkTemplateVariablesAccessEnabled(SubscriptionContext ctx) {
244+
var importedNamesCollector = new ImportedNamesCollector();
245+
importedNamesCollector.collect(ctx.syntaxNode());
246+
isTemplateVariablesAccessEnabled = importedNamesCollector.anyMatches("pandas"::equals);
247+
}
248+
249+
/**
250+
* Bottom-up approach, keeping track of which variables will be read by successor elements.
251+
*/
252+
private void verifyBlock(SubscriptionContext ctx, CfgBlock block, LiveVariablesAnalysis.LiveVariables blockLiveVariables,
253+
Set<Symbol> readSymbols, FunctionDef functionDef) {
254+
255+
var stringLiteralValuesCollector = new StringLiteralValuesCollector();
256+
if (isTemplateVariablesAccessEnabled) {
257+
stringLiteralValuesCollector.collect(functionDef);
258+
}
259+
DeadStoreUtils.findUnnecessaryAssignments(block, blockLiveVariables, functionDef)
260+
.stream()
261+
// symbols should have at least one read usage (otherwise will be reported by S1481)
262+
.filter(unnecessaryAssignment -> readSymbols.contains(unnecessaryAssignment.symbol))
263+
.filter((unnecessaryAssignment -> !isException(unnecessaryAssignment.symbol, unnecessaryAssignment.element, functionDef,
264+
stringLiteralValuesCollector)))
265+
.forEach(unnecessaryAssignment -> raiseIssue(ctx, unnecessaryAssignment));
266+
}
267+
268268
private static class SideEffectDetector extends BaseTreeVisitor {
269269

270270
private boolean sideEffect = false;

‎python-checks/src/main/java/org/sonar/python/checks/IgnoredParameterCheck.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,11 @@ public void initialize(Context context) {
4747
if (cfg == null) {
4848
return;
4949
}
50-
LiveVariablesAnalysis lva = LiveVariablesAnalysis.analyze(cfg);
50+
LiveVariablesAnalysis lva = ctx.lva(functionDef);
51+
if (lva == null) {
52+
return;
53+
}
54+
5155
Set<CfgBlock> unreachableBlocks = CfgUtils.unreachableBlocks(cfg);
5256
cfg.blocks().forEach(block -> {
5357
var unnecessaryAssignments = DeadStoreUtils.findUnnecessaryAssignments(block, lva.getLiveVariables(block), functionDef);

‎python-frontend/src/main/java/org/sonar/plugins/python/api/PythonVisitorContext.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.sonar.sslr.api.RecognitionException;
2121
import java.io.File;
2222
import java.util.ArrayList;
23+
import java.util.HashMap;
2324
import java.util.List;
2425
import java.util.Map;
2526
import java.util.Optional;
@@ -37,6 +38,7 @@
3738
import org.sonar.plugins.python.api.tree.Tree;
3839
import org.sonar.plugins.python.api.types.v2.ModuleType;
3940
import org.sonar.python.caching.CacheContextImpl;
41+
import org.sonar.python.cfg.fixpoint.LiveVariablesAnalysis;
4042
import org.sonar.python.cfg.fixpoint.ReachingDefinitionsAnalysis;
4143
import org.sonar.python.semantic.ProjectLevelSymbolTable;
4244
import org.sonar.python.semantic.SymbolTableBuilder;
@@ -58,9 +60,12 @@ public class PythonVisitorContext extends PythonInputFileContext {
5860
private final ProjectConfiguration projectConfiguration;
5961
private final CallGraph callGraph;
6062
private final Map<Tree, ControlFlowGraph> cfgMap;
63+
private final Map<Tree, LiveVariablesAnalysis> lvaMap;
6164
private final ReachingDefinitionsAnalysis reachingDefinitionsAnalysis;
6265
private final TypeTable typeTable;
6366

67+
68+
6469
private PythonVisitorContext(FileInput rootTree,
6570
PythonFile pythonFile,
6671
@Nullable File workingDirectory,
@@ -79,6 +84,7 @@ private PythonVisitorContext(FileInput rootTree,
7984
this.callGraph = callGraph;
8085
this.cfgMap = cfgMap;
8186
this.reachingDefinitionsAnalysis = new ReachingDefinitionsAnalysis(pythonFile);
87+
this.lvaMap = new HashMap<>();
8288
this.rootTree = rootTree;
8389
this.parsingException = null;
8490
this.typeTable = typeTable;
@@ -96,6 +102,7 @@ public PythonVisitorContext(PythonFile pythonFile, RecognitionException parsingE
96102
this.callGraph = CallGraph.EMPTY;
97103
this.cfgMap = Map.of();
98104
this.reachingDefinitionsAnalysis = new ReachingDefinitionsAnalysis(pythonFile);
105+
this.lvaMap = new HashMap<>();
99106
this.issues = new ArrayList<>();
100107
this.moduleType = null;
101108
}
@@ -142,10 +149,20 @@ public CallGraph callGraph() {
142149
return callGraph;
143150
}
144151

152+
@CheckForNull
145153
public ControlFlowGraph cfg(Tree tree) {
146154
return cfgMap.get(tree);
147155
}
148156

157+
@CheckForNull
158+
public LiveVariablesAnalysis lva(Tree tree){
159+
ControlFlowGraph cfg = cfg(tree);
160+
if (cfg == null) {
161+
return null;
162+
}
163+
return lvaMap.computeIfAbsent(tree, t -> LiveVariablesAnalysis.analyze(cfg));
164+
}
165+
149166
public Optional<DjangoViewInfo> getDjangoViewInfo(String fqn) {
150167
return projectLevelSymbolTable().getDjangoViewInfo(fqn);
151168
}

‎python-frontend/src/main/java/org/sonar/plugins/python/api/SubscriptionContext.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.sonar.plugins.python.api.tree.Name;
3232
import org.sonar.plugins.python.api.tree.Token;
3333
import org.sonar.plugins.python.api.tree.Tree;
34+
import org.sonar.python.cfg.fixpoint.LiveVariablesAnalysis;
3435
import org.sonar.python.semantic.v2.callgraph.CallGraph;
3536
import org.sonar.python.semantic.v2.typetable.TypeTable;
3637
import org.sonar.python.types.v2.TypeChecker;
@@ -83,6 +84,8 @@ public interface SubscriptionContext {
8384

8485
ControlFlowGraph cfg(Tree tree);
8586

87+
LiveVariablesAnalysis lva(Tree tree);
88+
8689
Set<Expression> valuesAtLocation(Name name);
8790

8891
/**

‎python-frontend/src/main/java/org/sonar/python/SubscriptionVisitor.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import org.sonar.plugins.python.api.tree.Token;
5252
import org.sonar.plugins.python.api.tree.Tree;
5353
import org.sonar.plugins.python.api.tree.Tree.Kind;
54+
import org.sonar.python.cfg.fixpoint.LiveVariablesAnalysis;
5455
import org.sonar.python.regex.PythonAnalyzerRegexSource;
5556
import org.sonar.python.regex.PythonRegexIssueLocation;
5657
import org.sonar.python.regex.RegexContext;
@@ -212,6 +213,11 @@ public ControlFlowGraph cfg(Tree tree) {
212213
return pythonVisitorContext.cfg(tree);
213214
}
214215

216+
@Override
217+
public LiveVariablesAnalysis lva(Tree tree) {
218+
return pythonVisitorContext.lva(tree);
219+
}
220+
215221
@Override
216222
public Set<Expression> valuesAtLocation(Name name) {
217223
return pythonVisitorContext.valuesAtLocation(name);

‎python-frontend/src/test/java/org/sonar/plugins/python/api/PythonVisitorContextTest.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.sonar.plugins.python.api.tree.Tree;
3030
import org.sonar.python.PythonTestUtils;
3131
import org.sonar.python.caching.CacheContextImpl;
32+
import org.sonar.python.cfg.fixpoint.LiveVariablesAnalysis;
3233
import org.sonar.python.index.Descriptor;
3334
import org.sonar.python.index.VariableDescriptor;
3435
import org.sonar.python.parser.PythonParser;
@@ -125,6 +126,40 @@ def foo():
125126

126127
}
127128

129+
@Test
130+
void lva_returns_null_when_no_cfg() {
131+
FileInput fileInput = PythonTestUtils.parse("def foo():\n x = 1");
132+
PythonFile pythonFile = pythonFile("my_module.py");
133+
var ctx = new PythonVisitorContext.Builder(fileInput, pythonFile).build();
134+
135+
FunctionDef functionDef = (FunctionDef) PythonTestUtils.getAllDescendant(fileInput, t -> t.is(Tree.Kind.FUNCDEF)).get(0);
136+
// A Name node has no CFG entry, so lva should return null
137+
assertThat(ctx.lva(functionDef.name())).isNull();
138+
}
139+
140+
@Test
141+
void lva_returns_analysis_for_function_def() {
142+
FileInput fileInput = PythonTestUtils.parse("def foo():\n x = 1\n return x");
143+
PythonFile pythonFile = pythonFile("my_module.py");
144+
var ctx = new PythonVisitorContext.Builder(fileInput, pythonFile).build();
145+
146+
FunctionDef functionDef = (FunctionDef) PythonTestUtils.getAllDescendant(fileInput, t -> t.is(Tree.Kind.FUNCDEF)).get(0);
147+
LiveVariablesAnalysis lva = ctx.lva(functionDef);
148+
assertThat(lva).isNotNull();
149+
}
150+
151+
@Test
152+
void lva_caches_result() {
153+
FileInput fileInput = PythonTestUtils.parse("def foo():\n x = 1\n return x");
154+
PythonFile pythonFile = pythonFile("my_module.py");
155+
var ctx = new PythonVisitorContext.Builder(fileInput, pythonFile).build();
156+
157+
FunctionDef functionDef = (FunctionDef) PythonTestUtils.getAllDescendant(fileInput, t -> t.is(Tree.Kind.FUNCDEF)).get(0);
158+
LiveVariablesAnalysis first = ctx.lva(functionDef);
159+
LiveVariablesAnalysis second = ctx.lva(functionDef);
160+
assertThat(first).isSameAs(second);
161+
}
162+
128163
@Test
129164
void sonar_product() {
130165
CacheContextImpl cacheContext = CacheContextImpl.dummyCache();

0 commit comments

Comments
 (0)