Copilot commented on code in PR #15571:
URL: https://github.com/apache/grails-core/pull/15571#discussion_r3068344048
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/CompilePlugin.groovy:
##########
@@ -111,6 +114,33 @@ class CompilePlugin implements Plugin<Project> {
}
}
+ @CompileStatic(TypeCheckingMode.SKIP)
+ private static void configureAnnotationProcessors(Project project) {
+ // Configure Spring Boot configuration processor for modules with
@ConfigurationProperties.
+ // Groovy modules intentionally avoid annotation processors due to
incompatibility
+ // with incremental Groovy compilation (see issue #15211).
+ // Modules must opt-in via 'enableAnnotationProcessor = true' property.
Review Comment:
The comment claims Groovy modules “intentionally avoid annotation
processors”, but this implementation only gates on the
`enableAnnotationProcessor` opt-in and doesn’t check whether the Groovy plugin
is applied (and `grails-databinding` applies `groovy`). Please adjust the
comment to match the actual behavior/constraints to avoid misleading future
maintainers.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/CompilePlugin.groovy:
##########
@@ -111,6 +114,33 @@ class CompilePlugin implements Plugin<Project> {
}
}
+ @CompileStatic(TypeCheckingMode.SKIP)
+ private static void configureAnnotationProcessors(Project project) {
+ // Configure Spring Boot configuration processor for modules with
@ConfigurationProperties.
+ // Groovy modules intentionally avoid annotation processors due to
incompatibility
+ // with incremental Groovy compilation (see issue #15211).
+ // Modules must opt-in via 'enableAnnotationProcessor = true' property.
+ project.afterEvaluate {
+ try {
+ // Check if this module has opted in to annotation processor
support
+ def ext = project.extensions.extraProperties
+ def enableProcessor = ext.has('enableAnnotationProcessor') &&
ext.get('enableAnnotationProcessor') == true
+
+ if (enableProcessor) {
+ // Add annotation processor dependencies for modules with
@ConfigurationProperties
+
project.configurations.getByName('annotationProcessor').dependencies.add(
+
project.dependencies.platform(project.project(':grails-bom'))
+ )
+
project.configurations.getByName('annotationProcessor').dependencies.add(
+
project.dependencies.create('org.springframework.boot:spring-boot-configuration-processor')
+ )
+ }
+ } catch (Exception ignored) {
+ // Configuration not available for this module
+ }
Review Comment:
Catching and silently ignoring all Exceptions here can hide real
configuration errors (e.g., missing `:grails-bom`, typos in configuration
names) and can lead to metadata generation being skipped without any signal.
Prefer catching the specific Gradle exceptions you expect (e.g., unknown
configuration) and otherwise failing fast or logging a clear message when
opt-in is enabled but setup can’t be applied.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/CompilePlugin.groovy:
##########
@@ -23,11 +23,13 @@ import java.nio.charset.StandardCharsets
import java.util.concurrent.atomic.AtomicBoolean
import groovy.transform.CompileStatic
+import groovy.transform.TypeCheckingMode
import org.gradle.api.Plugin
import org.gradle.api.Project
import org.gradle.api.file.DuplicatesStrategy
import org.gradle.api.plugins.JavaPluginExtension
+import org.gradle.api.tasks.SourceSetContainer
Review Comment:
Remove the unused `SourceSetContainer` import. It isn’t referenced anywhere
in this class, and this repo’s CodeNarc ruleset enables `UnusedImport`, which
will flag unused imports during style checks.
##########
build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/CompilePlugin.groovy:
##########
@@ -111,6 +114,33 @@ class CompilePlugin implements Plugin<Project> {
}
}
+ @CompileStatic(TypeCheckingMode.SKIP)
+ private static void configureAnnotationProcessors(Project project) {
+ // Configure Spring Boot configuration processor for modules with
@ConfigurationProperties.
+ // Groovy modules intentionally avoid annotation processors due to
incompatibility
+ // with incremental Groovy compilation (see issue #15211).
+ // Modules must opt-in via 'enableAnnotationProcessor = true' property.
+ project.afterEvaluate {
+ try {
+ // Check if this module has opted in to annotation processor
support
+ def ext = project.extensions.extraProperties
+ def enableProcessor = ext.has('enableAnnotationProcessor') &&
ext.get('enableAnnotationProcessor') == true
+
Review Comment:
The boolean opt-in check is overly strict:
`ext.get('enableAnnotationProcessor') == true` will be false if the property is
set to a truthy non-Boolean value (e.g., `'true'`). Consider normalizing to a
boolean (e.g., `Boolean.TRUE.equals(...)` or parsing string values) so the
opt-in behaves predictably.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
