jdaugherty commented on code in PR #16134:
URL: https://github.com/apache/grails-core/pull/16134#discussion_r3827012338
##########
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:
This section documents the behaviour the last commit removed.
`CompiledTagInvocation.invoke` now hands a tag the running application has not
registered back to the namespace dispatcher, which ends in
`MissingMethodException` — exactly the cases this section lists (excluded
plugin, `nonEnhancedTagLibClasses`, a unit test mocking only some tag
libraries), and the method's own javadoc says resolving the call must not turn
the exception into something else.
The advice here — switch `catch (MissingMethodException)` to `catch
(GrailsTagException)` — is now wrong; following it would break the very
fallback the change preserved. `GrailsTagException` still arises when a page
has no tag library lookup at all, but that is a different case and not what
this section describes.
Suggest deleting the section, or shrinking it to a sentence stating that a
resolved call reports an unregistered tag exactly as the dynamic path did, so
`respondsTo` probes and `MissingMethodException` handlers keep working.
##########
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:
This paragraph records the explanation that turned out to be wrong. The
re-added convention case in `ControllerTagCallRewriteSpec` passes on macOS,
Linux and Windows, and its trait assertion always held on both platforms —
recognising a controller by its location does not depend on where the
compilation happens. What varied was the rewrite, because at `CANONICALIZATION`
the transform read the class before a locally-arriving trait had been applied.
The spec still earns its keep as the whole-build check — index generated,
packaged, on the compile classpath, transform applied — but this second
paragraph should tell the true story rather than the one the fix disproved.
##########
grails-gsp/grails-web-taglib/src/test/groovy/org/grails/taglib/index/TagLibraryIndexGeneratorSpec.groovy:
##########
@@ -0,0 +1,300 @@
+/*
+ * 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.nio.file.Files
+import java.nio.file.Path
+
+import spock.lang.Specification
+import spock.lang.TempDir
+
+/**
+ * Generating the index for a whole source set, rather than accumulating it as
each class compiles,
+ * is what allows a renamed or deleted tag library to disappear from it.
+ */
+class TagLibraryIndexGeneratorSpec extends Specification {
+
+ @TempDir
+ Path tempDir
+
+ Path sources
+ Path output
+
+ def setup() {
+ sources = Files.createDirectories(tempDir.resolve('src'))
+ output = Files.createDirectories(tempDir.resolve('out'))
+ }
+
+ void 'a closure based tag is recorded as such'() {
+ given:
+ write('Legacy.groovy', '''
+ import grails.gsp.TagLib
+ @TagLib
+ class LegacyTagLib {
+ static namespace = 'legacy'
+ def asMethod(Map attrs) { }
+ Closure asClosure = { Map attrs -> }
+ }
+ ''')
+
+ when:
+ TagLibraryIndexGenerator.generate(sources.toFile(), output.toFile(),
true, 'UTF-8')
+
+ then: 'the closure form is marked so that callers keep dispatching it
dynamically'
Review Comment:
The label still says the closure form is "marked", but the marking is what
this round removed — the assertion now checks a plain name list. While here:
the new `a class pulled in only to resolve a type is not described` method is
missing the blank line separating it from the helper above it.
##########
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:
This claim is no longer true, and the behaviour it justifies now diverges
from the path it cites. `methodMissingForTagLib` no longer reduces an unmatched
argument list to an empty invocation: `matchesTagShape` declines it, and the
call falls through to the overload lookup and then to `MissingMethodException`.
Here an unmatched list still becomes `attrs = [:]`, `body = null`, and runs the
tag with nothing.
The rewriter's `forwardableShape` means no bytecode this branch emits can
reach the default case any more, but
`invokeArguments`/`invokeArgumentsInContext` are public entry points. Either
make the unmatched shapes match the new dynamic behaviour — decline and
dispatch dynamically, the way an unregistered tag already is — or rewrite the
comment to say the empty invocation survives only for direct callers of the
public API, and why that is acceptable.
##########
grails-gsp/grails-taglib/src/main/groovy/org/grails/taglib/discovery/TagDiscoveryRules.java:
##########
@@ -0,0 +1,187 @@
+/*
+ * 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.LinkedHashSet;
+import java.util.Set;
+
+/**
+ * Decides whether a method is a tag.
+ *
+ * <p>The single statement of those rules. Both the reflective discovery an
application performs at
+ * startup and the syntax-tree discovery a build performs while compiling a
tag library route through
+ * here, so the two cannot drift apart: a method is a tag for a compiler
exactly when it is a tag for
+ * the runtime.
+ *
+ * <p>The rules, in order:
+ * <ol>
+ * <li>plumbing — non-public, static, or compiler-generated methods are never
tags;</li>
+ * <li>{@code @NotATag} excludes, {@code @Tag} includes, each overriding
everything below;</li>
+ * <li>names belonging to Object, Groovy, or the framework traits are never
tags;</li>
+ * <li>property accessors are never tags;</li>
+ * <li>what remains is a tag if it can be called as {@code (attrs)} or {@code
(attrs, body)}.</li>
+ * </ol>
+ *
+ * @since 8.0.0
+ */
+public final class TagDiscoveryRules {
+
+ /**
+ * The name a {@link java.util.Map} parameter must carry to be the
attributes parameter, when the
+ * method retains parameter names.
+ */
+ public static final String ATTRS_PARAMETER_NAME = "attrs";
+
+ /**
+ * The name a {@link groovy.lang.Closure} parameter must carry to be the
body parameter, when the
+ * method retains parameter names.
+ */
+ public static final String BODY_PARAMETER_NAME = "body";
+
+ /**
+ * Names that are Groovy or Object plumbing on any class.
+ */
+ private static final Set<String> LANGUAGE_METHOD_NAMES = Set.of(
+ "invokeMethod", "methodMissing", "propertyMissing", "getProperty",
"setProperty",
+ "getMetaClass", "setMetaClass", "equals", "hashCode", "toString");
+
+ /**
+ * Names every tag library carries through the framework traits and
lifecycle interfaces.
+ */
+ private static final Set<String> FRAMEWORK_METHOD_NAMES = Set.of(
+ "afterPropertiesSet",
+ "currentRequestAttributes",
+ "destroy",
+ "initializeTagLibrary",
+ "onApplicationEvent",
+ "raw",
+ "throwTagError",
+ "withCodec");
+
+ private TagDiscoveryRules() {
+ }
+
+ /**
+ * @return the names that are never tags, whatever their shape
+ */
+ public static Set<String> getFrameworkMethodNames() {
+ return FRAMEWORK_METHOD_NAMES;
+ }
+
+ /**
+ * @param method the method to classify
+ * @return true if the method can be invoked as a tag
+ */
+ /**
+ * Finds every tag a tag library declares, from either view of it.
+ *
+ * <p>The two kinds are enumerated differently, because the runtime
dispatches them differently. A
+ * method tag is read from the declaring class alone, since dispatch scans
declared methods and an
+ * inherited one is not callable as a tag. A closure tag is read up the
whole hierarchy, since
+ * dispatch finds it as a property and a property is inherited.
+ *
+ * @param view the tag library, from a syntax tree or from a compiled class
+ * @return every tag name the library declares
+ */
+ public static Set<String> findTags(TagLibraryView view) {
Review Comment:
`findTags` was inserted between `isTagMethod`'s javadoc and `isTagMethod`
itself, so the `@param method the method to classify` block now dangles above
this method's own javadoc and `isTagMethod` is left undocumented.
The same slip happened twice more in this round:
`TagLibraryIndex.isStrict`'s javadoc (ending `@return true when the build set
grails.compileStatic.strictTags`) now sits stranded above
`rewritesUnqualifiedCalls`, and `GrailsCompileStaticOptions.strictTags`'s doc
(ending `@since 8.0`) sits above the new `unqualifiedTagCalls` property. Same
fix in each file: place the new member after the one whose javadoc it split, or
move the stranded block back onto its member.
##########
grails-gsp/grails-taglib/src/main/groovy/org/grails/taglib/TagLibraryMetaUtils.groovy:
##########
@@ -199,6 +220,30 @@ class TagLibraryMetaUtils {
existingMethod instanceof CachedMethod
}
+ /**
+ * Whether an argument list is one a tag can be called with.
+ *
+ * <p>A tag takes attributes, a body, or both, which is none, one, or two
arguments whose first is
+ * a Map. Anything else the switch below reduces to a call with no
attributes and no body, silently
+ * dropping what was written - so a name that is both a tag and an
ordinary overload, a tag
+ * {@code foo(Map)} beside a helper {@code foo(String, String)}, would run
the tag with nothing.
+ * Such a call is left to the method lookup further down, which finds the
overload.
+ *
+ * @param args the arguments the call was made with
+ * @return true when the call can be treated as a tag invocation
+ */
+ private static boolean matchesTagShape(Object[] args) {
Review Comment:
Non-blocking observation. The gate closes the shapes a tag cannot take, but
the overlapping shapes still prefer the tag over a real overload the old call
site reached. One CharSequence argument is a valid tag body, so `format('x')`
beside `def format(String)` takes the tag branch where
`tagLibrary.invokeMethod` used to find the overload; the same holds for no
arguments beside a zero-argument helper, and for `(Map, anything)` beside a
`(Map, List)` helper.
I don't think there is a better rule — those shapes are legitimate tag
calls, and preferring any matching overload would dispatch the tag's own
`(Map)` method without output capture — but the choice deserves to be pinned: a
spec row for the one-argument overload case, and a sentence here saying
overlapping shapes deliberately resolve to the tag, matching how a GSP has
always dispatched them.
--
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]
