GNATCheckWorker: unify duplicated rules when merging rules sets

This change improves how rules sets are merged by the worker by:

  * Improving diagnostics by giving precise instances locations.

  * Improving conflicts resolution when duplicate instances are
  merged together. In case the instances are the same (they share
  the same name and the same parameters), duplicates are simply
  ignored. In the case the instances run the same check (they have a
  different name but run the same check in the same configuration),
  a warning is emitted. Only the case where the instances share the
  same name but actually run different configurations of the check
  emits an error.

  * Producing more diagnostics when sets are merged,
  information-level diagnostics are emitted in verbose mode when an
  instance is added to the global instances set run by the driver,
  and, warning-level diagnostics are emitted when duplicate
  instances are added to the global set.

Author: thvnx

lkql_checker/src/gnatcheck-compiler.adb

@@ -600,6 +600,8 @@ package body Gnatcheck.Compiler is
                   end if;
                end;
             end if;
+         elsif Line_Len >= 16 and then Line (1 .. 13) = "WORKER_INFO: " then
+            Info (Line (14 .. Line_Len));
          elsif Line_Len >= 16 and then Line (1 .. 16) = "WORKER_WARNING: " then
             Warning (Line (17 .. Line_Len));
          elsif Line_Len >= 14 and then Line (1 .. 14) = "WORKER_ERROR: " then
@@ -616,8 +618,6 @@ package body Gnatcheck.Compiler is
             --  the line if that's the case.
             if Line_Len = Line'Length then
                Skip_Line (File);
-            else
-               null;
             end if;
          else
             Analyze_Line (Line (1 .. Line_Len));
@@ -1862,6 +1862,11 @@ package body Gnatcheck.Compiler is
       Num_Args := @ + 1;
       Args (Num_Args) := new String'(LKQL_RF_Name);
 
+      if Verbose_Mode then
+         Num_Args := @ + 1;
+         Args (Num_Args) := new String'("--verbose");
+      end if;
+
       --  Output the called command if in debug mode
       if Arg.Debug_Mode.Get then
          Put (Base_Name (Worker.all));

lkql_jit/cli/src/main/java/com/adacore/lkql_jit/cli/BaseLKQLChecker.java

@@ -173,7 +173,8 @@ protected Map<String, RuleInstance> getRuleInstances() {
                     ruleName,
                     Optional.empty(),
                     RuleInstance.SourceMode.GENERAL,
-                    instanceArgs.get(instanceId)
+                    instanceArgs.get(instanceId),
+                    null
                 )
             );
         }

lkql_jit/cli/src/main/java/com/adacore/lkql_jit/cli/GNATCheckWorker.java

@@ -201,7 +201,10 @@ protected int executeScript(Context.Builder contextBuilder) {
         // If a LKQL rule config file has been provided, parse it and display the result
         if (this.args.lkqlConfigFile != null) {
             try {
-                final var instances = parseLKQLRuleFile(this.args.lkqlConfigFile);
+                final var instances = parseLKQLRuleFile(
+                    this.args.lkqlConfigFile,
+                    this.args.verbose
+                );
                 final var jsonInstances = new JSONObject(
                     instances
                         .entrySet()
@@ -245,7 +248,7 @@ protected int executeScript(Context.Builder contextBuilder) {
         for (var rulesFrom : this.args.rulesFroms) {
             if (!rulesFrom.isEmpty()) {
                 try {
-                    instances.putAll(parseLKQLRuleFile(rulesFrom));
+                    instances.putAll(parseLKQLRuleFile(rulesFrom, this.args.verbose));
                 } catch (LKQLRuleFileError e) {
                     System.out.println(e.getMessage());
                     return 0;
@@ -317,8 +320,10 @@ private static void errorInLKQLRuleFile(
      * @throws LKQLRuleFileError If there is any error in the provided LKQL rule file, preventing
      *     the analysis to go further.
      */
-    private static Map<String, RuleInstance> parseLKQLRuleFile(final String lkqlRuleFileName)
-        throws LKQLRuleFileError {
+    private static Map<String, RuleInstance> parseLKQLRuleFile(
+        final String lkqlRuleFileName,
+        final boolean verbose
+    ) throws LKQLRuleFileError {
         final File lkqlFile = new File(lkqlRuleFileName);
         final String lkqlFileBasename = lkqlFile.getName();
         final Map<String, RuleInstance> res = new HashMap<>();
@@ -346,7 +351,8 @@ private static Map<String, RuleInstance> parseLKQLRuleFile(final String lkqlRule
                     lkqlFileBasename,
                     topLevel.getMember("rules"),
                     RuleInstance.SourceMode.GENERAL,
-                    res
+                    res,
+                    verbose
                 );
 
                 // Then get the optional Ada and SPARK instances
@@ -355,15 +361,17 @@ private static Map<String, RuleInstance> parseLKQLRuleFile(final String lkqlRule
                         lkqlFileBasename,
                         topLevel.getMember("ada_rules"),
                         RuleInstance.SourceMode.ADA,
-                        res
+                        res,
+                        verbose
                     );
                 }
                 if (topLevel.hasMember("spark_rules")) {
                     processInstancesObject(
                         lkqlFileBasename,
                         topLevel.getMember("spark_rules"),
                         RuleInstance.SourceMode.SPARK,
-                        res
+                        res,
+                        verbose
                     );
                 }
             } else {
@@ -390,7 +398,8 @@ private static void processInstancesObject(
         final String lkqlRuleFile,
         final Value instancesObject,
         final RuleInstance.SourceMode sourceMode,
-        final Map<String, RuleInstance> toPopulate
+        final Map<String, RuleInstance> toPopulate,
+        final boolean verbose
     ) throws LKQLRuleFileError {
         // Iterate on all instance object keys
         for (String ruleName : instancesObject.getMemberKeys()) {
@@ -399,13 +408,28 @@ private static void processInstancesObject(
 
             // Check that the value associated to the rule name is an array like value
             if (args.hasArrayElements()) {
-                // Else iterate over each argument object and create one instance for each
+                // Then iterate over each argument object and create one instance for each
                 for (long i = 0; i < args.getArraySize(); i++) {
                     var arg = args.getArrayElement(i);
                     if (arg.hasMembers()) {
-                        processArgsObject(lkqlRuleFile, arg, sourceMode, lowerRuleName, toPopulate);
+                        processArgsObject(
+                            lkqlRuleFile,
+                            arg,
+                            sourceMode,
+                            lowerRuleName,
+                            toPopulate,
+                            verbose
+                        );
                     } else if (acceptSoleArgs(ruleName) && arg.isString()) {
-                        processSoleArg(arg, sourceMode, ruleName, toPopulate);
+                        processSoleArg(
+                            lkqlRuleFile,
+                            instancesObject,
+                            arg,
+                            sourceMode,
+                            ruleName,
+                            toPopulate,
+                            verbose
+                        );
                     } else {
                         errorInLKQLRuleFile(lkqlRuleFile, "Arguments should be in an object value");
                     }
@@ -414,6 +438,39 @@ private static void processInstancesObject(
                 errorInLKQLRuleFile(lkqlRuleFile, "The value associated to a rule must be a list");
             }
         }
+
+        // Post-process instances map and look for instances that are identical
+        // but have different aliases.
+
+        // Build a new map to group all instances of the same rule into a list
+        Map<String, List<RuleInstance>> rules = new HashMap();
+        for (var instance : toPopulate.entrySet()) {
+            var ruleName = instance.getValue().ruleName();
+            var instancesList = rules.getOrDefault(ruleName, new ArrayList<>());
+            instancesList.add(instance.getValue());
+            rules.put(ruleName, instancesList);
+        }
+
+        // Look for duplicates to warn the user about duplicate checks
+        for (var rule : rules.entrySet()) {
+            for (int i = 0; i < rule.getValue().size(); i++) {
+                for (var instance : rule.getValue().subList(i + 1, rule.getValue().size())) {
+                    RuleInstance current = rule.getValue().get(i);
+                    if (current.isEquivalent(instance)) {
+                        System.err.println(
+                            "WORKER_WARNING: duplicate rules with different names, " +
+                            instance.instanceId() +
+                            " from " +
+                            instance.locationToGNATDiagnosisFormatString() +
+                            " runs the same check than rule " +
+                            current.instanceId() +
+                            " from " +
+                            current.locationToGNATDiagnosisFormatString()
+                        );
+                    }
+                }
+            }
+        }
     }
 
     /** Internal method to process an object value containing arguments for the given rule name. */
@@ -422,7 +479,8 @@ private static void processArgsObject(
         final Value argsObject,
         final RuleInstance.SourceMode sourceMode,
         final String ruleName,
-        final Map<String, RuleInstance> toPopulate
+        final Map<String, RuleInstance> toPopulate,
+        final boolean verbose
     ) throws LKQLRuleFileError {
         // Compute the instance arguments and optional instance name
         String instanceId = ruleName;
@@ -442,31 +500,103 @@ private static void processArgsObject(
             }
         }
 
-        // Add an instance in the instance map if it is not present
+        RuleInstance newInstance = new RuleInstance(
+            ruleName,
+            instanceName,
+            sourceMode,
+            arguments,
+            argsObject.getSourceLocation()
+        );
+
+        // Add an instance in the instance map
         if (toPopulate.containsKey(instanceId)) {
-            errorInLKQLRuleFile(
-                lkqlRuleFile,
-                "Multiple instances with the same name: " + instanceId
-            );
+            RuleInstance oldInstance = toPopulate.get(instanceId);
+            // If the instance is already present, compare parameters, if they
+            // are identical, skip the current instance and emit a warning.
+            if (oldInstance.arguments().equals(newInstance.arguments())) {
+                System.err.println(
+                    "WORKER_WARNING: duplicated rule: " +
+                    instanceId +
+                    " ignored from " +
+                    newInstance.locationToGNATDiagnosisFormatString() +
+                    ", previous declaration at " +
+                    oldInstance.locationToGNATDiagnosisFormatString()
+                );
+            } else {
+                // On the contrary, emit an error.
+                errorInLKQLRuleFile(
+                    lkqlRuleFile,
+                    "multiple instances with the same name but different configuration: " +
+                    instanceId +
+                    " at " +
+                    newInstance.locationToGNATDiagnosisFormatString() +
+                    ", previous declaration at " +
+                    oldInstance.locationToGNATDiagnosisFormatString() +
+                    " (instances should have unique names)"
+                );
+            }
         } else {
-            toPopulate.put(
-                instanceId,
-                new RuleInstance(ruleName, instanceName, sourceMode, arguments)
-            );
+            if (verbose) {
+                System.err.println(
+                    "WORKER_INFO: register new instance: " +
+                    instanceId +
+                    " from " +
+                    newInstance.locationToGNATDiagnosisFormatString()
+                );
+            }
+            toPopulate.put(instanceId, newInstance);
         }
     }
 
     /** Internal function to process a sole string argument for a compiler-based rule. */
     private static void processSoleArg(
+        final String lkqlRuleFile,
+        final Value instancesObject,
         final Value arg,
         final RuleInstance.SourceMode sourceMode,
         final String ruleName,
-        final Map<String, RuleInstance> toPopulate
-    ) {
+        final Map<String, RuleInstance> toPopulate,
+        final boolean verbose
+    ) throws LKQLRuleFileError {
         // Create the new rule instance and add it to the "global" map
         Map<String, String> args = new HashMap<>();
         args.put("arg", "\"" + arg + "\"");
-        toPopulate.put(ruleName, new RuleInstance(ruleName, Optional.empty(), sourceMode, args));
+        RuleInstance newInstance = new RuleInstance(
+            ruleName,
+            Optional.empty(),
+            sourceMode,
+            args,
+            // Since arg is a string literal, it doesn't have SourceSection
+            // information yet, use the parent instancesObject's location
+            // instead.
+            instancesObject.getSourceLocation()
+        );
+
+        if (!toPopulate.containsKey(ruleName)) {
+            toPopulate.put(ruleName, newInstance);
+            if (verbose) {
+                System.err.println(
+                    "WORKER_INFO: register new instance: " +
+                    ruleName +
+                    " from " +
+                    newInstance.locationToGNATDiagnosisFormatString()
+                );
+            }
+        } else {
+            errorInLKQLRuleFile(
+                lkqlRuleFile,
+                "cannot add instance " +
+                ruleName +
+                " twice using the shortcut argument format from instances set included in rules set at: " +
+                newInstance.locationToGNATDiagnosisFormatString() +
+                ". Previous instance has been declared in" +
+                ((newInstance
+                                .instanceLocation()
+                                .equals(toPopulate.get(ruleName).instanceLocation()))
+                        ? " the same set"
+                        : ": " + toPopulate.get(ruleName).locationToGNATDiagnosisFormatString())
+            );
+        }
     }
 
     /** Util function which returns whether a rule accepts sole argument. */

lkql_jit/options/src/main/java/com/adacore/lkql_jit/options/RuleInstance.java

@@ -7,6 +7,7 @@
 
 import java.util.Map;
 import java.util.Optional;
+import org.graalvm.polyglot.SourceSection;
 import org.json.JSONException;
 import org.json.JSONObject;
 
@@ -34,7 +35,8 @@ public record RuleInstance(
     String ruleName,
     Optional<String> instanceName,
     SourceMode sourceMode,
-    Map<String, String> arguments
+    Map<String, String> arguments,
+    SourceSection instanceLocation
 ) {
     // ----- Constructor -----
 
@@ -57,7 +59,8 @@ public static RuleInstance fromJson(JSONObject jsonObject) throws JSONException
             jsonObject.getString("ruleName"),
             Optional.ofNullable(jsonObject.optString("instanceName", null)),
             SourceMode.valueOf(jsonObject.getString("sourceMode")),
-            JSONUtils.parseStringMap(jsonObject.getJSONObject("arguments"))
+            JSONUtils.parseStringMap(jsonObject.getJSONObject("arguments")),
+            null
         );
     }
 
@@ -77,6 +80,34 @@ public JSONObject toJson() {
             .put("instanceName", this.instanceName.orElse(null))
             .put("sourceMode", this.sourceMode.toString())
             .put("arguments", new JSONObject(this.arguments));
+        // Do not output source location information for now
+    }
+
+    /** Return whether this instance is equivalent to the other one, i.e.: it
+     * runs the same checker with the same parameters (only the instance name
+     * can differ). */
+    public boolean isEquivalent(RuleInstance o) {
+        return (
+            this.ruleName.equals(o.ruleName) &&
+            this.sourceMode.equals(o.sourceMode) &&
+            this.arguments.equals(o.arguments)
+        );
+    }
+
+    /** Return a String holding the location of the instance in the GNAT
+     * Diagnosis format when possible. */
+    public String locationToGNATDiagnosisFormatString() {
+        if (instanceLocation != null) {
+            return (
+                instanceLocation.getSource().getName() +
+                ":" +
+                instanceLocation.getStartLine() +
+                ":" +
+                instanceLocation.getStartColumn()
+            );
+        } else {
+            return "undefined:0:0";
+        }
     }
 
     // ----- Override methods ------