Merge branch 'mr/thevenoux/gnatcheck#19' into 'master'

GNATcheck: Improve rule duplicates handling

Closes eng/codepeer/gnatcheck#19

See merge request eng/libadalang/langkit-query-language!616

Author: thvnx

lkql_checker/src/lkql_checker-rules-rule_table.adb

@@ -413,19 +413,53 @@ package body Lkql_Checker.Rules.Rule_Table is
    function Check_Instance_Is_Unique
      (Instance_Name, Instantiation_Location : String) return Boolean
    is
+      Is_Legacy_File_Kind : constant Boolean :=
+        Instantiation_Location'Length > 0;
+
       Instance : constant Rule_Instance_Access := Get_Instance (Instance_Name);
    begin
       if Instance /= null then
-         Error
-           ("rule instance with the same name already exists: """
-            & Instance_Name
-            & """ previously instantiated at "
-            & (if Instance.Defined_At /= ""
+         declare
+            Location : constant String :=
+              (if Instance.Defined_At /= ""
                then To_String (Instance.Defined_At)
-               else "command line"),
-            Location => Instantiation_Location);
-         Bad_Rule_Detected := True;
-         return False;
+               else "command line");
+         begin
+
+            if not Is_Legacy_File_Kind
+              and then not Instance.Is_Alias
+              and then not Instance.Has_Parameters
+            then
+               --  If the instance to check for uniqueness comes from the
+               --  command line and is not an alias, we can salefy skip
+               --  duplicates if the rule has no parameters (we could also
+               --  compare parameters but this will require to add support to
+               --  compare them directly from the driver). Emit a simple
+               --  warning.
+
+               Warning
+                 ("skipping rule instance """
+                  & Instance_Name
+                  & """ (from command line) previously instantiated at "
+                  & Location);
+               return True;
+            else
+               --  Otherwise, emit an error.
+
+               Error
+                 ("cannot add rule instance named """
+                  & Instance_Name
+                  & """"
+                  & (if not Is_Legacy_File_Kind
+                     then " (specified from command line)"
+                     else "")
+                  & ", already instantiated at "
+                  & Location,
+                  Location => Instantiation_Location);
+               Bad_Rule_Detected := True;
+               return False;
+            end if;
+         end;
       end if;
       return True;
    end Check_Instance_Is_Unique;

lkql_checker/src/lkql_checker-rules.adb

@@ -3007,6 +3007,22 @@ package body Lkql_Checker.Rules is
 
    --  == Overriding operations on rule instances
 
+   --------------------
+   -- Has_Parameters --
+   --------------------
+
+   overriding
+   function Has_Parameters (Instance : Custom_Instance) return Boolean is
+   begin
+      return not Instance.Arguments.Is_Empty;
+   end Has_Parameters;
+
+   overriding
+   function Has_Parameters (Instance : Compiler_Instance) return Boolean is
+   begin
+      return not Instance.Arguments.Is_Empty;
+   end Has_Parameters;
+
    ------------------------------------
    -- Process_Instance_Params_Object --
    ------------------------------------

lkql_checker/src/lkql_checker-rules.ads

@@ -225,6 +225,10 @@ package Lkql_Checker.Rules is
    -- Operations that may be redefined by specific rule instances --
    -----------------------------------------------------------------
 
+   function Has_Parameters (Instance : Rule_Instance) return Boolean
+   is (False);
+   --  Return whether the Instance has some parameters set.
+
    procedure Process_Instance_Params_Object
      (Instance : in out Rule_Instance; Params_Object : in out JSON_Value)
    is null;
@@ -290,6 +294,10 @@ package Lkql_Checker.Rules is
    end record;
    --  Represents an instance of a rule with only one integer parameter
 
+   function Has_Parameters
+     (Instance : One_Integer_Parameter_Instance) return Boolean
+   is (True);
+
    overriding
    procedure Process_Instance_Params_Object
      (Instance      : in out One_Integer_Parameter_Instance;
@@ -321,6 +329,10 @@ package Lkql_Checker.Rules is
    end record;
    --  Represents an instance of a rule with only one boolean parameter
 
+   function Has_Parameters
+     (Instance : One_Boolean_Parameter_Instance) return Boolean
+   is (True);
+
    overriding
    procedure Process_Instance_Params_Object
      (Instance      : in out One_Boolean_Parameter_Instance;
@@ -351,6 +363,10 @@ package Lkql_Checker.Rules is
    end record;
    --  Represents an instance of a rule with only one string parameter
 
+   function Has_Parameters
+     (Instance : One_String_Parameter_Instance) return Boolean
+   is (True);
+
    overriding
    procedure Process_Instance_Params_Object
      (Instance      : in out One_String_Parameter_Instance;
@@ -385,6 +401,10 @@ package Lkql_Checker.Rules is
    with null record;
    --  Represents an instance of a rule with only on array parameter
 
+   function Has_Parameters
+     (Instance : One_Array_Parameter_Instance) return Boolean
+   is (True);
+
    overriding
    procedure Process_Instance_Params_Object
      (Instance      : in out One_Array_Parameter_Instance;
@@ -408,6 +428,10 @@ package Lkql_Checker.Rules is
    end record;
    --  Represents an instance of a rule with one integer and boolean parameters
 
+   function Has_Parameters
+     (Instance : One_Integer_Or_Booleans_Parameter_Instance) return Boolean
+   is (True);
+
    overriding
    procedure Process_Instance_Params_Object
      (Instance      : in out One_Integer_Or_Booleans_Parameter_Instance;
@@ -447,6 +471,10 @@ package Lkql_Checker.Rules is
    end record;
    --  Represents an instance of a rule about identifier suffixes
 
+   function Has_Parameters
+     (Instance : Identifier_Suffixes_Instance) return Boolean
+   is (True);
+
    overriding
    procedure Process_Instance_Params_Object
      (Instance      : in out Identifier_Suffixes_Instance;
@@ -486,6 +514,10 @@ package Lkql_Checker.Rules is
    end record;
    --  Represents an instance of a rule about identifier prefixes
 
+   function Has_Parameters
+     (Instance : Identifier_Prefixes_Instance) return Boolean
+   is (True);
+
    overriding
    procedure Process_Instance_Params_Object
      (Instance      : in out Identifier_Prefixes_Instance;
@@ -522,6 +554,10 @@ package Lkql_Checker.Rules is
    end record;
    --  Represents an instance of a rule about identifiers casing
 
+   function Has_Parameters
+     (Instance : Identifier_Casing_Instance) return Boolean
+   is (True);
+
    overriding
    procedure Process_Instance_Params_Object
      (Instance      : in out Identifier_Casing_Instance;
@@ -553,6 +589,9 @@ package Lkql_Checker.Rules is
    end record;
    --  Represents an instance of a rule about forbiddening things
 
+   function Has_Parameters (Instance : Forbidden_Instance) return Boolean
+   is (True);
+
    overriding
    procedure Process_Instance_Params_Object
      (Instance : in out Forbidden_Instance; Params_Object : in out JSON_Value);
@@ -582,6 +621,10 @@ package Lkql_Checker.Rules is
    end record;
    --  Represents an instance of a rule about silent exception handlers
 
+   function Has_Parameters
+     (Instance : Silent_Exception_Handlers_Instance) return Boolean
+   is (True);
+
    overriding
    procedure Process_Instance_Params_Object
      (Instance      : in out Silent_Exception_Handlers_Instance;
@@ -612,6 +655,9 @@ package Lkql_Checker.Rules is
    end record;
    --  Represents an instance of a rule with arbitrary parameters
 
+   overriding
+   function Has_Parameters (Instance : Custom_Instance) return Boolean;
+
    overriding
    procedure Process_Instance_Params_Object
      (Instance : in out Custom_Instance; Params_Object : in out JSON_Value);
@@ -639,4 +685,7 @@ package Lkql_Checker.Rules is
       Arguments : String_Vector;
    end record;
 
+   overriding
+   function Has_Parameters (Instance : Compiler_Instance) return Boolean;
+
 end Lkql_Checker.Rules;

testsuite/drivers/gnatcheck_driver.py

@@ -726,4 +726,15 @@ def output_refiners(self) -> list[OutputRefiner]:
                 "project search path: <gpr_path>",
             )
         )
+
+        # Remove sloc information for gnatcheck rules temporary files (instances
+        # order is not deterministic)
+        if self.test_env.get("canonicalize_gnatcheck_tmp_files", False):
+            result.append(
+                PatternSubstitute(
+                    "gnatcheck-rules[0-9]+.TMP:[0-9]+:[0-9]+",
+                    "gnatcheck-rules<N>.TMP:<L>:<C>",
+                )
+            )
+
         return result

testsuite/tests/gnatcheck_errors/same_name_instances/rules.lkql

@@ -0,0 +1,3 @@
+val rules = @{
+  Slices
+}

testsuite/tests/gnatcheck_errors/same_name_instances/test.out

@@ -1,21 +1,41 @@
 In rule options
 ===============
 
-gnatcheck: error: rule instance with the same name already exists: "same_alias" previously instantiated at command line
+gnatcheck: error: cannot add rule instance named "same_alias" (specified from command line), already instantiated at command line
 gnatcheck: info: if you want to pass multiple parameters to a rule you should use the comma separated notation: e.g. +RMy_Rule:Param1,Param2
-gnatcheck: error: rule instance with the same name already exists: "comp_check" previously instantiated at command line
-gnatcheck: error: rule instance with the same name already exists: "comp_check" previously instantiated at command line
+gnatcheck: error: cannot add rule instance named "comp_check" (specified from command line), already instantiated at command line
+gnatcheck: error: cannot add rule instance named "comp_check" (specified from command line), already instantiated at command line
 >>>program returned status code 5
 
-In command-line
-===============
+In rule options (with alias named after another rule)
+=====================================================
+
+gnatcheck: error: cannot add rule instance named "Slices_Alias" (specified from command line), already instantiated at command line
+gnatcheck: info: if you want to pass multiple parameters to a rule you should use the comma separated notation: e.g. +RMy_Rule:Param1,Param2
+gnatcheck: error: cannot add rule instance named "Goto_Statements" (specified from command line), already instantiated at command line
+gnatcheck: warning: skipping rule instance "slices" (from command line) previously instantiated at command line
+gnatcheck-rules<N>.TMP:<L>:<C>: warning: instance slices runs the same check than instance slices_alias declared at gnatcheck-rules<N>.TMP:<L>:<C>
+>>>program returned status code 5
+
+In command-line (rules with default arguments)
+==============================================
 
-gnatcheck: error: rule instance with the same name already exists: "goto_statements" previously instantiated at command line
+gnatcheck: error: cannot add rule instance named "goto_statements" (specified from command line), already instantiated at command line
 >>>program returned status code 5
 
+In command-line (rules without arguments)
+=========================================
+
+gnatcheck: warning: skipping rule instance "slices" (from command line) previously instantiated at command line
+
+In command-line and rules file (rules without arguments)
+========================================================
+
+gnatcheck: warning: skipping rule instance "slices" (from command line) previously instantiated at rules.lkql:1:1
+
 In command-line and rule options
 ================================
 
-rules.txt:1:1: error: rule instance with the same name already exists: "goto_statements" previously instantiated at command line
+rules.txt:1:1: error: cannot add rule instance named "goto_statements", already instantiated at command line
 gnatcheck: info: if you want to pass multiple parameters to a rule you should use the comma separated notation: e.g. +RMy_Rule:Param1,Param2
 >>>program returned status code 5

testsuite/tests/gnatcheck_errors/same_name_instances/test.yaml

@@ -1,5 +1,6 @@
 driver: gnatcheck
 format: brief
+canonicalize_gnatcheck_tmp_files: true
 input_sources:
   - main.adb
 tests:
@@ -10,12 +11,32 @@ tests:
       - +R:comp_check:Style_Checks:xz
       - +R:comp_check:Warnings:u
       - +R:comp_check:Restrictions:No_Access_Subprograms
-  - label: In command-line
+  - label: In rule options (with alias named after another rule)
+    rules:
+      - +R:Slices_Alias:Slices
+      - +R:Slices_Alias:Slices
+      - +RGoto_Statements
+      - +R:Goto_Statements:Slices
+      - +RSlices
+      - +RSlices
+  - label: In command-line (rules with default arguments)
     extra_args:
       - -r
       - goto_statements
       - -r
       - goto_statements
+  - label: In command-line (rules without arguments)
+    extra_args:
+      - -r
+      - slices
+      - -r
+      - slices
+  - label: In command-line and rules file (rules without arguments)
+    extra_args:
+      - -r
+      - slices
+      - --rule-file
+      - rules.lkql
   - label: In command-line and rule options
     extra_args:
       - -r