codeconsole commented on code in PR #16134: URL: https://github.com/apache/grails-core/pull/16134#discussion_r3793083447
########## grails-gsp/grails-web-taglib/src/main/groovy/grails/gsp/taglib/compiler/CompiledTagCallRewriter.java: ########## @@ -0,0 +1,519 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package grails.gsp.taglib.compiler; + +import java.util.Collections; +import java.util.List; +import java.util.Set; + +import org.codehaus.groovy.ast.AnnotationNode; +import org.codehaus.groovy.ast.ClassCodeExpressionTransformer; +import org.codehaus.groovy.ast.ClassHelper; +import org.codehaus.groovy.ast.ClassNode; +import org.codehaus.groovy.ast.ConstructorNode; +import org.codehaus.groovy.ast.DynamicVariable; +import org.codehaus.groovy.ast.FieldNode; +import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.Parameter; +import org.codehaus.groovy.ast.Variable; +import org.codehaus.groovy.ast.expr.ArgumentListExpression; +import org.codehaus.groovy.ast.expr.ClosureExpression; +import org.codehaus.groovy.ast.expr.ConstantExpression; +import org.codehaus.groovy.ast.expr.Expression; +import org.codehaus.groovy.ast.expr.MapExpression; +import org.codehaus.groovy.ast.expr.MethodCallExpression; +import org.codehaus.groovy.ast.expr.PropertyExpression; +import org.codehaus.groovy.ast.expr.StaticMethodCallExpression; +import org.codehaus.groovy.ast.expr.TupleExpression; +import org.codehaus.groovy.ast.expr.VariableExpression; +import org.codehaus.groovy.ast.stmt.Statement; +import org.codehaus.groovy.control.SourceUnit; + +import org.grails.compiler.injection.GrailsASTUtils; +import org.grails.taglib.CompiledTagInvocation; +import org.grails.taglib.discovery.TagLibraryAstDiscovery; +import org.grails.taglib.index.TagLibraryIndex; + +/** + * Rewrites a call to a known tag into a direct invocation. + * + * <p>Writing {@code g.message(code: 'x')} reaches the tag library through {@code propertyMissing} to + * find the namespace and {@code invokeMethod} to find the tag, which is a dynamic call site even in a + * statically compiled class. Both the namespace and the tag name are fixed in the source, and the tag + * library index says whether that tag exists, so the call is replaced with a call to + * {@link CompiledTagInvocation}, an ordinary static method call. + * + * <p>The tag is still selected by name at runtime, through the same lookup the dynamic path uses, so + * a tag library registered later, one that overrides another, and the order tag libraries are + * registered in all decide the outcome exactly as they did before. Nothing is bound to a particular + * tag library class. + * + * <p>A namespace the index does not know is left alone, which is what keeps a tag library registered + * at runtime working, as is a name that something else in scope already answers to. + * + * @since 8.0.0 + */ +public class CompiledTagCallRewriter extends ClassCodeExpressionTransformer { + + private static final ClassNode INVOCATION_TYPE = ClassHelper.make(CompiledTagInvocation.class); + private static final String LOOKUP_ACCESSOR = "getTagLibraryLookup"; + private static final String OUTPUT_CONTEXT_ACCESSOR = "getOutputContext"; + private static final String INVOKE = "invoke"; + private static final String INVOKE_ARGUMENTS = "invokeArguments"; + private static final String INVOKE_ARGUMENTS_IN_CONTEXT = "invokeArgumentsInContext"; + private static final String GROOVY_PAGE_TYPE = "org.grails.gsp.GroovyPage"; + private static final String COMPILE_STATIC_TYPE = "groovy.transform.CompileStatic"; + private static final String GRAILS_COMPILE_STATIC_TYPE = "grails.compiler.GrailsCompileStatic"; + private static final String MARKUP_TAG_CALL = "invokeTag"; + private static final String DEFAULT_NAMESPACE = "g"; + + /** + * Names the dispatch treats as its own before it ever considers a tag, so an unqualified call to + * one of them is not a tag call however the index reads. + */ + private static final Set<String> RESERVED_NAMES = Set.of("body", "render"); + + private static final String REWRITTEN_MARKER = CompiledTagCallRewriter.class.getName(); + + private final SourceUnit sourceUnit; + private final TagLibraryIndex index; + private final ClassNode classNode; + private final String callerNamespace; + private final boolean page; + private final boolean rewritingPermitted; + private Set<String> localNames = Collections.emptySet(); + private Set<String> pageBindings = Collections.emptySet(); + private int rewritten; + + public CompiledTagCallRewriter(SourceUnit sourceUnit, TagLibraryIndex index, ClassNode classNode) { + this.sourceUnit = sourceUnit; + this.index = index; + this.classNode = classNode; + this.page = isGroovyPage(classNode); + // A page resolves a name against the model it was rendered with before it reaches a tag + // library, and that model is not visible here, so rewriting a page's tag call can only be + // sound where the page has given up dynamic resolution. Declaring compileStatic is that: it + // reserves the namespace names for tag libraries. A page that has not declared it keeps + // resolving its tags exactly as before. + this.rewritingPermitted = !this.page || isCompileStatic(classNode); + // An unqualified call is offered to the caller's own namespace before the default one, which is + // what a tag library declaring a namespace does at runtime. A page and a controller have no + // namespace of their own, so for them the two are the same. + String declared = this.page ? DEFAULT_NAMESPACE : TagLibraryAstDiscovery.resolveNamespace(classNode); + this.callerNamespace = declared != null ? declared : DEFAULT_NAMESPACE; + } + + /** + * @return how many calls were rewritten, for tests to assert against + */ + public int getRewrittenCount() { + return rewritten; + } + + public void rewrite() { + // A tag library is reached both as an artefact and as a class carrying the invoker trait, so + // rewriting can be asked for twice. Rewriting again would be harmless, but reporting a + // misspelled tag twice would not be. + if (classNode.getNodeMetaData(REWRITTEN_MARKER) != null) { + return; + } + classNode.putNodeMetaData(REWRITTEN_MARKER, Boolean.TRUE); + if (page) { + pageBindings = PageBindingCollector.collect(classNode); + } + for (MethodNode method : classNode.getMethods()) { + // getMethods() reaches inherited methods, whose bodies belong to the class that declared + // them. Rewriting one here would change a superclass through a subclass that happens to be + // able to call tags. Trait methods are woven as declarations on this class and so remain. + if (method.getDeclaringClass() != null && !classNode.equals(method.getDeclaringClass())) { + continue; + } + if (method.getCode() != null && !method.isAbstract()) { + rewriteBody(method.getCode(), method.getParameters()); + } + } + for (ConstructorNode constructor : classNode.getDeclaredConstructors()) { + if (constructor.getCode() != null) { + rewriteBody(constructor.getCode(), constructor.getParameters()); + } + } + for (FieldNode field : classNode.getFields()) { + if (field.getDeclaringClass() != null && !classNode.equals(field.getDeclaringClass())) { + continue; + } + Expression initial = field.getInitialExpression(); + if (initial != null) { + localNames = Collections.emptySet(); + field.setInitialValueExpression(transform(initial)); + } + } + for (Statement statement : classNode.getObjectInitializerStatements()) { + rewriteBody(statement, null); + } + } + + private void rewriteBody(Statement code, Parameter[] parameters) { + // An unqualified call reaches a tag only when nothing nearer answers to the name, and a local + // holding a closure answers to it. Which locals are in scope at a given point is not tracked + // here: a name declared anywhere in the body is treated as claimed throughout it, which can + // leave a call dispatched dynamically but never sends one to the wrong place. + localNames = LocalNameCollector.collect(code, parameters); + visitClassCodeContainer(code); + } + + @Override + protected SourceUnit getSourceUnit() { + return sourceUnit; + } + + @Override + public Expression transform(Expression expression) { + if (expression instanceof ClosureExpression closure) { + // ClassCodeExpressionTransformer deliberately does not descend into closures, and documents + // this override as the way to reach them. Without it a tag call written in a tag body, in a + // withFormat block, or in anything else taking a closure is never resolved - which is most + // of the tag calls in a real tag library. + closure.visit(this); + return closure; + } + if (expression instanceof MethodCallExpression call) { + Expression rewrite = rewriteTagCall(call); + if (rewrite != null) { + rewritten++; + return rewrite; + } + validateMarkupTagCall(call); + } + return super.transform(expression); + } + + private Expression rewriteTagCall(MethodCallExpression call) { + if (!(call.getMethod() instanceof ConstantExpression methodName) || + methodName.getValue() == null) { + return null; + } + String tagName = methodName.getValue().toString(); + String namespace = namespaceOf(call.getObjectExpression()); + if (namespace != null) { + // A namespace the build declared as filled in at runtime is left alone entirely: that + // declaration is how an application says its tags are decided while it runs, whether by a + // tag library registered then or by metaprogramming, and binding a call now would settle + // what it asked to keep open. + if (index.isDynamicNamespace(namespace)) { + return null; + } + if (!index.hasNamespace(namespace) || isShadowed(call.getObjectExpression(), namespace) || + pageBindings.contains(namespace)) { + return null; + } + if (!index.isKnown(namespace, tagName)) { + // Only where the name means a tag library for certain. In a page that has not given up + // dynamic resolution the receiver may just as well be the model it was rendered with, + // and reporting there would reject a call this release deliberately still allows. + if (this.rewritingPermitted) { + reportUnknownTag(namespace, tagName, call); + } + return null; + } + } + else { + namespace = unqualifiedNamespaceOf(call, tagName); + if (namespace == null) { + return null; + } + } + if (!this.rewritingPermitted) { + return null; + } + Expression invocation = invocation(namespace, tagName, call.getArguments()); + if (invocation != null) { + // Kept where the tag was written, so a stack trace and any later diagnostic still point at + // the line the author wrote rather than at the start of the file. + setSourcePosition(invocation, call); + } + return invocation; + } + + /** + * The namespace an unqualified call such as {@code message(code: 'x')} resolves in. + * + * <p>Only a name nothing else answers to reaches a tag at all: a real method of the class, an + * inherited one, a field, a property or a local wins, and whether such a member exists is what + * decides the call. Where nothing claims the name, dispatch offers it to the caller's own + * namespace and then to the default one, which is the order reproduced here. + * + * @return the namespace to invoke in, or {@code null} when the call is not resolvably a tag + */ + private String unqualifiedNamespaceOf(MethodCallExpression call, String tagName) { + if (page) { + // A page resolves an unqualified name against its binding before it reaches a tag, and what + // a page's binding holds - the model it was rendered with - is not visible here. A call + // written with its namespace says which tag library it means and is rewritten; one without + // is left to resolve as it did. + return null; + } + if (!call.isImplicitThis() || RESERVED_NAMES.contains(tagName)) { + return null; + } + if (declaresMember(tagName) || localNames.contains(tagName)) { + return null; + } + if (index.isDynamicNamespace(callerNamespace) || index.isDynamicNamespace(DEFAULT_NAMESPACE)) { + // The namespaces an unqualified call could reach were declared as decided at runtime. + return null; + } + if (index.isKnown(callerNamespace, tagName)) { + return callerNamespace; + } + if (index.isKnown(DEFAULT_NAMESPACE, tagName)) { + return DEFAULT_NAMESPACE; + } + // Not a tag this build knows about. It is not reported: an unqualified name in a controller is + // as likely to be a dynamic finder, an injected service method or anything else contributed at + // runtime as it is a misspelled tag. + return null; + } + + /** + * Builds the invocation, passing the attributes and body directly where the source says what they + * are and forwarding the arguments as written where it does not. + */ + private Expression invocation(String namespace, String tagName, Expression arguments) { + if (!(arguments instanceof TupleExpression tuple)) { + return null; + } + ArgumentListExpression invocationArgs = new ArgumentListExpression(); + invocationArgs.addExpression(new MethodCallExpression(VariableExpression.THIS_EXPRESSION, + LOOKUP_ACCESSOR, MethodCallExpression.NO_ARGUMENTS)); + invocationArgs.addExpression(new ConstantExpression(namespace)); + invocationArgs.addExpression(new ConstantExpression(tagName)); + + Expression[] attrsAndBody = attributesAndBody(tuple); + if (attrsAndBody != null) { + invocationArgs.addExpression(attrsAndBody[0]); + invocationArgs.addExpression(attrsAndBody[1]); + if (page) { + invocationArgs.addExpression(outputContext()); + } + return new StaticMethodCallExpression(INVOCATION_TYPE, INVOKE, invocationArgs); + } + + // The shape is only known once the arguments have been evaluated - a map held in a variable, a + // single value the tag reads under its own name, and so on - so they are forwarded as written + // and sorted out by the same rules the dynamic path applies. + if (page) { + invocationArgs.addExpression(outputContext()); + } + for (Expression argument : tuple.getExpressions()) { + invocationArgs.addExpression(transform(argument)); + } + return new StaticMethodCallExpression(INVOCATION_TYPE, + page ? INVOKE_ARGUMENTS_IN_CONTEXT : INVOKE_ARGUMENTS, invocationArgs); + } + + private Expression outputContext() { + return new MethodCallExpression(VariableExpression.THIS_EXPRESSION, OUTPUT_CONTEXT_ACCESSOR, + MethodCallExpression.NO_ARGUMENTS); + } + + /** + * @return the attributes and body to pass, or {@code null} when the source does not say what they + * are and the arguments have to be forwarded instead + */ + private Expression[] attributesAndBody(TupleExpression tuple) { + List<Expression> args = tuple.getExpressions(); + Expression noAttributes = new MapExpression(); + Expression noBody = ConstantExpression.NULL; + switch (args.size()) { + case 0: + return new Expression[] { noAttributes, noBody }; + case 1: + if (args.get(0) instanceof MapExpression) { + return new Expression[] { transform(args.get(0)), noBody }; + } + if (args.get(0) instanceof ClosureExpression) { + return new Expression[] { noAttributes, transform(args.get(0)) }; + } + return null; + case 2: + if (args.get(0) instanceof MapExpression && args.get(1) instanceof ClosureExpression) { + return new Expression[] { transform(args.get(0)), transform(args.get(1)) }; + } + return null; + default: + return null; + } + } + + /** + * Checks a tag written as markup, which a page compiles into a call naming the tag and namespace + * directly. Such a call is already an ordinary method call and needs no rewriting, but the names in + * it are worth the same check as the ones written in an expression. + */ + private void validateMarkupTagCall(MethodCallExpression call) { + if (!page || !MARKUP_TAG_CALL.equals(call.getMethodAsString()) || + !(call.getArguments() instanceof TupleExpression tuple) || + tuple.getExpressions().size() < 2) { + return; + } + if (!(tuple.getExpression(0) instanceof ConstantExpression tagName) || + !(tuple.getExpression(1) instanceof ConstantExpression namespace) || + tagName.getValue() == null || namespace.getValue() == null) { + return; + } + String namespaceName = namespace.getValue().toString(); + String tag = tagName.getValue().toString(); + if (index.isDynamicNamespace(namespaceName)) { + return; + } + if (index.hasNamespace(namespaceName) && !index.isKnown(namespaceName, tag)) { + reportUnknownTag(namespaceName, tag, call); + } + } + + /** + * Reports a tag that no compiled tag library declares. + * + * <p>Silent unless the build declared its tag libraries complete. A namespace holding some + * compiled tag libraries is not the same as one holding all of them: a plugin built before + * descriptors existed contributes tags to {@code g} without one, and a tag library registered + * while an application runs contributes more. Reporting by default would mean warning about calls + * that are perfectly correct - this framework calls one such tag itself - so a build says when it + * knows better. + */ + private void reportUnknownTag(String namespace, String tagName, Expression call) { + if (!index.isStrict() || index.isDynamicNamespace(namespace)) { + return; + } + if (!index.isNamespaceComplete(namespace)) { + // Something contributing to this namespace could not be described. A tag missing from it + // is as likely to be one of those as a misspelling, and reporting it would fail a build + // over code that is correct. + return; + } + String message = "No such tag [" + tagName + "] in namespace [" + namespace + "]. Known tags: " + + String.join(", ", index.getTagNames(namespace)); + // Collected rather than fatal, so that every misspelling in a file is reported at once instead + // of one per build. + GrailsASTUtils.error(sourceUnit, call, message, false); + } + + /** + * @return the namespace a call is made through, or {@code null} when the receiver is not a plain + * name that could be one + */ + private static String namespaceOf(Expression objectExpression) { + if (objectExpression instanceof VariableExpression variable) { + return variable.isThisExpression() || variable.isSuperExpression() ? null : variable.getName(); + } + if (objectExpression instanceof PropertyExpression property && + property.getObjectExpression() instanceof VariableExpression receiver && + receiver.isThisExpression()) { + return property.getPropertyAsString(); + } + return null; + } + + /** + * Whether something in scope has already claimed the name, in which case it is that thing rather + * than a tag library namespace. + * + * <p>A namespace is not declared anywhere: it is reached because nothing else answers to the name. + * A local variable, a parameter or a field called {@code g} does answer to it, and rewriting such a + * call would silently send it to a tag library instead of the object the author wrote. + */ + private boolean isShadowed(Expression objectExpression, String namespace) { + if (objectExpression instanceof VariableExpression variable) { + Variable accessed = variable.getAccessedVariable(); + // A name that resolves to something - a local, a parameter, a field, a property - is that + // thing. Only a name nothing has claimed is left to mean a namespace. + if (accessed != null && !(accessed instanceof DynamicVariable)) { + return true; + } + } + // Reached as this.g, or as a bare name resolved dynamically: a field or property of that name + // anywhere in the hierarchy is the member, not a namespace. + return declaresProperty(namespace); + } + + /** + * Whether the class, or anything it inherits from, reads a property of this name. A method of the + * same name does not count: {@code g.link()} reads {@code g} as a property whatever methods exist. + */ + private boolean declaresProperty(String name) { + return classNode.getField(name) != null || + classNode.getProperty(name) != null || + hasGetter(name); + } + + /** + * Whether the class, or anything it inherits from, already answers to a name at all. An + * unqualified call reaches a tag only when nothing else does, so here a method counts too. + */ + private boolean declaresMember(String name) { Review Comment: Fixed. `RESERVED_NAMES` is now seeded from `GroovySystem.getMetaClassRegistry().getMetaClass(Object)`, so every name the metaclass answers to for an arbitrary receiver — DGM plus any extension module on the compiling classpath — is left alone. Only unqualified calls are affected; a call naming its namespace is unchanged. Not hypothetical, as you say: grails-fields already declares `f:with`. `GroovyMethodNameCollisionSpec` covers it, and I checked it bites — with the old two-name set both `with` and `each` fail, with the fix both pass, and the guard cases (namespaced call, non-colliding tag) pass either way. ########## grails-gsp/grails-web-taglib/src/main/groovy/grails/gsp/taglib/compiler/CompiledTagCallRewriter.java: ########## @@ -0,0 +1,519 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package grails.gsp.taglib.compiler; + +import java.util.Collections; +import java.util.List; +import java.util.Set; + +import org.codehaus.groovy.ast.AnnotationNode; +import org.codehaus.groovy.ast.ClassCodeExpressionTransformer; +import org.codehaus.groovy.ast.ClassHelper; +import org.codehaus.groovy.ast.ClassNode; +import org.codehaus.groovy.ast.ConstructorNode; +import org.codehaus.groovy.ast.DynamicVariable; +import org.codehaus.groovy.ast.FieldNode; +import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.Parameter; +import org.codehaus.groovy.ast.Variable; +import org.codehaus.groovy.ast.expr.ArgumentListExpression; +import org.codehaus.groovy.ast.expr.ClosureExpression; +import org.codehaus.groovy.ast.expr.ConstantExpression; +import org.codehaus.groovy.ast.expr.Expression; +import org.codehaus.groovy.ast.expr.MapExpression; +import org.codehaus.groovy.ast.expr.MethodCallExpression; +import org.codehaus.groovy.ast.expr.PropertyExpression; +import org.codehaus.groovy.ast.expr.StaticMethodCallExpression; +import org.codehaus.groovy.ast.expr.TupleExpression; +import org.codehaus.groovy.ast.expr.VariableExpression; +import org.codehaus.groovy.ast.stmt.Statement; +import org.codehaus.groovy.control.SourceUnit; + +import org.grails.compiler.injection.GrailsASTUtils; +import org.grails.taglib.CompiledTagInvocation; +import org.grails.taglib.discovery.TagLibraryAstDiscovery; +import org.grails.taglib.index.TagLibraryIndex; + +/** + * Rewrites a call to a known tag into a direct invocation. + * + * <p>Writing {@code g.message(code: 'x')} reaches the tag library through {@code propertyMissing} to + * find the namespace and {@code invokeMethod} to find the tag, which is a dynamic call site even in a + * statically compiled class. Both the namespace and the tag name are fixed in the source, and the tag + * library index says whether that tag exists, so the call is replaced with a call to + * {@link CompiledTagInvocation}, an ordinary static method call. + * + * <p>The tag is still selected by name at runtime, through the same lookup the dynamic path uses, so + * a tag library registered later, one that overrides another, and the order tag libraries are + * registered in all decide the outcome exactly as they did before. Nothing is bound to a particular + * tag library class. + * + * <p>A namespace the index does not know is left alone, which is what keeps a tag library registered + * at runtime working, as is a name that something else in scope already answers to. + * + * @since 8.0.0 + */ +public class CompiledTagCallRewriter extends ClassCodeExpressionTransformer { + + private static final ClassNode INVOCATION_TYPE = ClassHelper.make(CompiledTagInvocation.class); + private static final String LOOKUP_ACCESSOR = "getTagLibraryLookup"; + private static final String OUTPUT_CONTEXT_ACCESSOR = "getOutputContext"; + private static final String INVOKE = "invoke"; + private static final String INVOKE_ARGUMENTS = "invokeArguments"; + private static final String INVOKE_ARGUMENTS_IN_CONTEXT = "invokeArgumentsInContext"; + private static final String GROOVY_PAGE_TYPE = "org.grails.gsp.GroovyPage"; + private static final String COMPILE_STATIC_TYPE = "groovy.transform.CompileStatic"; + private static final String GRAILS_COMPILE_STATIC_TYPE = "grails.compiler.GrailsCompileStatic"; + private static final String MARKUP_TAG_CALL = "invokeTag"; + private static final String DEFAULT_NAMESPACE = "g"; + + /** + * Names the dispatch treats as its own before it ever considers a tag, so an unqualified call to + * one of them is not a tag call however the index reads. + */ + private static final Set<String> RESERVED_NAMES = Set.of("body", "render"); + + private static final String REWRITTEN_MARKER = CompiledTagCallRewriter.class.getName(); + + private final SourceUnit sourceUnit; + private final TagLibraryIndex index; + private final ClassNode classNode; + private final String callerNamespace; + private final boolean page; + private final boolean rewritingPermitted; + private Set<String> localNames = Collections.emptySet(); + private Set<String> pageBindings = Collections.emptySet(); + private int rewritten; + + public CompiledTagCallRewriter(SourceUnit sourceUnit, TagLibraryIndex index, ClassNode classNode) { + this.sourceUnit = sourceUnit; + this.index = index; + this.classNode = classNode; + this.page = isGroovyPage(classNode); + // A page resolves a name against the model it was rendered with before it reaches a tag + // library, and that model is not visible here, so rewriting a page's tag call can only be + // sound where the page has given up dynamic resolution. Declaring compileStatic is that: it + // reserves the namespace names for tag libraries. A page that has not declared it keeps + // resolving its tags exactly as before. + this.rewritingPermitted = !this.page || isCompileStatic(classNode); + // An unqualified call is offered to the caller's own namespace before the default one, which is + // what a tag library declaring a namespace does at runtime. A page and a controller have no + // namespace of their own, so for them the two are the same. + String declared = this.page ? DEFAULT_NAMESPACE : TagLibraryAstDiscovery.resolveNamespace(classNode); + this.callerNamespace = declared != null ? declared : DEFAULT_NAMESPACE; + } + + /** + * @return how many calls were rewritten, for tests to assert against + */ + public int getRewrittenCount() { + return rewritten; + } + + public void rewrite() { + // A tag library is reached both as an artefact and as a class carrying the invoker trait, so + // rewriting can be asked for twice. Rewriting again would be harmless, but reporting a + // misspelled tag twice would not be. + if (classNode.getNodeMetaData(REWRITTEN_MARKER) != null) { + return; + } + classNode.putNodeMetaData(REWRITTEN_MARKER, Boolean.TRUE); + if (page) { + pageBindings = PageBindingCollector.collect(classNode); + } + for (MethodNode method : classNode.getMethods()) { + // getMethods() reaches inherited methods, whose bodies belong to the class that declared + // them. Rewriting one here would change a superclass through a subclass that happens to be + // able to call tags. Trait methods are woven as declarations on this class and so remain. + if (method.getDeclaringClass() != null && !classNode.equals(method.getDeclaringClass())) { + continue; + } + if (method.getCode() != null && !method.isAbstract()) { + rewriteBody(method.getCode(), method.getParameters()); + } + } + for (ConstructorNode constructor : classNode.getDeclaredConstructors()) { + if (constructor.getCode() != null) { + rewriteBody(constructor.getCode(), constructor.getParameters()); + } + } + for (FieldNode field : classNode.getFields()) { + if (field.getDeclaringClass() != null && !classNode.equals(field.getDeclaringClass())) { + continue; + } + Expression initial = field.getInitialExpression(); + if (initial != null) { + localNames = Collections.emptySet(); + field.setInitialValueExpression(transform(initial)); + } + } + for (Statement statement : classNode.getObjectInitializerStatements()) { + rewriteBody(statement, null); + } + } + + private void rewriteBody(Statement code, Parameter[] parameters) { + // An unqualified call reaches a tag only when nothing nearer answers to the name, and a local + // holding a closure answers to it. Which locals are in scope at a given point is not tracked + // here: a name declared anywhere in the body is treated as claimed throughout it, which can + // leave a call dispatched dynamically but never sends one to the wrong place. + localNames = LocalNameCollector.collect(code, parameters); + visitClassCodeContainer(code); + } + + @Override + protected SourceUnit getSourceUnit() { + return sourceUnit; + } + + @Override + public Expression transform(Expression expression) { + if (expression instanceof ClosureExpression closure) { + // ClassCodeExpressionTransformer deliberately does not descend into closures, and documents + // this override as the way to reach them. Without it a tag call written in a tag body, in a + // withFormat block, or in anything else taking a closure is never resolved - which is most + // of the tag calls in a real tag library. + closure.visit(this); + return closure; + } + if (expression instanceof MethodCallExpression call) { + Expression rewrite = rewriteTagCall(call); + if (rewrite != null) { + rewritten++; + return rewrite; + } + validateMarkupTagCall(call); + } + return super.transform(expression); + } + + private Expression rewriteTagCall(MethodCallExpression call) { + if (!(call.getMethod() instanceof ConstantExpression methodName) || + methodName.getValue() == null) { + return null; + } + String tagName = methodName.getValue().toString(); + String namespace = namespaceOf(call.getObjectExpression()); + if (namespace != null) { + // A namespace the build declared as filled in at runtime is left alone entirely: that + // declaration is how an application says its tags are decided while it runs, whether by a + // tag library registered then or by metaprogramming, and binding a call now would settle + // what it asked to keep open. + if (index.isDynamicNamespace(namespace)) { + return null; + } + if (!index.hasNamespace(namespace) || isShadowed(call.getObjectExpression(), namespace) || + pageBindings.contains(namespace)) { + return null; + } + if (!index.isKnown(namespace, tagName)) { + // Only where the name means a tag library for certain. In a page that has not given up + // dynamic resolution the receiver may just as well be the model it was rendered with, + // and reporting there would reject a call this release deliberately still allows. + if (this.rewritingPermitted) { + reportUnknownTag(namespace, tagName, call); + } + return null; + } + } + else { + namespace = unqualifiedNamespaceOf(call, tagName); + if (namespace == null) { + return null; + } + } + if (!this.rewritingPermitted) { + return null; + } + Expression invocation = invocation(namespace, tagName, call.getArguments()); + if (invocation != null) { + // Kept where the tag was written, so a stack trace and any later diagnostic still point at + // the line the author wrote rather than at the start of the file. + setSourcePosition(invocation, call); + } + return invocation; + } + + /** + * The namespace an unqualified call such as {@code message(code: 'x')} resolves in. + * + * <p>Only a name nothing else answers to reaches a tag at all: a real method of the class, an + * inherited one, a field, a property or a local wins, and whether such a member exists is what + * decides the call. Where nothing claims the name, dispatch offers it to the caller's own + * namespace and then to the default one, which is the order reproduced here. + * + * @return the namespace to invoke in, or {@code null} when the call is not resolvably a tag + */ + private String unqualifiedNamespaceOf(MethodCallExpression call, String tagName) { + if (page) { + // A page resolves an unqualified name against its binding before it reaches a tag, and what + // a page's binding holds - the model it was rendered with - is not visible here. A call + // written with its namespace says which tag library it means and is rewritten; one without + // is left to resolve as it did. + return null; + } + if (!call.isImplicitThis() || RESERVED_NAMES.contains(tagName)) { + return null; + } + if (declaresMember(tagName) || localNames.contains(tagName)) { + return null; + } + if (index.isDynamicNamespace(callerNamespace) || index.isDynamicNamespace(DEFAULT_NAMESPACE)) { + // The namespaces an unqualified call could reach were declared as decided at runtime. + return null; + } + if (index.isKnown(callerNamespace, tagName)) { + return callerNamespace; + } + if (index.isKnown(DEFAULT_NAMESPACE, tagName)) { + return DEFAULT_NAMESPACE; + } + // Not a tag this build knows about. It is not reported: an unqualified name in a controller is + // as likely to be a dynamic finder, an injected service method or anything else contributed at + // runtime as it is a misspelled tag. + return null; + } + + /** + * Builds the invocation, passing the attributes and body directly where the source says what they + * are and forwarding the arguments as written where it does not. + */ + private Expression invocation(String namespace, String tagName, Expression arguments) { + if (!(arguments instanceof TupleExpression tuple)) { + return null; + } + ArgumentListExpression invocationArgs = new ArgumentListExpression(); + invocationArgs.addExpression(new MethodCallExpression(VariableExpression.THIS_EXPRESSION, Review Comment: Fixed — `new VariableExpression("this")` and `new ConstantExpression(null)` per call site, matching `TagLibraryTransformer`. ########## grails-gsp/plugin/src/ast/groovy/grails/compiler/traits/CompiledTagCallTransformation.groovy: ########## @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package grails.compiler.traits + +import groovy.transform.CompileStatic +import org.codehaus.groovy.ast.ASTNode +import org.codehaus.groovy.ast.ClassHelper +import org.codehaus.groovy.ast.ClassNode +import org.codehaus.groovy.ast.ModuleNode +import org.codehaus.groovy.control.CompilePhase +import org.codehaus.groovy.control.SourceUnit +import org.codehaus.groovy.transform.ASTTransformation +import org.codehaus.groovy.transform.GroovyASTTransformation + +import grails.artefact.gsp.TagLibraryInvoker +import grails.gsp.taglib.compiler.CompiledTagCallRewriter +import org.grails.taglib.index.TagLibraryIndex + +/** + * Compiles a call to a known tag into a direct invocation, wherever tags can be called from. + * + * <p>A tag library rewrites its own calls as it is compiled, but a controller can call tags too, and + * gains that ability from the {@link TagLibraryInvoker} trait rather than from being a tag library. + * Any class carrying that trait is therefore a candidate, which covers controllers without naming + * them and without a second copy of the rewriting rules. A compiled GSP calls tags as well, and + * reaches them through {@code GroovyPage} rather than through the trait, so it is matched separately. + * + * <p>Runs after trait injection, since whether a class can call tags is only settled once its traits + * have been applied. + * + * @since 8.0.0 + */ +@CompileStatic +@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION) +class CompiledTagCallTransformation implements ASTTransformation { Review Comment: Half fixed, half documented. `TransformWithPriority` is implemented with a `COMPILED_TAG_CALL_ORDER` slot in `GroovyTransformOrder`, and `CompiledTagCallTransformationOrderSpec` pins the relationship to artefact trait injection rather than the number. The `@Artefact('Controller')` gap I documented rather than fixed. Hooking into `ArtefactTypeAstTransformation` would mean grails-core referencing the GSP rewriter, and rewriting after type checking risks `@CompileStatic`. It degrades to dynamic dispatch, so behaviour is correct — it just misses the faster path. `ControllerTagCallRewriteSpec` pins the annotated case so the difference is explicit, and the guide says so. ########## grails-gsp/grails-web-taglib/src/main/groovy/grails/gsp/taglib/compiler/TagLibArtefactTypeAstTransformation.java: ########## @@ -45,9 +52,79 @@ public class TagLibArtefactTypeAstTransformation extends ArtefactTypeAstTransfor @Override protected String resolveArtefactType(SourceUnit sourceUnit, AnnotationNode annotationNode, ClassNode classNode) { addClosureTagDeprecationWarnings(sourceUnit, classNode); + writeIndexEntry(sourceUnit, classNode); + rewriteResolvedTagCalls(sourceUnit, classNode); return "TagLibrary"; } + /** + * Records the namespace and tag names this tag library declares, so that a GSP compiled later can + * resolve a tag call without loading the tag library or consulting its metaclass. + * + * <p>Failure to write is never fatal: the index is an optimisation, and a missing descriptor + * degrades to the runtime resolution that applies when a tag library is registered dynamically. + */ + protected void writeIndexEntry(SourceUnit sourceUnit, ClassNode classNode) { Review Comment: Confirmed, and reworded rather than fixed. The guide now says the fallback reaches `@TagLib`-annotated tag libraries only, that a convention-declared one contributes no descriptor without the Gradle plugin, and that this degrades to dynamic dispatch rather than breaking. Also noted that a self-written descriptor is never removed, which you raised separately. ########## grails-doc/src/en/guide/theWebLayer/gsp/taglibs/compiledTags.adoc: ########## @@ -0,0 +1,237 @@ +//// +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + +https://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +//// + +Tag libraries are described when they are compiled, and that description is used to resolve tag calls +in pages, tag libraries and controllers compiled afterwards. + +==== Defining tags as methods + +Define a tag as a method: + +[source,groovy] +---- +class GreetingTagLib { + + static namespace = 'greet' + + def hello(Map attrs) { + out << "Hello ${attrs.name}" + } + + def wrapped(Map attrs, Closure body) { + out << '<div>' << body() << '</div>' + } +} +---- + +The attributes parameter must be a `Map` named `attrs`, and the body parameter a `Closure` named +`body`. A method taking anything else is an ordinary method of the tag library rather than a tag. +Where that convention does not suit, `@Tag` marks a method as a tag whatever its signature and +`@NotATag` excludes one that would otherwise match. + +The older form, a `Closure` field, still works: + +[source,groovy] +---- +// Deprecated +Closure hello = { Map attrs -> + out << "Hello ${attrs.name}" +} +---- + +A tag declared this way is described and called like any other — the tag is selected by name when the +call runs, and a closure answers to a name as readily as a method does. The form remains deprecated +because a closure carries no signature, so nothing about the call can be checked. Compiling a tag +library that declares one produces a warning naming the tag. + +==== Calling tags + +In a tag library or a controller, a call to a tag whose namespace and name are known is compiled into +a direct invocation rather than being dispatched through the metaclass: + +[source,groovy] +---- +class BookController { + def index() { + String markup = g.createLink(controller: 'book') // compiled into a direct invocation + String other = greet.hello(name: 'Grails') // likewise + } +} +---- + +The attributes and body are passed straight through where the call says what they are. Where it does +not — attributes assembled at runtime, or a single value the tag reads under its own name — the +arguments are forwarded as written and sorted out by the same rules dynamic dispatch applies: + +[source,groovy] +---- +Map attrs = buildAttributes() +g.createLink(attrs) // still compiled into an invocation +---- + +The tag is always selected by name when the call runs, through the same lookup dynamic dispatch uses. +A tag library that overrides another, one registered while the application is running, and the order +tag libraries are registered in all decide the outcome exactly as they did before. Nothing is bound to +a particular tag library class, so a tag declared by more than one of them is compiled the same way. + +A call into a namespace no compiled tag library declares is left alone, which is what allows a tag +library registered while an application is running to keep working. + +A name that something else in scope already answers to is not a namespace. A local variable, a +parameter or a property called `g` is that thing, and a call on it is left alone: + +[source,groovy] +---- +def index() { + def g = someClient + g.createLink(controller: 'book') // someClient.createLink, not the tag +} +---- + +A call written without a namespace follows the same rule. It reaches a tag only when nothing nearer +answers to the name — not a method of the class, not one it inherits, not a field, property or local — +and is then offered to the calling tag library's own namespace before the default one, which is the +order dispatch uses at runtime: + +[source,groovy] +---- +class BookController { + def index() { + String markup = createLink(controller: 'book') // compiled into an invocation + } +} +---- + +Tags called from within a closure — a tag body, a `withFormat` block, anything taking a block — are +compiled the same way as tags called directly. + +==== Tags in pages + +A page resolves a name against the model it was rendered with before it reaches a tag library, and +that model is not known when the page is compiled. A page therefore keeps resolving its tags as it +always has, unless it declares `compileStatic`: + +[source,html] +---- +<%@ page compileStatic="true" %> +${g.createLink(controller: 'book')} <%-- compiled into a direct invocation --%> +---- + +Declaring `compileStatic` on a page reserves the namespace names for tag libraries: a model attribute +called `g` no longer shadows the `g` namespace there. Without it, an expression is dispatched exactly +as before. Set `grails.views.gsp.compileStatic` in configuration to apply it to every page. + +Two things hold in a page either way. A call written without a namespace, as +`${createLink(controller: 'book')}` is, is always left to resolve against the binding. And a name the +page puts into its own binding is that variable rather than a namespace: + +[source,html] +---- +<g:set var="g" value="${someObject}"/> +${g.createLink(controller: 'book')} <%-- someObject.createLink, not the tag --%> +---- + +A tag written as markup, as `<g:createLink controller="book"/>` is, already compiles into a direct +call naming the tag and needs no rewriting. It is unambiguously a tag whatever the page does, so under +strict checking it is checked in every page. An expression is checked only where it is resolved, in a +page declaring `compileStatic`, since elsewhere the receiver may be part of the model. + +==== Reporting unknown tags + +By default nothing is reported: a tag that no compiled tag library declares is left to resolve at +runtime, exactly as it did before. A namespace can hold tag libraries that were not compiled with a +description — a plugin built against an earlier version of Grails contributes tags to `g` without one, +and a tag library registered at runtime contributes more — so a tag missing from the description is +not necessarily a misspelling, and reporting one by default would mean complaining about correct code. + +An application whose tag libraries are all described can ask for an error instead: + +[source,groovy] +.build.gradle +---- +grails { + compileStatic { + strictTags = true + dynamicTagNamespaces = ['legacy'] // <1> + } +} +---- +<1> namespaces genuinely filled in while the application runs + +Strict checking applies where the source says a call is a tag: one naming its namespace, as +`g.message(code: 'x')` does, and one written as markup, as `<g:message/>` is. A call written without a +namespace is never checked — such a name may equally be a method contributed while the application +runs, and in a page it may come from the model. A namespaced expression in a page is checked only when +that page declares `compileStatic`, for the same reason its calls are only rewritten there. + +`dynamicTagNamespaces` names the namespaces whose tags are decided while the application runs rather +than described when it is compiled. It turns compile-time resolution off for them completely: a call +into such a namespace is never rewritten, never reported, and is dispatched exactly as it was before +this release, whether or not a compiled tag library also declares the namespace. Declare a namespace +here when a tag library is registered at runtime, or when the tags in it are contributed by +metaprogramming. + +Both settings are read from the build, not from a system property, so changing either recompiles what +depends on it. + +==== Where the description lives + +Each tag library contributes one file under `META-INF/grails/taglibs` in the artifact it is packaged +in. Descriptions from every jar on the classpath are combined, so a plugin's tag libraries are +resolvable by an application that depends on it without any extra build configuration. + +Under the Grails Gradle plugin the description is written twice, because the two things that read it +need different guarantees. + +`generateTagLibraryIndex` runs before compilation and reads the sources under `grails-app/taglib`, so +tags an application declares are resolvable in the same compilation that defines them. Being read from +source it cannot describe everything: a tag library referring to a type written in Java, or generated +by the build, is left out. What it missed is recorded, and nothing in a namespace it could not fully +describe is ever reported as an unknown tag, so strict checking cannot fail a build over a tag that +does exist. This index is used only to compile this project, and is never packaged. + +`packageTagLibraryIndex` runs after compilation, with the project's own classes on its classpath, +where every tag library resolves whatever language its collaborators were written in. That index is +the authoritative one: pages are compiled against it, it travels with the artifact, and a project +depending on this one reads it. Every run replaces the directory, so a tag library that is renamed or +deleted disappears from it. + +A project keeping tag libraries elsewhere adds those directories once, by task type, so that both +indexes describe the same set — configuring them separately would let the index this project compiles +against differ from the one it publishes: + +[source,groovy] +.build.gradle +---- +import org.grails.gradle.plugin.views.gsp.GenerateTagLibraryIndexTask + +tasks.withType(GenerateTagLibraryIndexTask).configureEach { + sourceDirectories.from(file('src/main/groovy')) +} +---- + +Before compilation, a tag library referring to something the same project declares — a service it +injects, a base class it extends, a trait it carries — is described too, by reading that Groovy source +alongside it, so an inherited namespace, a tag a trait contributes and an attributes parameter of a +project-declared type are all read rather than guessed. A tag library naming a type that does not +exist is left out, as it would be by the compiler. + +Where no build writes the index — a plain Groovy compilation, or one that does not apply the Grails +Gradle plugin — each tag library describes itself as it is compiled instead, which makes it resolvable Review Comment: Reworded — it now says what it actually does, including that the fallback misses convention-declared tag libraries and that the result is dynamic dispatch rather than a failure. -- 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]
