jamesfredley commented on code in PR #15398:
URL: https://github.com/apache/grails-core/pull/15398#discussion_r2824971738
##########
grails-gsp/core/src/main/groovy/org/grails/gsp/compiler/GroovyPageParser.java:
##########
@@ -463,14 +463,11 @@ public void writeLineNumbers(File filename) throws
IOException {
}
private void declare(boolean gsp) {
- if (finalPass) {
- return;
- }
-
- out.println();
- write(scan.getToken().trim(), gsp);
Review Comment:
The old `declare()` was **not** throwing an error. It was calling
`write(scan.getToken().trim(), gsp)` during pass 1, which silently emits
content to the output stream. Because pass 2 (where the class body is
generated) hasn't run yet, this content ends up placed between the `package`
statement and the generated `class` declaration:
```groovy
// Generated code (broken):
package com.example.demo
int counter = 0; // written by declare() in pass 1
class DemoPage extends GroovyPage {
void run() { ... } // written in pass 2
}
```
Groovy compiles this without error (it's a valid script+class hybrid), but
at runtime the class body can't see the script-level variable, causing a
`MissingPropertyException` (500 error with no diagnostic pointing to the
declaration block).
### This is not a 7.x regression
The `declare()` method body is identical to the original 2009 commit
(`3e17890a22`). David Estes's 7.x optimization commit (`85148d4612`, Oct 2025)
only touched the class javadoc and renamed `printHtmlPart(` to `h(` in
`htmlPartPrintlnRaw()` - he did not touch `declare()` or the two-pass logic at
all.
The core issue is that `declare()` has **inverted** `finalPass` logic
compared to every other code-generating method in the parser:
| Method | Guard | Runs in | Purpose |
|--------|-------|---------|---------|
| `script()` | `if (!finalPass) return` | Pass 2 | Code generation |
| `expr()` | `if (!finalPass) return` | Pass 2 | Code generation |
| `html()` | `if (!finalPass) return` | Pass 2 | Code generation |
| `scriptletExpr()` | `if (!finalPass) return` | Pass 2 | Code generation |
| `startTag()` | `if (!finalPass) return` | Pass 2 | Code generation |
| `endTag()` | `if (!finalPass) return` | Pass 2 | Code generation |
| `direct()` | `if (finalPass) return` | Pass 1 | Directive processing
(correct - configures parser state) |
| **`declare()`** | **`if (finalPass) return`** | **Pass 1** | **Writes code
output (broken - ends up before class definition)** |
`declare()` writes code like `script()` does, but runs in pass 1 like
`direct()` does. That mismatch is the bug, and it has existed since Grails 1.1
(March 2009).
--
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]
