jdaugherty commented on code in PR #16134: URL: https://github.com/apache/grails-core/pull/16134#discussion_r3816364894
########## 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)) { + if (!isTagLibrary(classNode)) { + continue; + } + String namespace = TagLibraryAstDiscovery.resolveNamespace(classNode); + if (namespace == null) { + // Only knowable once the initialiser runs, so recording it would file the tags under + // a guess. Left out, which leaves the tag library to runtime resolution. + continue; + } + TagLibraryIndexWriter.write(outputDir, classNode.getName(), namespace, + TagLibraryAstDiscovery.findTags(classNode, parameterNamesRetained)); + } + recordWhatWasMissed(outputDir, skipped, encoding); + } + + /** + * Records the namespaces left incomplete by whatever could not be read, so that a call to a tag of + * one of them is never reported as a misspelling. + */ + private static void recordWhatWasMissed(File outputDir, List<File> skipped, String encoding) + throws IOException { + Set<String> namespaces = new TreeSet<>(); + boolean everything = false; + for (File source : skipped) { + String namespace = declaredNamespace(source, encoding); + if (namespace != null) { + namespaces.add(namespace); + } + else { + everything = true; + } + } + TagLibraryIndexWriter.writeIncomplete(outputDir, namespaces, everything); + } + + /** + * Parses the sources far enough to describe them. + * + * <p>A tag library referring to something outside this directory and off the classpath given here, + * such as a service in the same project, cannot be resolved before that project is compiled. Those + * are parsed on their own and skipped when they still fail, rather than losing the index for every + * other tag library alongside them. What was skipped is recorded, so that nothing in a namespace + * missing some of its tags is reported; a build that writes the index describes it again once the + * project has been compiled, and until then its tags resolve dynamically, exactly as a tag library + * with no descriptor does. + */ + private static List<ClassNode> parse(List<File> sources, List<File> resolutionRoots, + boolean parameterNamesRetained, String encoding, List<File> skippedOut) { + try { + return collectClassNodes(compile(sources, resolutionRoots, parameterNamesRetained, encoding)); + } catch (Exception wholeSourceSetFailed) { + List<ClassNode> classNodes = new ArrayList<>(); + for (File source : sources) { + try { + classNodes.addAll(collectClassNodes( + compile(List.of(source), resolutionRoots, parameterNamesRetained, encoding))); + } catch (Exception singleSourceFailed) { + skippedOut.add(source); + } + } + if (!skippedOut.isEmpty()) { + List<String> names = new ArrayList<>(); + for (File skipped : skippedOut) { + names.add(skipped.getName()); + } + System.out.println("Tag library index: could not read " + String.join(", ", names) + + " before compilation; their tags resolve dynamically until they are compiled."); + } + classNodes.sort(Comparator.comparing(ClassNode::getName)); + return classNodes; + } + } + + /** + * The namespace a source that could not be described declares, read from its syntax tree. + * + * <p>Only ever used to record which namespace is missing some of its tags. Naming the wrong one + * leaves the real one looking complete, which is exactly when a call to a tag that does exist gets + * reported as one that does not, so this claims a namespace only where the source leaves no room + * for doubt: one tag library in the file, declaring its own namespace as a constant. + * + * <p>Anything else - a namespace field on some other class in the file, more than one tag library, + * a namespace inherited from a base class that may not have resolved, or none stated at all - + * yields nothing, and every namespace is then treated as incomplete. That costs diagnostics rather + * than inventing an error. + * + * @return the namespace, or {@code null} when this source does not plainly state one + */ + private static String declaredNamespace(File source, String encoding) { + try { + CompilerConfiguration configuration = new CompilerConfiguration(); + configuration.setSourceEncoding(encoding); + CompilationUnit unit = new CompilationUnit(configuration); + unit.addSource(source); + // Conversion builds the tree and stops before resolving anything, so a type this project + // has not compiled yet cannot make it fail. + unit.compile(Phases.CONVERSION); + + ClassNode candidate = null; + for (ClassNode classNode : collectClassNodes(unit)) { + if (!isTagLibrary(classNode)) { + continue; + } + if (candidate != null) { + // Which of them failed is not knowable, so neither is claimed. + return null; + } + candidate = classNode; + } + if (candidate == null) { + return null; + } + + FieldNode field = candidate.getDeclaredField(NAMESPACE_FIELD); + if (field == null || !field.isStatic()) { + // Either the default namespace or one inherited from a base class whose resolution is + // the very thing in doubt. Not distinguishable here, so not claimed. + return null; + } + if (!(field.getInitialExpression() instanceof ConstantExpression constant) || + constant.getValue() == null) { + return null; + } + String namespace = constant.getValue().toString().trim(); + return namespace.isEmpty() ? null : namespace; + } + catch (Exception unparseable) { + return null; + } + } + + private static CompilationUnit compile(List<File> sources, List<File> resolutionRoots, + boolean parameterNamesRetained, String encoding) { + CompilerConfiguration configuration = new CompilerConfiguration(); + configuration.setParameters(parameterNamesRetained); + configuration.setSourceEncoding(encoding); + CompilationUnit unit = new CompilationUnit(configuration); + if (!resolutionRoots.isEmpty()) { + unit.setClassNodeResolver(new SourceRootClassNodeResolver(resolutionRoots)); + } + for (File source : sources) { + unit.addSource(source); + } + // Canonicalization is the last phase before bytecode, by which point traits are applied and + // annotations resolved, and it stops short of generating or loading any class. + unit.compile(Phases.CANONICALIZATION); + return unit; + } + + /** + * Resolves a type this project declares by compiling its source alongside the tag library that + * refers to it. + * + * <p>A tag library commonly refers to something the same project declares - a service it injects, + * a base class it extends, a trait it carries - and none of those exist as classes yet when the + * index is generated. Compiling their source too is what the Groovy compiler does for types within + * one compilation, and is what lets a tag library be described exactly as it will be once built. + * + * <p>Deliberately not a stand-in class node. What is missing decides what a tag library declares: + * a base class carries the namespace, a trait carries tags, and a parameter type decides whether a + * method is a tag at all. Answering with a placeholder would file a tag library under the wrong + * namespace, or leave out tags the running application has, and the index would then disagree with + * what the application does - which is the one thing it must never do. A type that cannot be found + * in source is left unresolved, and the tag library referring to it is skipped as before. + */ + private static final class SourceRootClassNodeResolver extends ClassNodeResolver { + + private final List<File> roots; + + private SourceRootClassNodeResolver(List<File> roots) { + this.roots = roots; + } + + @Override + public LookupResult resolveName(String name, CompilationUnit compilationUnit) { + LookupResult onTheClasspath = super.resolveName(name, compilationUnit); + if (onTheClasspath != null) { + return onTheClasspath; + } + File source = findSource(name); + if (source == null) { + // Not something this project declares. Left unresolved so that resolution carries on + // to the next candidate a star import offers, and so that a name that is simply + // misspelled still fails rather than being quietly invented. + return null; + } + return new LookupResult(compilationUnit.addSource(source), null); + } + + private File findSource(String name) { + String relativePath = name.replace('.', File.separatorChar) + ".groovy"; + for (File root : roots) { + File candidate = new File(root, relativePath); + if (candidate.isFile()) { + return candidate; + } + } + return null; + } + } + + private static List<ClassNode> collectClassNodes(CompilationUnit unit) { + List<ClassNode> classNodes = new ArrayList<>(); + unit.getAST().getModules().forEach(module -> classNodes.addAll(module.getClasses())); + // Sorted so that the index is identical for identical sources regardless of the order the + // file system enumerated them, keeping the build reproducible. + classNodes.sort(Comparator.comparing(ClassNode::getName)); + return classNodes; + } + + private static boolean isTagLibrary(ClassNode classNode) { + for (AnnotationNode annotation : classNode.getAnnotations()) { + String annotationName = annotation.getClassNode().getName(); + if (TAG_LIB_ANNOTATION.equals(annotationName)) { + return true; + } + if (ARTEFACT_ANNOTATION.equals(annotationName)) { + var member = annotation.getMember("value"); + if (member != null && TAG_LIB_ARTEFACT.equals(member.getText())) { + return true; + } + } + } + return classNode.getName().endsWith(TAG_LIB_ARTEFACT); Review Comment: **Abstract classes are described, but can never be tag libraries at runtime.** Reproduced by running the generator over a temp `grails-app/taglib`: ```groovy // grails-app/taglib/demo/BaseTagLib.groovy abstract class BaseTagLib { def common(Map attrs) { 'shared' } } // grails-app/taglib/demo/MyTagLib.groovy @TagLib class MyTagLib extends BaseTagLib { static namespace = 'my' } ``` produces `demo.BaseTagLib.properties` with `namespace=g` and `tags=common:METHOD`. At runtime that tag exists nowhere: `ArtefactHandlerAdapter.isArtefactClass` rejects abstract classes (`allowAbstract` is false and `TagLibArtefactHandler` does not set it), so `BaseTagLib` is never registered — and `MyTagLib`'s method tags are declared-only, so `common` is not a tag of `my` either. Every `g.common(...)` in the project then compiles into a direct invocation that throws `GrailsTagException` when it runs, and under `strictTags` a real misspelling that happens to match a phantom name is no longer reported. Restricting the loop to `sourceDirs` (sbglasius's comment above) does not cover this case: `grails-app/taglib` *is* a source dir, and an abstract base living there for its subclasses to share is exactly the shape `IndexEdgeCaseTagLib`'s own `BaseEdgeTagLib` fixture has. Suggest skipping abstract `ClassNode`s here, which is the same rule the runtime applies — and it covers traits and interfaces for free, since their class nodes are abstract too. The `@TagLib`-annotated abstract case in `TagLibArtefactTypeAstTransformation.writeIndexEntry` wants the same check. Worth a test asserting that an abstract base with a `(Map)`-shaped method produces no descriptor. ########## grails-gsp/grails-taglib/src/main/groovy/org/grails/taglib/index/TagLibraryIndex.java: ########## @@ -0,0 +1,486 @@ +/* + * 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.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.Reader; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.util.Collections; +import java.util.Enumeration; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Properties; +import java.util.Set; +import java.util.TreeMap; +import java.util.TreeSet; +import java.util.WeakHashMap; + +/** + * The set of tag libraries and tag names known at compile time. + * + * <p>Each tag library contributes one descriptor under {@value #INDEX_LOCATION}, written by the + * {@code TagLib} AST transformation as the tag library is compiled. Descriptors are per class rather + * than per module so that libraries packaged in separate jars merge on the classpath without any + * build step having to combine them, in the same way {@code META-INF/services} entries do. + * + * <p>Reading the index answers "which tags exist in namespace x" without loading or reflecting over a + * single tag library class, which is what allows GSP expressions to be resolved when a page is + * compiled rather than dispatched dynamically when it renders. + * + * @since 8.0.0 + */ +public final class TagLibraryIndex { + + /** + * Classpath directory holding one descriptor per compiled tag library. + */ + public static final String INDEX_LOCATION = "META-INF/grails/taglibs/"; + + /** + * Descriptor format this build writes and understands. A descriptor carrying anything else was + * produced by a different version of Grails and is ignored, so its tags resolve dynamically rather + * than being read under the wrong set of rules. + */ + public static final int FORMAT_VERSION = 1; + + /** + * Settings the build states for the compilation the index is read in, written alongside the + * descriptors by the build and deliberately not packaged into the artifact: they describe how this + * project is compiled, not what its tag libraries declare. + */ + public static final String SETTINGS_LOCATION = INDEX_LOCATION + "compile-settings.properties"; + + /** + * What the index could not describe, written by whatever produced it. + * + * <p>An index generated before its project is compiled cannot always read every tag library: one + * referring to a type that does not exist yet, in a language it cannot parse, or generated by the + * build itself, is left out. A namespace missing some of its tags must not have a call to one of + * them reported as a misspelling, so what was missed is recorded rather than left to be inferred + * from the absence. + */ + public static final String INCOMPLETE_LOCATION = INDEX_LOCATION + "incomplete.properties"; + + static final String VERSION_KEY = "version"; + static final String NAMESPACE_KEY = "namespace"; + static final String CLASS_KEY = "class"; + static final String TAGS_KEY = "tags"; + static final String STRICT_KEY = "strictTags"; + static final String INCOMPLETE_NAMESPACES_KEY = "namespaces"; + static final String INCOMPLETE_ALL_KEY = "all"; + static final String DYNAMIC_NAMESPACES_KEY = "dynamicTagNamespaces"; + + /** + * One index per class loader. A compilation gets a class loader of its own, so this is read once + * per compilation rather than once per source file, and is not held after that compilation ends. + * Caching in a plain static field instead would carry one project's tag libraries into the next + * compilation in the same Gradle daemon. + */ + private static final Map<ClassLoader, TagLibraryIndex> BY_CLASS_LOADER = + Collections.synchronizedMap(new WeakHashMap<>()); + + private final Map<String, Map<String, TagLibraryIndexEntry>> byNamespace; + private final Map<String, Set<String>> ambiguousByNamespace; + private final Map<String, Set<String>> tagNamesByClass; + private final boolean strict; + private final Set<String> dynamicNamespaces; + private final Set<String> incompleteNamespaces; + private final boolean everythingIncomplete; + + private TagLibraryIndex(Map<String, Map<String, TagLibraryIndexEntry>> byNamespace, + Map<String, Set<String>> ambiguousByNamespace, Map<String, Set<String>> tagNamesByClass, + boolean strict, Set<String> dynamicNamespaces, Set<String> incompleteNamespaces, + boolean everythingIncomplete) { + this.byNamespace = byNamespace; + this.ambiguousByNamespace = ambiguousByNamespace; + this.tagNamesByClass = tagNamesByClass; + this.strict = strict; + this.dynamicNamespaces = dynamicNamespaces; + this.incompleteNamespaces = incompleteNamespaces; + this.everythingIncomplete = everythingIncomplete; + } + + /** + * Reads the index for a class loader, reusing the one already read for it. + * + * <p>Reading walks every jar on the classpath, so a compiler that consults the index for each + * source file it compiles would walk it once per file. Use this from compilation; use + * {@link #load(ClassLoader)} where a fresh read is wanted. + * + * @param classLoader the loader to scan; when {@code null} the thread context loader is used + * @return the merged index, never {@code null} + */ + public static TagLibraryIndex forClassLoader(ClassLoader classLoader) { + ClassLoader loader = classLoader != null ? classLoader : Thread.currentThread().getContextClassLoader(); + if (loader == null) { + return load(null); + } + return BY_CLASS_LOADER.computeIfAbsent(loader, TagLibraryIndex::load); + } + + /** + * Reads every tag library descriptor visible to the given class loader. + * + * @param classLoader the loader to scan; when {@code null} the thread context loader is used + * @return the merged index, never {@code null} + */ + public static TagLibraryIndex load(ClassLoader classLoader) { + ClassLoader loader = classLoader != null ? classLoader : Thread.currentThread().getContextClassLoader(); + Map<String, Map<String, TagLibraryIndexEntry>> merged = new TreeMap<>(); + Map<String, Set<String>> ambiguous = new TreeMap<>(); + Map<String, Set<String>> byClass = new TreeMap<>(); + if (loader == null) { + return new TagLibraryIndex(merged, ambiguous, byClass, false, Collections.emptySet(), + Collections.emptySet(), false); + } + // A directory resource enumerates its children on some classpath layouts but not inside jars, + // so the descriptors are discovered through the manifest of names each descriptor records + // rather than by listing the directory. + for (URL url : listDescriptors(loader)) { + Properties properties = read(url); + if (properties == null) { + continue; + } + if (!String.valueOf(FORMAT_VERSION).equals(properties.getProperty(VERSION_KEY))) { + continue; + } + String namespace = properties.getProperty(NAMESPACE_KEY); + String className = properties.getProperty(CLASS_KEY); + String tags = properties.getProperty(TAGS_KEY, ""); + if (namespace == null || namespace.isEmpty() || className == null || className.isEmpty()) { + continue; + } + // Recorded from the descriptor rather than from its tags, so that a tag library declaring + // none of them is still known to have been described. Deciding that from the tags alone + // would have such a tag library described twice. + byClass.computeIfAbsent(className, k -> new TreeSet<>()); + Map<String, TagLibraryIndexEntry> tagsForNamespace = + merged.computeIfAbsent(namespace, k -> new TreeMap<>()); + for (String encodedTag : tags.split(",")) { + String trimmed = encodedTag.trim(); + if (trimmed.isEmpty()) { + continue; + } + // Recorded as "name:KIND"; an unrecognised kind is treated as the dynamic one so that a + // descriptor from a later version cannot cause a call to be bound wrongly. + int separator = trimmed.lastIndexOf(':'); + String tagName = separator > 0 ? trimmed.substring(0, separator) : trimmed; + TagLibraryIndexEntry.Kind kind = TagLibraryIndexEntry.Kind.LEGACY_CLOSURE; + if (separator > 0) { + try { + kind = TagLibraryIndexEntry.Kind.valueOf(trimmed.substring(separator + 1)); + } catch (IllegalArgumentException unknownKind) { + kind = TagLibraryIndexEntry.Kind.LEGACY_CLOSURE; + } + } + trimmed = tagName; + // Recorded against the declaring class before ambiguity is considered, so that asking + // what one tag library declares is answered from its own descriptor and is unaffected + // by whether some other tag library happens to declare the same name. + byClass.computeIfAbsent(className, k -> new TreeSet<>()).add(trimmed); + TagLibraryIndexEntry existing = tagsForNamespace.get(trimmed); + if (existing != null && !existing.tagLibraryClassName().equals(className)) { + // At runtime the tag library registered last wins, and registration order comes + // from artefact scanning rather than from classpath order, so which of these two + // will win cannot be known here. Resolving it either way risks compiling against + // one implementation and dispatching to the other, so the tag is marked ambiguous + // and left to runtime resolution. + ambiguous.computeIfAbsent(namespace, k -> new TreeSet<>()).add(trimmed); + continue; + } + tagsForNamespace.put(trimmed, + new TagLibraryIndexEntry(namespace, trimmed, className, kind, true)); + } + } + Properties settings = readSettings(loader); + boolean strict = Boolean.parseBoolean(settings.getProperty(STRICT_KEY, "false")); + Set<String> dynamic = new TreeSet<>(); + for (String namespace : settings.getProperty(DYNAMIC_NAMESPACES_KEY, "").split(",")) { + String trimmed = namespace.trim(); + if (!trimmed.isEmpty()) { + dynamic.add(trimmed); + } + } + Set<String> incomplete = new TreeSet<>(); + boolean allIncomplete = false; + for (URL url : urls(loader, INCOMPLETE_LOCATION)) { + Properties recorded = read(url); + if (recorded == null) { + continue; + } + allIncomplete |= Boolean.parseBoolean(recorded.getProperty(INCOMPLETE_ALL_KEY, "false")); + for (String namespace : recorded.getProperty(INCOMPLETE_NAMESPACES_KEY, "").split(",")) { + String trimmed = namespace.trim(); + if (!trimmed.isEmpty()) { + incomplete.add(trimmed); + } + } + } + return new TagLibraryIndex(merged, ambiguous, byClass, strict, + Collections.unmodifiableSet(dynamic), Collections.unmodifiableSet(incomplete), + allIncomplete); + } + + private static Set<URL> urls(ClassLoader loader, String location) { + Set<URL> found = new LinkedHashSet<>(); + try { + Enumeration<URL> resources = loader.getResources(location); + while (resources.hasMoreElements()) { + found.add(resources.nextElement()); + } + } + catch (IOException unreadable) { + return found; + } + return found; + } + + /** + * Reads the settings the build states for this compilation. Only the project being compiled + * contributes them, so the first one found wins rather than several being merged. + */ + private static Properties readSettings(ClassLoader loader) { + URL url = loader.getResource(SETTINGS_LOCATION); + if (url == null) { + return new Properties(); + } + Properties settings = read(url); + return settings != null ? settings : new Properties(); + } + + private static Set<URL> listDescriptors(ClassLoader loader) { + Set<URL> urls = new LinkedHashSet<>(); + try { + Enumeration<URL> manifests = loader.getResources(INDEX_LOCATION + "index.properties"); + while (manifests.hasMoreElements()) { + URL manifest = manifests.nextElement(); + Properties names = read(manifest); + if (names == null) { + continue; + } + for (String className : names.stringPropertyNames()) { + // Resolved against the manifest that names it rather than searched for on the + // classpath. A descriptor always sits beside its own manifest, and asking the + // loader instead would walk every classpath entry once per tag library - a few + // hundred full walks for an application with a few hundred of them. + URL descriptor = resolveSibling(manifest, className + ".properties"); + if (descriptor != null) { + urls.add(descriptor); + } + } + } + } catch (IOException e) { + // A classpath that cannot be enumerated yields no statically known tags, which degrades to + // the dynamic dispatch that was in place before the index existed. + return urls; + } + return urls; + } + + /** + * @param manifest the manifest naming the descriptor + * @param fileName the descriptor's file name + * @return the descriptor beside that manifest, or {@code null} when it cannot be addressed + */ + private static URL resolveSibling(URL manifest, String fileName) { + try { + return new URL(manifest, fileName); Review Comment: **This reintroduces the javac deprecation note the last round removed.** Every `java.net.URL` constructor is deprecated since JDK 20, so on the JDK 21 baseline `./gradlew :grails-taglib:compileGroovy` now prints: ``` Note: .../org/grails/taglib/index/TagLibraryIndex.java uses or overrides a deprecated API. ``` The awkward part is that the constructor is the right tool here: `URI.resolve` cannot resolve a relative name against a `jar:` URI (it is opaque), and round-tripping the manifest URL through `URI` breaks on characters `getResources` does not encode. So rather than replacing it, suppress it deliberately — `@SuppressWarnings("deprecation")` on `resolveSibling` with a sentence saying why the constructor stays — so the module builds quietly without the reason getting lost. ########## 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: Confirmed the core of this — the loop does describe everything `parse()` returned, resolver-pulled sources included, and the bare-name fallback then files a `src/main/groovy` helper's `(Map)` methods under `g`. One correction on the first outcome, worth having before anyone fixes it: a collision does not stop rewriting. `isKnown` deliberately answers true for an ambiguous tag and the invocation binds by name at runtime, so the colliding case is benign — the harm is confined to the non-colliding phantom (compiles into an invocation that throws `GrailsTagException` at runtime) and to `strictTags` no longer reporting a real misspelling that matches a phantom name. Restricting the loop to `sourceDirs` is still the right fix; see the new comment on `isTagLibrary` for the abstract-class case that restriction does not cover. ########## grails-gsp/grails-taglib/src/main/groovy/org/grails/taglib/index/TagLibraryIndexEntry.java: ########## @@ -0,0 +1,57 @@ +/* + * 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; + +/** + * One tag recorded in the {@link TagLibraryIndex} at compile time. + * + * @param namespace the tag library namespace the tag is reachable through + * @param tagName the tag name within that namespace + * @param tagLibraryClassName the binary name of the tag library declaring the tag + * @param kind how the tag is implemented, which decides whether a call to it can be resolved + * @param acceptsBody whether the tag can be called with a body + * @since 8.0.0 + */ +public record TagLibraryIndexEntry(String namespace, String tagName, String tagLibraryClassName, Review Comment: Keep `getTagNamesForClass` and `isClassDescribed` — each pins an invariant nothing else states. `lookup` from the original list is in the same boat (still spec-only); given the agreement spec reads through it and it is the natural read API next to `isAmbiguous`, keeping all three is fine. ########## grails-gsp/core/src/main/groovy/org/grails/gsp/GroovyPage.java: ########## @@ -296,6 +310,23 @@ public Object getProperty(String property) { return resolveProperty(property); } + /** + * Resolves a tag called without a namespace, as {@code ${message(code: 'x')}} is. + * + * <p>A real method rather than one installed onto this page's metaclass. Installing it, along with + * a method for every tag and a property for every namespace, meant writing to an + * ExpandoMetaClass for every page compiled and made every later tag call a read of an initialised + * metaclass, which is guarded by a lock. + * + * @param name the tag name + * @param args the arguments the tag was called with + * @return whatever the tag produces + */ + public Object methodMissing(String name, Object args) { + return TagLibraryMetaUtils.methodMissingForTagLib(getMetaClass(), getClass(), gspTagLibraryLookup, Review Comment: Keep the guard — stating the contract beats leaning on how dynamic dispatch happens to treat a null receiver, and the reworded comment plus `GroovyPageMethodMissingSpec` now say exactly what it does. ########## 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: Confirmed, including the before: at the merge-base this line was `tagLibrary.invokeMethod(methodName, args)`, which dispatched `format('2026-08-19', 'yyyy')` to the two-String overload on the bean. `methodMissingForTagLib` itself is unchanged by this PR — its tag branch and argument switch predate it — so the regression is purely that this call site now routes into it. Whatever the fix looks like, it needs to land in both halves: guarding only the rewriter (the sibling comment) leaves this runtime path discarding the same arguments. -- 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]
