Browse Source

SONAR-17287 Fix quality issues and adopt changes to API

tags/9.7.0.61563
Duarte Meneses 1 year ago
parent
commit
f51e80efde

+ 3
- 2
plugins/sonar-xoo-plugin/src/test/java/org/sonar/xoo/rule/MultilineIssuesSensorTest.java View File

@@ -35,8 +35,9 @@ 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.NewIssue;
import org.sonar.api.batch.sensor.issue.NewIssue.FlowType;
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;

@@ -77,7 +78,7 @@ public class MultilineIssuesSensorTest {

List<DefaultIssueFlow> defaultIssueFlows = flows.stream().map(DefaultIssueFlow.class::cast).collect(Collectors.toList());

assertThat(defaultIssueFlows).extracting(DefaultIssueFlow::getType).containsExactlyInAnyOrder(Type.DATA, Type.EXECUTION);
assertThat(defaultIssueFlows).extracting(DefaultIssueFlow::type).containsExactlyInAnyOrder(FlowType.DATA, FlowType.EXECUTION);
}

private DefaultInputFile newTestFile(String content) {

+ 19
- 4
server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactoryTest.java View File

@@ -48,9 +48,9 @@ import org.sonar.core.issue.tracking.Input;
import org.sonar.db.protobuf.DbIssues;
import org.sonar.scanner.protocol.Constants;
import org.sonar.scanner.protocol.output.ScannerReport;
import org.sonar.scanner.protocol.output.ScannerReport.FlowType;
import org.sonar.scanner.protocol.output.ScannerReport.IssueType;
import org.sonar.server.rule.CommonRuleKeys;
import org.sonarqube.ws.Common;

import static java.util.Arrays.asList;
import static java.util.Collections.emptyMap;
@@ -167,10 +167,13 @@ public class TrackerRawInputFactoryTest {
.setRuleRepository(ruleKey.repository())
.setRuleKey(ruleKey.rule())
.addFlow(ScannerReport.Flow.newBuilder()
.setType(ScannerReport.FlowType.DATA)
.setType(FlowType.DATA)
.setDescription("flow1")
.addLocation(ScannerReport.IssueLocation.newBuilder().setMsg("loc1").setComponentRef(1).build())
.addLocation(ScannerReport.IssueLocation.newBuilder().setMsg("loc2").setComponentRef(1).build()))
.addFlow(ScannerReport.Flow.newBuilder()
.setType(FlowType.EXECUTION)
.addLocation(ScannerReport.IssueLocation.newBuilder().setTextRange(newTextRange(2)).setComponentRef(1).build()))
.addFlow(ScannerReport.Flow.newBuilder()
.addLocation(ScannerReport.IssueLocation.newBuilder().setTextRange(newTextRange(2)).setComponentRef(1).build()))
.build();
@@ -179,14 +182,18 @@ public class TrackerRawInputFactoryTest {

Collection<DefaultIssue> issues = input.getIssues();
DbIssues.Locations locations = Iterators.getOnlyElement(issues.iterator()).getLocations();
assertThat(locations.getFlowCount()).isEqualTo(2);
assertThat(locations.getFlowCount()).isEqualTo(3);
assertThat(locations.getFlow(0).getDescription()).isEqualTo("flow1");
assertThat(locations.getFlow(0).getType()).isEqualTo(DbIssues.FlowType.DATA);
assertThat(locations.getFlow(0).getLocationList()).hasSize(2);

assertThat(locations.getFlow(1).hasDescription()).isFalse();
assertThat(locations.getFlow(1).hasType()).isFalse();
assertThat(locations.getFlow(1).getType()).isEqualTo(DbIssues.FlowType.EXECUTION);
assertThat(locations.getFlow(1).getLocationList()).hasSize(1);

assertThat(locations.getFlow(2).hasDescription()).isFalse();
assertThat(locations.getFlow(2).hasType()).isFalse();
assertThat(locations.getFlow(2).getLocationList()).hasSize(1);
}

@Test
@@ -291,6 +298,7 @@ public class TrackerRawInputFactoryTest {
.setSeverity(Constants.Severity.BLOCKER)
.setEffort(20L)
.setType(issueType)
.addFlow(ScannerReport.Flow.newBuilder().setType(FlowType.DATA).addLocation(ScannerReport.IssueLocation.newBuilder().build()).build())
.build();
reportReader.putExternalIssues(FILE.getReportAttributes().getRef(), asList(reportIssue));
Input<DefaultIssue> input = underTest.create(FILE);
@@ -307,6 +315,13 @@ public class TrackerRawInputFactoryTest {
assertThat(issue.message()).isEqualTo("the message");
assertThat(issue.type()).isEqualTo(expectedRuleType);

DbIssues.Locations locations = Iterators.getOnlyElement(issues.iterator()).getLocations();
assertThat(locations.getFlowCount()).isEqualTo(1);
assertThat(locations.getFlow(0).getType()).isEqualTo(DbIssues.FlowType.DATA);
assertThat(locations.getFlow(0).getLocationList()).hasSize(1);



// fields set by compute engine
assertThat(issue.checksum()).isEqualTo(input.getLineHashSequence().getHashForLine(2));
assertThat(issue.tags()).isEmpty();

+ 37
- 49
server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ShowActionTest.java View File

@@ -53,8 +53,10 @@ import org.sonar.db.DbTester;
import org.sonar.db.component.BranchType;
import org.sonar.db.component.ComponentDto;
import org.sonar.db.issue.IssueDto;
import org.sonar.db.protobuf.DbCommons;
import org.sonar.db.protobuf.DbCommons.TextRange;
import org.sonar.db.protobuf.DbIssues;
import org.sonar.db.protobuf.DbIssues.Flow;
import org.sonar.db.protobuf.DbIssues.FlowType;
import org.sonar.db.rule.RuleDescriptionSectionContextDto;
import org.sonar.db.rule.RuleDescriptionSectionDto;
import org.sonar.db.rule.RuleDto;
@@ -205,49 +207,31 @@ public class ShowActionTest {
ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
ComponentDto anotherFile = dbTester.components().insertComponent(newFileDto(project));
DbIssues.Locations.Builder locations = DbIssues.Locations.newBuilder()
.addFlow(DbIssues.Flow.newBuilder()
.addFlow(Flow.newBuilder()
.setDescription("FLOW DESCRIPTION")
.setType(DbIssues.FlowType.DATA)
.setType(FlowType.DATA)
.addAllLocation(Arrays.asList(
DbIssues.Location.newBuilder()
.setComponentId(file.uuid())
.setMsg("FLOW MESSAGE")
.setTextRange(DbCommons.TextRange.newBuilder()
.setStartLine(1)
.setEndLine(1)
.setStartOffset(0)
.setEndOffset(12)
.build())
.build(),
.setTextRange(textRange(1, 1, 0, 12)).build(),
DbIssues.Location.newBuilder()
.setComponentId(anotherFile.uuid())
.setMsg("ANOTHER FLOW MESSAGE")
.setTextRange(DbCommons.TextRange.newBuilder()
.setStartLine(1)
.setEndLine(1)
.setStartOffset(0)
.setEndOffset(12)
.build())
.build(),
.setTextRange(textRange(1, 1, 0, 12)).build(),
DbIssues.Location.newBuilder()
.setMsg("FLOW MESSAGE WITHOUT FILE UUID")
.setTextRange(DbCommons.TextRange.newBuilder()
.setStartLine(1)
.setEndLine(1)
.setStartOffset(0)
.setEndOffset(12)
.build())
.build())))
.addFlow(DbIssues.Flow.newBuilder()
.addAllLocation(Arrays.asList(
DbIssues.Location.newBuilder()
.setComponentId(file.uuid())
.setTextRange(DbCommons.TextRange.newBuilder()
.setStartLine(1)
.setStartOffset(0)
.setEndOffset(12)
.build())
.build())));
.setTextRange(textRange(1, 1, 0, 12)).build())))
.addFlow(Flow.newBuilder()
.addLocation(DbIssues.Location.newBuilder()
.setComponentId(file.uuid())
.setTextRange(TextRange.newBuilder().setStartLine(1).setStartOffset(0).setEndOffset(12).build())
.build()))
.addFlow(Flow.newBuilder()
.setType(FlowType.EXECUTION)
.addLocation(DbIssues.Location.newBuilder()
.setComponentId(file.uuid())
.build()));
RuleDto rule = newRule(SECURITY_HOTSPOT);
var hotspot = dbTester.issues().insertHotspot(rule, project, file, i -> i.setLocations(locations.build()));
mockChangelogAndCommentsFormattingContext();
@@ -258,7 +242,7 @@ public class ShowActionTest {
.executeProtobuf(Hotspots.ShowWsResponse.class);

assertThat(response.getKey()).isEqualTo(hotspot.getKey());
assertThat(response.getFlowsCount()).isEqualTo(2);
assertThat(response.getFlowsCount()).isEqualTo(3);
assertThat(response.getFlows(0).getDescription()).isEqualTo("FLOW DESCRIPTION");
assertThat(response.getFlows(0).getType()).isEqualTo(Common.FlowType.DATA);
assertThat(response.getFlows(0).getLocationsList())
@@ -270,6 +254,8 @@ public class ShowActionTest {

assertThat(response.getFlows(1).getDescription()).isEmpty();
assertThat(response.getFlows(1).hasType()).isFalse();

assertThat(response.getFlows(2).getType()).isEqualTo(Common.FlowType.EXECUTION);
}

@Test
@@ -674,14 +660,7 @@ public class ShowActionTest {
ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
RuleDto rule = newRule(SECURITY_HOTSPOT);
IssueDto hotspot = dbTester.issues().insertHotspot(rule, project, file,
t -> t.setLocations(DbIssues.Locations.newBuilder()
.setTextRange(DbCommons.TextRange.newBuilder()
.setStartLine(startLine)
.setEndLine(endLine)
.setStartOffset(startOffset)
.setEndOffset(endOffset)
.build())
.build()));
t -> t.setLocations(DbIssues.Locations.newBuilder().setTextRange(textRange(startLine, endLine, startOffset, endOffset)).build()));
mockChangelogAndCommentsFormattingContext();

Hotspots.ShowWsResponse response = newRequest(hotspot)
@@ -857,7 +836,7 @@ public class ShowActionTest {
RuleDto rule = newRule(SECURITY_HOTSPOT);
IssueDto hotspot = dbTester.issues().insertHotspot(rule, project, file,
t -> t.setLocations(DbIssues.Locations.newBuilder()
.setTextRange(DbCommons.TextRange.newBuilder().build())
.setTextRange(TextRange.newBuilder().build())
.build()));
mockChangelogAndCommentsFormattingContext();

@@ -882,7 +861,7 @@ public class ShowActionTest {
RuleDto rule = newRule(SECURITY_HOTSPOT, t -> t.setSecurityStandards(standards));
IssueDto hotspot = dbTester.issues().insertHotspot(rule, project, file,
t -> t.setLocations(DbIssues.Locations.newBuilder()
.setTextRange(DbCommons.TextRange.newBuilder().build())
.setTextRange(TextRange.newBuilder().build())
.build()));
mockChangelogAndCommentsFormattingContext();

@@ -917,7 +896,7 @@ public class ShowActionTest {
RuleDto rule = newRule(SECURITY_HOTSPOT);
IssueDto hotspot = dbTester.issues().insertHotspot(rule, project, project,
t -> t.setLocations(DbIssues.Locations.newBuilder()
.setTextRange(DbCommons.TextRange.newBuilder().build())
.setTextRange(TextRange.newBuilder().build())
.build()));
mockChangelogAndCommentsFormattingContext();

@@ -937,7 +916,7 @@ public class ShowActionTest {
RuleDto rule = newRule(SECURITY_HOTSPOT);
IssueDto hotspot = dbTester.issues().insertHotspot(rule, branch, file,
t -> t.setLocations(DbIssues.Locations.newBuilder()
.setTextRange(DbCommons.TextRange.newBuilder().build())
.setTextRange(TextRange.newBuilder().build())
.build()));
mockChangelogAndCommentsFormattingContext();

@@ -958,7 +937,7 @@ public class ShowActionTest {
RuleDto rule = newRule(SECURITY_HOTSPOT);
IssueDto hotspot = dbTester.issues().insertHotspot(rule, pullRequest, file,
t -> t.setLocations(DbIssues.Locations.newBuilder()
.setTextRange(DbCommons.TextRange.newBuilder().build())
.setTextRange(TextRange.newBuilder().build())
.build()));
mockChangelogAndCommentsFormattingContext();

@@ -977,7 +956,7 @@ public class ShowActionTest {
ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
IssueDto hotspot = dbTester.issues().insertHotspot(rule, project, file,
t -> t.setLocations(DbIssues.Locations.newBuilder()
.setTextRange(DbCommons.TextRange.newBuilder().build())
.setTextRange(TextRange.newBuilder().build())
.build()));
List<Common.Changelog> changelog = IntStream.range(0, 1 + new Random().nextInt(12))
.mapToObj(i -> Common.Changelog.newBuilder().setUser("u" + i).build())
@@ -1181,6 +1160,15 @@ public class ShowActionTest {
}
}

private static TextRange textRange(int startLine, int endLine, int startOffset, int endOffset) {
return TextRange.newBuilder()
.setStartLine(startLine)
.setEndLine(endLine)
.setStartOffset(startOffset)
.setEndOffset(endOffset)
.build();
}

private TestRequest newRequest(IssueDto hotspot) {
return actionTester.newRequest()
.setParam("hotspot", hotspot.getKey());

+ 6
- 10
sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/AbstractDefaultIssue.java View File

@@ -34,7 +34,7 @@ import org.sonar.api.batch.sensor.internal.DefaultStorable;
import org.sonar.api.batch.sensor.internal.SensorStorage;
import org.sonar.api.batch.sensor.issue.Issue.Flow;
import org.sonar.api.batch.sensor.issue.IssueLocation;
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.utils.PathUtils;

@@ -72,25 +72,21 @@ public abstract class AbstractDefaultIssue<T extends AbstractDefaultIssue> exten
}

public T addLocation(NewIssueLocation secondaryLocation) {
flows.add(new DefaultIssueFlow(List.of(rewriteLocation((DefaultIssueLocation) secondaryLocation)), DefaultIssueFlow.Type.UNDEFINED, null));
flows.add(new DefaultIssueFlow(List.of(rewriteLocation((DefaultIssueLocation) secondaryLocation)), FlowType.UNDEFINED, null));
return (T) this;
}

public T addFlow(Iterable<NewIssueLocation> locations) {
return addFlow(locations, DefaultIssueFlow.Type.UNDEFINED, null);
return addFlow(locations, FlowType.UNDEFINED, null);
}

public T addFlow(Iterable<NewIssueLocation> flowLocations, NewIssue.FlowType flowType, @Nullable String flowDescription) {
return addFlow(flowLocations, DefaultIssueFlow.Type.valueOf(flowType.name()), flowDescription);
}

private T addFlow(Iterable<NewIssueLocation> locations, DefaultIssueFlow.Type type, @Nullable String description) {
public T addFlow(Iterable<NewIssueLocation> flowLocations, FlowType type, @Nullable String flowDescription) {
checkArgument(type != null, "Type can't be null");
List<IssueLocation> flowAsList = new ArrayList<>();
for (NewIssueLocation issueLocation : locations) {
for (NewIssueLocation issueLocation : flowLocations) {
flowAsList.add(rewriteLocation((DefaultIssueLocation) issueLocation));
}
flows.add(new DefaultIssueFlow(flowAsList, type, description));
flows.add(new DefaultIssueFlow(flowAsList, type, flowDescription));
return (T) this;
}


+ 7
- 8
sonar-plugin-api-impl/src/main/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueFlow.java View File

@@ -24,14 +24,15 @@ import javax.annotation.CheckForNull;
import javax.annotation.Nullable;
import org.sonar.api.batch.sensor.issue.Issue;
import org.sonar.api.batch.sensor.issue.IssueLocation;
import org.sonar.api.batch.sensor.issue.NewIssue.FlowType;

public class DefaultIssueFlow implements Issue.Flow {
private final List<IssueLocation> locations;
private final Type type;
private final FlowType type;
@Nullable
private final String description;

public DefaultIssueFlow(List<IssueLocation> locations, Type type, @Nullable String description) {
public DefaultIssueFlow(List<IssueLocation> locations, FlowType type, @Nullable String description) {
this.locations = locations;
this.type = type;
this.description = description;
@@ -42,16 +43,14 @@ public class DefaultIssueFlow implements Issue.Flow {
return locations;
}

public Type getType() {
@Override
public FlowType type() {
return type;
}

@CheckForNull
public String getDescription() {
@Override
public String description() {
return description;
}

public enum Type {
UNDEFINED, DATA, EXECUTION;
}
}

+ 42
- 6
sonar-plugin-api-impl/src/test/java/org/sonar/api/batch/sensor/issue/internal/DefaultIssueTest.java View File

@@ -37,10 +37,14 @@ import org.sonar.api.batch.fs.internal.DefaultTextRange;
import org.sonar.api.batch.fs.internal.TestInputFileBuilder;
import org.sonar.api.batch.rule.Severity;
import org.sonar.api.batch.sensor.internal.SensorStorage;
import org.sonar.api.batch.sensor.issue.Issue;
import org.sonar.api.batch.sensor.issue.Issue.Flow;
import org.sonar.api.batch.sensor.issue.NewIssue;
import org.sonar.api.batch.sensor.issue.NewIssue.FlowType;
import org.sonar.api.rule.RuleKey;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.Assertions.tuple;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
@@ -94,16 +98,35 @@ public class DefaultIssueTest {

DefaultIssue issue = new DefaultIssue(project, storage)
.at(new DefaultIssueLocation().on(inputFile))
.addFlow(List.of(new DefaultIssueLocation().message("loc1").on(inputFile)), NewIssue.FlowType.DATA, "desc")
.addFlow(List.of(new DefaultIssueLocation().message("loc1").on(inputFile)), FlowType.DATA, "desc")
.addFlow(List.of(new DefaultIssueLocation().message("loc1").on(inputFile).at(range1), new DefaultIssueLocation().message("loc1").on(inputFile).at(range2)))
.forRule(RULE_KEY);

assertThat(issue.flows)
.extracting(DefaultIssueFlow::getType, DefaultIssueFlow::getDescription)
.containsExactly(tuple(DefaultIssueFlow.Type.DATA, "desc"), tuple(DefaultIssueFlow.Type.UNDEFINED, null));
assertThat(issue.flows())
.extracting(Flow::type, Flow::description)
.containsExactly(tuple(FlowType.DATA, "desc"), tuple(FlowType.UNDEFINED, null));

assertThat(issue.flows.get(0).locations()).hasSize(1);
assertThat(issue.flows.get(1).locations()).hasSize(2);
assertThat(issue.flows().get(0).locations()).hasSize(1);
assertThat(issue.flows().get(1).locations()).hasSize(2);
}

@Test
public void build_issue_with_secondary_locations() {
TextRange range1 = new DefaultTextRange(new DefaultTextPointer(1, 1), new DefaultTextPointer(1, 2));
TextRange range2 = new DefaultTextRange(new DefaultTextPointer(2, 1), new DefaultTextPointer(2, 2));

DefaultIssue issue = new DefaultIssue(project, storage)
.at(new DefaultIssueLocation().on(inputFile))
.addLocation(new DefaultIssueLocation().on(inputFile).at(range1).message("loc1"))
.addLocation(new DefaultIssueLocation().on(inputFile).at(range2).message("loc2"))
.forRule(RULE_KEY);

assertThat(issue.flows())
.extracting(Flow::type, Flow::description)
.containsExactly(tuple(FlowType.UNDEFINED, null), tuple(FlowType.UNDEFINED, null));

assertThat(issue.flows().get(0).locations()).hasSize(1);
assertThat(issue.flows().get(1).locations()).hasSize(1);
}

@Test
@@ -177,6 +200,19 @@ public class DefaultIssueTest {
verify(storage).store(issue);
}

@Test
public void at_fails_if_called_twice() {
DefaultIssueLocation loc = new DefaultIssueLocation().on(inputFile);
DefaultIssue issue = new DefaultIssue(project, storage).at(loc);
assertThatThrownBy(() -> issue.at(loc)).isInstanceOf(IllegalStateException.class);
}

@Test
public void at_fails_if_location_is_null() {
DefaultIssue issue = new DefaultIssue(project, storage);
assertThatThrownBy(() -> issue.at(null)).isInstanceOf(IllegalArgumentException.class);
}

@Test
public void default_issue_has_no_quickfix() {
DefaultIssue issue = new DefaultIssue(project, storage);

+ 5
- 4
sonar-scanner-engine/src/main/java/org/sonar/scanner/issue/IssuePublisher.java View File

@@ -32,6 +32,7 @@ import org.sonar.api.batch.rule.ActiveRules;
import org.sonar.api.batch.sensor.issue.ExternalIssue;
import org.sonar.api.batch.sensor.issue.Issue;
import org.sonar.api.batch.sensor.issue.Issue.Flow;
import org.sonar.api.batch.sensor.issue.NewIssue.FlowType;
import org.sonar.api.batch.sensor.issue.internal.DefaultIssueFlow;
import org.sonar.scanner.protocol.Constants.Severity;
import org.sonar.scanner.protocol.output.ScannerReport;
@@ -181,15 +182,15 @@ public class IssuePublisher {
}
flowBuilder.addLocation(locationBuilder.build());
}
if (flow.getDescription() != null) {
flowBuilder.setDescription(flow.getDescription());
if (flow.description() != null) {
flowBuilder.setDescription(flow.description());
}
flowBuilder.setType(toProtobufFlowType(flow.getType()));
flowBuilder.setType(toProtobufFlowType(flow.type()));
consumer.accept(flowBuilder.build());
}
}

private static ScannerReport.FlowType toProtobufFlowType(DefaultIssueFlow.Type flowType) {
private static ScannerReport.FlowType toProtobufFlowType(FlowType flowType) {
switch (flowType) {
case EXECUTION:
return ScannerReport.FlowType.EXECUTION;

Loading…
Cancel
Save