codeconsole commented on code in PR #16292:
URL: https://github.com/apache/grails-core/pull/16292#discussion_r3972535853
##########
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 bodies that construct an anonymous class now compile and run under
`@CompileStatic` on both host kinds, along with the `@CompileStatic` descriptor
shape the deferral broke. Fix and tests in 67d20dc, on the re-homing thread.
##########
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:
Groups are covered in 191830e — the check runs for each group as well as the
sibling, with the stricter reachable set a static nested class implies. Detail
on that thread.
--
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]
