codeconsole commented on code in PR #16019:
URL: https://github.com/apache/grails-core/pull/16019#discussion_r3679574264
##########
grails-core/build.gradle:
##########
@@ -51,6 +51,8 @@ dependencies {
api 'jakarta.persistence:jakarta.persistence-api'
api 'jakarta.annotation:jakarta.annotation-api'
+ api project(':grails-beans-dsl')
Review Comment:
Resolved by `138d014236` along the lines of your follow-up: this `api` is
the one declaration, and it now carries a comment recording *why* — that
`compileBeansDsl` loads the transform reflectively and returns silently when
absent, so this line is what makes the implicit `def beans = { }` convention
reach a third-party plugin author.
I did not change `@GrailsBeans` to `CLASS` retention. It is a reasonable
point that nothing reads it at runtime, but it is a separate question from the
scope story and I would rather not fold an unrelated retention change into this
PR. Say the word if you want it here.
##########
grails-core/src/main/groovy/org/grails/compiler/injection/GlobalGrailsClassInjectorTransformation.groovy:
##########
@@ -179,6 +188,44 @@ class GlobalGrailsClassInjectorTransformation implements
ASTTransformation, Comp
/**
* @return {@code true} when the {@code grails.isolated.build} system
property is {@code true}.
*/
+ /**
+ * Compiles a plugin descriptor's or application class's {@code beans}
closure into {@code @Bean}
+ * factory methods, so {@code @GrailsBeans} does not have to be written
out - the {@code beans}
+ * property is a convention here in the same way {@code doWithSpring} and
{@code watchedResources}
+ * already are.
+ *
+ * <p>The transformation is invoked directly rather than by adding the
annotation: annotation-driven
+ * transformations are collected during semantic analysis, so an
annotation added at
+ * {@code CANONICALIZATION} would never fire. A class that already
declares {@code @GrailsBeans}
+ * is skipped, since its own transformation has run; a class without a
{@code beans} property is
+ * skipped too, which is every plugin that does not use the DSL.</p>
+ */
+ private void compileBeansDsl(ClassNode classNode, SourceUnit source) {
+ if (classNode.getProperty(BEANS_PROPERTY) == null) {
+ return
+ }
+ if (!classNode.getAnnotations(GRAILS_BEANS_ANNOTATION).isEmpty()) {
+ return
+ }
+
+ ASTTransformation transformation
+ try {
+ transformation = (ASTTransformation) getClass().classLoader
+
.loadClass('org.grails.compiler.beans.GrailsBeansASTTransformation')
+ .getDeclaredConstructor()
+ .newInstance()
+ }
+ catch (ClassNotFoundException ignored) {
Review Comment:
Both addressed in `94e25dee20`.
The silent branch now records why it is allowed to be silent — it is
unreachable only because `grails-core` declares the module `api`, and narrowing
that scope would turn it into a live path where a DSL-shaped block registers
nothing and reports nothing.
On the second: I narrowed the trigger rather than documenting the break. The
implicit path now requires a closure whose every top-level statement is rooted
in a `bean`/`field`/`method` call, so a pre-existing descriptor with `def beans
= [...]`, or a closure of something else, is left alone. Writing `@GrailsBeans`
explicitly still opts in to the strict errors, which is right where the author
has said what they mean. An empty block is still claimed, so both spellings
agree.
Three tests: a Map-valued property, a non-DSL closure, and a DSL-shaped one.
##########
settings.gradle:
##########
@@ -107,6 +107,9 @@ def skipMicronautProjects = explicitlySkipMicronaut ||
(!buildJdkSupportsMicrona
include(
'grails-bootstrap',
+ 'grails-beans-dsl',
+ 'grails-beans-dsl-example',
+ 'grails-beans-dsl-plugin-example',
Review Comment:
Drift, not a decision — thank you for checking rather than assuming. Both
moved under `grails-test-examples/` in `0224df0ecd`, per the table in your
later comment.
Your point that they are more than samples is taken: all 7 specs run at the
new locations, `FarewellGrailsPluginAutoDiscoverySpec` included. And since that
spec now sits outside core-only CI, the `autoconfiguration-imports` plugin
gained a production consumer (`grails-databinding`) so that coverage is not the
only thing exercising it.
##########
grails-core/build.gradle:
##########
@@ -51,6 +51,8 @@ dependencies {
api 'jakarta.persistence:jakarta.persistence-api'
api 'jakarta.annotation:jakarta.annotation-api'
+ api project(':grails-beans-dsl')
Review Comment:
Agreed, and it stays. It now carries a comment recording the reasoning you
set out here — that `compileBeansDsl` loads the transform reflectively and
returns silently when it is absent, so this `api` is what makes the implicit
`def beans = { }` convention reach a third-party plugin author (`138d014236`).
The other seven are deleted, and each removal was verified by its module's
generated sibling class rather than by the build merely succeeding — see my
reply on `grails-cache/build.gradle`.
##########
grails-cache/build.gradle:
##########
@@ -49,6 +49,7 @@ dependencies {
api "org.codehaus.gpars:gpars:$gparsVersion"
api
"com.googlecode.concurrentlinkedhashmap:concurrentlinkedhashmap-lru:$concurrentlinkedhashmapLruVersion"
+ implementation project(':grails-beans-dsl')
Review Comment:
Removed in `138d014236`.
Worth recording how that was checked, because compiling is *not* sufficient
evidence here:
`compileBeansDsl` fails silently, so a module that lost the transform would
still compile and
simply leave its `beans` block as an untouched property. Each of the seven
was verified by its
generated sibling class existing, with forced no-cache recompiles of
`grails-i18n` and
`grails-url-mappings`, which reach `grails-core` only transitively through
`grails-web-core`.
The other six are `grails-databinding`, `grails-domain-class`, `grails-i18n`,
`grails-url-mappings`, `grails-sitemesh3` and `grails-mail`.
##########
grails-databinding/build.gradle:
##########
@@ -38,6 +38,7 @@ dependencies {
implementation platform(project(':grails-bom'))
+ implementation project(':grails-beans-dsl')
Review Comment:
Removed in `138d014236`, along with the other six. See my reply on
`grails-cache/build.gradle` for how each removal was verified — compiling is
not sufficient evidence, since `compileBeansDsl` fails silently, so
`grails-databinding`'s generated sibling class was checked instead.
##########
grails-domain-class/build.gradle:
##########
@@ -38,6 +38,7 @@ dependencies {
implementation platform(project(':grails-bom'))
+ implementation project(':grails-beans-dsl')
Review Comment:
Removed in `138d014236`, along with the other six. See my reply on
`grails-cache/build.gradle` for how each removal was verified — compiling is
not sufficient evidence, since `compileBeansDsl` fails silently, so
`grails-domain-class`'s generated sibling class was checked instead.
##########
grails-i18n/build.gradle:
##########
@@ -41,6 +41,8 @@ dependencies {
api project(':grails-web-core')
api 'org.apache.groovy:groovy'
+ implementation project(':grails-beans-dsl')
Review Comment:
Removed in `138d014236`, along with the other six. See my reply on
`grails-cache/build.gradle` for how each removal was verified — compiling is
not sufficient evidence, since `compileBeansDsl` fails silently, so
`grails-i18n`'s generated sibling class was checked instead.
##########
grails-url-mappings/build.gradle:
##########
@@ -43,6 +43,8 @@ dependencies {
api 'org.apache.groovy:groovy'
api 'org.springframework.boot:spring-boot-servlet'
+ implementation project(':grails-beans-dsl')
Review Comment:
Removed in `138d014236`, along with the other six. See my reply on
`grails-cache/build.gradle` for how each removal was verified — compiling is
not sufficient evidence, since `compileBeansDsl` fails silently, so
`grails-url-mappings`'s generated sibling class was checked instead.
##########
grails-gsp/grails-sitemesh3/build.gradle:
##########
@@ -52,6 +52,7 @@ dependencies {
api project(':grails-core')
implementation 'org.apache.groovy:groovy'
+ implementation project(':grails-beans-dsl')
Review Comment:
Removed in `138d014236`, along with the other six. See my reply on
`grails-cache/build.gradle` for how each removal was verified — compiling is
not sufficient evidence, since `compileBeansDsl` fails silently, so
`grails-sitemesh3`'s generated sibling class was checked instead.
--
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]
