jdaugherty commented on code in PR #16292:
URL: https://github.com/apache/grails-core/pull/16292#discussion_r3960763892
##########
grails-beans-dsl/src/main/java/org/grails/compiler/beans/GrailsBeansASTTransformation.java:
##########
@@ -786,16 +862,178 @@ private void processStatement(ClassNode classNode,
Statement statement, SourceUn
}
if (isBean) {
- processBeanStatement(classNode, outerCall, baseCall,
qualifierCalls, source, usedNames);
+ processBeanStatement(classNode, declaringClass, outerCall,
baseCall, qualifierCalls, source, usedNames);
+ }
+ else if (GROUP_CALL.equals(rootName)) {
+ processGroupStatement(classNode, declaringClass, outerCall,
baseCall, qualifierCalls, source, usedNames);
}
else if (FIELD_CALL.equals(rootName)) {
- processFieldStatement(classNode, baseCall, qualifierCalls, source,
usedNames);
+ processFieldStatement(classNode, declaringClass, baseCall,
qualifierCalls, source, usedNames);
}
else {
- processMethodStatement(classNode, outerCall, baseCall,
qualifierCalls, source, usedNames);
+ processMethodStatement(classNode, declaringClass, outerCall,
baseCall, qualifierCalls, source, usedNames);
}
}
+ /**
+ * Compiles {@code group("name").<conditions> { ... }} into a nested static
+ * {@code @Configuration(proxyBeanMethods = false)} class holding the
declarations in its body,
+ * with the chained qualifiers attached to that class rather than to each
bean.
+ *
+ * <p>This is the shape real auto-configurations take, and the one a
condition on an optional
+ * type has to take. Spring reads a condition from the bytecode before
loading anything, but a
+ * {@code @Bean} method's parameter and return types are resolved when its
configuration class
+ * is parsed - so a bean whose own signature names a class that may be
absent cannot be guarded
+ * on the method. Moving it into a nested class moves the guard with it,
and the nested class is
+ * never parsed when the condition fails. Spring Boot writes exactly this:
JacksonAutoConfiguration
+ * carries four nested {@code @ConditionalOnClass} configuration
classes.</p>
+ *
+ * <p>Spring finds the nested class itself - {@code
ConfigurationClassParser} processes the member
+ * classes of a configuration class - so nothing has to import or register
it.</p>
+ */
+ private void processGroupStatement(ClassNode classNode, ClassNode
declaringClass, MethodCallExpression outerCall,
+ MethodCallExpression baseCall, List<MethodCallExpression>
qualifierCalls, SourceUnit source,
+ Set<String> usedNames) {
+ List<Expression> closureCallArgs = flatten(outerCall.getArguments());
+ if (closureCallArgs.isEmpty() ||
!(closureCallArgs.get(closureCallArgs.size() - 1) instanceof
ClosureExpression)) {
+ addError(outerCall, source, "group(...) must end with a body
closure: group(\"name\") { ... }");
+ return;
+ }
+ ClosureExpression body = (ClosureExpression)
closureCallArgs.get(closureCallArgs.size() - 1);
+ if (body.getParameters() != null && body.getParameters().length > 0) {
+ addError(outerCall, source, "group(...) takes no closure
parameters - a group declares a class, " +
+ "not a bean, so there is nothing to inject into. Put the
parameters on the bean(...) " +
+ "declarations inside it");
+ return;
+ }
+
+ List<Expression> baseArgs = flatten(baseCall.getArguments());
+ if (baseCall == outerCall && !baseArgs.isEmpty()) {
+ baseArgs = baseArgs.subList(0, baseArgs.size() - 1);
+ }
+ if (baseArgs.size() != 1) {
+ addError(baseCall, source, "group(...) takes a name, e.g.
group(\"imageServing\") { ... }");
+ return;
+ }
+ String name = resolveStringConstant(baseArgs.get(0), declaringClass);
+ if (name == null ||
!isValidJavaIdentifier(BeanUtils.capitalize(name))) {
+ addError(baseArgs.get(0), source, "group(name) requires the name
to be a String literal or a " +
+ "compile-time String constant that is a valid Java
identifier - it becomes the nested " +
+ "class's name, e.g. group(\"imageServing\")");
+ return;
+ }
+
+ // JacksonObjectMapperConfiguration rather than JacksonObjectMapper:
the suffix is what says
+ // this is a configuration class when it turns up in a stack trace or
/actuator/beans.
+ String simpleName = BeanUtils.capitalize(name);
+ if (!simpleName.endsWith("Configuration")) {
+ simpleName = simpleName + "Configuration";
+ }
+ if (!registerName(simpleName, baseCall, source, usedNames,
+ "is already used by another member of the class - generated
member names must be unique")) {
+ return;
+ }
+
+ List<Statement> statements = beanStatements(body);
+ if (statements.isEmpty()) {
+ addError(outerCall, source, "group(\"" + name + "\") declares
nothing - a group exists to put a " +
+ "condition on the declarations inside it");
+ return;
+ }
+ for (Statement statement : statements) {
+ if (isGroupRootedStatement(statement)) {
+ addError(statement, source, "group(...) cannot be nested -
flatten it, or give the inner " +
+ "group its own conditions at the top level");
+ return;
+ }
+ }
+
+ InnerClassNode group = new InnerClassNode(classNode,
classNode.getName() + "$" + simpleName,
+ Modifier.PUBLIC | Modifier.STATIC, ClassHelper.OBJECT_TYPE);
+ group.setSourcePosition(baseCall);
+ source.getAST().addClass(group);
+
+ // proxyBeanMethods = false, matching what Spring Boot's own nested
configuration classes
+ // carry - and keeping the sibling-call check below meaningful inside
the group.
+ AnnotationNode configuration = new
AnnotationNode(ClassHelper.make(Configuration.class));
+ configuration.setMember(PROXY_BEAN_METHODS_MEMBER, new
ConstantExpression(Boolean.FALSE));
+ group.addAnnotation(withPosition(configuration, baseCall));
+
+ for (MethodCallExpression qualifierCall : qualifierCalls) {
+ List<Expression> qualifierArgs =
flatten(qualifierCall.getArguments());
+ if (qualifierCall == outerCall) {
+ qualifierArgs = qualifierArgs.subList(0, qualifierArgs.size()
- 1);
+ }
+ if (!applyGroupQualifier(group, qualifierCall, qualifierArgs,
source)) {
+ return;
+ }
+ }
+
+ Set<String> groupNames = existingMemberNames(group);
+ List<MethodNode> preExisting = new ArrayList<>(group.getMethods());
+ validateSharedBeanNames(statements, source);
+ for (Statement statement : statements) {
+ if (!isBeanRootedStatement(statement)) {
+ processStatement(group, declaringClass, statement, source,
groupNames);
+ }
+ }
+ for (Statement statement : statements) {
+ if (isBeanRootedStatement(statement)) {
+ processStatement(group, declaringClass, statement, source,
groupNames);
+ }
+ }
+ List<MethodNode> generated = generatedMembers(group, preExisting);
+ rejectUnproxiedSiblingBeanCalls(group, generated, source);
+ dumpGeneratedMembers(group, generated, new
ArrayList<>(group.getFields()), source);
+
+ // The group is compiled as its own class, so it needs the host's
static-compilation
+ // treatment in its own right - otherwise its bodies are dynamic
inside a @CompileStatic file.
+ applyStaticCompilation(classNode, group, source);
Review Comment:
`group(...)` does not survive `@CompileStatic` on the host, for a plain host
or for a plugin descriptor. A valid body:
```groovy
@GrailsBeans
@CompileStatic
@AutoConfiguration
class Host {
def beans = {
group('extras').conditionalOnProperty('probe.enabled') {
bean('greeting', String) { 'hello'.toUpperCase() }
}
}
}
```
fails in class generation with `GroovyBugError:
StaticTypesCallSiteWriter#makeCallSite should not have been called. Call site
lacked method target for static compilation`, reported against `new
MissingMethodException(notFound.method, this.getClass(), notFound.arguments)`.
That expression is from the MOP dispatch methods Groovy's
`InnerClassCompletionVisitor` adds to every inner class, and that visitor is
registered as a post-transform CANONICALIZATION operation, so it runs after
this transform. Statically compiling the nested class here marks it for static
compilation before those methods exist, and they are then generated without
ever having been type-checked. The same body compiles and runs without
`@CompileStatic`.
On a plugin descriptor this particular call is a no-op (the sibling only
receives `@CompileStatic` after the statements are processed), but the
sibling's own pass visits the nested class at the same point in the pipeline
and fails the same way. One way out is to defer the static-compilation pass to
INSTRUCTION_SELECTION via `compilationUnit.addPhaseOperation`, the point a
hand-written class's annotation would run at, after the MOP methods exist.
All four group tests use dynamic hosts; a `@CompileStatic` case for a plain
host and for a plugin descriptor would have caught this, and given that every
in-tree descriptor is `@GrailsCompileStatic` it is the case that matters most.
##########
grails-beans-dsl/src/main/java/org/grails/compiler/beans/GrailsBeansASTTransformation.java:
##########
@@ -913,6 +1514,164 @@ private Statement synthesizedConstruction(ClassNode
beanType, Parameter[] parame
return returnStatement;
}
+ /**
+ * Whether {@code candidate} is a {@code target}. Used for the
implementation type, where
+ * checking it here rather than leaving it to the generated {@code return
new Impl()} means the
+ * failure names both types and points at the {@code bean(...)} statement
instead of surfacing as
+ * an assignment error inside a body the author never wrote.
+ */
+ private boolean isSubtypeOf(ClassNode candidate, ClassNode target) {
+ ClassNode resolved = target.redirect();
+ return candidate.redirect().equals(resolved) ||
candidate.isDerivedFrom(resolved) ||
+ candidate.implementsInterface(resolved);
+ }
+
+ private boolean hasExplicitTypeArguments(List<MethodCallExpression>
qualifierCalls) {
+ for (MethodCallExpression qualifierCall : qualifierCalls) {
+ if (TYPE_ARGUMENTS_CALL.equals(qualifierCall.getMethodAsString()))
{
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * The type a factory closure constructs, when its body is exactly that
and nothing else: a last
+ * statement that is a {@code new ...} expression, which in Groovy is the
closure's return value.
+ * Anything else - a local, a method call, a conditional - is not evidence
of anything, and this
+ * returns null rather than guess.
+ */
+ private ClassNode constructedTypeFromBody(ClosureExpression factory) {
+ if (factory == null || !(factory.getCode() instanceof BlockStatement))
{
+ return null;
+ }
+ List<Statement> statements = ((BlockStatement)
factory.getCode()).getStatements();
+ if (statements.isEmpty()) {
+ return null;
+ }
+ Statement last = statements.get(statements.size() - 1);
+ Expression expression = null;
+ if (last instanceof ReturnStatement) {
+ expression = ((ReturnStatement) last).getExpression();
+ }
+ else if (last instanceof ExpressionStatement) {
+ expression = ((ExpressionStatement) last).getExpression();
+ }
+ return expression instanceof ConstructorCallExpression ?
expression.getType() : null;
+ }
+
+ /**
+ * The declared type parameterized by what {@code evidence} binds it to,
or null when that cannot
+ * be answered concretely - {@code evidence} is unrelated, the declared
type is not generic, or
+ * the binding is itself a type variable ({@code class Box<T> implements
Holder<T>} proves
+ * nothing about a {@code Holder} bean). Inference only ever adds
information the compiler could
+ * already see; where it cannot, the raw type stands exactly as before and
+ * {@code .typeArguments(...)} remains the way to say it.
+ */
+ private ClassNode inferTypeArguments(ClassNode declaredRaw, ClassNode
evidence) {
+ GenericsType[] declared = declaredRaw.redirect().getGenericsTypes();
+ if (evidence == null || declared == null || declared.length == 0) {
+ return null;
+ }
+ if (!isSubtypeOf(evidence, declaredRaw)) {
+ return null;
+ }
+ // A raw construction of a generic type proves nothing: Groovy
resolves its parameters to
+ // their bounds, so new GenericBox() would infer Holder<Object> - not
merely uninformative
+ // but wrong, since a bean typed Holder<Object> no longer matches a
Holder<String> injection
+ // point it previously did as a raw Holder.
+ GenericsType[] evidenceParameters =
evidence.redirect().getGenericsTypes();
+ if (evidenceParameters != null && evidenceParameters.length > 0 &&
+ (evidence.getGenericsTypes() == null ||
evidence.getGenericsTypes().length == 0)) {
+ return null;
+ }
+ ClassNode parameterized;
+ try {
+ parameterized = GenericsUtils.parameterizeType(evidence,
declaredRaw.redirect());
+ }
+ catch (RuntimeException ignored) {
+ // parameterizeType is best-effort on partially resolved
hierarchies; an unusable answer
+ // is the same as no answer.
+ return null;
+ }
+ GenericsType[] resolved = parameterized == null ? null :
parameterized.getGenericsTypes();
+ if (resolved == null || resolved.length != declared.length) {
+ return null;
+ }
+ for (GenericsType candidate : resolved) {
+ if (candidate.isPlaceholder() || candidate.isWildcard() ||
candidate.getType() == null ||
+ candidate.getType().isGenericsPlaceHolder()) {
+ return null;
+ }
+ }
+ return GenericsUtils.makeClassSafeWithGenerics(declaredRaw, resolved);
+ }
+
+ /**
+ * Re-homes anonymous inner classes in a body lifted out of the {@code
beans} closure.
+ *
+ * <p>Groovy's {@code InnerClassVisitor} runs at SEMANTIC_ANALYSIS, before
this transform, and
+ * gives an anonymous class its enclosing instance from wherever it was
written: a class
+ * declared inside a closure gets {@code final Closure this$0} and a
constructor taking a
+ * {@code Closure}, where one declared in a method gets the declaring
class. Lifting the body
+ * into a method moves the code and not that decision, so the generated
call passes {@code this}
+ * - the configuration class - to a constructor still expecting the
closure. It compiles, and
+ * fails at runtime with a {@code GroovyCastException} naming neither the
bean nor the DSL.</p>
+ *
+ * <p>So the three places that decision landed are corrected here: the
{@code this$0} field, the
+ * synthetic constructor's first parameter, and the argument at the call
site.</p>
+ */
+ private boolean rehomeAnonymousInnerClasses(Statement body, ClassNode
host, boolean staticMethod,
+ String beanName, SourceUnit source) {
+ List<ConstructorCallExpression> anonymous = new ArrayList<>();
+ body.visit(new CodeVisitorSupport() {
Review Comment:
This visitor descends into nested closures, and an anonymous class
constructed inside a nested closure still has that closure as its enclosing
instance at runtime - the lift did not move it out of one. Re-homing it anyway
breaks two shapes that compile and run on `8.0.x` today:
```groovy
bean('greeter', Greeter) {
List<Greeter> made = ['x'].collect { String tag ->
new Greeter() { String greet() { 'hello ' + tag } }
}
made[0]
}
```
now fails at runtime with `GroovyCastException: Cannot cast object
'Host$_greeter_closure1' ... to class 'Host'`: the rewritten `this` argument,
evaluated inside the closure, still loads the closure instance. The same body
under `.staticMethod()` now fails to compile with the "needs an enclosing
instance" error, though it worked before because the nested closure supplies
the instance.
Stopping at closure boundaries - the empty `visitClosureExpression` override
`rejectUnproxiedSiblingBeanCalls` already uses - restores both shapes while
keeping the direct-in-body rejection; I confirmed that locally with the full
spec still green. Please also add coverage for an anonymous class inside a
nested closure, for an instance bean and for a `.staticMethod()` bean, since
neither shape is exercised by the spec today.
##########
grails-beans-dsl/src/main/java/org/grails/compiler/beans/GrailsBeansASTTransformation.java:
##########
@@ -582,8 +635,8 @@ private void validateSharedBeanNames(List<Statement>
statements, SourceUnit sour
addError(use.baseCall, source, "\"" + use.beanName + "\"
is already used as the Spring " +
Review Comment:
Anchoring on the check's error since the gap is in `parseBeanNameUse` just
below, which is outside the diff. `parseBeanNameUse` still reads the `[name, ]
Type` head with the old rules - at most two arguments and a literal name - so
the declaration forms this PR adds fall out of shared-name validation:
`bean('x', Iface, Impl)` has three arguments and returns null here, and
`bean(CONST, Type)` has a `VariableExpression` name and returns null at the
`ConstantExpression` check below. Two declarations of one name in either form
compile without the discriminating-condition error, and Spring keeps the first
and silently drops the rest, which is exactly what this check exists to prevent.
Routing this through the same head parsing `processBeanStatement` uses (the
implementation split and `resolveStringConstant`) would keep the two in step; a
test for a duplicate three-argument declaration and a duplicate constant-named
declaration would pin it.
##########
grails-beans-dsl/src/main/java/org/grails/compiler/beans/GrailsBeansASTTransformation.java:
##########
@@ -913,6 +1514,164 @@ private Statement synthesizedConstruction(ClassNode
beanType, Parameter[] parame
return returnStatement;
}
+ /**
+ * Whether {@code candidate} is a {@code target}. Used for the
implementation type, where
+ * checking it here rather than leaving it to the generated {@code return
new Impl()} means the
+ * failure names both types and points at the {@code bean(...)} statement
instead of surfacing as
+ * an assignment error inside a body the author never wrote.
+ */
+ private boolean isSubtypeOf(ClassNode candidate, ClassNode target) {
+ ClassNode resolved = target.redirect();
+ return candidate.redirect().equals(resolved) ||
candidate.isDerivedFrom(resolved) ||
+ candidate.implementsInterface(resolved);
+ }
+
+ private boolean hasExplicitTypeArguments(List<MethodCallExpression>
qualifierCalls) {
+ for (MethodCallExpression qualifierCall : qualifierCalls) {
+ if (TYPE_ARGUMENTS_CALL.equals(qualifierCall.getMethodAsString()))
{
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * The type a factory closure constructs, when its body is exactly that
and nothing else: a last
+ * statement that is a {@code new ...} expression, which in Groovy is the
closure's return value.
+ * Anything else - a local, a method call, a conditional - is not evidence
of anything, and this
+ * returns null rather than guess.
+ */
+ private ClassNode constructedTypeFromBody(ClosureExpression factory) {
+ if (factory == null || !(factory.getCode() instanceof BlockStatement))
{
+ return null;
+ }
+ List<Statement> statements = ((BlockStatement)
factory.getCode()).getStatements();
+ if (statements.isEmpty()) {
+ return null;
+ }
+ Statement last = statements.get(statements.size() - 1);
+ Expression expression = null;
+ if (last instanceof ReturnStatement) {
+ expression = ((ReturnStatement) last).getExpression();
+ }
+ else if (last instanceof ExpressionStatement) {
+ expression = ((ExpressionStatement) last).getExpression();
+ }
+ return expression instanceof ConstructorCallExpression ?
expression.getType() : null;
+ }
+
+ /**
+ * The declared type parameterized by what {@code evidence} binds it to,
or null when that cannot
+ * be answered concretely - {@code evidence} is unrelated, the declared
type is not generic, or
+ * the binding is itself a type variable ({@code class Box<T> implements
Holder<T>} proves
+ * nothing about a {@code Holder} bean). Inference only ever adds
information the compiler could
+ * already see; where it cannot, the raw type stands exactly as before and
+ * {@code .typeArguments(...)} remains the way to say it.
+ */
+ private ClassNode inferTypeArguments(ClassNode declaredRaw, ClassNode
evidence) {
+ GenericsType[] declared = declaredRaw.redirect().getGenericsTypes();
+ if (evidence == null || declared == null || declared.length == 0) {
+ return null;
+ }
+ if (!isSubtypeOf(evidence, declaredRaw)) {
+ return null;
+ }
+ // A raw construction of a generic type proves nothing: Groovy
resolves its parameters to
+ // their bounds, so new GenericBox() would infer Holder<Object> - not
merely uninformative
+ // but wrong, since a bean typed Holder<Object> no longer matches a
Holder<String> injection
+ // point it previously did as a raw Holder.
+ GenericsType[] evidenceParameters =
evidence.redirect().getGenericsTypes();
+ if (evidenceParameters != null && evidenceParameters.length > 0 &&
+ (evidence.getGenericsTypes() == null ||
evidence.getGenericsTypes().length == 0)) {
+ return null;
+ }
+ ClassNode parameterized;
+ try {
+ parameterized = GenericsUtils.parameterizeType(evidence,
declaredRaw.redirect());
+ }
+ catch (RuntimeException ignored) {
+ // parameterizeType is best-effort on partially resolved
hierarchies; an unusable answer
+ // is the same as no answer.
+ return null;
+ }
+ GenericsType[] resolved = parameterized == null ? null :
parameterized.getGenericsTypes();
+ if (resolved == null || resolved.length != declared.length) {
+ return null;
+ }
+ for (GenericsType candidate : resolved) {
+ if (candidate.isPlaceholder() || candidate.isWildcard() ||
candidate.getType() == null ||
+ candidate.getType().isGenericsPlaceHolder()) {
+ return null;
+ }
+ }
+ return GenericsUtils.makeClassSafeWithGenerics(declaredRaw, resolved);
+ }
+
+ /**
+ * Re-homes anonymous inner classes in a body lifted out of the {@code
beans} closure.
+ *
+ * <p>Groovy's {@code InnerClassVisitor} runs at SEMANTIC_ANALYSIS, before
this transform, and
+ * gives an anonymous class its enclosing instance from wherever it was
written: a class
+ * declared inside a closure gets {@code final Closure this$0} and a
constructor taking a
+ * {@code Closure}, where one declared in a method gets the declaring
class. Lifting the body
+ * into a method moves the code and not that decision, so the generated
call passes {@code this}
+ * - the configuration class - to a constructor still expecting the
closure. It compiles, and
+ * fails at runtime with a {@code GroovyCastException} naming neither the
bean nor the DSL.</p>
+ *
+ * <p>So the three places that decision landed are corrected here: the
{@code this$0} field, the
+ * synthetic constructor's first parameter, and the argument at the call
site.</p>
+ */
+ private boolean rehomeAnonymousInnerClasses(Statement body, ClassNode
host, boolean staticMethod,
+ String beanName, SourceUnit source) {
+ List<ConstructorCallExpression> anonymous = new ArrayList<>();
+ body.visit(new CodeVisitorSupport() {
+ @Override
+ public void
visitConstructorCallExpression(ConstructorCallExpression call) {
+ if (call.isUsingAnonymousInnerClass()) {
+ anonymous.add(call);
+ }
+ super.visitConstructorCallExpression(call);
+ }
+ });
+ for (ConstructorCallExpression call : anonymous) {
+ ClassNode inner = call.getType();
+ FieldNode outerField = inner.getDeclaredField("this$0");
+ if (outerField == null ||
!ClassHelper.CLOSURE_TYPE.equals(outerField.getType())) {
+ continue; // already homed somewhere real, or static - nothing
the lift broke
+ }
+ // A static factory method has no enclosing instance to give it,
and the field cannot be
+ // dropped here: InnerClassVisitor added it and the constructor
body assigns it.
+ if (staticMethod) {
+ addError(call, source, "\"" + beanName + "\" is declared
.staticMethod() and its body " +
+ "constructs an anonymous inner class, which needs an
enclosing instance the " +
+ "static method has not got - give the anonymous class
a name and declare it " +
+ "as a static nested class, or drop .staticMethod()");
+ return false;
+ }
+ ClassNode enclosing = host.getPlainNodeReference();
Review Comment:
On a plugin descriptor the re-homing is only half of the move. The anonymous
class's outer class is still the plugin class (`InnerClassNode` fixes it at
construction), so the MOP dispatch methods Groovy generates for it read
`this$0` with the plugin class's descriptor and call `this$dist$invoke$N` on
the plugin class, while the field is now typed as the sibling. Any unqualified
reference from the anonymous class body to a member of the configuration fails
at runtime:
```groovy
class ProbeGrailsPlugin extends Plugin {
def beans = {
method('suffix', String) { '!' }
bean('greeter', Greeter) {
new Greeter() { String greet() { 'hello' + suffix() } }
}
}
}
```
`greet()` throws `NoSuchFieldError: Class ProbeGrailsPlugin$1 does not have
member field 'ProbeGrailsPlugin this$0'`. Under `@CompileStatic` the same body
is rejected at compile time (`Cannot find matching method
ProbeGrailsPlugin$1#suffix()`), which is at least visible. An anonymous class
that touches nothing outside itself works on both.
This shape was not reachable before - the construction failed outright - so
it is not a regression, but the Javadoc now advertises anonymous classes in
bodies without the caveat. If the outer class cannot be moved along with
`this$0`, then a compile-time error for an implicit-this call from an anonymous
class on a plugin host, or at minimum a documented limitation and a test
pinning the current behaviour, would keep this from surfacing as a
`NoSuchFieldError` inside an application.
##########
grails-doc/src/en/guide/plugins/hookingIntoRuntimeConfiguration.adoc:
##########
@@ -193,6 +204,30 @@ bean('greeting', Greeting) {
}
----
+A third statement kind, `group(["name"]).<conditions> { ... }`, declares a
nested static `@Configuration(proxyBeanMethods = false)` class holding the
declarations in its body, with the chained conditions on the class rather than
on each bean:
+
+[source,groovy]
+----
+group('imageServing').conditionalOnClass(name:
'com.example.ManagedFileAccessProvider') {
+ bean('roomFileAccessProvider', RoomFileAccessProvider)
+ bean('chatFileAccessProvider', ChatFileAccessProvider)
+}
+----
+
+This is the shape real auto-configurations take — Spring Boot's own
`JacksonAutoConfiguration` carries four nested `@ConditionalOnClass`
configuration classes — and it is the only shape that works for a bean whose
*own signature* names a class that may be absent. A condition is read from the
bytecode before anything is loaded, but a `@Bean` method's parameter and return
types are resolved when its configuration class is parsed, so guarding such a
bean on the method is not reliably safe; moving it into a group moves the guard
onto a class that is never parsed when the condition fails. Spring finds the
nested class unaided, since `ConfigurationClassParser` processes a
configuration class's member classes. Groups take the condition qualifiers and
`.annotate(...)`; they do not nest, and the closure takes no parameters, since
it declares a class rather than a bean.
+
+=== Diagnostics
+
+Three mistakes the DSL used to accept are now compile errors, because each of
them fails at runtime in a way that is hard to trace back:
+
+* A `beans` closure containing any top-level `bean`/`field`/`method` call must
be entirely such calls. A stray statement among real declarations — a typo in a
call name, or an `if` wrapped around some beans — used to make the whole block
register *nothing*, silently, with the failure surfacing far away as beans that
are simply absent. To register a bean conditionally, put the condition on the
bean rather than around it; for state or logic shared between beans, use
`field(...)` or `method(...)`. A `beans` closure containing no such calls at
all is not the DSL and is left alone, as before.
+* Calling one bean from another does not return the registered singleton
unless the host is a proxied `@Configuration` class — and the DSL's usual hosts
are not: `@AutoConfiguration` is `@Configuration(proxyBeanMethods = false)`, a
generated plugin sibling carries exactly that, and an `Application` class is a
configuration source without being annotated at all. Such a call silently
constructs a second instance, so it is rejected; inject the bean as a closure
parameter instead. A static `@Bean` method is never intercepted even on a
proxied class, so `.staticMethod()` beans are checked everywhere.
+* A `BeanFactoryPostProcessor` or `BeanPostProcessor` bean must be
`.staticMethod()`.
+
+=== Seeing what a block compiled to
+
+Building with `-Dgrails.beans.dsl.dumpdir=<dir>` writes one `<qualified
name>.beans.txt` per host class, listing the generated members — bean names,
the annotations the qualifiers became, modifiers, declared types with any type
arguments they ended up carrying, and parameter annotations. Bodies are
omitted, being the closure bodies from the source. Nothing is written unless
the property is set.
Review Comment:
`grails.beans.dsl.dumpdir` is read with `System.getProperty` in the JVM that
runs the Groovy compiler. With Gradle's default `fork = true` for
`GroovyCompile`, `./gradlew build -Dgrails.beans.dsl.dumpdir=build/beans` sets
the property on the Gradle client and daemon, not on the compiler worker that
runs this transform, and nothing is written. Worth saying how to set it where
it counts - `tasks.withType(GroovyCompile).configureEach {
groovyOptions.forkOptions.jvmArgs += '-Dgrails.beans.dsl.dumpdir=...' }` - and
that a relative path resolves against that worker process's working directory,
so an absolute path is the safe form.
--
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]
