Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for multiple project-level issues #9407

Merged
merged 7 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@

namespace SonarAnalyzer.Rules
{
public abstract class MarkAssemblyWithAssemblyVersionAttributeBase : MarkAssemblyWithAttributeBase
public abstract class MarkAssemblyWithAssemblyVersionAttributeBase() : MarkAssemblyWithAttributeBase(DiagnosticId, MessageFormat)
{
private const string DiagnosticId = "S3904";
private const string MessageFormat = "Provide an 'AssemblyVersion' attribute for assembly '{0}'.";

private protected override KnownType AttributeToFind => KnownType.System_Reflection_AssemblyVersionAttribute;

protected MarkAssemblyWithAssemblyVersionAttributeBase() : base(DiagnosticId, MessageFormat) { }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@

namespace SonarAnalyzer.Rules
{
public abstract class MarkAssemblyWithClsCompliantAttributeBase : MarkAssemblyWithAttributeBase
public abstract class MarkAssemblyWithClsCompliantAttributeBase() : MarkAssemblyWithAttributeBase(DiagnosticId, MessageFormat)
{
protected const string DiagnosticId = "S3990";
private const string DiagnosticId = "S3990";
private const string MessageFormat = "Provide a 'CLSCompliant' attribute for assembly '{0}'.";

private protected override KnownType AttributeToFind => KnownType.System_CLSCompliantAttribute;

protected MarkAssemblyWithClsCompliantAttributeBase() : base(DiagnosticId, MessageFormat) { }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@

namespace SonarAnalyzer.Rules
{
public abstract class MarkAssemblyWithComVisibleAttributeBase : MarkAssemblyWithAttributeBase
public abstract class MarkAssemblyWithComVisibleAttributeBase() : MarkAssemblyWithAttributeBase(DiagnosticId, MessageFormat)
{
private const string DiagnosticId = "S3992";
private const string MessageFormat = "Provide a 'ComVisible' attribute for assembly '{0}'.";

private protected override KnownType AttributeToFind => KnownType.System_Runtime_InteropServices_ComVisibleAttribute;

protected MarkAssemblyWithComVisibleAttributeBase() : base(DiagnosticId, MessageFormat) { }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import java.io.IOException;
import java.nio.file.Path;
import java.util.List;
import java.util.stream.Collectors;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.io.TempDir;
Expand All @@ -39,14 +38,14 @@ class ProjectLevelDuplicationTest {
private static Path temp;

@Test
void containsOnlyOneProjectLevelIssue() throws IOException {
Tests.analyzeProject(temp, "ProjectLevelIssue");
void projectLevelIssuesAreRaisedOncePerProject() throws IOException {
Tests.analyzeProject(temp, "ProjectLevelIssue"); // A Solution with 2 projects

assertThat(getComponent("ProjectLevelIssue")).isNotNull();
List<Issues.Issue> projectLevelIssues = getIssues("ProjectLevelIssue")
.stream()
.filter(x -> x.getRule().equals("csharpsquid:S3904"))
.collect(Collectors.toList());
assertThat(projectLevelIssues).hasSize(1);
.toList();
assertThat(projectLevelIssues).hasSize(2);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,13 @@
*/
package org.sonarsource.dotnet.shared.plugins;

import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Consumer;
Expand Down Expand Up @@ -59,6 +57,7 @@ public class SarifParserCallbackImpl implements SarifParserCallback {
private final SensorContext context;
private final Map<String, String> repositoryKeyByRoslynRuleKey;
private final Set<Issue> savedIssues = new HashSet<>();
private final Set<ProjectIssue> projectIssues = new HashSet<>();
private final boolean ignoreThirdPartyIssues;
private final Set<String> bugCategories;
private final Set<String> codeSmellCategories;
Expand All @@ -78,9 +77,10 @@ public SarifParserCallbackImpl(SensorContext context, Map<String, String> reposi

@Override
public void onProjectIssue(String ruleId, @Nullable String level, InputProject inputProject, String message) {
// De-duplicate issues
Issue issue = new Issue(ruleId, inputProject.toString(), true);
if (!savedIssues.add(issue)) {
// Remove duplicate issues.
// We do not have enough information (other than the message) to distinguish between different dotnet projects.
// Due to this, project level issues should always mention the assembly in their message (see: S3990, S3992, or S3904)
if (!projectIssues.add(new ProjectIssue(ruleId, message))) {
return;
}

Expand All @@ -104,7 +104,7 @@ private void createProjectLevelIssue(String ruleId, InputProject inputProject, S
@Override
public void onFileIssue(String ruleId, @Nullable String level, String absolutePath, Collection<Location> secondaryLocations, String message) {
// De-duplicate issues
Issue issue = new Issue(ruleId, absolutePath, false);
Issue issue = new Issue(ruleId, absolutePath);
if (!savedIssues.add(issue)) {
return;
}
Expand Down Expand Up @@ -336,54 +336,20 @@ private void logMissingInputFile(String ruleId, String filePath){
LOG.debug("Skipping issue {}, input file not found or excluded: {}", ruleId, filePath);
}

private static class Issue {
private String ruleId;
private String moduleId;
private String absolutePath;
private int startLine;
private int startColumn;
private int endLine;
private int endColumn;

Issue(String ruleId, String moduleOrPath, boolean isModuleId) {
this.ruleId = ruleId;
if (isModuleId) {
this.moduleId = moduleOrPath;
this.absolutePath = "";
} else {
this.absolutePath = moduleOrPath;
}
}
private record ProjectIssue(String ruleId, String message) { }

Issue(String ruleId, Location location) {
this.ruleId = ruleId;
this.absolutePath = location.getAbsolutePath();
this.startLine = location.getStartLine();
this.startColumn = location.getStartColumn();
this.endLine = location.getEndLine();
this.endColumn = location.getEndColumn();
private record Issue(String ruleId, String absolutePath, int startLine, int startColumn, int endLine, int endColumn) {
Issue(String ruleId, String path) {
this(ruleId, path, 0, 0, 0, 0);
}

@Override
public int hashCode() {
return Objects.hash(ruleId, moduleId, absolutePath, startLine, startColumn, endLine, endColumn);
}

@Override
public boolean equals(Object other) {
if (!(other instanceof Issue)) {
return false;
}
Issue o = (Issue) other;

// note that comparison of absolute path is done using Path.
return Objects.equals(ruleId, o.ruleId)
&& Objects.equals(moduleId, o.moduleId)
&& Objects.equals(startLine, o.startLine)
&& Objects.equals(startColumn, o.startColumn)
&& Objects.equals(endLine, o.endLine)
&& Objects.equals(endColumn, o.endColumn)
&& Paths.get(absolutePath).equals(Paths.get(o.absolutePath));
Issue(String ruleId, Location location) {
this(ruleId,
location.getAbsolutePath(),
location.getStartLine(),
location.getStartColumn(),
location.getEndLine(),
location.getEndColumn());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public class SarifParserCallbackImplTest {
public LogTester logTester = new LogTester();

private SensorContextTester ctx;
private Map<String, String> repositoryKeyByRoslynRuleKey = new HashMap<>();
private final Map<String, String> repositoryKeyByRoslynRuleKey = new HashMap<>();

private SarifParserCallbackImpl callback;

Expand Down Expand Up @@ -428,6 +428,19 @@ public void issue_with_invalid_precise_location_forCshtml_reports_on_line() {
assertIssueReportedOnLine("Dummy.cshtml");
}

@Test
public void project_level_issues_for_different_projects() {
callback = new SarifParserCallbackImpl(ctx, repositoryKeyByRoslynRuleKey, false, emptySet(), emptySet(), emptySet());
repositoryKeyByRoslynRuleKey.put("S3990", "S3990");

callback.onProjectIssue("S3990", "level", ctx.project(), "Provide a 'CLSCompliant' attribute for assembly 'First'.");
callback.onProjectIssue("S3990", "level", ctx.project(), "Provide a 'CLSCompliant' attribute for assembly 'First'.");
assertThat(ctx.allIssues()).hasSize(1); // Issues with the same id and message are considered duplicates.

callback.onProjectIssue("S3990", "level", ctx.project(), "Provide a 'CLSCompliant' attribute for assembly 'Second'.");
assertThat(ctx.allIssues()).hasSize(2); // A different message leads to a different issue
}

private void assertIssueReportedOnLine(String fileName) {
callback = new SarifParserCallbackImpl(ctx, repositoryKeyByRoslynRuleKey, false, emptySet(), emptySet(), emptySet());

Expand Down