codeconsole commented on code in PR #16139:
URL: https://github.com/apache/grails-core/pull/16139#discussion_r3836309278
##########
grails-gsp/core/src/main/groovy/org/grails/gsp/compiler/tags/GroovyDefTag.java:
##########
@@ -59,10 +59,35 @@ public void doStartTag() {
if (typeName.equals("def") || typeName.equals("Object")) {
out.println(expr);
} else {
- out.println(typeName + ".cast(" + expr + ")");
+ // A Groovy cast rather than Class.cast, which only accepts what
is already of the type
+ // and so rejects every conversion Groovy would otherwise have
made. The expression is
+ // parenthesised because it is written by the page and may be a
GString or an operation.
+ out.println("(" + typeName + ") (" + expr + ")");
}
}
+ /**
+ * The Groovy an attribute value stands for.
+ *
+ * <p>A lone <code>${...}</code> is the expression it holds, and text with
no expression in it is
+ * read as written, which is what an untyped tag naming a variable relies
on. Text mixing the two,
+ * or holding more than one expression, is neither: it is a GString, and
emitting it unquoted
+ * produced source that did not parse.
+ */
+ private static String groovyExpressionFor(String value) {
+ String text = value.trim();
+ if ((text.startsWith("\"") && text.endsWith("\"")) ||
(text.startsWith("'") && text.endsWith("'"))) {
+ text = text.substring(1, text.length() - 1).trim();
+ }
+ if (!text.contains("${")) {
+ return text;
+ }
+ if (text.startsWith("${") && text.endsWith("}") && text.indexOf("${",
2) < 0) {
+ return text.substring(2, text.length() - 1).trim();
+ }
+ return '"' + text.replace("\\", "\\\\").replace("\"", "\\\"") + '"';
Review Comment:
Both reproduce, and your fix is the right one — taken as written in 05e11cc.
The nested-quote page failed to compile and the escaped dollar rendered `cost:
\1`; both render correctly now, and the mixed-text, two-expression and
plain-variable cases still do. Two rows added to the render table.
You are right about the cause: I was escaping a value the parser had already
made Groovy.
##########
grails-gsp/core/src/main/groovy/org/grails/gsp/compiler/GroovyPageTypeCheckingExtension.groovy:
##########
@@ -39,40 +41,91 @@ import org.codehaus.groovy.transform.stc.StaticTypesMarker
*/
class GroovyPageTypeCheckingExtension extends
GroovyTypeCheckingExtensionSupport.TypeCheckingDSL {
+ /**
+ * The operators the class writer emits directly, rather than leaving to a
call site.
+ *
+ * <p>These are the ones that cannot be handed a receiver of no known
type. A comparison or a
+ * logical operator is not among them: those report a type error of their
own, which is an answer
+ * a page can act on, so the receiver of one is still resolved the way
anything else is.</p>
+ */
+ private static final Set<Integer> WRITTEN_INTO_THE_CLASS = [
+ Types.LEFT_SQUARE_BRACKET,
+ Types.PLUS, Types.MINUS, Types.MULTIPLY, Types.DIVIDE,
Types.INTDIV, Types.MOD, Types.POWER,
+ Types.PLUS_EQUAL, Types.MINUS_EQUAL, Types.MULTIPLY_EQUAL,
Types.DIVIDE_EQUAL] as Set
Review Comment:
All of it reproduces — `%` giving `remainder()`, the shifts, the bitwise
operators, the compound assignments, and `<=>` crashing canonicalization. Fixed
in 05e11cc by listing the tokens rather than the symbols: `REMAINDER`,
`COMPARE_TO`, the three shifts, the three bitwise operators and the compound
forms. `MOD` and `INTDIV` stay, harmlessly, since nothing emits them.
One correction to the last line of your comment. Adding `COMPARE_TO` does
not turn `<=>` into the page-level message — it puts that message *first*, and
the transformer still fails afterwards with the same index out of bounds.
Reporting the receiver does not stop the transformer that reads it from
running. A test states that ordering rather than asserting a clean failure.
Thirteen operators are pinned reporting against an untyped receiver, and
twelve compiling once it has a type. `**=` is the exception either way: with a
type it fails with `Cannot assign value of type java.lang.Number`, which is
ordinary static-compilation behaviour since `power()` returns `Number`, not
over-reporting.
--
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]
