diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 63266ba1..77ccf6a6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -8,12 +8,12 @@ jobs: build: strategy: matrix: - java: [8, 11] + java: [11] sonar: - "" - "8.9.0.43852" include: - - java: 8 + - java: 11 sonar: "" archive: "yes" - java: 11 diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 920cbb55..91677cbf 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -18,7 +18,7 @@ jobs: - name: Set up JDK uses: actions/setup-java@v3 with: - java-version: '8' + java-version: '11' distribution: 'adopt' - name: Publish package diff --git a/codenarc-converter/CodeNarc b/codenarc-converter/CodeNarc index 89ced7a9..e6a1477c 160000 --- a/codenarc-converter/CodeNarc +++ b/codenarc-converter/CodeNarc @@ -1 +1 @@ -Subproject commit 89ced7a90051d109a3992d77eaf000ec4ef18f74 +Subproject commit e6a1477cf4159144dabdb9badafbbb875731302e diff --git a/pom.xml b/pom.xml index 2e49a10b..c655e9fa 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.sonarsource.parent parent - 59.0.29 + 64.0.211 org.sonarsource.groovy @@ -98,7 +98,7 @@ org.codenarc CodeNarc - 2.2.0 + 3.3.0 org.codehaus.groovy @@ -130,7 +130,7 @@ org.slf4j slf4j-simple - 1.7.36 + 2.0.9 @@ -161,7 +161,7 @@ com.diffplug.spotless spotless-maven-plugin - 2.29.0 + 2.41.1 diff --git a/sonar-groovy-plugin/pom.xml b/sonar-groovy-plugin/pom.xml index f0d186eb..32fb7578 100644 --- a/sonar-groovy-plugin/pom.xml +++ b/sonar-groovy-plugin/pom.xml @@ -23,7 +23,7 @@ com.github.spotbugs spotbugs-annotations - 4.5.3 + 4.8.3 provided @@ -93,7 +93,7 @@ org.mockito mockito-core - 3.12.4 + 5.8.0 test @@ -118,7 +118,7 @@ - 15000000 + 20000000 11000000 ${project.build.directory}/${project.build.finalName}.jar diff --git a/sonar-groovy-plugin/src/main/resources/org/sonar/plugins/groovy/rules.xml b/sonar-groovy-plugin/src/main/resources/org/sonar/plugins/groovy/rules.xml index c7012a05..db83989d 100644 --- a/sonar-groovy-plugin/src/main/resources/org/sonar/plugins/groovy/rules.xml +++ b/sonar-groovy-plugin/src/main/resources/org/sonar/plugins/groovy/rules.xml @@ -1,4 +1,4 @@ - + @@ -631,7 +631,7 @@ put @Override on a method implementing an interface. Use with Java MINOR - Checks for empty try blocks. Empty try blocks are confusing and serve no purpose.

+ Checks for empty try blocks. Empty try blocks are confusing and serve no purpose. This rule ignores all try-with-resources statements.

Here is an example of code that produces a violation:

    def myMethod() {
         try {
@@ -1482,6 +1482,68 @@ property applyToNonMainClasses to true.

+ + org.codenarc.rule.comments.SpaceAfterCommentDelimiterRule + MAJOR + + + <Since CodeNarc 3.3.0>

+

Checks that there is whitespace after comment delimiter (// and /*).

+

Notes:

+
    +
  • Comments are not available as part of the Groovy AST, so this rule is implemented using regular expression searches. The rule uses some heuristics to avoid false positives, but can also miss some actual violations (i.e., false negatives).
  • +
  • May cause violation for commented-out code (if there is no space after the comment delimiter)
  • +
+

Example of violations:

+
    /**Bad1                                             // violation
+     *Ignored .. not a violation
+     *    @author Some Developer
+     */
+    class MyClass {//Bad4                               // violation
+        int countThings(int start) { //some comment     // violation
+            //Do stuff                                  // violation
+        } //comment                                     // violation
+    }
+    //Other comment                                     // violation
+    /*Single line comment*/                             // violation
+
+]]>
+ bug +
+ + + org.codenarc.rule.comments.SpaceBeforeCommentDelimiterRule + MAJOR + + + <Since CodeNarc 3.3.0>

+

Checks that there is whitespace before comment characters (// and /*)

+

Notes:

+
    +
  • Comments are not available as part of the Groovy AST, so this rule is implemented using regular expression searches. The rule uses some heuristics to avoid false positives, but can also miss some actual violations (i.e., false negatives).
  • +
+

Example of violations:

+
    import org.acme.Stuff/**                            // violation
+     *    @author Some Developer
+     */
+    class MyClass {// class comment                     // violation
+        /**
+         *Return the calculated count of some stuff,
+         */
+        int countThings(int startIndex) {// comment     // violation
+            int count = 3 +// first part                // violation
+            reduce(23)
+            amount = 3 + amount/*violation*/            // violation
+        }//comment                                      // violation
+        
+        int count = 3//Other comment                    // violation
+        String x = doStuff()/*Single line comment*/     // violation
+    }
+
+]]>
+ bug +
+ @@ -1710,6 +1772,30 @@ within an outer synchronized statement does not cause a violation ( multi-threading + + org.codenarc.rule.concurrency.NoScriptBindingsRule + MAJOR + + + This rule reports occurrences of global variables that are bound to a script.

+

These should be avoided as concurrent executions can modify and read the shared +variable, leading to concurrency bugs.

+

Examples (within a script):

+
    b = 1                       // violation
+    def myMethod() {
+        a = 1                   // violation
+    }
+
+    // These are fields and local variables; they are OK
+    int b = 1
+    def myMethod2() {
+        Integer a = 1
+    }
+
+]]>
+ multi-threading +
+ org.codenarc.rule.concurrency.StaticCalendarFieldRule @@ -3145,18 +3231,21 @@ out of the ternary expression.

MAJOR - Check whether list and map literals contain optional trailing comma. -Rationale: Putting this comma in make is easier + Check whether list and map literals contain an optional trailing comma. +Rationale: Putting this comma in makes it easier to change the order of the elements or add new elements on the end.

This is valid code:

-
  int[] array1 = [] // one line declaration
-  int[] array2 = [ // empty list
+
  int[] array1 = []         // one line declaration
+  int[] array2 = [          // empty list
                  ]
-  int[] array3 = [1,2,3] // one line declaration
+  int[] array3 = [1,2,3]    // one line declaration
   int[] array4 = [1,
                   2,
-                  3, // contains trailing comma
+                  3,        // contains trailing comma
                  ]
+  int[] array5 = [1,
+                  2]        // last element followed by closing bracket on the same line
+                            // NOTE: This style actually violates the intention of this rule
 

Example of violations:

  int[] array2 = [1,
@@ -4797,6 +4886,11 @@ missed when reading the code.

Makes sure there are no consecutive lines that are either blank or whitespace only. This reduces the need to scroll further than necessary when reading code, and increases the likelihood that a logical block of code will fit on one screen for easier comprehension.

+

Known Limitations:

+
    +
  • Will create false positive violation for consecutive blank lines within string literals. For those cases, +configure the rule to skip that file (using doNotApplyToFileNames or doNotApplyToFilesMatching rule properties). See Standard Properties for Configuring Rules. Note that the class-based rule properties are not available on this rule.
  • +

Example of violation:

    def name
 
@@ -5014,15 +5108,11 @@ for parameter list.

MAJOR - Checks that there is no whitespace after the method name when a method call contains parenthesis or that there -is at most one space after the method name if the call does not contain parenthesis.

+ Checks that there is no whitespace after the method name when a method call contains parenthesis.

Examples of violations:

-
    aMethod ("arg") //violation
-    
-    aMethod  "arg" //violation
-    
-    throw new Exception () //violation
+
    aMethod ("arg")         // violation
     
+    throw new Exception ()  // violation
 
]]> convention @@ -5322,8 +5412,15 @@ properties below.

MAJOR - Check that there is at least one space (blank) or whitespace around each binary operator, -including: +, -, *, /, **, **, &&, ||, &, |, ?:, =, "as".

+ Check that there is at least one space (blank) or whitespace around each binary operator, including:

+
    +
  • Arithmetic operators: +, -, *, /, **
  • +
  • Assignment operators: =, +=, -=, *=, /=, **=, ?=
  • +
  • Relational operators: ==, !=, >, >=, <, <=, ===, !==
  • +
  • Logical operators: &&, ||
  • +
  • Bitwise operators: &, |
  • +
  • Other: ?:, "as"
  • +

Do not check dot ('.') operator. Do not check unary operators (!, +, -, ++, --, ?.). Do not check array ('[') operator.

Known limitations:

@@ -6784,6 +6881,15 @@ Consider calling key.toString().

In many case collectMany() yields the same result as collect{}.flatten(). It is easier to understand and more clearly conveys the intent.

+

Limitations:

+
    +
  • The collectMany() method does not recursively flatten nested collections, unlike collect().flatten(). +e.g.
  • +
+
     def l = [1, [2, 22], 3]
+     println l.collect{ [it, it*2] }.flatten() // [1, 2, 2, 22, 2, 22, 2, 22, 3, 6]
+     println l.collectMany{ [it, it*2] }       // [1, 2, [2, 22], [2, 22, 2, 22], 3, 6]
+

Example of violations:

def l = [1, 2, 3, 4]
 
@@ -7343,7 +7449,7 @@ default value is this comma separated list of regular expressions:

MAJOR - Rule that checks checks for JUnit setUp() methods that contain only a call to + Rule that checks for JUnit setUp() methods that contain only a call to super.setUp(). The method is then unnecessary.

This rule sets the default value of the applyToClassNames property to only match class names ending in 'Spec', 'Test', 'Tests' or 'TestCase'.

@@ -7363,7 +7469,7 @@ ending in 'Spec', 'Test', 'Tests' or 'TestCase'.

MAJOR - Rule that checks checks for JUnit tearDown() methods that contain only a call to + Rule that checks for JUnit tearDown() methods that contain only a call to super.tearDown(). The method is then unnecessary.

This rule sets the default value of the applyToClassNames property to only match class names ending in 'Spec', 'Test', 'Tests' or 'TestCase'.

@@ -7447,6 +7553,48 @@ Spock Specification classes.

+ + + org.codenarc.rule.junit.SpockMissingAssertRule + MAJOR + + + Spock treats all expressions on the first level of a then or expect block as an implicit assertion. +However, everything inside if/for/switch/... blocks is not an implicit assert, just a useless comparison (unless wrapped by a with or verifyAll).

+

This rule finds such expressions, where an explicit call to assert would be required. Please note that the rule might +produce false positives, as it relies on method names to determine whether an expression has a boolean type or not.

+

Example of violations:

+
    public class MySpec extends spock.lang.Specification {
+        def "test passes - does not behave as expected"() {
+            expect:
+            if (true) {
+                true == false // violation - is inside an if block, and therefore not treated as an implicit assertion by spock
+            }
+        }
+
+        def "test fails - behaves as expected"() {
+            expect:
+            if (true) {
+                with(new Object()) {
+                    true == false // no violation - expressions in with are treated as implicit assertions by spock
+                }
+            }
+        }
+    }
+
+]]>
+ junit + + specificationClassNames + + + + specificationSuperclassNames + + *Specification + +
+ org.codenarc.rule.junit.UnnecessaryFailRule @@ -7870,7 +8018,7 @@ popular Builder Pattern. If the 'build' method is in a class named *Builder then regex - (build.|create.) + (build.*|create.*) @@ -8013,7 +8161,7 @@ package name consists of only lowercase letters and numbers, separated by period regex - [a-z]+[a-z0-9](\.[a-z0-9]+) + [a-z]+[a-z0-9]*(\.[a-z0-9]+)* @@ -8486,7 +8634,6 @@ declarations: = *= /= %= += **= **= &= |= ^= `= eXcentia Sonar ABC Metric Plugin (for Java) includes a table of risk classifications for ABC scores for both methods and classes.
  • See the GMetrics ABC metric. This includes a discussion of guidelines for interpreting ABC scores.
  • -
  • This rule requires Groovy 1.6 (or later).
  • This rule requires the GMetrics jar on the classpath. See GMetrics.
  • ]]>
    @@ -8614,7 +8761,6 @@ Add one (1) for each occurrence of each of the following:

  • See the original paper describing Cyclomatic Complexity
  • See the GMetrics Cyclomatic Complexity metric. This includes a discussion of guidelines for interpreting cyclomatic complexity values.
  • -
  • This rule requires Groovy 1.6 (or later).
  • This rule requires the GMetrics jar on the classpath. See GMetrics.
  • ]]>
    @@ -9735,15 +9881,15 @@ definitions, for older versions of Groovy (* 1.7.10?). For instance, The 'public' modifier is not required on methods, constructors or classes.

    -

    Because of Groovy parsing limitations, this rule ignores methods (and constructors) that include Generic types in the method declaration.

    +

    Limitations:

    +
      +
    • Because of Groovy parsing limitations, this rule ignores methods (and constructors) that include Generic types in the method declaration.
    • +
    • This rule will not catch the public modifier if it is on a separate line from the rest of the declaration.
    • +

    Example of violations:

    -
        // violation on class
    -    public class MyClass {
    -        // violation on constructor
    -        public MyClass() {}
    -
    -        // violation on method
    -        public void myMethod() {}
    +
        public class MyClass {              // violation on class
    +        public MyClass() {}             // violation on constructor
    +        public void myMethod() {}       // violation on method
         }
     
    ]]> @@ -9862,7 +10008,8 @@ Any source line that even looks like it is a comment is ignored.

    Checks for explicit calls to setter methods which can, for the most part, be replaced by assignment to property. A setter is defined as a method call that matches set[A-Z] but not set[A-Z][A-Z] such as setURL(). -Setters take one method argument. Setter calls within an expression are ignored.

    +Setters take one method argument. Setter calls within an expression are ignored. Calls to static setter methods +within the same class are ignored.

    These bits of code produce violations:

      x.setProperty(1)
       x.setProperty(this.getA())
    @@ -9880,26 +10027,6 @@ Setters take one method argument. Setter calls within an expression are ignored.
         clumsy
       
     
    -  
    -  
    -    org.codenarc.rule.unnecessary.UnnecessarySubstringRule
    -    MAJOR
    -    
    -    
    -    NOTE: This rule is deprecated and will be removed in a future CodeNarc version. Its recommendation to use subscripts on
    -strings is not always safe/valid. See #562.

    -

    This rule finds usages of String.substring(int) and String.substring(int, int) that can be replaced by use of the -subscript operator. For instance, var.substring(5) can be replaced with var[5..-1].

    -

    Note that the String.substring(beginIndex,endIndex) method specifies a range of beginIndex..endIndex-1, while -Groovy's String subscript specifies an inclusive range. So, "123456".substring(1, 5) is equivalent to "123456"[1..4].

    -

    Example of violations:

    -
        myVar.substring(5)          // can use myVar[5..-1] instead
    -    myVar.substring(1, 5)       // can use myVar[1..4] instead
    -
    -]]>
    - clumsy -
    - org.codenarc.rule.unnecessary.UnnecessaryStringInstantiationRule @@ -10166,6 +10293,11 @@ For instance, to also ignore fields named 'fieldx', set the property to the 'fie true + + ignoreClassesAnnotatedWithNames + + Entity + ignoreFieldNames diff --git a/sonar-groovy-plugin/src/test/java/org/sonar/plugins/groovy/codenarc/CodeNarcRulesDefinitionTest.java b/sonar-groovy-plugin/src/test/java/org/sonar/plugins/groovy/codenarc/CodeNarcRulesDefinitionTest.java index 2300cfc8..8a0bb769 100644 --- a/sonar-groovy-plugin/src/test/java/org/sonar/plugins/groovy/codenarc/CodeNarcRulesDefinitionTest.java +++ b/sonar-groovy-plugin/src/test/java/org/sonar/plugins/groovy/codenarc/CodeNarcRulesDefinitionTest.java @@ -1,6 +1,6 @@ /* * Sonar Groovy Plugin - * Copyright (C) 2010-2021 SonarQube Community + * Copyright (C) 2010-2023 SonarQube Community * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -41,7 +41,7 @@ public void test() { assertThat(repository.language()).isEqualTo(Groovy.KEY); List rules = repository.rules(); - assertThat(rules).hasSize(394); + assertThat(rules).hasSize(397); List missingDebt = new LinkedList<>(); for (Rule rule : rules) { @@ -55,7 +55,13 @@ public void test() { } // From SONARGROOV-36, 'org.codenarc.rule.generic.IllegalSubclassRule' does not have debt by // purpose - assertThat(missingDebt).containsOnly("org.codenarc.rule.generic.IllegalSubclassRule.fixed"); + assertThat(missingDebt) + .containsOnly( + "org.codenarc.rule.generic.IllegalSubclassRule.fixed", + "org.codenarc.rule.junit.SpockMissingAssertRule", + "org.codenarc.rule.concurrency.NoScriptBindingsRule", + "org.codenarc.rule.comments.SpaceAfterCommentDelimiterRule", + "org.codenarc.rule.comments.SpaceBeforeCommentDelimiterRule"); Rule rule = repository.rule("org.codenarc.rule.braces.ElseBlockBracesRule"); assertThat(rule.params()).hasSize(1); diff --git a/sonar-groovy-plugin/src/test/java/org/sonar/plugins/groovy/surefire/GroovySurefireSensorTest.java b/sonar-groovy-plugin/src/test/java/org/sonar/plugins/groovy/surefire/GroovySurefireSensorTest.java index e1a2e933..db83de0d 100644 --- a/sonar-groovy-plugin/src/test/java/org/sonar/plugins/groovy/surefire/GroovySurefireSensorTest.java +++ b/sonar-groovy-plugin/src/test/java/org/sonar/plugins/groovy/surefire/GroovySurefireSensorTest.java @@ -1,6 +1,6 @@ /* * Sonar Groovy Plugin - * Copyright (C) 2010-2021 SonarQube Community + * Copyright (C) 2010-2023 SonarQube Community * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -19,7 +19,7 @@ package org.sonar.plugins.groovy.surefire; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Matchers.anyString; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; @@ -131,7 +131,7 @@ public void shouldHandleTestSuiteDetails() throws URISyntaxException { context .measure(":org.sonar.core.ExtensionsFinderTest", CoreMetrics.SKIPPED_TESTS) .value()) - .isEqualTo(0); + .isZero(); assertThat(context.measure(":org.sonar.core.ExtensionsFinderTest2", CoreMetrics.TESTS).value()) .isEqualTo(2); @@ -144,17 +144,17 @@ public void shouldHandleTestSuiteDetails() throws URISyntaxException { context .measure(":org.sonar.core.ExtensionsFinderTest2", CoreMetrics.TEST_FAILURES) .value()) - .isEqualTo(0); + .isZero(); assertThat( context .measure(":org.sonar.core.ExtensionsFinderTest2", CoreMetrics.TEST_ERRORS) .value()) - .isEqualTo(0); + .isZero(); assertThat( context .measure(":org.sonar.core.ExtensionsFinderTest2", CoreMetrics.SKIPPED_TESTS) .value()) - .isEqualTo(0); + .isZero(); assertThat(context.measure(":org.sonar.core.ExtensionsFinderTest3", CoreMetrics.TESTS).value()) .isEqualTo(1); @@ -167,12 +167,12 @@ public void shouldHandleTestSuiteDetails() throws URISyntaxException { context .measure(":org.sonar.core.ExtensionsFinderTest3", CoreMetrics.TEST_FAILURES) .value()) - .isEqualTo(0); + .isZero(); assertThat( context .measure(":org.sonar.core.ExtensionsFinderTest3", CoreMetrics.TEST_ERRORS) .value()) - .isEqualTo(0); + .isZero(); assertThat( context .measure(":org.sonar.core.ExtensionsFinderTest3", CoreMetrics.SKIPPED_TESTS) @@ -266,7 +266,7 @@ public void measuresShouldNotIncludeSkippedTests() throws URISyntaxException { assertThat(context.measure(":org.sonar.Foo", CoreMetrics.TESTS).value()).isEqualTo(2); assertThat(context.measure(":org.sonar.Foo", CoreMetrics.TEST_FAILURES).value()).isEqualTo(1); - assertThat(context.measure(":org.sonar.Foo", CoreMetrics.TEST_ERRORS).value()).isEqualTo(0); + assertThat(context.measure(":org.sonar.Foo", CoreMetrics.TEST_ERRORS).value()).isZero(); assertThat(context.measure(":org.sonar.Foo", CoreMetrics.SKIPPED_TESTS).value()).isEqualTo(1); } @@ -284,9 +284,9 @@ public void noSuccessRatioIfNoTests() throws URISyntaxException { "/org/sonar/plugins/groovy/surefire/SurefireSensorTest/noSuccessRatioIfNoTests/") .toURI()))); - assertThat(context.measure(":org.sonar.Foo", CoreMetrics.TESTS).value()).isEqualTo(0); - assertThat(context.measure(":org.sonar.Foo", CoreMetrics.TEST_FAILURES).value()).isEqualTo(0); - assertThat(context.measure(":org.sonar.Foo", CoreMetrics.TEST_ERRORS).value()).isEqualTo(0); + assertThat(context.measure(":org.sonar.Foo", CoreMetrics.TESTS).value()).isZero(); + assertThat(context.measure(":org.sonar.Foo", CoreMetrics.TEST_FAILURES).value()).isZero(); + assertThat(context.measure(":org.sonar.Foo", CoreMetrics.TEST_ERRORS).value()).isZero(); assertThat(context.measure(":org.sonar.Foo", CoreMetrics.SKIPPED_TESTS).value()).isEqualTo(2); } @@ -307,7 +307,7 @@ public void ignoreSuiteAsInnerClass() throws URISyntaxException { // ignore TestHandler$Input.xml assertThat( context.measure(":org.apache.shindig.protocol.TestHandler", CoreMetrics.TESTS).value()) - .isEqualTo(0); + .isZero(); assertThat( context .measure(":org.apache.shindig.protocol.TestHandler", CoreMetrics.SKIPPED_TESTS)