Skip to content

Commit c8de0ec

Browse files
committed
SONARPY-201 Simplify FileLinesVisitor
1 parent 7bcab61 commit c8de0ec

6 files changed

Lines changed: 57 additions & 129 deletions

File tree

python-squid/src/main/java/org/sonar/python/metrics/FileLinesVisitor.java

Lines changed: 5 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,8 @@
2626
import com.sonar.sslr.api.GenericTokenType;
2727
import com.sonar.sslr.api.Token;
2828
import com.sonar.sslr.api.Trivia;
29-
import java.util.HashMap;
30-
import java.util.Map;
3129
import java.util.Set;
32-
import org.sonar.api.batch.fs.FileSystem;
33-
import org.sonar.api.batch.fs.InputFile;
3430
import org.sonar.api.measures.CoreMetrics;
35-
import org.sonar.api.measures.FileLinesContext;
36-
import org.sonar.api.measures.FileLinesContextFactory;
3731
import org.sonar.python.DocstringExtractor;
3832
import org.sonar.python.PythonVisitor;
3933
import org.sonar.python.TokenLocation;
@@ -46,8 +40,6 @@ public class FileLinesVisitor extends PythonVisitor {
4640

4741
private static final PythonCommentAnalyser COMMENT_ANALYSER = new PythonCommentAnalyser();
4842

49-
private final FileLinesContextFactory fileLinesContextFactory;
50-
5143
private boolean seenFirstToken;
5244

5345
private final boolean ignoreHeaderComments;
@@ -56,15 +48,8 @@ public class FileLinesVisitor extends PythonVisitor {
5648
private Set<Integer> linesOfCode = Sets.newHashSet();
5749
private Set<Integer> linesOfComments = Sets.newHashSet();
5850
private Set<Integer> linesOfDocstring = Sets.newHashSet();
59-
private final FileSystem fileSystem;
60-
private final Map<InputFile, Set<Integer>> allLinesOfCode = new HashMap<>();
61-
62-
public FileLinesVisitor(
63-
FileLinesContextFactory fileLinesContextFactory,
64-
FileSystem fileSystem,
65-
boolean ignoreHeaderComments) {
66-
this.fileLinesContextFactory = fileLinesContextFactory;
67-
this.fileSystem = fileSystem;
51+
52+
public FileLinesVisitor(boolean ignoreHeaderComments) {
6853
this.ignoreHeaderComments = ignoreHeaderComments;
6954
}
7055

@@ -141,43 +126,23 @@ public void visitComment(Trivia trivia) {
141126

142127
@Override
143128
public void leaveFile(AstNode astNode) {
144-
InputFile inputFile = fileSystem.inputFile(fileSystem.predicates().is(getContext().pythonFile().file()));
145-
if (inputFile == null){
146-
throw new IllegalStateException("InputFile is null, but it should not be.");
147-
}
148-
FileLinesContext fileLinesContext = fileLinesContextFactory.createFor(inputFile);
149-
150129
// account for the docstring lines
151130
for (Integer line : linesOfDocstring) {
152131
linesOfCode.remove(line);
153132
linesOfComments.add(line);
154133
}
155-
156-
for (int line : linesOfCode) {
157-
fileLinesContext.setIntValue(CoreMetrics.NCLOC_DATA_KEY, line, 1);
158-
}
159-
for (int line : linesOfComments) {
160-
fileLinesContext.setIntValue(CoreMetrics.COMMENT_LINES_DATA_KEY, line, 1);
161-
}
162-
fileLinesContext.save();
163-
164-
allLinesOfCode.put(inputFile, ImmutableSet.copyOf(linesOfCode));
165134
}
166135

167136
public Set<Integer> getLinesWithNoSonar() {
168-
return noSonar;
137+
return ImmutableSet.copyOf(noSonar);
169138
}
170139

171140
public Set<Integer> getLinesOfCode() {
172-
return linesOfCode;
141+
return ImmutableSet.copyOf(linesOfCode);
173142
}
174143

175144
public Set<Integer> getLinesOfComments() {
176-
return linesOfComments;
177-
}
178-
179-
public Map<InputFile, Set<Integer>> linesOfCodeByFile() {
180-
return allLinesOfCode;
145+
return ImmutableSet.copyOf(linesOfComments);
181146
}
182147

183148
private static class PythonCommentAnalyser {

python-squid/src/main/java/org/sonar/python/metrics/MetricVisitor.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@
2222
import com.sonar.sslr.api.AstNode;
2323
import java.util.ArrayList;
2424
import java.util.List;
25-
import org.sonar.api.batch.fs.FileSystem;
26-
import org.sonar.api.measures.FileLinesContextFactory;
2725
import org.sonar.python.PythonVisitor;
2826
import org.sonar.python.PythonVisitorContext;
2927
import org.sonar.python.api.PythonGrammar;
@@ -36,8 +34,8 @@ public class MetricVisitor extends PythonVisitor {
3634
private final FileLinesVisitor fileLinesVisitor;
3735
private List<Integer> functionComplexities;
3836

39-
public MetricVisitor(FileLinesContextFactory fileLinesContextFactory, FileSystem fileSystem, boolean ignoreHeaderComments) {
40-
fileLinesVisitor = new FileLinesVisitor(fileLinesContextFactory, fileSystem, ignoreHeaderComments);
37+
public MetricVisitor(boolean ignoreHeaderComments) {
38+
fileLinesVisitor = new FileLinesVisitor(ignoreHeaderComments);
4139
}
4240

4341
@Override

python-squid/src/test/java/org/sonar/python/FileLinesVisitorTest.java

Lines changed: 9 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -20,84 +20,38 @@
2020
package org.sonar.python;
2121

2222
import java.io.File;
23-
import java.nio.file.Paths;
2423
import org.junit.Test;
25-
import org.sonar.api.batch.fs.internal.DefaultFileSystem;
26-
import org.sonar.api.batch.fs.internal.DefaultInputFile;
27-
import org.sonar.api.measures.CoreMetrics;
28-
import org.sonar.api.measures.FileLinesContext;
29-
import org.sonar.api.measures.FileLinesContextFactory;
3024
import org.sonar.python.metrics.FileLinesVisitor;
3125

3226
import static org.assertj.core.api.Assertions.assertThat;
33-
import static org.mockito.Mockito.mock;
34-
import static org.mockito.Mockito.verify;
35-
import static org.mockito.Mockito.verifyNoMoreInteractions;
36-
import static org.mockito.Mockito.when;
3727

3828
public class FileLinesVisitorTest {
3929

4030
private static final File BASE_DIR = new File("src/test/resources/metrics");
4131

42-
private DefaultFileSystem fileSystem = new DefaultFileSystem(Paths.get(""));
43-
44-
private FileLinesContextFactory fileLinesContextFactory = mock(FileLinesContextFactory.class);
45-
46-
private FileLinesContext fileLinesContext = mock(FileLinesContext.class);
47-
4832
@Test
4933
public void test() {
50-
DefaultInputFile inputFile = initFile("file_lines.py");
34+
FileLinesVisitor visitor = new FileLinesVisitor(false);
5135

52-
FileLinesVisitor visitor = new FileLinesVisitor(fileLinesContextFactory, fileSystem, false);
53-
54-
TestPythonVisitorRunner.scanFile(inputFile.file(), visitor);
36+
TestPythonVisitorRunner.scanFile(new File(BASE_DIR, "file_lines.py"), visitor);
5537

5638
assertThat(visitor.getLinesOfCode()).hasSize(12);
57-
assertThat(visitor.linesOfCodeByFile()).hasSize(1);
58-
assertThat(visitor.linesOfCodeByFile().get(inputFile)).as("Lines of codes").containsOnly(2, 4, 7, 8, 9, 10, 11, 12, 14, 15, 17, 21);
59-
verifyInvocation(fileLinesContext, CoreMetrics.NCLOC_DATA_KEY, 2, 4, 7, 8, 9, 10, 11, 12, 14, 15, 17, 21);
39+
assertThat(visitor.getLinesOfCode()).containsOnly(2, 4, 7, 8, 9, 10, 11, 12, 14, 15, 17, 21);
6040

6141
assertThat(visitor.getLinesOfComments()).hasSize(9);
62-
verifyInvocation(fileLinesContext, CoreMetrics.COMMENT_LINES_DATA_KEY, 1, 4, 6, 13, 14, 17, 18, 19, 20);
42+
assertThat(visitor.getLinesOfComments()).containsOnly(1, 4, 6, 13, 14, 17, 18, 19, 20);
6343

64-
verify(fileLinesContext).save();
65-
verifyNoMoreInteractions(fileLinesContext);
44+
assertThat(visitor.getLinesWithNoSonar()).containsOnly(11);
6645
}
6746

6847
@Test
6948
public void test_ignoreHeaderComments() {
70-
DefaultInputFile inputFile = initFile("file_lines_header_comments.py");
71-
72-
FileLinesVisitor visitor = new FileLinesVisitor(fileLinesContextFactory, fileSystem, true);
49+
FileLinesVisitor visitor = new FileLinesVisitor(true);
7350

74-
TestPythonVisitorRunner.scanFile(inputFile.file(), visitor);
75-
76-
assertThat(visitor.linesOfCodeByFile()).hasSize(1);
77-
assertThat(visitor.linesOfCodeByFile().get(inputFile)).as("Lines of codes").containsOnly(2, 4);
78-
79-
verifyInvocation(fileLinesContext, CoreMetrics.NCLOC_DATA_KEY, 2, 4);
80-
verifyInvocation(fileLinesContext, CoreMetrics.COMMENT_LINES_DATA_KEY, 4);
81-
verify(fileLinesContext).save();
82-
verifyNoMoreInteractions(fileLinesContext);
83-
}
84-
85-
private DefaultInputFile initFile(String fileName) {
86-
File file = new File(BASE_DIR, fileName);
87-
DefaultInputFile inputFile = new DefaultInputFile("", file.getPath());
88-
fileSystem.add(inputFile);
89-
when(fileLinesContextFactory.createFor(inputFile)).thenReturn(fileLinesContext);
90-
return inputFile;
91-
}
51+
TestPythonVisitorRunner.scanFile(new File(BASE_DIR, "file_lines_header_comments.py"), visitor);
9252

93-
/**
94-
* Checks that method fileLinesContext.setIntValue has been invoked for the specified
95-
* metrics and for every specified line.
96-
*/
97-
private void verifyInvocation(FileLinesContext fileLinesContext, String metric, int... lines) {
98-
for (int i = 0; i < lines.length; i++) {
99-
verify(fileLinesContext).setIntValue(metric, lines[i], 1);
100-
}
53+
assertThat(visitor.getLinesOfCode()).containsOnly(2, 4);
54+
assertThat(visitor.getLinesOfComments()).containsOnly(4);
10155
}
10256

10357
}

python-squid/src/test/java/org/sonar/python/metrics/MetricVisitorTest.java

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,9 @@
2121

2222
import java.io.File;
2323
import org.junit.Test;
24-
import org.sonar.api.batch.fs.internal.DefaultFileSystem;
25-
import org.sonar.api.batch.fs.internal.DefaultInputFile;
26-
import org.sonar.api.measures.FileLinesContext;
27-
import org.sonar.api.measures.FileLinesContextFactory;
2824
import org.sonar.python.TestPythonVisitorRunner;
2925

3026
import static org.fest.assertions.Assertions.assertThat;
31-
import static org.mockito.Mockito.mock;
32-
import static org.mockito.Mockito.when;
3327

3428
public class MetricVisitorTest {
3529

@@ -60,14 +54,8 @@ public void function_complexities() throws Exception {
6054

6155
private MetricVisitor metrics(String fileName) {
6256
File baseDir = new File("src/test/resources/metrics/");
63-
DefaultFileSystem fileSystem = new DefaultFileSystem(baseDir);
64-
FileLinesContextFactory fileLinesContextFactory = mock(FileLinesContextFactory.class);
6557
File file = new File(baseDir, fileName);
66-
DefaultInputFile inputFile = new DefaultInputFile("", file.getPath());
67-
fileSystem.add(inputFile);
68-
FileLinesContext fileLinesContext = mock(FileLinesContext.class);
69-
when(fileLinesContextFactory.createFor(inputFile)).thenReturn(fileLinesContext);
70-
MetricVisitor visitor = new MetricVisitor(fileLinesContextFactory, fileSystem, true);
58+
MetricVisitor visitor = new MetricVisitor(true);
7159
TestPythonVisitorRunner.scanFile(file, visitor);
7260
return visitor;
7361
}

sonar-python-plugin/src/main/java/org/sonar/plugins/python/PythonScanner.java

Lines changed: 37 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@
2222
import com.sonar.sslr.api.Grammar;
2323
import com.sonar.sslr.api.RecognitionException;
2424
import com.sonar.sslr.impl.Parser;
25+
import java.util.HashMap;
2526
import java.util.List;
27+
import java.util.Map;
28+
import java.util.Set;
2629
import org.sonar.api.batch.fs.InputFile;
2730
import org.sonar.api.batch.fs.TextRange;
2831
import org.sonar.api.batch.rule.Checks;
@@ -32,6 +35,8 @@
3235
import org.sonar.api.ce.measure.RangeDistributionBuilder;
3336
import org.sonar.api.issue.NoSonarFilter;
3437
import org.sonar.api.measures.CoreMetrics;
38+
import org.sonar.api.measures.FileLinesContext;
39+
import org.sonar.api.measures.FileLinesContextFactory;
3540
import org.sonar.api.measures.Metric;
3641
import org.sonar.api.rule.RuleKey;
3742
import org.sonar.api.utils.log.Logger;
@@ -54,19 +59,22 @@ public class PythonScanner {
5459

5560
private final SensorContext context;
5661
private final Parser<Grammar> parser;
57-
private final MetricVisitor metricVisitor;
5862
private final List<InputFile> inputFiles;
5963
private final Checks<PythonCheck> checks;
64+
private final FileLinesContextFactory fileLinesContextFactory;
6065
private final NoSonarFilter noSonarFilter;
66+
private Map<InputFile, Set<Integer>> linesOfCodeByFile;
6167

62-
public PythonScanner(SensorContext context, Checks<PythonCheck> checks, MetricVisitor metricVisitor,
63-
NoSonarFilter noSonarFilter, List<InputFile> inputFiles) {
68+
69+
public PythonScanner(SensorContext context, Checks<PythonCheck> checks,
70+
FileLinesContextFactory fileLinesContextFactory, NoSonarFilter noSonarFilter, List<InputFile> inputFiles) {
6471
this.context = context;
6572
this.checks = checks;
66-
this.metricVisitor = metricVisitor;
73+
this.fileLinesContextFactory = fileLinesContextFactory;
6774
this.noSonarFilter = noSonarFilter;
6875
this.inputFiles = inputFiles;
6976
this.parser = PythonParser.create(new PythonConfiguration(context.fileSystem().encoding()));
77+
this.linesOfCodeByFile = new HashMap<>();
7078
}
7179

7280
public void scanFiles() {
@@ -137,20 +145,35 @@ private static NewIssueLocation newLocation(InputFile inputFile, NewIssue issue,
137145
}
138146

139147
private void saveMeasures(InputFile inputFile, PythonVisitorContext visitorContext) {
148+
boolean ignoreHeaderComments = new PythonConfiguration(context.fileSystem().encoding()).getIgnoreHeaderComments();
149+
MetricVisitor metricVisitor = new MetricVisitor(ignoreHeaderComments);
140150
metricVisitor.scanFile(visitorContext);
141151
FileLinesVisitor fileLinesVisitor = metricVisitor.fileLinesVisitor();
142152

143153
noSonarFilter.noSonarInFile(inputFile, fileLinesVisitor.getLinesWithNoSonar());
144154

145-
saveFilesComplexityDistribution(inputFile);
146-
saveFunctionsComplexityDistribution(inputFile);
155+
saveFilesComplexityDistribution(inputFile, metricVisitor);
156+
saveFunctionsComplexityDistribution(inputFile, metricVisitor);
147157

148-
saveMetricOnFile(inputFile, CoreMetrics.NCLOC, fileLinesVisitor.getLinesOfCode().size());
158+
Set<Integer> linesOfCode = fileLinesVisitor.getLinesOfCode();
159+
Set<Integer> linesOfComments = fileLinesVisitor.getLinesOfComments();
160+
saveMetricOnFile(inputFile, CoreMetrics.NCLOC, linesOfCode.size());
149161
saveMetricOnFile(inputFile, CoreMetrics.STATEMENTS, metricVisitor.numberOfStatements());
150162
saveMetricOnFile(inputFile, CoreMetrics.FUNCTIONS, metricVisitor.numberOfFunctions());
151163
saveMetricOnFile(inputFile, CoreMetrics.CLASSES, metricVisitor.numberOfClasses());
152164
saveMetricOnFile(inputFile, CoreMetrics.COMPLEXITY, metricVisitor.complexity());
153-
saveMetricOnFile(inputFile, CoreMetrics.COMMENT_LINES, fileLinesVisitor.getLinesOfComments().size());
165+
saveMetricOnFile(inputFile, CoreMetrics.COMMENT_LINES, linesOfComments.size());
166+
167+
FileLinesContext fileLinesContext = fileLinesContextFactory.createFor(inputFile);
168+
for (int line : linesOfCode) {
169+
fileLinesContext.setIntValue(CoreMetrics.NCLOC_DATA_KEY, line, 1);
170+
}
171+
for (int line : linesOfComments) {
172+
fileLinesContext.setIntValue(CoreMetrics.COMMENT_LINES_DATA_KEY, line, 1);
173+
}
174+
fileLinesContext.save();
175+
176+
linesOfCodeByFile.put(inputFile, linesOfCode);
154177
}
155178

156179
private void saveMetricOnFile(InputFile inputFile, Metric<Integer> metric, Integer value) {
@@ -161,7 +184,7 @@ private void saveMetricOnFile(InputFile inputFile, Metric<Integer> metric, Integ
161184
.save();
162185
}
163186

164-
private void saveFunctionsComplexityDistribution(InputFile inputFile) {
187+
private void saveFunctionsComplexityDistribution(InputFile inputFile, MetricVisitor metricVisitor) {
165188
RangeDistributionBuilder complexityDistribution = new RangeDistributionBuilder(FUNCTIONS_DISTRIB_BOTTOM_LIMITS);
166189
for (Integer functionComplexity : metricVisitor.functionComplexities()) {
167190
complexityDistribution.add(functionComplexity);
@@ -174,7 +197,7 @@ private void saveFunctionsComplexityDistribution(InputFile inputFile) {
174197
.save();
175198
}
176199

177-
private void saveFilesComplexityDistribution(InputFile inputFile) {
200+
private void saveFilesComplexityDistribution(InputFile inputFile, MetricVisitor metricVisitor) {
178201
RangeDistributionBuilder complexityDistribution = new RangeDistributionBuilder(FILES_DISTRIB_BOTTOM_LIMITS);
179202
complexityDistribution.add(metricVisitor.complexity());
180203
context.<String>newMeasure()
@@ -183,4 +206,8 @@ private void saveFilesComplexityDistribution(InputFile inputFile) {
183206
.withValue(complexityDistribution.build())
184207
.save();
185208
}
209+
210+
public Map<InputFile, Set<Integer>> linesOfCodeByFile() {
211+
return linesOfCodeByFile;
212+
}
186213
}

sonar-python-plugin/src/main/java/org/sonar/plugins/python/PythonSquidSensor.java

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,7 @@
3535
import org.sonar.api.utils.Version;
3636
import org.sonar.plugins.python.coverage.PythonCoverageSensor;
3737
import org.sonar.python.PythonCheck;
38-
import org.sonar.python.PythonConfiguration;
3938
import org.sonar.python.checks.CheckList;
40-
import org.sonar.python.metrics.MetricVisitor;
4139

4240
public final class PythonSquidSensor implements Sensor {
4341

@@ -65,17 +63,15 @@ public void describe(SensorDescriptor descriptor) {
6563

6664
@Override
6765
public void execute(SensorContext context) {
68-
PythonConfiguration conf = new PythonConfiguration(context.fileSystem().encoding());
69-
70-
MetricVisitor metricVisitor = new MetricVisitor(fileLinesContextFactory, context.fileSystem(), conf.getIgnoreHeaderComments());
7166
FilePredicates p = context.fileSystem().predicates();
7267
List<InputFile> inputFiles = ImmutableList.copyOf(
7368
context.fileSystem().inputFiles(p.and(p.hasType(InputFile.Type.MAIN), p.hasLanguage(Python.KEY))));
7469

75-
new PythonScanner(context, checks, metricVisitor, noSonarFilter, inputFiles).scanFiles();
70+
PythonScanner scanner = new PythonScanner(context, checks, fileLinesContextFactory, noSonarFilter, inputFiles);
71+
scanner.scanFiles();
7672

7773
if (!isSonarLint(context)) {
78-
(new PythonCoverageSensor()).execute(context, metricVisitor.fileLinesVisitor().linesOfCodeByFile());
74+
(new PythonCoverageSensor()).execute(context, scanner.linesOfCodeByFile());
7975
}
8076
}
8177

0 commit comments

Comments
 (0)