Handle conflicting SARIF results (#473)

If a user accidentally feeds multiple identical SARIFs, we should
gracefully handle the situation.

Author: nahsra

framework/codemodder-base/src/main/java/io/codemodder/CLI.java

@@ -366,14 +366,14 @@ public Integer call() throws IOException {
       log.debug("excluding paths: {}", pathExcludes);
 
       // get all files that match
-      log.debug("Listing source directories");
+      log.trace("Listing source directories");
       List<SourceDirectory> sourceDirectories =
           sourceDirectoryLister.listJavaSourceDirectories(List.of(projectDirectory));
 
-      log.debug("Listing files");
+      log.trace("Listing files");
       List<Path> filePaths = fileFinder.findFiles(projectPath, includesExcludes);
 
-      log.debug("Creating codemod regulator");
+      log.trace("Creating codemod regulator");
 
       // get codemod includes/excludes
       final CodemodRegulator regulator;

framework/codemodder-base/src/main/java/io/codemodder/DefaultSarifParser.java

@@ -46,7 +46,7 @@ private Optional<Map.Entry<String, RuleSarif>> tryToBuild(
       }
     }
 
-    log.info("Found SARIF from unsupported tool: {}", toolName);
+    log.info("Found SARIF rule entries from unsupported tool: {}", toolName);
     return Optional.empty();
   }
 

plugins/codemodder-plugin-codeql/src/main/java/io/codemodder/providers/sarif/codeql/CodeQLModule.java

@@ -4,12 +4,10 @@
 import io.codemodder.CodeChanger;
 import io.codemodder.RuleSarif;
 import java.lang.reflect.Constructor;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Optional;
-import java.util.stream.Collectors;
+import java.util.*;
 import java.util.stream.Stream;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /** Responsible for distributing the SARIFS to CodeQL based codemods based on rules. */
 public final class CodeQLModule extends AbstractModule {
@@ -27,9 +25,16 @@ public final class CodeQLModule extends AbstractModule {
   protected void configure() {
     // What if there are multiple sarif files with a given rule?
     // We can safely ignore this case for now.
-    final Map<String, RuleSarif> map =
-        allCodeqlRuleSarifs.stream()
-            .collect(Collectors.toUnmodifiableMap(RuleSarif::getRule, rs -> rs));
+    final Map<String, RuleSarif> map = new HashMap<>();
+    allCodeqlRuleSarifs.forEach(
+        rs -> {
+          if (!map.containsKey(rs.getRule())) {
+            map.put(rs.getRule(), rs);
+          } else {
+            log.warn(
+                "Multiple SARIFs found for rule: {}, ignoring results after first", rs.getRule());
+          }
+        });
 
     for (final Class<? extends CodeChanger> codemodType : codemodTypes) {
       final Constructor<?>[] constructors = codemodType.getDeclaredConstructors();
@@ -49,4 +54,6 @@ protected void configure() {
                   .toInstance(map.getOrDefault(providedCodeQLScan.ruleId(), RuleSarif.EMPTY)));
     }
   }
+
+  private static final Logger log = LoggerFactory.getLogger(CodeQLModule.class);
 }

plugins/codemodder-plugin-codeql/src/main/java/io/codemodder/providers/sarif/codeql/CodeQLRuleSarif.java

@@ -1,16 +1,12 @@
 package io.codemodder.providers.sarif.codeql;
 
-import com.contrastsecurity.sarif.Region;
-import com.contrastsecurity.sarif.Result;
-import com.contrastsecurity.sarif.Run;
-import com.contrastsecurity.sarif.SarifSchema210;
+import com.contrastsecurity.sarif.*;
 import io.codemodder.CodeDirectory;
 import io.codemodder.RuleSarif;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.*;
-import java.util.stream.Collectors;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -39,12 +35,8 @@ private String extractRuleId(final Result result, final Run run) {
               .skip(toolIndex)
               .findFirst()
               .flatMap(tool -> tool.getRules().stream().skip(ruleIndex).findFirst())
-              .map(rd -> rd.getId());
-      if (maybeRule.isPresent()) {
-        return maybeRule.get();
-      } else {
-        return null;
-      }
+              .map(ReportingDescriptor::getId);
+      return maybeRule.orElse(null);
     }
     return result.getRuleId();
   }
@@ -53,7 +45,7 @@ private String extractRuleId(final Result result, final Run run) {
   public List<Region> getRegionsFromResultsByRule(final Path path) {
     return getResultsByLocationPath(path).stream()
         .map(result -> result.getLocations().get(0).getPhysicalLocation().getRegion())
-        .collect(Collectors.toUnmodifiableList());
+        .toList();
   }
 
   @Override

plugins/codemodder-plugin-codeql/src/test/java/io/codemodder/providers/sarif/codeql/ConflictingSarifTest.java

@@ -0,0 +1,44 @@
+package io.codemodder.providers.sarif.codeql;
+
+import io.codemodder.*;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.Map;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+final class ConflictingSarifTest {
+
+  /**
+   * Test that conflicting SARIFs can be combined, and will fail gracefully, only honoring the first
+   * set of results found.
+   */
+  @Test
+  void it_combines_sarifs_with_overlapping_keys(@TempDir Path tempDir) {
+    List<Path> sarifFiles =
+        List.of(
+            Path.of("src/test/resources/conflicting-sarifs/codeql-0.sarif"),
+            Path.of("src/test/resources/conflicting-sarifs/codeql-1.sarif"),
+            Path.of("src/test/resources/conflicting-sarifs/codeql-2.sarif"),
+            Path.of("src/test/resources/conflicting-sarifs/codeql-3.sarif"),
+            Path.of("src/test/resources/conflicting-sarifs/codeql-4.sarif"),
+            Path.of("src/test/resources/conflicting-sarifs/codeql-5.sarif"));
+
+    Map<String, List<RuleSarif>> pathSarifMap =
+        SarifParser.create().parseIntoMap(sarifFiles, CodeDirectory.from(tempDir));
+
+    new CodemodLoader(
+        List.of(),
+        CodemodRegulator.of(DefaultRuleSetting.ENABLED, List.of()),
+        tempDir,
+        List.of(),
+        List.of(),
+        List.of(),
+        pathSarifMap,
+        List.of(),
+        List.of(),
+        List.of(),
+        null,
+        null);
+  }
+}