codeconsole commented on code in PR #16134:
URL: https://github.com/apache/grails-core/pull/16134#discussion_r3827397688
##########
grails-doc/src/en/guide/upgrading/upgrading80x.adoc:
##########
@@ -2632,3 +2632,190 @@ used to apply to its own message source.
Adding or removing a base name now needs a restart, because Spring Boot reads
the configured base names
once when it builds the message source.
+
+==== 48. Tag Libraries Are Described When Compiled
+
+Tag calls are resolved against a description each tag library contributes as
it is compiled, and a
+resolved call is compiled into a direct invocation. The tag itself is still
selected by name when the
+call runs, through the same lookup dynamic dispatch uses, so a tag library
that overrides another and
+the order tag libraries are registered in behave as before. A call into a
namespace no compiled tag
+library declares — a tag library from a plugin built against an earlier
version of Grails, or one
+registered while the application runs — is left to dispatch exactly as it did.
+
+Three things are worth knowing when upgrading.
+
+A tag that no compiled tag library declares is left to resolve at runtime, and
nothing is reported.
+Setting `grails { compileStatic { strictTags = true } }` makes it a
compilation error instead, which
+is worth doing once to find misspelled tags, but is not the default because a
namespace can
+legitimately hold tag libraries that carry no description. Where an
application registers tag
+libraries while it runs, name their namespaces in
+`grails { compileStatic { dynamicTagNamespaces = [...] } }` so that they are
never checked.
+
+Strict checking applies only where the source says a call is a tag: one naming
its namespace, and one
+written as markup. A call written without a namespace is never checked, and a
namespaced expression in
+a page is checked only when that page declares `compileStatic`.
+
+A call written without a namespace is not compiled at all unless the build
asks for it with
+`grails { compileStatic { unqualifiedTagCalls = true } }`. Whether a bare name
is a tag depends on what
+else answers to it, and not all of that is visible when compiling — a method
Groovy gives every object,
+a delegate an enclosing closure is handed, an overload the tag library also
declares — so by default
+such a call is dispatched exactly as it was before this release.
+
+Where it is turned on, a call written without a namespace reaches a tag only
when nothing nearer
+answers to the name: not a method of the class, not one it inherits, not a
field, property or local,
+not a method Groovy provides, and not a name inside a closure, whose delegate
is only known when it
+runs. A method added to a controller or tag library *while the application
runs*, through a plugin's
+`doWithDynamicMethods`, is not visible when the calling code is compiled, so a
call that used to reach
+such a method and shares its name with a tag would reach the tag instead.
Declare the method on the
+class, name the namespace in `dynamicTagNamespaces`, or call the tag with its
namespace.
+
+Naming a namespace in `dynamicTagNamespaces` turns rewriting off for it
entirely, not just the
+reporting: calls into it are dispatched exactly as they were before this
release. That is the escape
+hatch for a namespace whose tags are decided while the application runs.
+
+Tags defined as closures now warn at compile time. They still work and are
called the same way, but a
+closure carries no signature, so nothing about a call to such a tag can be
checked. This covers the
+`def` form as well as the explicitly typed one, and the `def` form is the one
most tag libraries are
+written in:
+
+[source,groovy]
+----
+// Before - both forms warn
+def hello = { attrs ->
+ out << "Hello ${attrs.name}"
+}
+
+Closure goodbye = { Map attrs ->
+ out << "Goodbye ${attrs.name}"
+}
+
+// After
+def hello(Map attrs) {
+ out << "Hello ${attrs.name}"
+}
+
+def goodbye(Map attrs) {
+ out << "Goodbye ${attrs.name}"
+}
+----
+
+A tag taking a body becomes a method with a second `Closure body` parameter:
+
+[source,groovy]
+----
+// Before
+def wrapped = { attrs, body ->
+ out << '<div>' << body() << '</div>'
+}
+
+// After
+def wrapped(Map attrs, Closure body) {
+ out << '<div>' << body() << '</div>'
+}
+----
+
+===== Tags Are No Longer Installed Onto Metaclasses
+
+Dispatching a tag no longer works by installing a method for every tag, and a
property for every
+namespace, onto the metaclass of every tag library, controller and page. Tags
are resolved through the
+tag library lookup instead. Calling a tag — from a page, a tag library or a
controller, with or
+without its namespace — is unaffected.
+
+What changes is code that inspected the metaclass to find tags. A check such as
+
+[source,groovy]
+----
+tagLib.metaClass.respondsTo(tagLib, 'someTag')
+----
+
+answered `true` before because the tag had been installed there, and now
answers `false`. Call the tag,
+or consult the tag library lookup, instead of asking the metaclass what it
holds.
+
+The methods that performed the installation are deprecated or removed:
+
+[cols="2,3"]
+|===
+|Member |Replacement
+
+|`NamespacedTagDispatcher.initializeMetaClass()`
+|Removed. Nothing needs to be initialised.
+
+|`NamespacedTagDispatcher.registerTagMetaMethods(ExpandoMetaClass)`
+|Removed.
+
+|`TemplateNamespacedTagDispatcher.registerTagMetaMethods(ExpandoMetaClass)`
+|Removed.
+
+|`GroovyPagesMetaUtils.registerMethodMissingForGSP(...)`
+|Retained but does nothing. A page now declares `methodMissing` itself.
+
+|`TagLibraryMetaUtils.enhanceTagLibMetaClass`, `registerTagMetaMethods`,
`registerMethodMissingForTags`, `registerNamespaceMetaProperties`,
`registerPropertyMissingForTag`, `addTagLibMethodToMetaClass`
+|Deprecated. Unit test support still uses them so that a tag method can be
called directly on a tag library under test.
+|===
+
+`TagLibraryMetaUtils.methodMissingForTagLib` is not deprecated — it is the
dynamic dispatch path a call
+into an undescribed namespace still takes.
+
+===== A Method-Declared Tag Called Without a Namespace Returns Its Output
+
+A class that can call tags but is not a tag library — a controller — reaches a
tag written without a
+namespace through `methodMissing` on the `TagLibraryInvoker` trait. That used
to end in a direct call
+on the tag library bean. For a tag declared as a closure this made no
difference, because the call
+landed on a generated wrapper that captured output anyway; for one declared as
a method there was no
+wrapper, so the method ran with nothing captured and its own return value came
back.
+
+Both forms now capture, so a method-declared tag returns what it wrote:
+
+[source,groovy]
+----
+class ReportTagLib {
+ static namespace = 'g'
+
+ def summary(Map attrs) {
+ out << 'the output'
+ 'the return value' // <1>
+ }
+}
+
+class ReportController {
+ def index() {
+ String result = summary(id: 1) // 'the output', previously 'the
return value'
+ }
+}
+----
+<1> a tag's return value is not what a caller receives; what it writes is
+
+A tag called *with* its namespace, and any tag called from a GSP, already
captured, so only an
+unqualified call from a controller to a method-declared tag changes.
+
+===== A Tag the Runtime Cannot Resolve Reports a Different Exception
Review Comment:
Deleted in c2c45fb. There is no upgrade note left to write: a resolved call
reports an unregistered tag exactly as the dynamic path did, so nothing changes
for a caller.
##########
grails-gsp/grails-taglib/src/main/groovy/org/grails/taglib/CompiledTagInvocation.java:
##########
@@ -0,0 +1,192 @@
+/*
+ * 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;
+
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.Map;
+
+import groovy.lang.Closure;
+import groovy.lang.MissingMethodException;
+
+import org.grails.taglib.encoder.OutputContext;
+import org.grails.taglib.encoder.OutputContextLookupHelper;
+
+/**
+ * Invokes a tag whose namespace and name are known without going through
Groovy's method dispatch.
+ *
+ * <p>Calling a tag as {@code g.message(code: 'x')} reaches the tag library
through {@code
+ * invokeMethod}, which means a dynamic call site in the caller's bytecode
even when that caller is
+ * statically compiled. The tag being called is fixed in the source, so once
it has been resolved
+ * against the tag library index there is nothing left to decide at runtime
beyond which bean holds it.
+ *
+ * <p>This is the entry point such a call is expressed as: an ordinary method
call taking the
+ * namespace and name as arguments. It applies the same attribute and body
handling, output capture,
+ * encoding and return-object behaviour as the dynamic path, because both end
at
+ * {@link TagOutput#captureTagOutput}.
+ *
+ * @since 8.0.0
+ */
+public final class CompiledTagInvocation {
+
+ private static final Object[] EMPTY_ARGUMENTS = new Object[0];
+
+ private CompiledTagInvocation() {
+ }
+
+ /**
+ * Invokes a tag with attributes and a body.
+ *
+ * @param lookup the tag libraries available to the caller
+ * @param namespace the tag library namespace
+ * @param tagName the tag name within that namespace
+ * @param attrs the tag attributes, treated as empty when {@code null}
+ * @param body the tag body as a closure or as text, or {@code null} when
there is none
+ * @return whatever the tag produces, which for a tag that writes to the
output is its output
+ */
+ public static Object invoke(TagLibraryLookup lookup, String namespace,
String tagName,
+ Map<?, ?> attrs, Object body) {
+ return invoke(lookup, namespace, tagName, attrs, body,
+ OutputContextLookupHelper.lookupOutputContext());
+ }
+
+ /**
+ * Invokes a tag against a known output context, for a caller that already
has one to hand.
+ *
+ * @param lookup the tag libraries available to the caller
+ * @param namespace the tag library namespace
+ * @param tagName the tag name within that namespace
+ * @param attrs the tag attributes, treated as empty when {@code null}
+ * @param body the tag body as a closure or as text, or {@code null} when
there is none
+ * @param outputContext where the tag writes
+ * @return whatever the tag produces
+ */
+ public static Object invoke(TagLibraryLookup lookup, String namespace,
String tagName,
+ Map<?, ?> attrs, Object body, OutputContext outputContext) {
+ if (lookup == null) {
+ throw new GrailsTagException("Tag [" + tagName + "] cannot be
invoked without a tag library lookup");
+ }
+ Map<?, ?> attributes = attrs != null ? attrs : Collections.emptyMap();
+ // A body may be a closure or the text a caller wrote directly, which
the dynamic path accepted
+ // through overloads that wrapped the text. Narrowing this to Closure
would turn a string body
+ // into a cast failure.
+ Object tagBody = body instanceof CharSequence ? new
TagOutput.ConstantClosure((CharSequence) body) : body;
+ if (lookup.lookupTagLibrary(namespace, tagName) == null) {
+ return dispatchUnregistered(lookup, namespace, tagName,
attributes, tagBody);
+ }
+ return TagOutput.captureTagOutput(lookup, namespace, tagName,
attributes, tagBody, outputContext);
+ }
+
+ /**
+ * Hands a tag the index knows but the running application has not
registered back to the dispatch
+ * that would have run had the call never been resolved.
+ *
+ * <p>The index describes what a tag library declares when it is compiled,
which is not the same
+ * question as what a running application registers: a plugin can be
excluded, a tag library can be
+ * named in {@code nonEnhancedTagLibClasses}, and a unit test can mock
some tag libraries and not
+ * others. The dynamic path reported that as a {@link
MissingMethodException}, and code written
+ * around a tag call catches it or probes with {@code respondsTo}, so
resolving the call must not
+ * turn it into something else. Dispatching through the namespace rather
than raising the exception
+ * here also keeps the type it names the one that path named.
+ */
+ private static Object dispatchUnregistered(TagLibraryLookup lookup, String
namespace, String tagName,
+ Map<?, ?> attrs, Object body) {
+ Object[] arguments;
+ if (body != null) {
+ arguments = new Object[] {attrs, body};
+ }
+ else if (!attrs.isEmpty()) {
+ arguments = new Object[] {attrs};
+ }
+ else {
+ arguments = EMPTY_ARGUMENTS;
+ }
+ NamespacedTagDispatcher dispatcher =
lookup.lookupNamespaceDispatcher(namespace);
+ if (dispatcher == null) {
+ throw new MissingMethodException(tagName,
CompiledTagInvocation.class, arguments);
+ }
+ return dispatcher.invokeMethod(tagName, arguments);
+ }
+
+ /**
+ * Invokes a tag with whatever arguments the call was written with.
+ *
+ * <p>A tag call is written in more shapes than attributes and a body:
with nothing, with a body
+ * alone, or with a single value that the tag reads under its own name.
Where the shape is not
+ * evident in the source - a map held in a variable, say - the arguments
are only known once they
+ * have been evaluated, which is what this takes.
+ *
+ * @param lookup the tag libraries available to the caller
+ * @param namespace the tag library namespace
+ * @param tagName the tag name within that namespace
+ * @param args the evaluated arguments, in the order they were written
+ * @return whatever the tag produces
+ */
+ public static Object invokeArguments(TagLibraryLookup lookup, String
namespace, String tagName,
+ Object... args) {
+ return invokeArgumentsInContext(lookup, namespace, tagName,
+ OutputContextLookupHelper.lookupOutputContext(), args);
+ }
+
+ /**
+ * Invokes a tag with whatever arguments the call was written with,
against a known output context.
+ *
+ * @param lookup the tag libraries available to the caller
+ * @param namespace the tag library namespace
+ * @param tagName the tag name within that namespace
+ * @param outputContext where the tag writes
+ * @param args the evaluated arguments, in the order they were written
+ * @return whatever the tag produces
+ */
+ public static Object invokeArgumentsInContext(TagLibraryLookup lookup,
String namespace,
+ String tagName, OutputContext outputContext, Object... args) {
+ Object[] arguments = args != null ? args : EMPTY_ARGUMENTS;
+ Map<?, ?> attrs = Collections.emptyMap();
+ Object body = null;
+ // Deliberately the same shapes, in the same order, as the dynamic
dispatch in
+ // TagLibraryMetaUtils.methodMissingForTagLib, including its treatment
of argument lists that
+ // match none of them: a call that produced an empty invocation there
must produce one here.
Review Comment:
Confirmed and fixed in c2c45fb — I chose the first option. Two arguments
whose first is not a Map, or three or more, now go back through the namespace
dispatcher, which is where an unregistered tag already goes, so a shape a tag
cannot take resolves as it would have unresolved. Two spec rows pin it.
You are right that no bytecode this branch emits reaches it, but leaving
public entry points running a tag with nothing where dispatch would have found
an overload was the wrong half to keep.
##########
grails-test-examples/app1/src/test/groovy/functionaltests/CompiledTagCallSpec.groovy:
##########
@@ -0,0 +1,65 @@
+/*
+ * 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 functionaltests
+
+import spock.lang.Specification
+
+/**
+ * A tag call in a controller declared by convention is compiled into a direct
invocation.
+ *
+ * <p>Everything else that asserts this compiles a source in isolation, which
proves the transform
+ * works but not that a real project reaches it: the index has to be
generated, packaged, placed on
+ * the compile classpath and read, and the transform has to run after the
trait that makes the class
+ * able to call tags has been applied. This reads the class file this project
actually produced.
+ *
+ * <p>It also covers ground a synthetic compilation cannot. An earlier spec
drove the convention path
+ * by writing a source into a temporary {@code grails-app/controllers}
directory; it passed on macOS
+ * and failed on Linux and Windows, because recognising a controller by its
location depends on where
Review Comment:
Rewritten in 79a69b3. It now says the rewrite read the class before a
locally-arriving trait had been applied, and that this spec is what reported
it, rather than the location explanation the fix disproved.
--
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]
