sbglasius commented on PR #16139:
URL: https://github.com/apache/grails-core/pull/16139#issuecomment-5360379358
Re-checked `1f63d1a784` by compiling probe pages against `86c4f060ef` and
the fix commit rather than reading the diff.
**Confirmed fixed:** the `var`/`status` regex (`<g:set value="${1 > 0}"
var="flag"/>` failed with *"The variable [flag] is undeclared."* before,
compiles now — and the quote-aware alternation is disjoint on its first
character, so no backtracking risk); the operator-error dedup; the misplaced
javadoc.
One correction to my earlier list: my "same expression reported twice" point
was wrong. `<g:set var="rows" value="${[1,2]}"/>${rows[0]}` emits exactly one
error both before and after. Disregard it.
**New regression from the typed-`g:set` fix.** `typedSetVariables` is a flat
per-page set with no notion of the generated block scope, so a second typed
`g:set` in a sibling scope emits a bare assignment where the variable was never
declared — it resolves through the page's `setProperty` and the type is
silently lost:
```gsp
<g:if test="${true}"><g:set type="int" var="n" value="${1}"/>${n + 1}</g:if>
<g:else><g:set type="int" var="n" value="${2}"/>${n + 1}</g:else>
```
Compiles at `86c4f060ef`; at `1f63d1a784` the else branch fails with *"The
type of [n] is not known here, and an operator cannot be applied to it."*
Declaring inside a `<g:if>` and typing again at top level fails the same way.
Keying by emission depth (`tagMetaStack` size / `closureLevel`) and only
reusing the assignment form when the earlier declaration encloses the current
position would handle both and keep the same-scope case the new test covers.
**On framework names as model keys** — agreed, breaking change to document
rather than a blocker. Two things I checked that shape the note:
A model key does shadow the implicit variable today:
`TemplateVariableBinding.getVariable` checks the page's own binding (where
`GroovyPageWritable.createBinding` puts the model) before the parent
`WebRequestTemplateVariableBinding`. With a real web request, `application`,
`session`, `namespace`, `servletContext`, `params`, `flash` and `response` all
returned the model value.
But the exposure is narrower than I first said: on the base, `<%@ page
compileStatic="true" %>${application}` already fails with *"The variable
[application] is undeclared."* for all seven, model key or not. So this is a
pure improvement for pages already on `compileStatic="true"`; only dynamic
pages newly opted in via `grails.views.gsp.compileStatic` are affected.
Worth calling out in the note: `${application.name}` is a compile error the
build catches, but bare `${application}` compiles clean and throws
`GroovyCastException` at render — `compileGroovyPages` stays green.
Also: the guide's GSP list is `application, applicationContext, flash,
grailsApplication, out, params, request, response, session, webRequest`.
`controllerName`, `actionName`, `namespace` and `servletContext` are documented
only for tag libraries. Being `String`, the first three only fail on a
non-String model value.
Suggested addition to the existing NOTE under "Enabling Static Compilation
for Every Page":
> A model variable sharing a name with one the framework supplies (`params`,
`request`, `response`, `session`, `application`, `servletContext`, `flash`,
`webRequest`, `controllerName`, `actionName`, `namespace`) is no longer read as
the model supplied it — the page reads the framework value's type. Reading a
member the framework type does not have is a compilation error; passing a value
of another type fails at render with a `GroovyCastException`, so it will not be
caught by precompilation. Rename the model key, or declare it in the `model`
directive to keep your own type.
**Minor, still open:** the `compileOnly` comment in
`grails-gsp/core/build.gradle` sits above an empty block (only
`testImplementation` entries were added);
`GrailsGspCompileStaticProvider.isStrict()` is `@Input` but is ignored by
`asArguments()` when `gsp` is off, so toggling `strictGsp` invalidates tasks
for no change in output.
--
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]
