From 3fdcf61545c23a2ae219fd8c8f44b44085f19aef Mon Sep 17 00:00:00 2001 From: Lukasz Jarocki Date: Mon, 12 Sep 2022 15:55:30 +0200 Subject: [PATCH] SONAR-17288 add medium tests --- .../sonar/xoo/rule/MultilineIssuesSensor.java | 247 ++++++++++++------ .../xoo/rule/MultilineIssuesSensorTest.java | 90 +++++++ .../resources/org/sonar/xoo/rule/dataflow.xoo | 11 + .../issues/MultilineIssuesMediumTest.java | 19 +- .../xources/hello/FlowTypes.xoo | 11 + 5 files changed, 298 insertions(+), 80 deletions(-) create mode 100644 plugins/sonar-xoo-plugin/src/test/java/org/sonar/xoo/rule/MultilineIssuesSensorTest.java create mode 100644 plugins/sonar-xoo-plugin/src/test/resources/org/sonar/xoo/rule/dataflow.xoo create mode 100644 sonar-scanner-engine/test-resources/mediumtest/xoo/sample-multiline/xources/hello/FlowTypes.xoo diff --git a/plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/rule/MultilineIssuesSensor.java b/plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/rule/MultilineIssuesSensor.java index e56b89f06ff..933bd2490ca 100644 --- a/plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/rule/MultilineIssuesSensor.java +++ b/plugins/sonar-xoo-plugin/src/main/java/org/sonar/xoo/rule/MultilineIssuesSensor.java @@ -19,17 +19,17 @@ */ package org.sonar.xoo.rule; -import com.google.common.collect.HashBasedTable; -import com.google.common.collect.Lists; -import com.google.common.collect.Table; import java.io.IOException; -import java.nio.file.Files; -import java.util.Collections; +import java.util.Collection; import java.util.HashMap; +import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.TreeMap; import java.util.regex.Matcher; import java.util.regex.Pattern; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; import org.sonar.api.batch.fs.FilePredicates; import org.sonar.api.batch.fs.FileSystem; import org.sonar.api.batch.fs.InputFile; @@ -39,19 +39,30 @@ import org.sonar.api.batch.sensor.Sensor; import org.sonar.api.batch.sensor.SensorContext; import org.sonar.api.batch.sensor.SensorDescriptor; import org.sonar.api.batch.sensor.issue.NewIssue; +import org.sonar.api.batch.sensor.issue.NewIssue.FlowType; import org.sonar.api.batch.sensor.issue.NewIssueLocation; import org.sonar.api.rule.RuleKey; import org.sonar.xoo.Xoo; +import static org.sonar.api.batch.sensor.issue.NewIssue.FlowType.DATA; +import static org.sonar.api.batch.sensor.issue.NewIssue.FlowType.EXECUTION; + public class MultilineIssuesSensor implements Sensor { public static final String RULE_KEY = "MultilineIssue"; + private static final Pattern START_ISSUE_PATTERN = Pattern.compile("\\{xoo-start-issue:([0-9]+)\\}"); private static final Pattern END_ISSUE_PATTERN = Pattern.compile("\\{xoo-end-issue:([0-9]+)\\}"); private static final Pattern START_FLOW_PATTERN = Pattern.compile("\\{xoo-start-flow:([0-9]+):([0-9]+):([0-9]+)\\}"); private static final Pattern END_FLOW_PATTERN = Pattern.compile("\\{xoo-end-flow:([0-9]+):([0-9]+):([0-9]+)\\}"); + private static final Pattern START_DATA_FLOW_PATTERN = Pattern.compile("\\{xoo-start-data-flow:([0-9]+):([0-9]+):([0-9]+)\\}"); + private static final Pattern END_DATA_FLOW_PATTERN = Pattern.compile("\\{xoo-end-data-flow:([0-9]+):([0-9]+):([0-9]+)\\}"); + + private static final Pattern START_EXECUTION_FLOW_PATTERN = Pattern.compile("\\{xoo-start-execution-flow:([0-9]+):([0-9]+):([0-9]+)\\}"); + private static final Pattern END_EXECUTION_FLOW_PATTERN = Pattern.compile("\\{xoo-end-execution-flow:([0-9]+):([0-9]+):([0-9]+)\\}"); + @Override public void describe(SensorDescriptor descriptor) { descriptor @@ -69,83 +80,39 @@ public class MultilineIssuesSensor implements Sensor { } } - private static void createIssues(InputFile file, SensorContext context) { - Map startIssuesPositions = new HashMap<>(); - Map endIssuesPositions = new HashMap<>(); - Map> startFlowsPositions = new HashMap<>(); - Map> endFlowsPositions = new HashMap<>(); - - parseIssues(file, context, startIssuesPositions, endIssuesPositions); - parseFlows(file, startFlowsPositions, endFlowsPositions); - createIssues(file, context, startIssuesPositions, endIssuesPositions, startFlowsPositions, endFlowsPositions); - } - - private static void parseFlows(InputFile file, Map> startFlowsPositions, - Map> endFlowsPositions) { - int currentLine = 0; - try { - for (String lineStr : Files.readAllLines(file.path(), file.charset())) { - currentLine++; - - Matcher m = START_FLOW_PATTERN.matcher(lineStr); - while (m.find()) { - Integer issueId = Integer.parseInt(m.group(1)); - Integer issueFlowId = Integer.parseInt(m.group(2)); - Integer issueFlowNum = Integer.parseInt(m.group(3)); - TextPointer newPointer = file.newPointer(currentLine, m.end()); - if (!startFlowsPositions.containsKey(issueId)) { - startFlowsPositions.put(issueId, HashBasedTable.create()); - } - startFlowsPositions.get(issueId).row(issueFlowId).put(issueFlowNum, newPointer); - } - - m = END_FLOW_PATTERN.matcher(lineStr); - while (m.find()) { - Integer issueId = Integer.parseInt(m.group(1)); - Integer issueFlowId = Integer.parseInt(m.group(2)); - Integer issueFlowNum = Integer.parseInt(m.group(3)); - TextPointer newPointer = file.newPointer(currentLine, m.start()); - if (!endFlowsPositions.containsKey(issueId)) { - endFlowsPositions.put(issueId, HashBasedTable.create()); - } - endFlowsPositions.get(issueId).row(issueFlowId).put(issueFlowNum, newPointer); - } - } - } catch (IOException e) { - throw new IllegalStateException("Unable to read file", e); - } + private void createIssues(InputFile file, SensorContext context) { + Collection issues = parseIssues(file); + FlowIndex flowIndex = new FlowIndex(); + parseFlows(flowIndex, file, START_FLOW_PATTERN, END_FLOW_PATTERN, null); + parseFlows(flowIndex, file, START_DATA_FLOW_PATTERN, END_DATA_FLOW_PATTERN, DATA); + parseFlows(flowIndex, file, START_EXECUTION_FLOW_PATTERN, END_EXECUTION_FLOW_PATTERN, EXECUTION); + createIssues(file, context, issues, flowIndex); } - private static void createIssues(InputFile file, SensorContext context, Map startPositions, - Map endPositions, Map> startFlowsPositions, - Map> endFlowsPositions) { + private static void createIssues(InputFile file, SensorContext context, Collection parsedIssues, FlowIndex flowIndex) { RuleKey ruleKey = RuleKey.of(XooRulesDefinition.XOO_REPOSITORY, RULE_KEY); - for (Map.Entry entry : startPositions.entrySet()) { + for (ParsedIssue parsedIssue : parsedIssues) { NewIssue newIssue = context.newIssue().forRule(ruleKey); - Integer issueId = entry.getKey(); NewIssueLocation primaryLocation = newIssue.newLocation() .on(file) - .at(file.newRange(entry.getValue(), endPositions.get(issueId))); + .at(file.newRange(parsedIssue.start, parsedIssue.end)); newIssue.at(primaryLocation.message("Primary location")); - if (startFlowsPositions.containsKey(issueId)) { - Table flows = startFlowsPositions.get(issueId); - for (Map.Entry> flowEntry : flows.rowMap().entrySet()) { - Integer flowId = flowEntry.getKey(); - List flowLocations = Lists.newArrayList(); - List flowNums = Lists.newArrayList(flowEntry.getValue().keySet()); - Collections.sort(flowNums); - for (Integer flowNum : flowNums) { - TextPointer start = flowEntry.getValue().get(flowNum); - TextPointer end = endFlowsPositions.get(issueId).row(flowId).get(flowNum); - NewIssueLocation newLocation = newIssue.newLocation() + + IssueFlows issueFlows = flowIndex.getFlows(parsedIssue.issueId); + if (issueFlows != null) { + for (ParsedFlow flow : issueFlows.getFlows()) { + List flowLocations = new LinkedList<>(); + + for (ParsedFlowLocation flowLocation : flow.getLocations()) { + flowLocations.add(newIssue.newLocation() .on(file) - .at(file.newRange(start, end)) - .message("Flow step #" + flowNum); - flowLocations.add(newLocation); + .at(file.newRange(flowLocation.start, flowLocation.end)) + .message("Flow step #" + flowLocation.flowLocationId)); } - if (flowLocations.size() == 1) { - newIssue.addLocation(flowLocations.get(0)); + + if (flow.getType() != null) { + newIssue.addFlow(flowLocations, flow.getType(), "flow #" + flow.getFlowId()); } else { newIssue.addFlow(flowLocations); } @@ -155,30 +122,152 @@ public class MultilineIssuesSensor implements Sensor { } } - private static void parseIssues(InputFile file, SensorContext context, Map startPositions, - Map endPositions) { + private static Collection parseIssues(InputFile file) { + Map issuesById = new HashMap<>(); + int currentLine = 0; try { - for (String lineStr : Files.readAllLines(file.path(), file.charset())) { + for (String lineStr : file.contents().split("\\r?\\n")) { currentLine++; Matcher m = START_ISSUE_PATTERN.matcher(lineStr); while (m.find()) { Integer issueId = Integer.parseInt(m.group(1)); - TextPointer newPointer = file.newPointer(currentLine, m.end()); - startPositions.put(issueId, newPointer); + issuesById.computeIfAbsent(issueId, ParsedIssue::new).start = file.newPointer(currentLine, m.end()); } m = END_ISSUE_PATTERN.matcher(lineStr); while (m.find()) { Integer issueId = Integer.parseInt(m.group(1)); - TextPointer newPointer = file.newPointer(currentLine, m.start()); - endPositions.put(issueId, newPointer); + issuesById.computeIfAbsent(issueId, ParsedIssue::new).end = file.newPointer(currentLine, m.start()); } } } catch (IOException e) { throw new IllegalStateException("Unable to read file", e); } + return issuesById.values(); } + private void parseFlows(FlowIndex flowIndex, InputFile file, Pattern flowStartPattern, Pattern flowEndPattern, @Nullable FlowType flowType) { + int currentLine = 0; + try { + for (String lineStr : file.contents().split("\\r?\\n")) { + currentLine++; + + Matcher m = flowStartPattern.matcher(lineStr); + while (m.find()) { + ParsedFlowLocation flowLocation = new ParsedFlowLocation(Integer.parseInt(m.group(1)), Integer.parseInt(m.group(2)), Integer.parseInt(m.group(3))); + flowLocation.start = file.newPointer(currentLine, m.end()); + flowIndex.addLocation(flowLocation, flowType); + } + + m = flowEndPattern.matcher(lineStr); + while (m.find()) { + ParsedFlowLocation flowLocation = new ParsedFlowLocation(Integer.parseInt(m.group(1)), Integer.parseInt(m.group(2)), Integer.parseInt(m.group(3))); + flowLocation.end = file.newPointer(currentLine, m.start()); + flowIndex.addLocation(flowLocation, flowType); + } + } + } catch (IOException e) { + throw new IllegalStateException("Unable to read file", e); + } + } + + private static class ParsedIssue { + private final int issueId; + + private TextPointer start; + private TextPointer end; + + private ParsedIssue(int issueId) { + this.issueId = issueId; + } + } + + private static class FlowIndex { + private final Map flowsByIssueId = new HashMap<>(); + + public void addLocation(ParsedFlowLocation flowLocation, @Nullable FlowType type) { + flowsByIssueId.computeIfAbsent(flowLocation.issueId, issueId -> new IssueFlows()).addLocation(flowLocation, type); + } + + @CheckForNull + public IssueFlows getFlows(int issueId) { + return flowsByIssueId.get(issueId); + } + } + + private static class IssueFlows { + private final Map flowById = new TreeMap<>(); + + private void addLocation(ParsedFlowLocation flowLocation, @Nullable FlowType type) { + flowById.computeIfAbsent(flowLocation.flowId, flowId -> new ParsedFlow(flowId, type)).addLocation(flowLocation); + } + + Collection getFlows() { + return flowById.values(); + } + } + + private static class ParsedFlow { + private final int id; + private final FlowType type; + private final Map locationsById = new TreeMap<>(); + private String description; + + private ParsedFlow(int id, @Nullable FlowType type) { + this.id = id; + this.type = type; + } + + private void addLocation(ParsedFlowLocation flowLocation) { + if (locationsById.containsKey(flowLocation.flowLocationId)) { + if (flowLocation.start != null) { + locationsById.get(flowLocation.flowLocationId).start = flowLocation.start; + } else if (flowLocation.end != null) { + locationsById.get(flowLocation.flowLocationId).end = flowLocation.end; + } + } else { + locationsById.put(flowLocation.flowLocationId, flowLocation); + } + } + + public Collection getLocations() { + return locationsById.values(); + + } + + public void setDescription(@Nullable String description) { + this.description = description; + } + + @CheckForNull + public String getDescription() { + return description; + } + + @CheckForNull + FlowType getType() { + return type; + } + + int getFlowId() { + return id; + } + } + + private static class ParsedFlowLocation { + private final int issueId; + private final int flowId; + private final int flowLocationId; + + private TextPointer start; + private TextPointer end; + + public ParsedFlowLocation(int issueId, int flowId, int flowLocationId) { + this.issueId = issueId; + this.flowId = flowId; + this.flowLocationId = flowLocationId; + } + } } diff --git a/plugins/sonar-xoo-plugin/src/test/java/org/sonar/xoo/rule/MultilineIssuesSensorTest.java b/plugins/sonar-xoo-plugin/src/test/java/org/sonar/xoo/rule/MultilineIssuesSensorTest.java new file mode 100644 index 00000000000..449ed220f5b --- /dev/null +++ b/plugins/sonar-xoo-plugin/src/test/java/org/sonar/xoo/rule/MultilineIssuesSensorTest.java @@ -0,0 +1,90 @@ +/* + * SonarQube + * Copyright (C) 2009-2022 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.xoo.rule; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.stream.Collectors; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.sonar.api.batch.fs.InputFile; +import org.sonar.api.batch.fs.internal.DefaultFileSystem; +import org.sonar.api.batch.fs.internal.DefaultInputFile; +import org.sonar.api.batch.fs.internal.TestInputFileBuilder; +import org.sonar.api.batch.rule.ActiveRule; +import org.sonar.api.batch.rule.ActiveRules; +import org.sonar.api.batch.sensor.internal.SensorContextTester; +import org.sonar.api.batch.sensor.issue.Issue; +import org.sonar.api.batch.sensor.issue.internal.DefaultIssueFlow; +import org.sonar.api.batch.sensor.issue.internal.DefaultIssueFlow.Type; +import org.sonar.api.internal.apachecommons.io.IOUtils; +import org.sonar.xoo.Xoo; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class MultilineIssuesSensorTest { + + @Rule + public TemporaryFolder temp = new TemporaryFolder(); + + private final ActiveRules activeRules = mock(ActiveRules.class); + + private final MultilineIssuesSensor sensor = new MultilineIssuesSensor(); + + @Before + public void before() { + when(activeRules.find(any())).thenReturn(mock(ActiveRule.class)); + } + + @Test + public void execute_dataAndExecutionFlowsAreDetected() throws IOException { + DefaultInputFile inputFile = newTestFile(IOUtils.toString(getClass().getResource("dataflow.xoo"), StandardCharsets.UTF_8)); + + DefaultFileSystem fs = new DefaultFileSystem(temp.newFolder()); + fs.add(inputFile); + + SensorContextTester sensorContextTester = SensorContextTester.create(fs.baseDir()); + sensorContextTester.setFileSystem(fs); + sensor.execute(sensorContextTester); + + assertThat(sensorContextTester.allIssues()).hasSize(1); + + List flows = sensorContextTester.allIssues().iterator().next().flows(); + assertThat(flows).hasSize(2); + + List defaultIssueFlows = flows.stream().map(DefaultIssueFlow.class::cast).collect(Collectors.toList()); + + assertThat(defaultIssueFlows).extracting(DefaultIssueFlow::getType).containsExactlyInAnyOrder(Type.DATA, Type.EXECUTION); + } + + private DefaultInputFile newTestFile(String content) { + return new TestInputFileBuilder("foo", "src/Foo.xoo") + .setLanguage(Xoo.KEY) + .setType(InputFile.Type.MAIN) + .setContents(content) + .build(); + } +} diff --git a/plugins/sonar-xoo-plugin/src/test/resources/org/sonar/xoo/rule/dataflow.xoo b/plugins/sonar-xoo-plugin/src/test/resources/org/sonar/xoo/rule/dataflow.xoo new file mode 100644 index 00000000000..848b32dc3e2 --- /dev/null +++ b/plugins/sonar-xoo-plugin/src/test/resources/org/sonar/xoo/rule/dataflow.xoo @@ -0,0 +1,11 @@ +{xoo-start-issue:0} + +{xoo-start-execution-flow:0:2:1} + +{xoo-end-execution-flow:0:2:1} + +{xoo-start-data-flow:0:1:1} + +hello +{xoo-end-data-flow:0:1:1} +{xoo-end-issue:0} diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/issues/MultilineIssuesMediumTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/issues/MultilineIssuesMediumTest.java index 929e5d11ad6..3066e4a0b39 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/issues/MultilineIssuesMediumTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/mediumtest/issues/MultilineIssuesMediumTest.java @@ -26,15 +26,18 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; -import org.sonar.scanner.mediumtest.ScannerMediumTester; import org.sonar.scanner.mediumtest.AnalysisResult; +import org.sonar.scanner.mediumtest.ScannerMediumTester; +import org.sonar.scanner.protocol.output.ScannerReport; import org.sonar.scanner.protocol.output.ScannerReport.Flow; +import org.sonar.scanner.protocol.output.ScannerReport.FlowType; import org.sonar.scanner.protocol.output.ScannerReport.Issue; import org.sonar.scanner.protocol.output.ScannerReport.IssueLocation; import org.sonar.xoo.XooPlugin; import org.sonar.xoo.rule.XooRulesDefinition; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.tuple; public class MultilineIssuesMediumTest { @@ -118,4 +121,18 @@ public class MultilineIssuesMediumTest { assertThat(flow.getLocationList()).hasSize(2); // TODO more assertions } + + @Test + public void testFlowsWithTypes() { + List issues = result.issuesFor(result.inputFile("xources/hello/FlowTypes.xoo")); + assertThat(issues).hasSize(1); + Issue issue = issues.get(0); + assertThat(issue.getFlowList()).hasSize(3); + + assertThat(issue.getFlowList()).extracting(Flow::getType, Flow::getDescription, f -> f.getLocationList().size()) + .containsExactly( + tuple(FlowType.DATA, "flow #1", 1), + tuple(FlowType.UNDEFINED, "", 1), + tuple(FlowType.EXECUTION, "flow #3", 1)); + } } diff --git a/sonar-scanner-engine/test-resources/mediumtest/xoo/sample-multiline/xources/hello/FlowTypes.xoo b/sonar-scanner-engine/test-resources/mediumtest/xoo/sample-multiline/xources/hello/FlowTypes.xoo new file mode 100644 index 00000000000..7436df4b820 --- /dev/null +++ b/sonar-scanner-engine/test-resources/mediumtest/xoo/sample-multiline/xources/hello/FlowTypes.xoo @@ -0,0 +1,11 @@ +package hello; + +public class HelloJava { + + public static void main(String[] args) { + {xoo-start-issue:1}System.out.println("Hello"){xoo-end-issue:1}; + {xoo-start-data-flow:1:1:1}System.out.println("World"){xoo-end-data-flow:1:1:1}; + {xoo-start-flow:1:2:1}System.out.println("World"){xoo-end-flow:1:2:1}; + {xoo-start-execution-flow:1:3:1}System.out.println("World"){xoo-end-execution-flow:1:3:1}; + } +} -- 2.39.5