Allow for multiple project-level issues (#9407)

analyzers/src/SonarAnalyzer.Common/Rules/MarkAssemblyWithAssemblyVersionAttributeBase.cs

@@ -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) { }
     }
 }

analyzers/src/SonarAnalyzer.Common/Rules/MarkAssemblyWithClsCompliantAttributeBase.cs

@@ -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) { }
     }
 }

analyzers/src/SonarAnalyzer.Common/Rules/MarkAssemblyWithComVisibleAttributeBase.cs

@@ -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) { }
     }
 }

its/src/test/java/com/sonar/it/csharp/ProjectLevelDuplicationTest.java

@@ -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;
@@ -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);
   }
 }

sonar-dotnet-shared-library/src/main/java/org/sonarsource/dotnet/shared/plugins/SarifParserCallbackImpl.java

@@ -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;
@@ -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;
@@ -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;
     }
 
@@ -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;
     }
@@ -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());
     }
   }
 }

sonar-dotnet-shared-library/src/test/java/org/sonarsource/dotnet/shared/sarif/SarifParserCallbackImplTest.java

@@ -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;
 
@@ -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());