Resolve tag calls at compile time instead of through the metaclass

end-to-end/README.md

@@ -36,6 +36,7 @@ applications at via `GRAILS_REPO_URL`.
 | `legacy-commands-plugin` | A Grails 8 plugin whose legacy commands are recompiled under Groovy 5. |
 | `legacy-commands` | A Grails 8 application that consumes both and runs their commands through the registry. |
 | `spring-dependency-management` | A Grails 8 application that manages its versions with the legacy `io.spring.dependency-management` plugin instead of the Grails Gradle plugin's native `platform(grails-bom)`, as an upgraded Grails 7 application does. |
+| `taglib-index-incremental` | Builds a Grails 8 application **twice, without a clean**, to prove a renamed or deleted tag library cannot survive in the published tag library index. Incremental behaviour is the whole point, so it cannot be expressed by a project the core build builds once for itself. |
 
 `legacy-g7-command-plugin` is deliberately excluded from `settings.gradle`. An included build would
 substitute `org.apache.grails:grails-core` for this repository's Groovy 5 project, which is exactly

end-to-end/settings.gradle

@@ -104,6 +104,10 @@ rootProject.name = 'grails-end-to-end'
 include(
         'legacy-commands',
         'legacy-commands-plugin',
+        // Builds an application twice without a clean, to prove a renamed or deleted tag library
+        // cannot survive in the index that is published. Incremental behaviour against real
+        // published artifacts is not something the core build can express.
+        'taglib-index-incremental',
         // Belongs here rather than in grails-test-examples: it imports grails-bom as a Maven BOM
         // through io.spring.dependency-management, which resolves imports in its own detached
         // configuration. That bypasses any project substitution, so the import can only ever be

end-to-end/taglib-index-incremental/build.gradle

@@ -0,0 +1,54 @@
+/*
+ *  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.
+ */
+
+// Whether a tag library that has been renamed or deleted can survive in what a build publishes.
+//
+// Answering it needs a real application built twice against real published artifacts, without a
+// clean in between, because what is being tested is incremental behaviour: Gradle does not recompile
+// a source that has not changed, so anything written per class as it compiled would simply stay. The
+// core build cannot express that - its own test projects are built once, by the build running the
+// test - so it lives here, where an application resolves Grails the way an application does.
+plugins {
+    id 'groovy'
+    id 'org.apache.grails.buildsrc.properties'
+}
+
+dependencies {
+    // Versions come from the same BOM the framework publishes, so this harness never pins its own.
+    testImplementation platform("org.apache.grails:grails-bom:${project.findProperty('projectVersion') ?: version}")
+    // Deliberately not gradleTestKit(): it carries Gradle's own Groovy 4 onto the test classpath,
+    // which the Groovy 5 Spock this repository builds against cannot compile against. The nested
+    // build is driven through the repository's own wrapper instead, which is also the Gradle an
+    // application would use.
+    testImplementation 'org.spockframework:spock-core'
+    testImplementation 'org.apache.groovy:groovy'
+    testRuntimeOnly 'org.junit.platform:junit-platform-launcher'
+}
+
+tasks.named('test', Test) {
+    useJUnitPlatform()
+    // The application the test builds resolves Grails from the same place this build does.
+    systemProperty 'grails.e2e.localMavenRepo',
+            rootProject.layout.projectDirectory.dir('../build/local-maven').asFile.absolutePath
+    systemProperty 'grails.e2e.version', project.findProperty('projectVersion') ?: version
+    systemProperty 'grails.e2e.gradlew',
+            rootProject.layout.projectDirectory.file('../gradlew').asFile.absolutePath
+    // Each case builds an application from scratch, so this is slow by nature. It belongs to a build
+    // that is already opt-in and already requires a publish, so it is not gated further.
+}

end-to-end/taglib-index-incremental/src/test/groovy/org/apache/grails/e2e/taglib/TagLibraryIndexIncrementalSpec.groovy

@@ -0,0 +1,241 @@
+/*
+ *  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.apache.grails.e2e.taglib
+
+import java.util.zip.ZipFile
+
+import spock.lang.Specification
+import spock.lang.TempDir
+
+/**
+ * A tag library that has been renamed or deleted must not survive in what a build publishes.
+ *
+ * <p>The interesting case is the second build. Gradle does not recompile a source that has not
+ * changed, so anything written per class as it compiled would never be revisited and would simply
+ * stay - describing a tag library that no longer exists, and being packaged alongside the index that
+ * no longer describes it. Only a build run twice, without a clean, shows that.
+ *
+ * <p>Built against published artifacts rather than project dependencies, so what is exercised is the
+ * plugin an application actually applies.
+ */
+class TagLibraryIndexIncrementalSpec extends Specification {
+
+    private static final String INDEX = 'META-INF/grails/taglibs'
+
+    @TempDir
+    File projectDir
+
+    def setup() {
+        writeSettings()
+        writeBuild()
+        writeTagLib('AlphaTagLib', 'alpha', 'alphaTag')
+        writeTagLib('BetaTagLib', 'beta', 'betaTag')
+    }
+
+    void 'a deleted tag library is gone from the published index after a build with no clean'() {
+        given: 'a first build describing both'
+        build()
+
+        expect:
+        packagedDescriptor('demo.AlphaTagLib').isFile()
+        packagedDescriptor('demo.BetaTagLib').isFile()
+        packagedManifest().contains('demo.BetaTagLib')
+        jarNames().contains("${INDEX}/demo.BetaTagLib.properties" as String)
+
+        when: 'one is deleted and the project is built again, without a clean'
+        new File(projectDir, 'grails-app/taglib/demo/BetaTagLib.groovy').delete()
+        build()
+
+        then: 'it is described nowhere: not beside the index, not in it, not in the artifact'
+        !packagedDescriptor('demo.BetaTagLib').isFile()
+        !packagedManifest().contains('demo.BetaTagLib')
+        !jarNames().any { it.contains('BetaTagLib') }
+
+        and: 'and the one that remains is still described'
+        packagedDescriptor('demo.AlphaTagLib').isFile()
+        packagedManifest().contains('demo.AlphaTagLib')
+    }
+
+    void 'a renamed tag library does not leave its old name behind'() {
+        given:
+        build()
+
+        when: 'renamed in place, which to a build is a deletion and an addition'
+        new File(projectDir, 'grails-app/taglib/demo/BetaTagLib.groovy').delete()
+        writeTagLib('GammaTagLib', 'beta', 'betaTag')
+        build()
+
+        then:
+        !packagedDescriptor('demo.BetaTagLib').isFile()
+        packagedDescriptor('demo.GammaTagLib').isFile()
+        packagedManifest().contains('demo.GammaTagLib')
+        !packagedManifest().contains('demo.BetaTagLib')
+    }
+
+    void 'a tag removed from a tag library is gone from the index it is described by'() {
+        given:
+        build()
+
+        expect:
+        packagedDescriptor('demo.AlphaTagLib').text.contains('alphaTag')
+
+        when: 'the tag is removed and the project built again'
+        writeTagLib('AlphaTagLib', 'alpha', 'renamedTag')
+        build()
+
+        then:
+        !packagedDescriptor('demo.AlphaTagLib').text.contains('alphaTag:')
+        packagedDescriptor('demo.AlphaTagLib').text.contains('renamedTag')
+    }
+
+    void 'an executable archive carries the descriptors and not the settings'() {
+        given: 'a war copies whole directories off the runtime classpath, which is how the settings'
+        buildTask('war')
+
+        when: 'used to escape an exclusion declared on the archive task'
+        List<String> entries = archiveNames('build/libs', '.war')
+
+        then: 'the descriptors are there, where a page compiled at runtime can read them'
+        entries.any { it == "WEB-INF/classes/${INDEX}/demo.AlphaTagLib.properties" as String }
+        entries.any { it == "WEB-INF/classes/${INDEX}/index.properties" as String }
+
+        and: 'the settings are nowhere in it'
+        !entries.any { it.endsWith('compile-settings.properties') }
+
+        and: 'and no descriptor is carried twice, in two places that would then disagree'
+        entries.findAll { it.endsWith('demo.AlphaTagLib.properties') }.size() == 1
+    }
+
+    void 'the settings this build declared reach no archive'() {
+        given: 'they say how this project compiles; a consumer inheriting them would compile by them'
+        build()
+
+        expect: 'not in the jar'
+        !jarNames().any { it.endsWith('compile-settings.properties') }
+
+        and: 'and not anywhere in the tree an executable archive is built from, which copies whole'
+        !new File(projectDir, 'build/generated/grails-taglibs-packaged')
+                .listFiles({ File dir, String name -> name == 'META-INF' } as FilenameFilter)
+                .collect { new File(it, 'grails/taglibs/compile-settings.properties') }
+                .any { it.exists() }
+    }
+
+    void 'nothing writes a second index into the class output'() {
+        given: 'a build that writes the index owns it, so a copy there could only compete and go stale'
+        build()
+
+        expect:
+        !new File(projectDir, "build/classes/groovy/main/${INDEX}").exists()
+    }
+
+    private void build() {
+        buildTask('jar')
+    }
+
+    private void buildTask(String task) {
+        Process process = new ProcessBuilder(System.getProperty('grails.e2e.gradlew'),
+                '-p', projectDir.absolutePath, task, '--stacktrace')
+                .redirectErrorStream(true)
+                .start()
+        String output = process.inputStream.getText('UTF-8')
+        int status = process.waitFor()
+        assert status == 0 : "building the application failed:\n${output}"
+    }
+
+    private File packagedDescriptor(String className) {
+        new File(projectDir, "build/generated/grails-taglibs-packaged/${INDEX}/${className}.properties")
+    }
+
+    private String packagedManifest() {
+        File manifest = new File(projectDir,
+                "build/generated/grails-taglibs-packaged/${INDEX}/index.properties")
+        manifest.isFile() ? manifest.text : ''
+    }
+
+    private List<String> jarNames() {
+        archiveNames('build/libs', '.jar')
+    }
+
+    private List<String> archiveNames(String directory, String extension) {
+        File archive = new File(projectDir, directory).listFiles()?.find { it.name.endsWith(extension) }
+        assert archive != null : "the project produced no ${extension} in ${directory}"
+        new ZipFile(archive).withCloseable { zip -> zip.entries().collect { it.name } }
+    }
+
+    private void writeTagLib(String className, String namespace, String tagName) {
+        File dir = new File(projectDir, 'grails-app/taglib/demo')
+        dir.mkdirs()
+        new File(dir, "${className}.groovy").text = """
+            package demo
+
+            import grails.gsp.TagLib
+
+            @TagLib
+            class ${className} {
+                static namespace = '${namespace}'
+
+                def ${tagName}(Map attrs) {
+                    out << 'hello'
+                }
+            }
+        """
+    }
+
+    private void writeSettings() {
+        String repo = System.getProperty('grails.e2e.localMavenRepo')
+        new File(projectDir, 'settings.gradle').text = """
+            pluginManagement {
+                repositories {
+                    maven { url = uri('${repo.replace('\\\\', '/')}') }
+                    gradlePluginPortal()
+                    mavenCentral()
+                }
+            }
+            dependencyResolutionManagement {
+                repositories {
+                    maven { url = uri('${repo.replace('\\\\', '/')}') }
+                    mavenCentral()
+                }
+            }
+            rootProject.name = 'taglib-index-incremental-app'
+        """
+    }
+
+    private void writeBuild() {
+        String version = System.getProperty('grails.e2e.version')
+        new File(projectDir, 'build.gradle').text = """
+            plugins {
+                id 'groovy'
+                id 'war'
+                id 'org.apache.grails.gradle.grails-gsp' version '${version}'
+            }
+
+            version = '0.1'
+            group = 'demo'
+
+            dependencies {
+                // The gsp plugin alone applies no BOM, so this names it the way an application does.
+                implementation platform('org.apache.grails:grails-bom:${version}')
+                implementation 'org.apache.grails.views:grails-web-taglib'
+                implementation 'org.apache.grails.views:grails-taglib'
+                implementation 'org.apache.grails.views:grails-gsp-core'
+            }
+        """
+    }
+}

gradle/rat-root-config.gradle

@@ -124,6 +124,7 @@ tasks.named('rat') {
             'grails-forge/**/src/main/resources/**', // src/main/resources are included in generated application and should not include a license
             'grails-forge/**/src/test/resources/**', // src/test/resources are used in tests against files included in generated application and should not include a license
             'grails-gradle/**/build/**', // grails-gradle does not have a build package name so exclude any build directories
+            'end-to-end/**/build/**', // its own build, so its build directories are not covered by the root exclude
             'grails-forge/*/build/**', // grails-forge build directories
             'grails-forge/build/**', // grails-forge build directories
             'grails-spring-security/plugin/src/main/templates/**', // template files that people are expected to use in the end application

grails-common/src/main/groovy/org/apache/grails/common/compiler/GroovyTransformOrder.groovy

@@ -228,4 +228,11 @@ interface GroovyTransformOrder {
      * contention, but a deterministic order keeps compilation output reproducible.
      */
     static final int COMMAND_FACTORIES_ORDER = RX_SCHEDULER_ORDER + DECREMENT_PRIORITY
+
+    /**
+     * Rewrites a call to a known tag into a direct invocation. Runs last, because whether a class can
+     * call tags at all is only settled once the traits that let it have been applied, which is what
+     * the artefact transforms above do.
+     */
+    static final int COMPILED_TAG_CALL_ORDER = COMMAND_FACTORIES_ORDER + DECREMENT_PRIORITY
 }

grails-core/src/main/groovy/org/grails/compiler/TagLibraryInvokerTypeCheckingExtension.groovy

@@ -69,6 +69,13 @@ import org.grails.core.artefact.ControllerArtefactHandler
  * without error. Type-safety for method calls on <em>declared</em> fields and
  * local variables is fully preserved.
  *
+ * <p>Calls to tags the tag library index knows never reach this extension at all: they are compiled
+ * into direct invocations before type checking runs, so the type checker sees ordinary resolved method
+ * calls. A misspelled tag is reported there instead, but only where the source says the call is a tag
+ * - one naming its namespace. An unqualified call is left dynamic and unjudged, here and there, since
+ * such a name may equally be a dynamic finder, an injected service method or anything else contributed
+ * while the application runs.
+ *
  * <p><strong>Composition with other extensions:</strong> because this is a catch-all
  * handler for unresolved calls in controllers and tag libraries, it must run <em>after</em> any other
  * type-checking extension that resolves DSL-style calls (e.g. a criteria extension).

grails-doc/src/en/guide/introduction/whatsNew.adoc

@@ -364,3 +364,47 @@ base name an application configures itself, neither of which Spring Boot's own r
 Plugin message bundles must now be namespaced on the plugin name — `spring-security-core.properties`
 rather than `messages.properties` — so that two plugins cannot shadow one another. See
 <<upgrading80x>> for the details.
+
+=== Compiled Tag Resolution
+
+Tag libraries are now described when they are compiled, and that description resolves tag calls in
+pages, tag libraries and controllers compiled afterwards. In a tag library or a controller, a call
+whose namespace and tag are known is compiled into a direct invocation rather than being dispatched
+through the metaclass, and no tag methods are installed onto tag library, page or dispatcher
+metaclasses to make dispatch work:
+
+[source,groovy]
+----
+class BookController {
+    def index() {
+        String markup = g.link(controller: 'book')   // compiled into a direct invocation
+    }
+}
+----
+
+The same applies to a call written without a namespace, to calls written inside a closure such as a
+tag body, and to a tag expression in a GSP declaring `compileStatic`. The tag itself is still selected
+by name when the call runs, so a tag library that overrides another, one registered while the
+application is running, and the order tag libraries are registered in all behave exactly as before. A
+namespace no compiled tag library declares, and a name something else in scope answers to, are left to
+dispatch as they did.
+
+A tag that no compiled tag library declares is left to resolve at runtime with nothing reported, since
+a namespace can legitimately hold tag libraries carrying no description. An application whose tag
+libraries are all described can ask for an error instead:
+
+[source,groovy]
+.build.gradle
+----
+grails {
+    compileStatic {
+        strictTags = true
+        dynamicTagNamespaces = ['legacy']   // registered while the application runs
+    }
+}
+----
+
+Defining a tag as a `Closure` field remains supported but is deprecated and now warns at compile time:
+a closure carries no signature, so nothing about a call to such a tag can be checked. Define tags as
+methods taking `Map attrs` and, where a body is needed, `Closure body`. See
+link:theWebLayer.html#compiledTags[Compiled Tag Resolution].