codeconsole commented on code in PR #16134:
URL: https://github.com/apache/grails-core/pull/16134#discussion_r3824087803


##########
grails-gsp/plugin/src/ast/groovy/grails/compiler/traits/CompiledTagCallTransformation.groovy:
##########
@@ -0,0 +1,98 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package grails.compiler.traits
+
+import groovy.transform.CompileStatic
+import org.codehaus.groovy.ast.ASTNode
+import org.codehaus.groovy.ast.ClassHelper
+import org.codehaus.groovy.ast.ClassNode
+import org.codehaus.groovy.ast.ModuleNode
+import org.codehaus.groovy.control.CompilePhase
+import org.codehaus.groovy.control.SourceUnit
+import org.codehaus.groovy.transform.ASTTransformation
+import org.codehaus.groovy.transform.GroovyASTTransformation
+
+import grails.artefact.gsp.TagLibraryInvoker
+import grails.gsp.taglib.compiler.CompiledTagCallRewriter
+import org.grails.taglib.index.TagLibraryIndex
+
+/**
+ * Compiles a call to a known tag into a direct invocation, wherever tags can 
be called from.
+ *
+ * <p>A tag library rewrites its own calls as it is compiled, but a controller 
can call tags too, and
+ * gains that ability from the {@link TagLibraryInvoker} trait rather than 
from being a tag library.
+ * Any class carrying that trait is therefore a candidate, which covers 
controllers without naming
+ * them and without a second copy of the rewriting rules. A compiled GSP calls 
tags as well, and
+ * reaches them through {@code GroovyPage} rather than through the trait, so 
it is matched separately.
+ *
+ * <p>Runs after trait injection, since whether a class can call tags is only 
settled once its traits
+ * have been applied.
+ *
+ * @since 8.0.0
+ */
+@CompileStatic
+@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION)
+class CompiledTagCallTransformation implements ASTTransformation {

Review Comment:
   Fixed. The rewrite runs at `INSTRUCTION_SELECTION` now rather than at 
`CANONICALIZATION` with a priority. Priority orders only transforms that 
declare one, and a trait arriving from a local transform is applied after every 
global transform has run, so the class was read before it carried the trait. In 
a later phase every trait has been applied, whichever transform supplied it. 
Your `AnnotatedController` is rewritten; `ControllerTagCallRewriteSpec` pins 
that, and a new case in `CompiledTagCallTransformationOrderSpec` pins the phase.
   
   It was a live bug rather than only a gap for classes outside the directory: 
the same race decided the conventional case by platform. `CompiledTagCallSpec` 
passed on macOS and failed on Linux, which is what the functional jobs here 
were failing on.



##########
grails-gsp/plugin/src/test/groovy/org/grails/web/taglib/ControllerTagCallRewriteSpec.groovy:
##########
@@ -0,0 +1,88 @@
+/*
+ *  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.web.taglib
+
+import java.nio.file.Files
+import java.nio.file.Path
+
+import org.codehaus.groovy.control.CompilationUnit
+import org.codehaus.groovy.control.CompilerConfiguration
+import spock.lang.Specification
+import spock.lang.TempDir
+
+/**
+ * A controller can call tags too, through the tag library invoker trait 
rather than by being a tag
+ * library, so the same rewriting has to reach it.
+ *
+ * <p>Checked in the class file, because a rewritten call and a dynamic one 
produce the same output.
+ */
+class ControllerTagCallRewriteSpec extends Specification {
+
+    @TempDir
+    Path tempDir
+
+    void 'a class that can call tags has its tag calls compiled into 
invocations'() {
+        when: 'a class carrying the tag library invoker trait, as a controller 
does'
+        byte[] compiled = compile('''
+            import grails.artefact.gsp.TagLibraryInvoker
+            class TagCallingController implements TagLibraryInvoker {

Review Comment:
   The `@Artefact('Controller')` case asserts the rewrite now rather than its 
absence, since the transform moved to `INSTRUCTION_SELECTION`. The 
`grails-app/controllers` case is covered by `CompiledTagCallSpec`, which reads 
a real build's output rather than compiling into a temporary directory — that 
is the one that was failing on Linux.



-- 
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]

Reply via email to