refactor: Use full string key for Sonar rules (ASSERT-KTH#581)

* Use full SonarJava key in entire application

* Make backwards compatible by adding the S prefix in the CLI

* Update rule key regex in support script

* Update README with new rule keys

* Throw exception if no rule key can be found

README.md

@@ -113,50 +113,61 @@ Repair Sonar rule violations in a targeted project.
                                time (need to specify --maxFilesPerSegment if
                                not default)
       --rule-key=<ruleKey>   Choose one of the following rule keys:
-                             1217: "Thread.run()" should not be called directly
-                             1444: "public static" fields should be constant
+                             S1118: Utility classes should not have public
+                               constructors
+                             	(incomplete: Only handles implicit public
+                               constructor)
+                             S1132: Strings literals should be placed on the
+                               left side when checking for equality
+                             S1155: Collection.isEmpty() should be used to test
+                               for emptiness
+                             S1217: "Thread.run()" should not be called directly
+                             S1444: "public static" fields should be constant
                              	(incomplete: does not fix variable naming)
-                             1656: Variables should not be self-assigned
-                             1854: Unused assignments should be removed
-                             1860: Synchronization should not be based on
+                             S1481: Unused local variables should be removed
+                             S1596: "Collections.EMPTY_LIST", "EMPTY_MAP", and
+                               "EMPTY_SET" should not be used
+                             S1656: Variables should not be self-assigned
+                             S1854: Unused assignments should be removed
+                             S1860: Synchronization should not be based on
                                Strings or boxed primitives
-                             1948: Fields in a "Serializable" class should
+                             S1948: Fields in a "Serializable" class should
                                either be transient or serializable
-                             2057: Every class implementing Serializable should
-                               declare a static final serialVersionUID.
-                                (incomplete: This processor does not address the
+                             S2057: Every class implementing Serializable
+                               should declare a static final serialVersionUID.
+                             	(incomplete: This processor does not address the
                                case where the class already has a
                                serialVersionUID with a non long type.)
-                             2095: Resources should be closed
-                             2097: "equals(Object obj)" should test argument
+                             S2095: Resources should be closed
+                             S2097: "equals(Object obj)" should test argument
                                type
-                             2111: "BigDecimal(double)" should not be used
-                             2116: "hashCode" and "toString" should not be
+                             S2111: "BigDecimal(double)" should not be used
+                             S2116: "hashCode" and "toString" should not be
                                called on array instances
-                             2142: "InterruptedException" should not be ignored
-                             2164: Math should not be performed on floats
-                             2167: "compareTo" should not return "Integer.
+                             S2142: "InterruptedException" should not be ignored
+                             S2164: Math should not be performed on floats
+                             S2167: "compareTo" should not return "Integer.
                                MIN_VALUE"
-                             2184: Math operands should be cast before
+                             S2184: Math operands should be cast before
                                assignment
-                             2204: ".equals()" should not be used to test the
+                             S2204: ".equals()" should not be used to test the
                                values of "Atomic" classes
-                             2225: "toString()" and "clone()" methods should
+                             S2225: "toString()" and "clone()" methods should
                                not return null
                              	(incomplete: does not fix null returning clone())
-                             2272: "Iterator.next()" methods should throw
+                             S2272: "Iterator.next()" methods should throw
                                "NoSuchElementException"
-                             2755: XML parsers should not be vulnerable to XXE
+                             S2755: XML parsers should not be vulnerable to XXE
                                attacks
                              	(incomplete: This processor is a WIP and
                                currently supports a subset of rule 2755. See
                                Sorald's documentation for details.)
-                             3032: JEE applications should not "getClassLoader"
-                             3067: "getClass" should not be used for
+                             S3032: JEE applications should not "getClassLoader"
+                             S3067: "getClass" should not be used for
                                synchronization
-                             3984: Exception should not be created without
+                             S3984: Exception should not be created without
                                being thrown
-                             4973: Strings and Boxed types should be compared
+                             S4973: Strings and Boxed types should be compared
                                using "equals()"
       --stats-output-file=<statsOutputFile>
                              Path to a file to store execution statistics in

experimentation/tools/sorald/_helpers/soraldwrapper.py

@@ -94,7 +94,7 @@ def available_rule_keys(
         for line in itertools.takewhile(
             lambda l: not l.strip().startswith("--"), dropped_initial_lines
         )
-        if (match := re.search(r"^\d+(?=:)", line.strip()))
+        if (match := re.search(r"^[^\s]+(?=:)", line.strip()))
     ]
 
 

src/main/java/sorald/Processors.java

@@ -11,46 +11,46 @@ public class Processors {
     private Processors() {}
 
     // GENERATED FIELD
-    private static final Map<Integer, Class<? extends SoraldAbstractProcessor<?>>>
+    private static final Map<String, Class<? extends SoraldAbstractProcessor<?>>>
             RULE_KEY_TO_PROCESSOR =
                     new java.util.HashMap<>() {
                         {
-                            put(1118, UtilityClassWithPublicConstructorProcessor.class);
-                            put(1132, StringLiteralInsideEqualsProcessor.class);
-                            put(1155, CollectionIsEmptyProcessor.class);
-                            put(1217, ThreadRunProcessor.class);
-                            put(1444, PublicStaticFieldShouldBeFinalProcessor.class);
-                            put(1481, UnusedLocalVariableProcessor.class);
-                            put(1596, CollectionsEmptyConstantsProcessor.class);
-                            put(1656, SelfAssignementProcessor.class);
-                            put(1854, DeadStoreProcessor.class);
-                            put(1860, SynchronizationOnStringOrBoxedProcessor.class);
-                            put(1948, SerializableFieldInSerializableClassProcessor.class);
-                            put(2057, SerialVersionUidProcessor.class);
-                            put(2095, UnclosedResourcesProcessor.class);
-                            put(2097, EqualsArgumentTypeProcessor.class);
-                            put(2111, BigDecimalDoubleConstructorProcessor.class);
-                            put(2116, ArrayHashCodeAndToStringProcessor.class);
-                            put(2142, InterruptedExceptionProcessor.class);
-                            put(2164, MathOnFloatProcessor.class);
-                            put(2167, CompareToReturnValueProcessor.class);
-                            put(2184, CastArithmeticOperandProcessor.class);
-                            put(2204, EqualsOnAtomicClassProcessor.class);
-                            put(2225, ToStringReturningNullProcessor.class);
-                            put(2272, IteratorNextExceptionProcessor.class);
-                            put(2755, XxeProcessingProcessor.class);
-                            put(3032, GetClassLoaderProcessor.class);
-                            put(3067, SynchronizationOnGetClassProcessor.class);
-                            put(3984, UnusedThrowableProcessor.class);
-                            put(4973, CompareStringsBoxedTypesWithEqualsProcessor.class);
+                            put("S1118", UtilityClassWithPublicConstructorProcessor.class);
+                            put("S1132", StringLiteralInsideEqualsProcessor.class);
+                            put("S1155", CollectionIsEmptyProcessor.class);
+                            put("S1217", ThreadRunProcessor.class);
+                            put("S1444", PublicStaticFieldShouldBeFinalProcessor.class);
+                            put("S1481", UnusedLocalVariableProcessor.class);
+                            put("S1596", CollectionsEmptyConstantsProcessor.class);
+                            put("S1656", SelfAssignementProcessor.class);
+                            put("S1854", DeadStoreProcessor.class);
+                            put("S1860", SynchronizationOnStringOrBoxedProcessor.class);
+                            put("S1948", SerializableFieldInSerializableClassProcessor.class);
+                            put("S2057", SerialVersionUidProcessor.class);
+                            put("S2095", UnclosedResourcesProcessor.class);
+                            put("S2097", EqualsArgumentTypeProcessor.class);
+                            put("S2111", BigDecimalDoubleConstructorProcessor.class);
+                            put("S2116", ArrayHashCodeAndToStringProcessor.class);
+                            put("S2142", InterruptedExceptionProcessor.class);
+                            put("S2164", MathOnFloatProcessor.class);
+                            put("S2167", CompareToReturnValueProcessor.class);
+                            put("S2184", CastArithmeticOperandProcessor.class);
+                            put("S2204", EqualsOnAtomicClassProcessor.class);
+                            put("S2225", ToStringReturningNullProcessor.class);
+                            put("S2272", IteratorNextExceptionProcessor.class);
+                            put("S2755", XxeProcessingProcessor.class);
+                            put("S3032", GetClassLoaderProcessor.class);
+                            put("S3067", SynchronizationOnGetClassProcessor.class);
+                            put("S3984", UnusedThrowableProcessor.class);
+                            put("S4973", CompareStringsBoxedTypesWithEqualsProcessor.class);
                         }
                     };
 
     // GENERATED FIELD
     public static final String RULE_DESCRIPTIONS =
-            "1118: Utility classes should not have public constructors\n\t(incomplete: Only handles implicit public constructor)\n1132: Strings literals should be placed on the left side when checking for equality\n1155: Collection.isEmpty() should be used to test for emptiness\n1217: \"Thread.run()\" should not be called directly\n1444: \"public static\" fields should be constant\n\t(incomplete: does not fix variable naming)\n1481: Unused local variables should be removed\n1596: \"Collections.EMPTY_LIST\", \"EMPTY_MAP\", and \"EMPTY_SET\" should not be used\n1656: Variables should not be self-assigned\n1854: Unused assignments should be removed\n1860: Synchronization should not be based on Strings or boxed primitives\n1948: Fields in a \"Serializable\" class should either be transient or serializable\n2057: Every class implementing Serializable should declare a static final serialVersionUID.\n\t(incomplete: This processor does not address the case where the class already has a serialVersionUID with a non long type.)\n2095: Resources should be closed\n2097: \"equals(Object obj)\" should test argument type\n2111: \"BigDecimal(double)\" should not be used\n2116: \"hashCode\" and \"toString\" should not be called on array instances\n2142: \"InterruptedException\" should not be ignored\n2164: Math should not be performed on floats\n2167: \"compareTo\" should not return \"Integer.MIN_VALUE\"\n2184: Math operands should be cast before assignment\n2204: \".equals()\" should not be used to test the values of \"Atomic\" classes\n2225: \"toString()\" and \"clone()\" methods should not return null\n\t(incomplete: does not fix null returning clone())\n2272: \"Iterator.next()\" methods should throw \"NoSuchElementException\"\n2755: XML parsers should not be vulnerable to XXE attacks\n\t(incomplete: This processor is a WIP and currently supports a subset of rule 2755. See Sorald\'s documentation for details.)\n3032: JEE applications should not \"getClassLoader\"\n3067: \"getClass\" should not be used for synchronization\n3984: Exception should not be created without being thrown\n4973: Strings and Boxed types should be compared using \"equals()\"";
+            "S1118: Utility classes should not have public constructors\n\t(incomplete: Only handles implicit public constructor)\nS1132: Strings literals should be placed on the left side when checking for equality\nS1155: Collection.isEmpty() should be used to test for emptiness\nS1217: \"Thread.run()\" should not be called directly\nS1444: \"public static\" fields should be constant\n\t(incomplete: does not fix variable naming)\nS1481: Unused local variables should be removed\nS1596: \"Collections.EMPTY_LIST\", \"EMPTY_MAP\", and \"EMPTY_SET\" should not be used\nS1656: Variables should not be self-assigned\nS1854: Unused assignments should be removed\nS1860: Synchronization should not be based on Strings or boxed primitives\nS1948: Fields in a \"Serializable\" class should either be transient or serializable\nS2057: Every class implementing Serializable should declare a static final serialVersionUID.\n\t(incomplete: This processor does not address the case where the class already has a serialVersionUID with a non long type.)\nS2095: Resources should be closed\nS2097: \"equals(Object obj)\" should test argument type\nS2111: \"BigDecimal(double)\" should not be used\nS2116: \"hashCode\" and \"toString\" should not be called on array instances\nS2142: \"InterruptedException\" should not be ignored\nS2164: Math should not be performed on floats\nS2167: \"compareTo\" should not return \"Integer.MIN_VALUE\"\nS2184: Math operands should be cast before assignment\nS2204: \".equals()\" should not be used to test the values of \"Atomic\" classes\nS2225: \"toString()\" and \"clone()\" methods should not return null\n\t(incomplete: does not fix null returning clone())\nS2272: \"Iterator.next()\" methods should throw \"NoSuchElementException\"\nS2755: XML parsers should not be vulnerable to XXE attacks\n\t(incomplete: This processor is a WIP and currently supports a subset of rule 2755. See Sorald\'s documentation for details.)\nS3032: JEE applications should not \"getClassLoader\"\nS3067: \"getClass\" should not be used for synchronization\nS3984: Exception should not be created without being thrown\nS4973: Strings and Boxed types should be compared using \"equals()\"";
 
-    public static Class<? extends SoraldAbstractProcessor<?>> getProcessor(int key) {
+    public static Class<? extends SoraldAbstractProcessor<?>> getProcessor(String key) {
         return RULE_KEY_TO_PROCESSOR.get(key);
     }
 }

src/main/java/sorald/Repair.java

@@ -91,7 +91,7 @@ public SoraldAbstractProcessor<?> repair(Set<RuleViolation> ruleViolations) {
         String ruleKey = distinctRuleKeys.get(0);
         Path inputDir = Path.of(config.getSource());
 
-        SoraldAbstractProcessor<?> processor = createProcessor(Integer.parseInt(ruleKey));
+        SoraldAbstractProcessor<?> processor = createProcessor(ruleKey);
         Stream<CtModel> models = repair(inputDir, processor, ruleViolations);
 
         models.forEach(
@@ -314,7 +314,7 @@ private static Supplier<? extends DefaultJavaPrettyPrinter> createDefaultPrinter
         return () -> new DefaultJavaPrettyPrinter(env);
     }
 
-    private SoraldAbstractProcessor<?> createBaseProcessor(Integer ruleKey) {
+    private SoraldAbstractProcessor<?> createBaseProcessor(String ruleKey) {
         try {
             Class<?> processor = Processors.getProcessor(ruleKey);
             if (processor != null) {
@@ -330,7 +330,7 @@ private SoraldAbstractProcessor<?> createBaseProcessor(Integer ruleKey) {
         return null;
     }
 
-    private SoraldAbstractProcessor<?> createProcessor(Integer ruleKey) {
+    private SoraldAbstractProcessor<?> createProcessor(String ruleKey) {
         SoraldAbstractProcessor<?> processor = createBaseProcessor(ruleKey);
         if (processor != null) {
             return processor

src/main/java/sorald/annotations/ProcessorAnnotation.java

@@ -8,7 +8,7 @@
 @Retention(RetentionPolicy.RUNTIME)
 @Target(ElementType.TYPE)
 public @interface ProcessorAnnotation {
-    public int key();
+    public String key();
 
     public String description();
 }

src/main/java/sorald/annotations/ProcessorsClassGenerator.java

@@ -15,8 +15,8 @@ public class ProcessorsClassGenerator<T>
         extends AbstractAnnotationProcessor<ProcessorAnnotation, CtClass<T>> {
     private CtCompilationUnit cu = null;
     private CtType<?> processorsClass = null;
-    private final SortedMap<Integer, CtClass<?>> processorMap = new TreeMap<>();
-    private final SortedMap<Integer, String> descriptions = new TreeMap<>();
+    private final SortedMap<String, CtClass<?>> processorMap = new TreeMap<>();
+    private final SortedMap<String, String> descriptions = new TreeMap<>();
 
     @Override
     public void process(ProcessorAnnotation annotation, CtClass<T> element) {
@@ -51,9 +51,9 @@ private CtExpression<?> generateProductionProcessorMapInitializer() {
                         + processorMap.entrySet().stream()
                                 .map(
                                         entry ->
-                                                "put("
+                                                "put(\""
                                                         + entry.getKey()
-                                                        + ","
+                                                        + "\","
                                                         + entry.getValue().getSimpleName()
                                                         + ".class);")
                                 .collect(Collectors.joining("\n"))

src/main/java/sorald/cli/MineCommand.java

@@ -128,9 +128,7 @@ private static List<? extends JavaFileScanner> inferCheckInstances(
                         : checks.stream()
                                 .filter(
                                         sc -> {
-                                            int key =
-                                                    Integer.parseInt(
-                                                            Checks.getRuleKey(sc.getClass()));
+                                            String key = Checks.getRuleKey(sc.getClass());
                                             return Processors.getProcessor(key) != null;
                                         })
                                 .collect(Collectors.toList());