Merge branch 'feat/grails-code-analysis-plugin' into feat/grails-jacoco-plugin

Author: jamesfredley

.agents/skills/violation-fixer/SKILL.md

@@ -31,7 +31,7 @@ Activate this skill when:
 | Plugin | Applied to | Responsibility |
 |--------|-----------|----------------|
 | `org.apache.grails.gradle.grails-code-style` | Every subproject | Applies Checkstyle and CodeNarc; registers per-project `codeStyle` task; redirects XML reports to root `build/reports/codestyle/` |
-| `org.apache.grails.gradle.grails-code-analysis` | Every subproject | Applies PMD and SpotBugs (both opt-in); registers per-project `codeAnalysis` task; redirects XML reports to root `build/reports/codestyle/` |
+| `org.apache.grails.gradle.grails-code-analysis` | Every subproject | Applies PMD and SpotBugs (both opt-in); registers per-project `codeAnalysis` task; redirects XML reports to root `build/reports/codeanalysis/` |
 | `org.apache.grails.gradle.grails-jacoco` | Every subproject | Applies JaCoCo; wires `jacocoTestReport` to run after each `test` task |
 | `org.apache.grails.gradle.grails-violation-aggregation` | **Root project only** | Registers `aggregateViolations` and `aggregateJacocoCoverage` tasks; writes Markdown summaries to `build/reports/violations/` |
 
@@ -214,11 +214,13 @@ build/reports/codestyle/        ← XML inputs for style aggregation
 │   ├── grails-core-checkstyleMain.xml
 │   ├── grails-web-mvc-checkstyleMain.xml
 │   └── ...
-├── codenarc/
-│   ├── grails-core-codenarcMain.xml
-│   └── ...
-├── pmd/       (if enabled)
-└── spotbugs/  (if enabled)
+└── codenarc/
+    ├── grails-core-codenarcMain.xml
+    └── ...
+
+build/reports/codeanalysis/     ← XML inputs for analysis aggregation (if enabled)
+├── pmd/
+└── spotbugs/
 
 build/reports/violations/       ← Markdown summaries written by aggregateViolations
 ├── CODENARC_VIOLATIONS.md

.github/workflows/codeanalysis.yml

@@ -44,7 +44,7 @@ jobs:
         with:
           develocity-access-key: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
       - name: "🔎 Check Core Projects"
-        run: ./gradlew aggregateAnalysisViolations --continue
+        run: ./gradlew aggregateAnalysisViolations --continue -Pgrails.codeanalysis.enabled.pmd=true -Pgrails.codeanalysis.enabled.spotbugs=true
       - name: "📤 Upload Reports"
         if: always()
         uses: actions/upload-artifact@v7.0.1
@@ -78,7 +78,7 @@ jobs:
           develocity-access-key: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
       - name: "🔎 Check Gradle Plugin Projects"
         working-directory: grails-gradle
-        run: ./gradlew aggregateAnalysisViolations --continue
+        run: ./gradlew aggregateAnalysisViolations --continue -Pgrails.codeanalysis.enabled.pmd=true -Pgrails.codeanalysis.enabled.spotbugs=true
       - name: "📤 Upload Reports"
         if: always()
         uses: actions/upload-artifact@v7.0.1

build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsCodeAnalysisExtension.groovy

@@ -30,7 +30,7 @@ import org.gradle.api.model.ObjectFactory
 class GrailsCodeAnalysisExtension {
 
     /**
-     * Defaults to project.rootProject.buildDir/codestyle/pmd.
+     * Defaults to project.rootProject.buildDir/codeanalysis/pmd.
      * Directory for PMD configuration files (e.g. pmd.xml).
      */
     final DirectoryProperty pmdDirectory

build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsCodeStyleExtension.groovy

@@ -25,7 +25,6 @@ import groovy.transform.CompileStatic
 import org.gradle.api.Project
 import org.gradle.api.file.DirectoryProperty
 import org.gradle.api.model.ObjectFactory
-import org.gradle.api.provider.Property
 
 @CompileStatic
 class GrailsCodeStyleExtension {

build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsCodeStylePlugin.groovy

@@ -109,15 +109,17 @@ class GrailsCodeStylePlugin implements Plugin<Project> {
     }
 
     private static void createOrLoad(Path expectedPath, String defaultResource, Project project) {
-        boolean defaultPath = expectedPath.startsWith(project.rootProject.buildDir.toPath())
-        if (!Files.exists(expectedPath) || expectedPath.size() == 0 || defaultPath) {
-            def defaultValue = GrailsCodeStylePlugin.getResourceAsStream(defaultResource)
-            if (!defaultValue) {
-                throw new IllegalStateException("Could not locate default configuration file: ${defaultResource}")
-            }
-            // TODO: This really need to use gradle caching instead
-            project.logger.info("Replacing code style configuration")
-            expectedPath.text = defaultValue.text
+        def defaultValue = GrailsCodeStylePlugin.getResourceAsStream(defaultResource)
+        if (!defaultValue) {
+            throw new IllegalStateException("Could not locate default configuration file: ${defaultResource}")
+        }
+        String defaultText = defaultValue.text
+        boolean missing = !Files.exists(expectedPath) || expectedPath.size() == 0
+        // Only rewrite when missing or the on-disk content differs from the bundled
+        // resource, so repeated builds across many subprojects don't churn the file.
+        if (missing || expectedPath.text != defaultText) {
+            project.logger.debug("Writing code style configuration to ${expectedPath}")
+            expectedPath.text = defaultText
         }
     }
 
@@ -228,15 +230,23 @@ class GrailsCodeStylePlugin implements Plugin<Project> {
                     content = content.replaceAll(/(class\s+[^{]+\{\n)([ \t]*[^ \s\n\/])/, '$1\n$2')
 
                     // 2. SpaceAroundMapEntryColon
-                    content = content.replaceAll(/([\[,]\s*(?:[\w\-.]+|'[^']+'|"[^"]+")):([^\s\/])/, '$1: $2')
-                    content = content.replaceAll(/(\(\s*(?:[\w\-.]+|'[^']+'|"[^"]+")):([^\s\/])/, '$1: $2')
+                    // (?!:) prevents matching the first : in a :: method reference (e.g. String::trim)
+                    content = content.replaceAll(/([\[,]\s*(?:[\w\-.]+|'[^']+'|"[^"]+")):(?!:)([^\s\/])/, '$1: $2')
+                    content = content.replaceAll(/(\(\s*(?:[\w\-.]+|'[^']+'|"[^"]+")):(?!:)([^\s\/])/, '$1: $2')
 
                     // 3. UnnecessaryGString
-                    content = content.replaceAll(/(?<!\\)(?<!")"([^"$\n\\]*)"(?!")/) { all, inner ->
+                    // The alternation skips over single-quoted strings so their embedded double-quote
+                    // content is never touched. The (?<!}) lookbehind prevents fusing the closing "
+                    // of a GString with the opening " of an adjacent plain string (e.g. obj."${x}"("y")).
+                    content = content.replaceAll(/(?<!\\)'(?:[^'\\\n]|\\.)*'|(?<!\\)(?<!")(?<!})"([^"$\n\\]*)"(?!")/) { List<String> args ->
+                        if (args[1] == null) {
+                            return args[0]  // single-quoted string matched — leave it untouched
+                        }
+                        String inner = args[1]
                         if (!inner.contains("'")) {
                             return "'$inner'"
                         }
-                        return all
+                        return args[0]
                     }
 
                     // 4. UnnecessarySemicolon

build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/GrailsViolationAggregationPlugin.groovy

@@ -61,6 +61,20 @@ class GrailsViolationAggregationPlugin implements Plugin<Project> {
 
     private static final Logger LOGGER = Logging.getLogger(GrailsViolationAggregationPlugin)
 
+    /**
+     * Comma-separated list of fully-qualified class-name prefixes to exclude from the aggregated
+     * JaCoCo coverage report. Configure via {@code -Pgrails.jacoco.aggregation.excludedClassPrefixes=...}
+     * or in {@code gradle.properties}.
+     *
+     * <p>Defaults to {@link #DEFAULT_JACOCO_EXCLUDED_CLASS_PREFIXES}: the Hibernate 7 support classes
+     * share fully-qualified names with their Hibernate 5 counterparts, and JaCoCo cannot aggregate
+     * coverage for two different classes with the same name (it fails with
+     * "Can't add different class with same name"). Excluding one variant keeps the aggregate valid.
+     */
+    static final String JACOCO_EXCLUDED_CLASS_PREFIXES_PROPERTY = 'grails.jacoco.aggregation.excludedClassPrefixes'
+
+    static final String DEFAULT_JACOCO_EXCLUDED_CLASS_PREFIXES = 'org.grails.orm.hibernate.support.hibernate7.'
+
     @Override
     void apply(Project project) {
         if (project != project.rootProject) {
@@ -153,13 +167,23 @@ class GrailsViolationAggregationPlugin implements Plugin<Project> {
             root.allprojects.collect { Project p -> p.file('build/reports/jacoco/test/jacocoTestReport.csv') }
         )
 
+        // Resolve the excluded class-name prefixes as a Provider so the value is captured
+        // configuration-cache-safely and read at task execution time.
+        Provider<List<String>> excludedClassPrefixes = root.providers
+            .gradleProperty(JACOCO_EXCLUDED_CLASS_PREFIXES_PROPERTY)
+            .orElse(DEFAULT_JACOCO_EXCLUDED_CLASS_PREFIXES)
+            .map { String value ->
+                value.split(',').collect { it.trim() }.findAll { !it.isEmpty() }
+            }
+
         TaskProvider<Task> aggregateTask = root.tasks.register('aggregateJacocoCoverage') { Task task ->
             task.group = 'verification'
             task.description = 'Aggregates JaCoCo coverage reports from all subprojects into build/reports/violations/'
             task.inputs.files(jacocoCsvFiles).optional(true)
+            task.inputs.property('excludedClassPrefixes', excludedClassPrefixes)
             task.outputs.file(root.file('build/reports/violations/JACOCO_COVERAGE.md'))
             task.doLast {
-                parseJacocoCoverage(jacocoCsvFiles, violationsDir.get())
+                parseJacocoCoverage(jacocoCsvFiles, violationsDir.get(), excludedClassPrefixes.get())
             }
         }
         root.subprojects { Project sub ->
@@ -175,7 +199,10 @@ class GrailsViolationAggregationPlugin implements Plugin<Project> {
     private static void parseStyleViolations(Directory styleXmlDir, Directory violationsDir,
             boolean checkStyleTests, boolean codenarcEnabled, boolean checkstyleEnabled) {
         def slurper = new XmlSlurper()
+        slurper.setFeature('http://apache.org/xml/features/disallow-doctype-decl', true)
         slurper.setFeature('http://apache.org/xml/features/nonvalidating/load-external-dtd', false)
+        slurper.setFeature('http://xml.org/sax/features/external-general-entities', false)
+        slurper.setFeature('http://xml.org/sax/features/external-parameter-entities', false)
         slurper.setFeature('http://xml.org/sax/features/namespaces', false)
 
         def getModule = { String fileName ->
@@ -300,7 +327,10 @@ class GrailsViolationAggregationPlugin implements Plugin<Project> {
     private static void parseAnalysisViolations(Directory analysisXmlDir, Directory violationsDir,
             boolean checkAnalysisTests, boolean pmdEnabled, boolean spotbugsEnabled) {
         def slurper = new XmlSlurper()
+        slurper.setFeature('http://apache.org/xml/features/disallow-doctype-decl', true)
         slurper.setFeature('http://apache.org/xml/features/nonvalidating/load-external-dtd', false)
+        slurper.setFeature('http://xml.org/sax/features/external-general-entities', false)
+        slurper.setFeature('http://xml.org/sax/features/external-parameter-entities', false)
         slurper.setFeature('http://xml.org/sax/features/namespaces', false)
 
         def getModule = { String fileName ->
@@ -406,7 +436,7 @@ class GrailsViolationAggregationPlugin implements Plugin<Project> {
     }
 
     @CompileDynamic
-    private static void parseJacocoCoverage(FileCollection csvFiles, Directory violationsDir) {
+    private static void parseJacocoCoverage(FileCollection csvFiles, Directory violationsDir, List<String> excludedClassPrefixes) {
         def jacocoCoverage = []
         csvFiles.each { File csvReport ->
             if (csvReport.exists()) {
@@ -442,7 +472,11 @@ class GrailsViolationAggregationPlugin implements Plugin<Project> {
             return
         }
 
-        jacocoCoverage.removeIf { it.className.startsWith('org.grails.orm.hibernate.support.hibernate7.') }
+        // Drop classes whose fully-qualified names collide across Hibernate variants (see
+        // JACOCO_EXCLUDED_CLASS_PREFIXES_PROPERTY) so the aggregate report stays valid.
+        if (excludedClassPrefixes) {
+            jacocoCoverage.removeIf { entry -> excludedClassPrefixes.any { prefix -> entry.className.startsWith(prefix) } }
+        }
 
         def outDir = violationsDir.asFile
         outDir.mkdirs()

build-logic/plugins/src/test/groovy/org/apache/grails/buildsrc/GrailsCodeStylePluginSpec.groovy

@@ -132,4 +132,92 @@ class Test {
         and: "escaped quotes are NOT broken"
         groovyFile.text.contains('"\\"\\$it\\""')
     }
+
+    def "test codenarcFix task does not corrupt method references (::)"() {
+        given: "a file with :: method references"
+        groovyFile.text = """package org.test
+
+import java.util.Arrays
+
+class Test {
+    def result = Arrays.stream(items).map(String::trim).toList()
+    def other = items.collect(String::valueOf)
+    def cors = parseConfigList(config, 'allowedOrigins', corsConfig::setAllowedOrigins)
+}
+"""
+        when: "running codenarcFix"
+        def result = GradleRunner.create()
+                .withProjectDir(testProjectDir.toFile())
+                .withArguments('codenarcFix', '--stacktrace')
+                .withPluginClasspath()
+                .build()
+
+        then: "task finished successfully"
+        result.task(':codenarcFix').outcome == org.gradle.testkit.runner.TaskOutcome.SUCCESS
+
+        and: "method references are NOT broken"
+        def content = groovyFile.text
+        content.contains('String::trim')
+        content.contains('String::valueOf')
+        content.contains('corsConfig::setAllowedOrigins')
+        !content.contains('String: :trim')
+        !content.contains('String: :valueOf')
+        !content.contains('corsConfig: :setAllowedOrigins')
+    }
+
+    def "test codenarcFix task does not corrupt adjacent GString and plain string"() {
+        given: "a file with a GString method name followed by a plain string argument"
+        groovyFile.text = '''package org.test
+
+class Test {
+    def invoke(invoker, name) {
+        invoker."${name}"("plain arg")
+        invoker."${name}"("-Pargs=${someVar}")
+    }
+}
+'''
+        when: "running codenarcFix"
+        def result = GradleRunner.create()
+                .withProjectDir(testProjectDir.toFile())
+                .withArguments('codenarcFix', '--stacktrace')
+                .withPluginClasspath()
+                .build()
+
+        then: "task finished successfully"
+        result.task(':codenarcFix').outcome == org.gradle.testkit.runner.TaskOutcome.SUCCESS
+
+        and: "adjacent GString boundaries are NOT fused"
+        def content = groovyFile.text
+        content.contains('invoker."${name}"(\'plain arg\')')
+        content.contains('invoker."${name}"("-Pargs=${someVar}")')
+        !content.contains('invoker."${name}\'(\'plain arg")')
+        !content.contains('invoker."${name}\'(\'plain arg\')')
+    }
+
+    def "test codenarcFix task does not corrupt double quotes inside single-quoted strings"() {
+        given: "a file with single-quoted strings containing double quotes"
+        groovyFile.text = """package org.test
+
+class Test {
+    def d1 = description('Accepts format "hh:mm:ss"')
+    def d2 = writeLine('[cols="2,5,2", options="header"]')
+    def d3 = 'value with "double quotes" inside'
+}
+"""
+        when: "running codenarcFix"
+        def result = GradleRunner.create()
+                .withProjectDir(testProjectDir.toFile())
+                .withArguments('codenarcFix', '--stacktrace')
+                .withPluginClasspath()
+                .build()
+
+        then: "task finished successfully"
+        result.task(':codenarcFix').outcome == org.gradle.testkit.runner.TaskOutcome.SUCCESS
+
+        and: "double quotes inside single-quoted strings are NOT changed"
+        def content = groovyFile.text
+        content.contains('\'Accepts format "hh:mm:ss"\'')
+        content.contains('\'[cols="2,5,2", options="header"]\'')
+        content.contains('\'value with "double quotes" inside\'')
+    }
 }

build-logic/plugins/src/test/groovy/org/apache/grails/buildsrc/GrailsViolationAggregationPluginSpec.groovy

@@ -174,4 +174,68 @@ class GrailsViolationAggregationPluginSpec extends Specification {
         and: "no report file is created"
         !testProjectDir.resolve('build/reports/violations/JACOCO_COVERAGE.md').toFile().exists()
     }
+
+    def "aggregateJacocoCoverage excludes the default hibernate7 support classes"() {
+        given: "a root project with a jacoco csv containing an h7 support class and a normal class"
+        testProjectDir.resolve('settings.gradle').toFile().text = ''
+        testProjectDir.resolve('build.gradle').toFile().text = """
+            plugins {
+                id 'org.apache.grails.gradle.grails-violation-aggregation'
+            }
+        """
+        writeJacocoCsv([
+                'app,org.grails.orm.hibernate.support.hibernate7,HibernateSupport,10,0',
+                'app,org.example.kept,KeptClass,0,20',
+        ])
+
+        when:
+        def result = GradleRunner.create()
+                .withProjectDir(testProjectDir.toFile())
+                .withArguments('aggregateJacocoCoverage', '--stacktrace')
+                .withPluginClasspath()
+                .build()
+
+        then: "task succeeds and the report drops the colliding h7 class but keeps the normal one"
+        result.task(':aggregateJacocoCoverage').outcome == TaskOutcome.SUCCESS
+        def report = testProjectDir.resolve('build/reports/violations/JACOCO_COVERAGE.md').toFile()
+        report.exists()
+        def text = report.text
+        text.contains('org.example.kept.KeptClass')
+        !text.contains('org.grails.orm.hibernate.support.hibernate7.HibernateSupport')
+    }
+
+    def "aggregateJacocoCoverage exclusion prefixes are configurable via property"() {
+        given: "a root project and a custom exclusion prefix that keeps the h7 class and drops a custom one"
+        testProjectDir.resolve('settings.gradle').toFile().text = ''
+        testProjectDir.resolve('build.gradle').toFile().text = """
+            plugins {
+                id 'org.apache.grails.gradle.grails-violation-aggregation'
+            }
+        """
+        writeJacocoCsv([
+                'app,org.grails.orm.hibernate.support.hibernate7,HibernateSupport,10,0',
+                'app,com.example.skip,SkipMe,5,5',
+                'app,org.example.kept,KeptClass,0,20',
+        ])
+
+        when:
+        def result = GradleRunner.create()
+                .withProjectDir(testProjectDir.toFile())
+                .withArguments('aggregateJacocoCoverage', '-Pgrails.jacoco.aggregation.excludedClassPrefixes=com.example.skip', '--stacktrace')
+                .withPluginClasspath()
+                .build()
+
+        then: "the custom prefix is dropped while the default h7 class is now retained"
+        result.task(':aggregateJacocoCoverage').outcome == TaskOutcome.SUCCESS
+        def text = testProjectDir.resolve('build/reports/violations/JACOCO_COVERAGE.md').toFile().text
+        text.contains('org.example.kept.KeptClass')
+        text.contains('org.grails.orm.hibernate.support.hibernate7.HibernateSupport')
+        !text.contains('com.example.skip.SkipMe')
+    }
+
+    private void writeJacocoCsv(List<String> dataRows) {
+        def csv = testProjectDir.resolve('build/reports/jacoco/test/jacocoTestReport.csv').toFile()
+        csv.parentFile.mkdirs()
+        csv.text = (['GROUP,PACKAGE,CLASS,INSTRUCTION_MISSED,INSTRUCTION_COVERED'] + dataRows).join('\n') + '\n'
+    }
 }

grails-gradle/build.gradle

@@ -23,6 +23,7 @@ import java.time.format.DateTimeFormatter
 plugins {
     id 'org.apache.grails.buildsrc.properties'
     id 'org.apache.grails.buildsrc.dependency-validator'
+    id 'org.apache.grails.gradle.grails-violation-aggregation'
 }
 
 allprojects {