codeconsole commented on code in PR #16134: URL: https://github.com/apache/grails-core/pull/16134#discussion_r3816757883
########## grails-gsp/grails-taglib/src/main/groovy/org/grails/taglib/discovery/TagLibraryAstDiscovery.java: ########## @@ -0,0 +1,108 @@ +/* + * 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 org.grails.taglib.discovery; + +import java.util.LinkedHashMap; +import java.util.Map; + +import groovy.lang.Closure; +import org.codehaus.groovy.ast.ClassHelper; +import org.codehaus.groovy.ast.ClassNode; +import org.codehaus.groovy.ast.FieldNode; +import org.codehaus.groovy.ast.MethodNode; +import org.codehaus.groovy.ast.expr.ConstantExpression; +import org.codehaus.groovy.ast.expr.Expression; + +import org.grails.taglib.index.TagLibraryIndexEntry; + +/** + * Reads a tag library's namespace and tag names from its syntax tree. + * + * <p>Classification is delegated to {@link TagDiscoveryRules}, the same rules an application applies + * when it registers tag libraries, so the two cannot disagree about what a tag is. What remains here + * is reading the namespace and gathering the candidate members from the tree. + * + * @since 8.0.0 + */ +public final class TagLibraryAstDiscovery { + + public static final String DEFAULT_NAMESPACE = "g"; + + private static final String NAMESPACE_FIELD = "namespace"; + + private static final ClassNode CLOSURE_TYPE = ClassHelper.make(Closure.class); + + private TagLibraryAstDiscovery() { + } + + /** + * Resolves the namespace the way {@code DefaultGrailsTagLibClass} does at runtime, which reads the + * static {@code namespace} property through the class hierarchy. + * + * @param classNode the tag library + * @return the namespace, or {@code null} when it cannot be determined without running the code, in + * which case no descriptor should be written and the tag library resolves dynamically + */ + public static String resolveNamespace(ClassNode classNode) { + for (ClassNode current = classNode; current != null && !ClassHelper.isObjectType(current); + current = current.getSuperClass()) { + FieldNode namespaceField = current.getDeclaredField(NAMESPACE_FIELD); + if (namespaceField == null || !namespaceField.isStatic()) { + continue; + } + Expression initial = namespaceField.getInitialExpression(); + if (initial instanceof ConstantExpression constant && constant.getValue() != null) { + String value = constant.getValue().toString().trim(); + return value.isEmpty() ? DEFAULT_NAMESPACE : value; + } + // Declared, but its value is only known once the initialiser runs - a reference to a shared + // constant, a concatenation, and so on. Guessing "g" here would file the tags under the + // wrong namespace, so the tag library is left out of the index entirely. + return null; + } + return DEFAULT_NAMESPACE; + } + + /** + * @param classNode the tag library + * @param parameterNamesRetained whether this compilation writes parameter names into the class file + * @return each tag mapped to how it is implemented, so that a caller can tell a tag it can bind to + * from one it must dispatch dynamically + */ + public static Map<String, TagLibraryIndexEntry.Kind> findTags(ClassNode classNode, + boolean parameterNamesRetained) { + Map<String, TagLibraryIndexEntry.Kind> tags = new LinkedHashMap<>(); + for (MethodNode method : classNode.getMethods()) { + if (method.getDeclaringClass() != null && !classNode.equals(method.getDeclaringClass())) { + continue; + } + if (TagDiscoveryRules.isTagMethod(new AstTagMethodView(method, parameterNamesRetained))) { + tags.put(method.getName(), TagLibraryIndexEntry.Kind.METHOD); + } + } + for (FieldNode field : classNode.getFields()) { Review Comment: Fixed. Enumeration is now shared the way classification already was: a `TagLibraryView` over a syntax tree or a compiled class, and one walk in `TagDiscoveryRules` that both `TagLibraryAstDiscovery` and `DefaultGrailsTagLibClass` route through. Method tags are read from the declaring class, closure tags up the hierarchy, matching dispatch. `TagSetAgreementSpec` asserts the two views produce the same set; the three inherited-closure rows fail without the walk. Two smaller divergences went with it: the AST used `equals` on `Closure` where the runtime uses `isAssignableFrom`, and closure-vs-method precedence is now decided once rather than falling out of loop order. ########## grails-gsp/grails-taglib/src/main/groovy/org/grails/taglib/index/TagLibraryIndexGenerator.java: ########## @@ -0,0 +1,390 @@ +/* + * 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 org.grails.taglib.index; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Set; +import java.util.TreeSet; +import java.util.stream.Stream; + +import org.codehaus.groovy.ast.AnnotationNode; +import org.codehaus.groovy.ast.ClassNode; +import org.codehaus.groovy.ast.FieldNode; +import org.codehaus.groovy.ast.expr.ConstantExpression; +import org.codehaus.groovy.control.ClassNodeResolver; +import org.codehaus.groovy.control.CompilationUnit; +import org.codehaus.groovy.control.CompilerConfiguration; +import org.codehaus.groovy.control.Phases; + +import org.grails.taglib.discovery.TagLibraryAstDiscovery; + +/** + * Writes the tag library index for a source set. + * + * <p>Sources are parsed to the point where the syntax tree is complete and no further, so a tag + * library is never loaded or executed to find out what it declares. Reading the tree rather than the + * text means the answer follows Groovy's own understanding of the source. + * + * <p>The index is rewritten in full each time rather than added to. A tag library that has been + * renamed or deleted therefore disappears from it, where an index accumulated as each class compiled + * would keep describing tags that no longer exist until the build directory was cleaned. + * + * <p>Invoked in a forked process by the build, with the source set's own compile classpath, because + * the rules it applies belong to the framework rather than to the build tooling. + * + * @since 8.0.0 + */ +public final class TagLibraryIndexGenerator { + + private static final String TAG_LIB_ANNOTATION = "grails.gsp.TagLib"; + private static final String ARTEFACT_ANNOTATION = "grails.artefact.Artefact"; + private static final String TAG_LIB_ARTEFACT = "TagLib"; + + private static final String NAMESPACE_FIELD = "namespace"; + + private TagLibraryIndexGenerator() { + } + + /** + * @param args the output directory, whether parameter names are retained, the source encoding, how + * many source directories follow, those directories, and then the source roots a type this + * project declares may be resolved from + */ + public static void main(String[] args) throws IOException { + if (args.length < 4) { + throw new IllegalArgumentException("Usage: <outputDir> <parameterNamesRetained> " + + "<sourceEncoding> <sourceDirCount> <sourceDir>... <resolutionRoot>..."); + } + File outputDir = new File(args[0]); + boolean parameterNamesRetained = Boolean.parseBoolean(args[1]); + String encoding = args[2].isEmpty() ? "UTF-8" : args[2]; + int sourceDirCount = Integer.parseInt(args[3]); + List<File> sourceDirs = new ArrayList<>(sourceDirCount); + List<File> resolutionRoots = new ArrayList<>(); + for (int i = 4; i < args.length; i++) { + (i - 4 < sourceDirCount ? sourceDirs : resolutionRoots).add(new File(args[i])); + } + generate(sourceDirs, resolutionRoots, outputDir, parameterNamesRetained, encoding); + } + + /** + * Regenerates the index describing every tag library under a source directory. + * + * @param sourceDir the directory to scan for tag libraries + * @param outputDir the directory the index is written beneath + * @param parameterNamesRetained whether the compilation writes parameter names into class files + * @param encoding the source encoding + * @throws IOException if the index cannot be written + */ + public static void generate(File sourceDir, File outputDir, boolean parameterNamesRetained, + String encoding) throws IOException { + generate(Collections.singletonList(sourceDir), Collections.emptyList(), outputDir, + parameterNamesRetained, encoding); + } + + /** + * Regenerates the index, resolving a type this project declares from its source. + * + * @param sourceDirs the directories to scan for tag libraries + * @param outputDir the directory the index is written beneath + * @param parameterNamesRetained whether the compilation writes parameter names into class files + * @param encoding the source encoding + * @throws IOException if the index cannot be written + */ + public static void generate(List<File> sourceDirs, File outputDir, boolean parameterNamesRetained, + String encoding) throws IOException { + generate(sourceDirs, Collections.emptyList(), outputDir, parameterNamesRetained, encoding); + } + + /** + * Regenerates the index describing every tag library under any of several source directories. + * + * <p>All of them are described in one pass. Describing them one at a time would mean either + * erasing the previous directory's descriptors or leaving behind descriptors for tag libraries + * that have since been renamed or deleted. + * + * @param sourceDirs the directories to scan for tag libraries + * @param resolutionRoots the source roots a type this project declares may be resolved from, so + * that a base class, trait or parameter type it supplies is read rather than guessed + * @param outputDir the directory the index is written beneath + * @param parameterNamesRetained whether the compilation writes parameter names into class files + * @param encoding the source encoding + * @throws IOException if the index cannot be written + */ + public static void generate(List<File> sourceDirs, List<File> resolutionRoots, File outputDir, + boolean parameterNamesRetained, String encoding) throws IOException { + TagLibraryIndexWriter.clear(outputDir); + List<File> sources = new ArrayList<>(); + if (sourceDirs != null) { + for (File sourceDir : sourceDirs) { + if (sourceDir != null && sourceDir.isDirectory()) { + sources.addAll(findGroovySources(sourceDir)); + } + } + } + if (sources.isEmpty()) { + return; + } + + List<File> roots = resolutionRoots != null ? resolutionRoots : Collections.emptyList(); + List<File> skipped = new ArrayList<>(); + for (ClassNode classNode : parse(sources, roots, parameterNamesRetained, encoding, skipped)) { Review Comment: Fixed. Descriptors are written only for class nodes whose source file was in `sourceDirectories`; a collaborator the resolver added to read a type is no longer described. `TagLibraryIndexGeneratorSpec` covers a `*TagLib`-named helper under a resolution root, and fails without the filter. ########## grails-gsp/grails-web-taglib/src/main/groovy/grails/artefact/gsp/TagLibraryInvoker.groovy: ########## @@ -93,11 +91,14 @@ trait TagLibraryInvoker extends WebAttributes { } if (tagLibrary) { - if (!developmentMode) { - MetaClass thisMc = GrailsMetaClassUtils.getMetaClass(this) - TagLibraryMetaUtils.registerMethodMissingForTags(thisMc, lookup, usedNamespace, methodName) - } - return tagLibrary.invokeMethod(methodName, args) + // Resolving the tag used to install it onto this object's metaclass so that later + // calls bypassed methodMissing. That made every caller mutate its own + // ExpandoMetaClass the first time it used a tag, and made every later call pay the + // read lock guarding an initialised metaclass. The tag is dispatched through the + // lookup each time instead, which is a map read. + return TagLibraryMetaUtils.methodMissingForTagLib( Review Comment: Fixed. `methodMissingForTagLib` takes the tag branch only when the argument list is a shape a tag can be called with — none, one, or two whose first is a `Map`. Anything else falls through to the method lookup, which finds the overload. `TagLibraryInvokerDispatchSpec` covers `format(Map)` beside `format(String, String)` called with two Strings; it runs the tag with no attributes without the fix. ########## grails-gsp/grails-web-taglib/src/main/groovy/grails/gsp/taglib/compiler/CompiledTagCallRewriter.java: ########## @@ -0,0 +1,570 @@ +/* + * 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.HashSet; +import java.util.List; +import java.util.Set; + +import groovy.lang.GroovySystem; +import groovy.lang.MetaMethod; +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 an unqualified call never reaches a tag through, however the index reads. + * + * <p>Two kinds. {@code body} and {@code render} the dispatch treats as its own before it ever + * considers a tag. The rest is every name the metaclass answers to for an arbitrary receiver — + * {@code DefaultGroovyMethods} and any extension module on the compiler's classpath. Those are + * real methods on every object: an unqualified {@code each { }} or {@code with { }} reached one + * directly and never went near {@code methodMissing}, so a tag library declaring a tag of the same + * name must not capture the call. Nothing here restricts a call that names its namespace, where + * the source has said which tag library it means. + */ + private static final Set<String> RESERVED_NAMES = reservedNames(); + + 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(); + /** + * How many closures enclose the expression being transformed. An unqualified call inside + * one may belong to the closure's delegate, which is only known when it runs. + */ + private int closureDepth; + 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. + // + // A call written with its namespace still says which tag library it means, so it is + // resolved here as anywhere else. One written without a namespace is not: a closure is + // given a delegate when it runs, and a name the delegate answers to is that delegate's, + // not a tag. request.withFormat { form multipartForm { } } is the case that proves it - + // form there is a format in a DSL, and rewriting it into g:form sends the call somewhere + // the author never wrote. + closureDepth++; + try { + closure.visit(this); + } + finally { + closureDepth--; + } + 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 (closureDepth > 0) { + // Inside a closure the name may be answered by whatever delegate the closure is given. + 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(new VariableExpression("this"), + 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); Review Comment: Fixed, and then made narrower. The rewriting now declines to compile an argument shape the invocation cannot account for, so those calls stay on the dynamic path. This was the fourth bug of the same family in this branch — `DefaultGroovyMethods` names, closure delegates, `withFormat`, and this — so the rule changed rather than gaining a fourth exclusion: rewriting an unqualified call is now off unless a build sets `grails.compileStatic.unqualifiedTagCalls`. Namespaced calls, markup tags and statically compiled page expressions are unaffected, which is where the measured benefit came from. ########## grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/views/gsp/GroovyPagePlugin.groovy: ########## @@ -47,13 +50,101 @@ import org.grails.gradle.plugin.util.SourceSets @CompileStatic class GroovyPagePlugin implements Plugin<Project> { + /** + * The test source sets a Grails project may define, each of which renders pages. + */ + private static final List<String> TEST_SOURCE_SET_NAMES = ['test', 'integrationTest'] + @Override void apply(Project project) { project.pluginManager.withPlugin('groovy') { configureProject(project) } } + /** + * Whether compilation keeps parameter names, which decides whether a tag's attributes and body + * parameters have to carry those names to be dispatchable. The index has to be generated under the + * same setting the sources are compiled with, or it would describe a different set of tags. + */ + @CompileDynamic + private static Provider<Boolean> resolvePreserveParameterNames(Project project) { + project.provider { + Object grails = project.extensions.findByName('grails') + Object preserve = grails?.hasProperty('preserveParameterNames') ? grails.preserveParameterNames : null + if (preserve instanceof Provider) { + return ((Provider) preserve).getOrElse(true) as Boolean + } + preserve == null ? Boolean.TRUE : (preserve as Boolean) + } + } + + /** + * Whether the build declared that every tag library it uses is described at compile time, so that + * a tag missing from the index is a mistake rather than something contributed later. + */ + @CompileDynamic + private static Provider<Boolean> resolveStrictTags(Project project) { + project.provider { + Object compileStatic = project.extensions.findByName('grails')?.compileStatic + Object strict = compileStatic?.hasProperty('strictTags') ? compileStatic.strictTags : null + strict instanceof Provider ? ((Provider) strict).getOrElse(false) as Boolean : Boolean.FALSE + } + } + + /** + * The namespaces the build declared as filled in while the application runs. + */ + @CompileDynamic + private static Provider<Set<String>> resolveDynamicTagNamespaces(Project project) { + project.provider { + Object compileStatic = project.extensions.findByName('grails')?.compileStatic + Object namespaces = compileStatic?.hasProperty('dynamicTagNamespaces') ? + compileStatic.dynamicTagNamespaces : null + namespaces instanceof Provider ? + (((Provider) namespaces).getOrElse([] as Set) as Set<String>) : ([] as Set<String>) + } + } + + /** + * The encoding the project's Groovy sources are compiled with, which the generator has to read + * them with. Falls back to the generator's own default when the project has not set one. + */ + @CompileDynamic + private static Provider<String> resolveCompileEncoding(Project project) { + project.provider { + Object compile = project.tasks.findByName('compileGroovy') + (compile instanceof GroovyCompile) ? ((GroovyCompile) compile).options.encoding : null + } + } + + /** + * The Groovy source roots of a source set, which is where a type this project declares is found. + */ + @CompileDynamic + private static Set<File> resolveGroovySourceRoots(SourceSet sourceSet) { + Object groovy = sourceSet?.extensions?.findByName('groovy') + groovy ? (groovy.srcDirs as Set<File>) : ([] as Set<File>) + } + + /** + * Puts the packaged index onto the runtime classpath of every test source set, so that a page + * rendered by a test resolves its tags against the same index as the same page in production. + */ + @CompileDynamic + private static void addPackagedIndexToTestRuntime(Project project, FileCollection packagedTagLibIndex) { + SourceSetContainer sourceSets = project.extensions.findByType(SourceSetContainer) + if (sourceSets == null) { + return + } + for (String name : TEST_SOURCE_SET_NAMES) { Review Comment: Fixed. `sourceSets.matching { it.name in TEST_SOURCE_SET_NAMES }.configureEach { }` instead of the eager lookup, so `integrationTest` is picked up whenever it is registered. -- 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]
