codeconsole commented on code in PR #16292:
URL: https://github.com/apache/grails-core/pull/16292#discussion_r3972533969
##########
grails-beans-dsl/src/main/java/org/grails/compiler/beans/GrailsBeansASTTransformation.java:
##########
@@ -913,35 +1660,221 @@ private Statement synthesizedConstruction(ClassNode
beanType, Parameter[] parame
return returnStatement;
}
- private String syntheticBeanMethodName(ClassNode beanType, Set<String>
usedNames) {
- String base = decapitalize(beanType.getNameWithoutPackage());
- String candidate;
- int index = 0;
- do {
- candidate = base + "$" + index;
- index++;
- }
- while (usedNames.contains(candidate));
- return candidate;
+ /**
+ * 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 void processFieldStatement(ClassNode classNode,
MethodCallExpression baseCall,
- List<MethodCallExpression> qualifierCalls, SourceUnit source,
Set<String> usedNames) {
- List<Expression> baseArgs = flatten(baseCall.getArguments());
- TypeAndName typeAndName = parseNameAndType(baseArgs, baseCall, source,
FIELD_CALL, true);
- if (typeAndName == null) {
- return;
+ private boolean hasExplicitTypeArguments(List<MethodCallExpression>
qualifierCalls) {
+ for (MethodCallExpression qualifierCall : qualifierCalls) {
+ if (TYPE_ARGUMENTS_CALL.equals(qualifierCall.getMethodAsString()))
{
+ return true;
+ }
}
- if (!registerName(typeAndName.name, baseCall, source, usedNames,
+ 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>
+ *
+ * <p>What this cannot correct is the anonymous class's <i>outer
class</i>, which
+ * {@code InnerClassNode} fixes at construction. On a plugin descriptor
the members move to a
+ * sibling while the anonymous class stays homed on the descriptor, so the
MOP dispatch methods
+ * Groovy generates for it read a {@code this$0} typed as the descriptor
while the field now
+ * holds the sibling. An anonymous class that touches only its own members
and what it inherits
+ * is fine; one that reaches a member declared in the block is rejected by
+ * {@link #rejectAnonymousClassReachingMovedMembers}, rather than left to
fail with
+ * {@code NoSuchFieldError} inside a running application.</p>
+ */
+ private boolean rehomeAnonymousInnerClasses(Statement body, ClassNode
host, boolean staticMethod,
+ String beanName, SourceUnit source) {
+ List<ConstructorCallExpression> anonymous = new ArrayList<>();
+ body.visit(new CodeVisitorSupport() {
+ // Deliberately not descending. The lift moves the closure's own
body into a method, so
+ // an anonymous class written directly in it loses the closure it
was homed against -
+ // that is what this repairs. One written inside a NESTED closure
does not: that closure
+ // survives the lift and is still its enclosing instance, so
re-homing it would rewrite a
+ // correct `this` into the configuration class and fail at runtime
with the very
+ // GroovyCastException this method exists to prevent - or, under
.staticMethod(), reject
+ // a body that has an enclosing instance and compiles perfectly
well.
+ @Override
+ public void visitClosureExpression(ClosureExpression expression) {
+ }
+
+ @Override
+ public void
visitConstructorCallExpression(ConstructorCallExpression call) {
+ if (call.isUsingAnonymousInnerClass()) {
+ anonymous.add(call);
+ }
+ super.visitConstructorCallExpression(call);
+ }
+ });
+ for (ConstructorCallExpression call : anonymous) {
+ ClassNode inner = call.getType();
Review Comment:
Fixed in 67d20dc: `inner.setEnclosingMethod(liftedMethod)`, threaded in from
both call sites.
Tests: `@CompileStatic` descriptor with an anonymous class in a top-level
bean body; `group(...)` body constructing one, both host kinds ×
dynamic/`@CompileStatic`. All hit the `GroovyBugError` without the line.
##########
grails-beans-dsl/src/main/java/org/grails/compiler/beans/GrailsBeansASTTransformation.java:
##########
@@ -875,9 +1208,423 @@ private void processBeanStatement(ClassNode classNode,
MethodCallExpression oute
}
}
+ if (!rejectNonStaticPostProcessor(beanMethod, beanType, baseCall,
source)) {
+ return;
+ }
+
+ if (!rehomeAnonymousInnerClasses(beanBody, classNode,
Modifier.isStatic(beanMethod.getModifiers()),
+ typeAndName.name, source)) {
+ return;
+ }
+
classNode.addMethod(beanMethod);
}
+ private static final String BEAN_FACTORY_POST_PROCESSOR =
"org.springframework.beans.factory.config.BeanFactoryPostProcessor";
+ private static final String BEAN_POST_PROCESSOR =
"org.springframework.beans.factory.config.BeanPostProcessor";
+
+ /**
+ * A {@code BeanFactoryPostProcessor}/{@code BeanPostProcessor} bean must
be creatable without
+ * instantiating its declaring class, because Spring has to obtain it
before the ordinary bean
+ * lifecycle it participates in has started. Declared as an instance
method it still "works",
+ * which is the problem: the configuration class is instantiated far too
early, taking every bean
+ * its methods depend on with it, out of order and past the
post-processors that would have
+ * configured them - a class of startup bug that shows up as an unrelated
bean being unconfigured
+ * rather than as anything pointing here.
+ *
+ * <p>{@code .staticMethod()} is the fix and is already in the DSL; this
only stops the mistake
+ * being silent. An instance-bound post-processor, if one is genuinely
wanted, is still writable
+ * as an ordinary {@code @Bean} method on the same class - the block does
not claim them.</p>
+ */
+ private boolean rejectNonStaticPostProcessor(MethodNode beanMethod,
ClassNode beanType,
+ ASTNode location, SourceUnit source) {
+ if (Modifier.isStatic(beanMethod.getModifiers())) {
+ return true;
+ }
+ String postProcessorType = null;
+ if (isSubtypeOf(beanType,
ClassHelper.make(BEAN_FACTORY_POST_PROCESSOR))) {
+ postProcessorType = "BeanFactoryPostProcessor";
+ }
+ else if (isSubtypeOf(beanType, ClassHelper.make(BEAN_POST_PROCESSOR)))
{
+ postProcessorType = "BeanPostProcessor";
+ }
+ if (postProcessorType == null) {
+ return true;
+ }
+ addError(location, source, "a " + postProcessorType + " bean must be
declared " +
+ ".staticMethod(), so Spring can obtain it without
instantiating this class - as an " +
+ "instance method it forces that instantiation before the beans
it post-processes " +
+ "are configured");
+ return false;
+ }
+
+ /**
+ * Writes the members this block generated to {@code
-Dgrails.beans.dsl.dumpdir=<dir>}, one file
+ * per host class.
+ *
+ * <p>Everything the DSL decides that the source does not say is a
declaration, not a body: the
+ * bean name Spring will resolve by, the annotations the qualifiers
became, the modifiers, the
+ * declared type and whether it ended up carrying type arguments, and the
parameter annotations
+ * that make a dependency optional or qualified. Bodies are excluded on
purpose - a bean body is
+ * the author's own closure body, lifted verbatim, so it is already
readable where they wrote it.
+ *
+ * <p>Without this, the only way to see any of it is {@code javap} on the
compiled class, which
+ * is a poor place to answer "did that qualifier attach anything" while
writing the block.
+ * Grails already takes this shape for its other compile-time generator,
where
+ * {@code grails.views.gsp.keepgenerateddir} keeps the Groovy a GSP
compiles to.</p>
+ */
+ private void dumpGeneratedMembers(ClassNode host, List<MethodNode>
methods, List<FieldNode> fields,
+ SourceUnit source) {
+ String dir = System.getProperty(DUMP_DIR_PROPERTY);
+ if (dir == null || dir.isBlank()) {
+ return;
+ }
+ StringBuilder text = new StringBuilder();
+ text.append("// Generated from the 'beans' DSL in
").append(host.getName()).append('\n');
+ text.append("// Bodies are omitted: each is the closure body from that
source, lifted verbatim.\n");
+ for (FieldNode field : fields) {
+ text.append('\n');
+ for (AnnotationNode annotation : field.getAnnotations()) {
+ text.append(annotationText(annotation)).append('\n');
+ }
+
text.append(AstToTextHelper.getModifiersText(field.getModifiers())).append(' ')
+ .append(typeText(field.getType())).append('
').append(field.getName()).append('\n');
+ }
+ for (MethodNode method : methods) {
+ text.append('\n');
+ for (AnnotationNode annotation : method.getAnnotations()) {
+ text.append(annotationText(annotation)).append('\n');
+ }
+
text.append(AstToTextHelper.getModifiersText(method.getModifiers())).append(' ')
+ .append(typeText(method.getReturnType())).append('
').append(method.getName())
+
.append('(').append(parametersText(method.getParameters())).append(")\n");
+ }
+ try {
+ Path target = Paths.get(dir);
+ Files.createDirectories(target);
+ Files.writeString(target.resolve(host.getName() + ".beans.txt"),
text.toString(),
+ StandardCharsets.UTF_8);
+ }
+ catch (IOException | RuntimeException e) {
+ // Opt-in by definition, so this can only fire for someone who
asked for the dump and
+ // would otherwise be left looking for a file that was never
written.
+ addError(host, source, "could not write the beans DSL dump for " +
host.getName() + " to \"" +
+ dir + "\" (" + DUMP_DIR_PROPERTY + "): " + e);
+ }
+ }
+
+ private String parametersText(Parameter[] parameters) {
+ StringBuilder text = new StringBuilder();
+ for (Parameter parameter : parameters) {
+ if (text.length() > 0) {
+ text.append(", ");
+ }
+ for (AnnotationNode annotation : parameter.getAnnotations()) {
+ text.append(annotationText(annotation)).append(' ');
+ }
+ text.append(typeText(parameter.getType())).append('
').append(parameter.getName());
+ }
+ return text.toString();
+ }
+
+ // Type arguments are printed only when every one of them is concrete. A
raw declared type
+ // resolved from a class still reports its own type PARAMETERS here, and
printing those would
+ // read as <String> when nothing of the sort was declared.
+ private String typeText(ClassNode type) {
+ GenericsType[] generics = type.getGenericsTypes();
+ if (generics == null || generics.length == 0) {
+ return type.getName();
+ }
+ StringBuilder text = new StringBuilder(type.getName());
+ for (GenericsType generic : generics) {
+ if (generic.isPlaceholder() || generic.isWildcard()) {
+ return type.getName();
+ }
+ }
+ text.append('<');
+ for (int i = 0; i < generics.length; i++) {
+ text.append(i == 0 ? "" : ",
").append(generics[i].getType().getName());
+ }
+ return text.append('>').toString();
+ }
+
+ private String annotationText(AnnotationNode annotation) {
+ StringBuilder text = new
StringBuilder("@").append(annotation.getClassNode().getNameWithoutPackage());
+ Map<String, Expression> members = annotation.getMembers();
+ if (members.isEmpty()) {
+ return text.toString();
+ }
+ text.append('(');
+ boolean first = true;
+ for (Map.Entry<String, Expression> member : members.entrySet()) {
+ text.append(first ? "" : ", ").append(member.getKey()).append(" =
")
+ .append(memberValueText(member.getValue()));
+ first = false;
+ }
+ return text.append(')').toString();
+ }
+
+ // Expression.getText() renders a String constant bare, so
@DependsOn("names") would print as
+ // value = names and read as an identifier. Quote them, and descend into a
list so an
+ // array-valued attribute reads the way it was written.
+ private String memberValueText(Expression value) {
+ if (value instanceof ConstantExpression && ((ConstantExpression)
value).getValue() instanceof String) {
+ return "\"" + ((ConstantExpression) value).getValue() + "\"";
+ }
+ if (value instanceof ListExpression) {
+ StringBuilder text = new StringBuilder("[");
+ List<Expression> entries = ((ListExpression)
value).getExpressions();
+ for (int i = 0; i < entries.size(); i++) {
+ text.append(i == 0 ? "" : ",
").append(memberValueText(entries.get(i)));
+ }
+ return text.append(']').toString();
+ }
+ return value.getText();
+ }
+
+ // The methods this block just generated, in declaration order: everything
on the host that was
+ // not there before the two processing loops ran. MethodNode does not
override equals, so the
+ // removal is by identity and cannot drop a same-signature method the user
wrote.
+ private List<MethodNode> generatedMembers(ClassNode host, List<MethodNode>
preExisting) {
+ List<MethodNode> generated = new ArrayList<>(host.getMethods());
+ generated.removeAll(preExisting);
+ return generated;
+ }
+
+ /**
+ * Rejects a call from one generated method to another generated {@code
@Bean} method, on a host
+ * whose bean methods Spring does not proxy.
+ *
+ * <p>Calling a sibling {@code @Bean} method and getting the singleton
back is a CGLIB trick, and
+ * Spring only plays it for a full {@code @Configuration} class. On a
<i>lite</i> configuration
+ * source the same call is a plain Java call that constructs a second
instance - and lite is the
+ * common case for this DSL: {@code @AutoConfiguration} is
+ * {@code @Configuration(proxyBeanMethods = false)}, the sibling generated
for a plugin descriptor
+ * carries exactly that, and a Grails {@code Application} class is a
configuration source without
+ * being annotated {@code @Configuration} at all.</p>
+ *
+ * <p>A full {@code @Configuration} class is not wholly exempt: the
interception is CGLIB
+ * subclassing, so it cannot override a {@code static} method, and Spring
documents that calls to
+ * a static {@code @Bean} method are never intercepted - not even there. A
{@code .staticMethod()}
+ * bean is therefore checked on every host, and is the only thing checked
on a proxied one.</p>
+ *
+ * <p>Nothing about that failure is visible at runtime. The context
starts, every bean exists, and
+ * two objects live where the author meant one - so a listener registers
on the wrong instance, or
+ * configuration applied to one is missing from the other. It is also the
exact mistake a
+ * migration invites, since moving bean methods off a real {@code
@Configuration} class into this
+ * DSL silently changes what those calls mean.</p>
+ */
+ private void rejectUnproxiedSiblingBeanCalls(ClassNode host,
List<MethodNode> generated, SourceUnit source) {
+ boolean proxied = beanMethodsAreProxied(host);
+ Map<String, MethodNode> beanMethodsByName = new LinkedHashMap<>();
+ // Every @Bean method on the host, not only the ones this block
generated. A class that
+ // mixes hand-written @Bean methods with the DSL is what a migration
looks like midway
+ // through, and a call to one of those from a generated body misses
the singleton in exactly
+ // the same way - more easily, in fact, since it was correct in the
@Configuration class the
+ // beans are being moved out of. Only generated bodies are scanned:
what a hand-written
+ // method does is its author's business, not this transform's.
+ for (MethodNode method : host.getMethods()) {
+ if (method.getAnnotations(ClassHelper.make(Bean.class)).isEmpty())
{
+ continue;
+ }
+ // A proxied host still cannot intercept a .staticMethod() bean:
the interception is
+ // CGLIB subclassing, and a static method cannot be overridden. So
on a full
+ // @Configuration class those are the only sibling calls still
worth rejecting.
+ if (!proxied || method.isStatic()) {
+ beanMethodsByName.put(method.getName(), method);
+ }
+ }
+ if (beanMethodsByName.isEmpty()) {
+ return;
+ }
+ for (MethodNode method : generated) {
+ if (method.getCode() == null) {
+ continue;
+ }
+ MethodNode caller = method;
+ method.getCode().visit(new CodeVisitorSupport() {
+ // Deliberately not descending. Inside a closure an
unqualified call is resolved
+ // against the delegate first, so `new Registry().tap {
initialize() }` calls the
+ // registry - not this class - even though the AST records
implicit-this either way.
+ // Reading that as a sibling bean call would reject working
code, which is a far
+ // worse trade than missing the rare bean call written inside
a nested closure.
+ @Override
+ public void visitClosureExpression(ClosureExpression
expression) {
+ }
+
+ @Override
+ public void visitMethodCallExpression(MethodCallExpression
call) {
+ super.visitMethodCallExpression(call);
+ if (!isSelfCall(call)) {
+ return;
+ }
+ MethodNode target =
beanMethodsByName.get(call.getMethodAsString());
+ if (target == null || target == caller) {
+ return;
+ }
+ addError(call, source, siblingCallCause(host,
call.getMethodAsString(), proxied) +
+ ", so this call does not return the bean Spring
registered - it constructs a second " +
+ "instance. Inject it instead, by declaring it as a
parameter of this closure; if what " +
+ "you want is shared logic rather than the bean,
move it into a method(...) declaration.");
+ }
+ });
+ }
+ }
+
+ // Why this particular call misses the singleton. On a proxied host the
map holds only static
+ // bean methods, so reaching here means the target is one.
+ private String siblingCallCause(ClassNode host, String name, boolean
proxied) {
+ if (proxied) {
+ return "\"" + name + "(...)\" is another bean declared in this
block, and is declared " +
+ ".staticMethod(). A static @Bean method is never
intercepted by the container - not even " +
+ "on a proxied @Configuration class like " +
host.getNameWithoutPackage() + " - because " +
+ "that interception is CGLIB subclassing, which cannot
override a static method";
+ }
+ return "\"" + name + "(...)\" is another bean declared in this block,
and " +
+ host.getNameWithoutPackage() + " is not a proxied
@Configuration class";
+ }
+
+ /**
+ * Rejects an unqualified reference from an anonymous inner class in a
bean body to a member that
+ * moved to the generated sibling.
+ *
+ * <p>Only on a plugin descriptor, and only because the anonymous class
cannot follow. Groovy
+ * fixes an inner class's outer class when it creates the node and offers
no way to move it, so
+ * the class stays homed on the descriptor while the members it wants are
on the sibling. Its MOP
+ * dispatch methods then read a {@code this$0} typed as the descriptor
where the field holds the
+ * sibling, and the reference fails with {@code NoSuchFieldError} inside a
running application -
+ * or, under {@code @CompileStatic}, as a "cannot find matching method"
naming a synthetic class
+ * nobody wrote.</p>
+ *
+ * <p>The test is the narrow one: a name that is <i>both</i> a member this
block generated and
+ * not resolvable on the anonymous class itself or anything it inherits. A
call to the anonymous
+ * class's own method, or to one from the interface it implements, is left
alone - it is only the
+ * names that actually moved that cannot be reached.</p>
+ */
+ private void rejectAnonymousClassReachingMovedMembers(ClassNode host,
ClassNode declaringClass,
+ List<MethodNode> generatedMethods, List<FieldNode>
generatedFields, SourceUnit source) {
+ if (host == declaringClass) {
Review Comment:
Fixed in 191830e: the check runs per group as well as per sibling; the early
return moved to the sibling call site. A group passes no moved set — static
nested class, no enclosing instance, so anything unresolvable on the anonymous
class itself is rejected.
Tests: group-level member, host-level member, each on a plain host and a
descriptor; self-contained and captured-local controls.
##########
grails-beans-dsl/src/main/java/org/grails/compiler/beans/GrailsBeansASTTransformation.java:
##########
@@ -875,9 +1208,423 @@ private void processBeanStatement(ClassNode classNode,
MethodCallExpression oute
}
}
+ if (!rejectNonStaticPostProcessor(beanMethod, beanType, baseCall,
source)) {
+ return;
+ }
+
+ if (!rehomeAnonymousInnerClasses(beanBody, classNode,
Modifier.isStatic(beanMethod.getModifiers()),
+ typeAndName.name, source)) {
+ return;
+ }
+
classNode.addMethod(beanMethod);
}
+ private static final String BEAN_FACTORY_POST_PROCESSOR =
"org.springframework.beans.factory.config.BeanFactoryPostProcessor";
+ private static final String BEAN_POST_PROCESSOR =
"org.springframework.beans.factory.config.BeanPostProcessor";
+
+ /**
+ * A {@code BeanFactoryPostProcessor}/{@code BeanPostProcessor} bean must
be creatable without
+ * instantiating its declaring class, because Spring has to obtain it
before the ordinary bean
+ * lifecycle it participates in has started. Declared as an instance
method it still "works",
+ * which is the problem: the configuration class is instantiated far too
early, taking every bean
+ * its methods depend on with it, out of order and past the
post-processors that would have
+ * configured them - a class of startup bug that shows up as an unrelated
bean being unconfigured
+ * rather than as anything pointing here.
+ *
+ * <p>{@code .staticMethod()} is the fix and is already in the DSL; this
only stops the mistake
+ * being silent. An instance-bound post-processor, if one is genuinely
wanted, is still writable
+ * as an ordinary {@code @Bean} method on the same class - the block does
not claim them.</p>
+ */
+ private boolean rejectNonStaticPostProcessor(MethodNode beanMethod,
ClassNode beanType,
+ ASTNode location, SourceUnit source) {
+ if (Modifier.isStatic(beanMethod.getModifiers())) {
+ return true;
+ }
+ String postProcessorType = null;
+ if (isSubtypeOf(beanType,
ClassHelper.make(BEAN_FACTORY_POST_PROCESSOR))) {
+ postProcessorType = "BeanFactoryPostProcessor";
+ }
+ else if (isSubtypeOf(beanType, ClassHelper.make(BEAN_POST_PROCESSOR)))
{
+ postProcessorType = "BeanPostProcessor";
+ }
+ if (postProcessorType == null) {
+ return true;
+ }
+ addError(location, source, "a " + postProcessorType + " bean must be
declared " +
+ ".staticMethod(), so Spring can obtain it without
instantiating this class - as an " +
+ "instance method it forces that instantiation before the beans
it post-processes " +
+ "are configured");
+ return false;
+ }
+
+ /**
+ * Writes the members this block generated to {@code
-Dgrails.beans.dsl.dumpdir=<dir>}, one file
+ * per host class.
+ *
+ * <p>Everything the DSL decides that the source does not say is a
declaration, not a body: the
+ * bean name Spring will resolve by, the annotations the qualifiers
became, the modifiers, the
+ * declared type and whether it ended up carrying type arguments, and the
parameter annotations
+ * that make a dependency optional or qualified. Bodies are excluded on
purpose - a bean body is
+ * the author's own closure body, lifted verbatim, so it is already
readable where they wrote it.
+ *
+ * <p>Without this, the only way to see any of it is {@code javap} on the
compiled class, which
+ * is a poor place to answer "did that qualifier attach anything" while
writing the block.
+ * Grails already takes this shape for its other compile-time generator,
where
+ * {@code grails.views.gsp.keepgenerateddir} keeps the Groovy a GSP
compiles to.</p>
+ */
+ private void dumpGeneratedMembers(ClassNode host, List<MethodNode>
methods, List<FieldNode> fields,
+ SourceUnit source) {
+ String dir = System.getProperty(DUMP_DIR_PROPERTY);
+ if (dir == null || dir.isBlank()) {
+ return;
+ }
+ StringBuilder text = new StringBuilder();
+ text.append("// Generated from the 'beans' DSL in
").append(host.getName()).append('\n');
+ text.append("// Bodies are omitted: each is the closure body from that
source, lifted verbatim.\n");
+ for (FieldNode field : fields) {
+ text.append('\n');
+ for (AnnotationNode annotation : field.getAnnotations()) {
+ text.append(annotationText(annotation)).append('\n');
+ }
+
text.append(AstToTextHelper.getModifiersText(field.getModifiers())).append(' ')
+ .append(typeText(field.getType())).append('
').append(field.getName()).append('\n');
+ }
+ for (MethodNode method : methods) {
+ text.append('\n');
+ for (AnnotationNode annotation : method.getAnnotations()) {
+ text.append(annotationText(annotation)).append('\n');
+ }
+
text.append(AstToTextHelper.getModifiersText(method.getModifiers())).append(' ')
+ .append(typeText(method.getReturnType())).append('
').append(method.getName())
+
.append('(').append(parametersText(method.getParameters())).append(")\n");
+ }
+ try {
+ Path target = Paths.get(dir);
+ Files.createDirectories(target);
+ Files.writeString(target.resolve(host.getName() + ".beans.txt"),
text.toString(),
+ StandardCharsets.UTF_8);
+ }
+ catch (IOException | RuntimeException e) {
+ // Opt-in by definition, so this can only fire for someone who
asked for the dump and
+ // would otherwise be left looking for a file that was never
written.
+ addError(host, source, "could not write the beans DSL dump for " +
host.getName() + " to \"" +
+ dir + "\" (" + DUMP_DIR_PROPERTY + "): " + e);
+ }
+ }
+
+ private String parametersText(Parameter[] parameters) {
+ StringBuilder text = new StringBuilder();
+ for (Parameter parameter : parameters) {
+ if (text.length() > 0) {
+ text.append(", ");
+ }
+ for (AnnotationNode annotation : parameter.getAnnotations()) {
+ text.append(annotationText(annotation)).append(' ');
+ }
+ text.append(typeText(parameter.getType())).append('
').append(parameter.getName());
+ }
+ return text.toString();
+ }
+
+ // Type arguments are printed only when every one of them is concrete. A
raw declared type
+ // resolved from a class still reports its own type PARAMETERS here, and
printing those would
+ // read as <String> when nothing of the sort was declared.
+ private String typeText(ClassNode type) {
+ GenericsType[] generics = type.getGenericsTypes();
+ if (generics == null || generics.length == 0) {
+ return type.getName();
+ }
+ StringBuilder text = new StringBuilder(type.getName());
+ for (GenericsType generic : generics) {
+ if (generic.isPlaceholder() || generic.isWildcard()) {
+ return type.getName();
+ }
+ }
+ text.append('<');
+ for (int i = 0; i < generics.length; i++) {
+ text.append(i == 0 ? "" : ",
").append(generics[i].getType().getName());
+ }
+ return text.append('>').toString();
+ }
+
+ private String annotationText(AnnotationNode annotation) {
+ StringBuilder text = new
StringBuilder("@").append(annotation.getClassNode().getNameWithoutPackage());
+ Map<String, Expression> members = annotation.getMembers();
+ if (members.isEmpty()) {
+ return text.toString();
+ }
+ text.append('(');
+ boolean first = true;
+ for (Map.Entry<String, Expression> member : members.entrySet()) {
+ text.append(first ? "" : ", ").append(member.getKey()).append(" =
")
+ .append(memberValueText(member.getValue()));
+ first = false;
+ }
+ return text.append(')').toString();
+ }
+
+ // Expression.getText() renders a String constant bare, so
@DependsOn("names") would print as
+ // value = names and read as an identifier. Quote them, and descend into a
list so an
+ // array-valued attribute reads the way it was written.
+ private String memberValueText(Expression value) {
+ if (value instanceof ConstantExpression && ((ConstantExpression)
value).getValue() instanceof String) {
+ return "\"" + ((ConstantExpression) value).getValue() + "\"";
+ }
+ if (value instanceof ListExpression) {
+ StringBuilder text = new StringBuilder("[");
+ List<Expression> entries = ((ListExpression)
value).getExpressions();
+ for (int i = 0; i < entries.size(); i++) {
+ text.append(i == 0 ? "" : ",
").append(memberValueText(entries.get(i)));
+ }
+ return text.append(']').toString();
+ }
+ return value.getText();
+ }
+
+ // The methods this block just generated, in declaration order: everything
on the host that was
+ // not there before the two processing loops ran. MethodNode does not
override equals, so the
+ // removal is by identity and cannot drop a same-signature method the user
wrote.
+ private List<MethodNode> generatedMembers(ClassNode host, List<MethodNode>
preExisting) {
+ List<MethodNode> generated = new ArrayList<>(host.getMethods());
+ generated.removeAll(preExisting);
+ return generated;
+ }
+
+ /**
+ * Rejects a call from one generated method to another generated {@code
@Bean} method, on a host
+ * whose bean methods Spring does not proxy.
+ *
+ * <p>Calling a sibling {@code @Bean} method and getting the singleton
back is a CGLIB trick, and
+ * Spring only plays it for a full {@code @Configuration} class. On a
<i>lite</i> configuration
+ * source the same call is a plain Java call that constructs a second
instance - and lite is the
+ * common case for this DSL: {@code @AutoConfiguration} is
+ * {@code @Configuration(proxyBeanMethods = false)}, the sibling generated
for a plugin descriptor
+ * carries exactly that, and a Grails {@code Application} class is a
configuration source without
+ * being annotated {@code @Configuration} at all.</p>
+ *
+ * <p>A full {@code @Configuration} class is not wholly exempt: the
interception is CGLIB
+ * subclassing, so it cannot override a {@code static} method, and Spring
documents that calls to
+ * a static {@code @Bean} method are never intercepted - not even there. A
{@code .staticMethod()}
+ * bean is therefore checked on every host, and is the only thing checked
on a proxied one.</p>
+ *
+ * <p>Nothing about that failure is visible at runtime. The context
starts, every bean exists, and
+ * two objects live where the author meant one - so a listener registers
on the wrong instance, or
+ * configuration applied to one is missing from the other. It is also the
exact mistake a
+ * migration invites, since moving bean methods off a real {@code
@Configuration} class into this
+ * DSL silently changes what those calls mean.</p>
+ */
+ private void rejectUnproxiedSiblingBeanCalls(ClassNode host,
List<MethodNode> generated, SourceUnit source) {
+ boolean proxied = beanMethodsAreProxied(host);
+ Map<String, MethodNode> beanMethodsByName = new LinkedHashMap<>();
+ // Every @Bean method on the host, not only the ones this block
generated. A class that
+ // mixes hand-written @Bean methods with the DSL is what a migration
looks like midway
+ // through, and a call to one of those from a generated body misses
the singleton in exactly
+ // the same way - more easily, in fact, since it was correct in the
@Configuration class the
+ // beans are being moved out of. Only generated bodies are scanned:
what a hand-written
+ // method does is its author's business, not this transform's.
+ for (MethodNode method : host.getMethods()) {
+ if (method.getAnnotations(ClassHelper.make(Bean.class)).isEmpty())
{
+ continue;
+ }
+ // A proxied host still cannot intercept a .staticMethod() bean:
the interception is
+ // CGLIB subclassing, and a static method cannot be overridden. So
on a full
+ // @Configuration class those are the only sibling calls still
worth rejecting.
+ if (!proxied || method.isStatic()) {
+ beanMethodsByName.put(method.getName(), method);
+ }
+ }
+ if (beanMethodsByName.isEmpty()) {
+ return;
+ }
+ for (MethodNode method : generated) {
+ if (method.getCode() == null) {
+ continue;
+ }
+ MethodNode caller = method;
+ method.getCode().visit(new CodeVisitorSupport() {
+ // Deliberately not descending. Inside a closure an
unqualified call is resolved
+ // against the delegate first, so `new Registry().tap {
initialize() }` calls the
+ // registry - not this class - even though the AST records
implicit-this either way.
+ // Reading that as a sibling bean call would reject working
code, which is a far
+ // worse trade than missing the rare bean call written inside
a nested closure.
+ @Override
+ public void visitClosureExpression(ClosureExpression
expression) {
+ }
+
+ @Override
+ public void visitMethodCallExpression(MethodCallExpression
call) {
+ super.visitMethodCallExpression(call);
+ if (!isSelfCall(call)) {
+ return;
+ }
+ MethodNode target =
beanMethodsByName.get(call.getMethodAsString());
+ if (target == null || target == caller) {
+ return;
+ }
+ addError(call, source, siblingCallCause(host,
call.getMethodAsString(), proxied) +
+ ", so this call does not return the bean Spring
registered - it constructs a second " +
+ "instance. Inject it instead, by declaring it as a
parameter of this closure; if what " +
+ "you want is shared logic rather than the bean,
move it into a method(...) declaration.");
+ }
+ });
+ }
+ }
+
+ // Why this particular call misses the singleton. On a proxied host the
map holds only static
+ // bean methods, so reaching here means the target is one.
+ private String siblingCallCause(ClassNode host, String name, boolean
proxied) {
+ if (proxied) {
+ return "\"" + name + "(...)\" is another bean declared in this
block, and is declared " +
+ ".staticMethod(). A static @Bean method is never
intercepted by the container - not even " +
+ "on a proxied @Configuration class like " +
host.getNameWithoutPackage() + " - because " +
+ "that interception is CGLIB subclassing, which cannot
override a static method";
+ }
+ return "\"" + name + "(...)\" is another bean declared in this block,
and " +
+ host.getNameWithoutPackage() + " is not a proxied
@Configuration class";
+ }
+
+ /**
+ * Rejects an unqualified reference from an anonymous inner class in a
bean body to a member that
+ * moved to the generated sibling.
+ *
+ * <p>Only on a plugin descriptor, and only because the anonymous class
cannot follow. Groovy
+ * fixes an inner class's outer class when it creates the node and offers
no way to move it, so
+ * the class stays homed on the descriptor while the members it wants are
on the sibling. Its MOP
+ * dispatch methods then read a {@code this$0} typed as the descriptor
where the field holds the
+ * sibling, and the reference fails with {@code NoSuchFieldError} inside a
running application -
+ * or, under {@code @CompileStatic}, as a "cannot find matching method"
naming a synthetic class
+ * nobody wrote.</p>
+ *
+ * <p>The test is the narrow one: a name that is <i>both</i> a member this
block generated and
+ * not resolvable on the anonymous class itself or anything it inherits. A
call to the anonymous
+ * class's own method, or to one from the interface it implements, is left
alone - it is only the
+ * names that actually moved that cannot be reached.</p>
+ */
+ private void rejectAnonymousClassReachingMovedMembers(ClassNode host,
ClassNode declaringClass,
+ List<MethodNode> generatedMethods, List<FieldNode>
generatedFields, SourceUnit source) {
+ if (host == declaringClass) {
+ // Not a plugin descriptor: the members and the anonymous class
share a home, so an
+ // unqualified reference resolves the way it reads.
+ return;
+ }
+ Set<String> moved = new HashSet<>();
+ for (MethodNode method : generatedMethods) {
+ moved.add(method.getName());
+ }
+ for (FieldNode field : generatedFields) {
+ moved.add(field.getName());
+ }
+ if (moved.isEmpty()) {
+ return;
+ }
+ for (MethodNode method : generatedMethods) {
+ if (method.getCode() == null) {
+ continue;
+ }
+ List<ConstructorCallExpression> anonymous = new ArrayList<>();
+ method.getCode().visit(new CodeVisitorSupport() {
+ @Override
+ public void
visitConstructorCallExpression(ConstructorCallExpression call) {
+ if (call.isUsingAnonymousInnerClass()) {
+ anonymous.add(call);
+ }
+ super.visitConstructorCallExpression(call);
+ }
+ });
+ for (ConstructorCallExpression call : anonymous) {
+ reportMovedMemberReferences(call, moved, host, source);
+ }
+ }
+ }
+
+ private void reportMovedMemberReferences(ConstructorCallExpression call,
Set<String> moved,
+ ClassNode host, SourceUnit source) {
+ ClassNode inner = call.getType();
+ Set<String> own = existingMemberNames(inner);
+ for (MethodNode method : inner.getMethods()) {
+ if (method.getCode() == null) {
+ continue;
+ }
+ method.getCode().visit(new CodeVisitorSupport() {
+ @Override
+ public void visitMethodCallExpression(MethodCallExpression
inner) {
+ super.visitMethodCallExpression(inner);
+ if (inner.isImplicitThis()) {
+ report(inner.getMethodAsString(), inner, "()");
+ }
+ }
+
+ @Override
+ public void visitVariableExpression(VariableExpression
expression) {
+ super.visitVariableExpression(expression);
+ if (expression.getAccessedVariable() instanceof
DynamicVariable) {
+ report(expression.getName(), expression, "");
+ }
+ }
+
+ private void report(String name, ASTNode at, String
callSuffix) {
+ if (name == null || own.contains(name) ||
!moved.contains(name)) {
Review Comment:
Fixed in 635f284: a `DynamicVariable` matches on the `get`/`is`/`set`
spellings of its name as well as its own — the set `collectMethodNames`
reserves.
Test: `method('getSuffix', String)` + `suffix` in the body.
##########
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:
Fixed in 67d20dc (re-homing thread). Group bodies with an anonymous class
compile and run under `@CompileStatic` on both host kinds.
##########
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:
Covered in 191830e (that thread): the check runs per group, with the
stricter reachable set.
--
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]
