You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "jamesfredley (via GitHub)" <gi...@apache.org> on 2026/02/17 05:21:07 UTC

[PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

jamesfredley opened a new pull request, #15398:
URL: https://github.com/apache/grails-core/pull/15398

   ## Summary
   
   GSP declaration blocks (`<%! ... %>` and `!{ ... }!`) have never worked correctly in any Grails version. The `declare()` method in `GroovyPageParser.java` generates broken Groovy code that causes a 500 Internal Server Error at runtime with no useful diagnostic. This fix replaces the broken implementation with a clear compile-time `GrailsTagException` that tells the developer exactly what syntax is unsupported and suggests alternatives.
   
   ## Bug Description
   
   When a GSP contains a JSP-style declaration block:
   
   ```gsp
   <%! int counter = 0; %>
   ```
   
   The application compiles and starts normally, but rendering the page at runtime produces a 500 Internal Server Error. The error message gives no indication that the declaration block syntax is the cause.
   
   **Before (broken — 500 error, no diagnostic):**
   ```
   org.codehaus.groovy.runtime.InvokerInvocationException: ...
     Caused by: groovy.lang.MissingPropertyException: No such property: counter
   ```
   
   **After (clear compile-time error):**
   ```
   org.grails.taglib.GrailsTagException: JSP-style declaration blocks (<%! ... %>) are not supported
   in Groovy Server Pages. Use <% ... %> for scriptlet code or move logic to a controller or service.
   ```
   
   ## Root Cause
   
   The GSP parser (`GroovyPageParser.java`) uses a two-pass system:
   - **Pass 1** (`finalPass=false`): Pre-scan. Most handlers skip via `if (!finalPass) return;`
   - **Pass 2** (`finalPass=true`): Code generation. This is where the class body is written
   
   The `declare()` method uniquely has **inverted** logic — `if (finalPass) return;` — so it runs during Pass 1 only. This causes declaration content to be written to the output stream *before* the class definition, between the `package` statement and `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
   }
   ```
   
   This creates an invalid Groovy script+class hybrid. Groovy compiles it without error, but at runtime the class body cannot see the script-level variable, causing `MissingPropertyException`.
   
   **This has been broken since the very first commit in Grails 1.1 (March 2009).** The `declare()` method has never generated correct code. Zero real-world usage exists — searching all public GitHub repositories for `<%!` in `.gsp` files returns exactly 1 result (a Sublime Text syntax highlighting test file). The Groovy variant `!{ ... }!` has 0 results.
   
   ## Fix
   
   Replaced the broken `declare()` implementation with a `GrailsTagException` throw, following the same error pattern used throughout `GroovyPageParser.java`:
   
   ```java
   // Before (broken since Grails 1.1):
   private void declare(boolean gsp) {
       if (finalPass) {
           return;
       }
       out.println();
       write(scan.getToken().trim(), gsp);
       out.println();
       out.println();
   }
   
   // After (clear error):
   private void declare(boolean gsp) {
       String syntax = gsp ? "!{ ... }!" : "<%! ... %>";
       throw new GrailsTagException(
               "JSP-style declaration blocks (" + syntax + ") are not supported in Groovy Server Pages. " +
               "Use <% ... %> for scriptlet code or move logic to a controller or service.",
               pageName, getCurrentOutputLineNumber());
   }
   ```
   
   The error is thrown during parsing (before any code generation), so developers get immediate feedback with the exact file name, line number, and a suggestion for the correct alternative syntax.
   
   ## Files Changed
   
   | File | Change |
   |------|--------|
   | `grails-gsp/core/.../GroovyPageParser.java` | Changed `declare()` to throw `GrailsTagException` instead of writing broken code |
   | `grails-gsp/plugin/.../ParseSpec.groovy` | Added 3 Spock tests for declaration block rejection |
   
   ## Test Coverage
   
   3 new Spock tests added to `ParseSpec.groovy` (all 13 tests pass — 10 existing + 3 new):
   
   1. **`parse with JSP declaration block throws error`** — verifies `<%! int counter = 0; %>` throws `GrailsTagException` with message containing "declaration blocks", "<%! ... %>", and "not supported"
   2. **`parse with JSP declaration block containing method throws error`** — verifies `<%! String hello() { return "hi"; } %>` embedded in HTML throws `GrailsTagException`
   3. **`parse with Groovy declaration block throws error`** — verifies `!{ int counter = 0; }!` (Groovy variant) throws `GrailsTagException` with message containing "!{ ... }!"
   
   ## Example Application
   
   **Repository:** https://github.com/jamesfredley/grails-gsp-declaration-block-bug
   
   Grails 7.0.7 app with two pages:
   
   1. `http://localhost:8080/bugDemo/safe` — uses `<% int counter = 0; %>` (scriptlet) — **renders correctly**
   2. `http://localhost:8080/bugDemo/index` — uses `<%! int counter = 0; %>` (declaration block) — **500 error** (demonstrates the bug)
   
   ```bash
   ./gradlew bootRun
   # Visit http://localhost:8080/bugDemo/safe   → works
   # Visit http://localhost:8080/bugDemo/index  → 500 error (bug)
   ```
   
   ## Environment Information
   
   - Grails: 7.0.7
   - GORM: 7.0.7
   - Spring Boot: 3.5.10
   - Groovy: 4.0.30
   - JDK: 17 (Amazon Corretto)
   
   ## Version
   
   7.0.7


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15398:
URL: https://github.com/apache/grails-core/pull/15398#discussion_r2824961625


##########
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) {

Review Comment:
   The `finalPass` guard is irrelevant because the new code unconditionally throws a `GrailsTagException`. Since `declare()` is called during pass 1 (when `finalPass=false`), the exception fires immediately on the first encounter. There's no reason to let pass 1 succeed and then throw on pass 2 - the guard would be dead code after the `throw`.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15398:
URL: https://github.com/apache/grails-core/pull/15398#discussion_r2817964201


##########
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:
   This doesn't look like the right change to me.   This was throwing an error before - are you saying this declare worked in previous versions?  @davydotcom  did rework some of this in 7.x, is this really just a bug introduced in 7.x? 



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15398:
URL: https://github.com/apache/grails-core/pull/15398#discussion_r2829782038


##########
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:
   Corrected my comment, I originally said Grails 6 instead of Grails 7.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15398:
URL: https://github.com/apache/grails-core/pull/15398#discussion_r2829842845


##########
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:
   I did not, but that code has been present since Grails 1.1, so the outcome should be the same.  Users in general know `<%! int counter = 0; %>` is a bad idea, but if they try it, it compiles but then errors during runtime.    This PR is just trying to alert them during the build instead of at runtime.  The outcome is the same, an exception.  



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #15398:
URL: https://github.com/apache/grails-core/pull/15398#issuecomment-3915781839

   <!-- Bito Reply -->
   The final pass check was removed to change the behavior of the declare method. Previously, it would skip processing during the final pass, but now it unconditionally throws an exception to disable support for JSP-style declaration blocks in Groovy Server Pages.


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "matrei (via GitHub)" <gi...@apache.org>.
matrei commented on code in PR #15398:
URL: https://github.com/apache/grails-core/pull/15398#discussion_r2832280086


##########
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:
   Tested with Grails 6.2.3:
   
   <img width="903" height="868" alt="Screenshot_20260220_103505" src="https://github.com/user-attachments/assets/71200529-659b-4ea4-a1dc-795fd20e6be9" />
   



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15398:
URL: https://github.com/apache/grails-core/pull/15398#discussion_r2829632522


##########
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:
   With https://github.com/jamesfredley/grails-gsp-declaration-block-bug it can be reproduced.  The reason this was not brought up for fixed before is likely that `<%! int counter = 0; %>` has been a known anti-pattern for a long time in JSP and most user didn't try it in GSP because it is not a good idea.  This is a super edge case, but does produce a runtime exception if you do it.    



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15398:
URL: https://github.com/apache/grails-core/pull/15398#discussion_r2830953196


##########
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:
   My fear in this area is the GSP dependency structure was significantly changed in Grails 7 and the area needs a lot of work.  We should first confirm this isn't a regression before removing it.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty merged PR #15398:
URL: https://github.com/apache/grails-core/pull/15398


-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15398:
URL: https://github.com/apache/grails-core/pull/15398#discussion_r2832876692


##########
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:
   Thank you.  I am good to remove then.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15398:
URL: https://github.com/apache/grails-core/pull/15398#discussion_r2829780859


##########
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 app you linked is a Grails 6 app though.  Is there a branch where you tested this with Grails 6?



##########
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 app you linked is a Grails 7 app though.  Is there a branch where you tested this with Grails 6?



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15398:
URL: https://github.com/apache/grails-core/pull/15398#discussion_r2828797808


##########
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:
   have you actually tested that this is an issue in Grails 6 though?  This looks like an AI response and I want to understand that we actually created an app to ensure this isn't really a borken feature.



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #15398:
URL: https://github.com/apache/grails-core/pull/15398#discussion_r2824961625


##########
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) {

Review Comment:
   The finalPass guard is irrelevant because the new code unconditionally throws. Throwing on the first pass encountered (pass 1)



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix: reject unsupported JSP/Groovy declaration blocks in GSP parser [grails-core]

Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #15398:
URL: https://github.com/apache/grails-core/pull/15398#discussion_r2817960298


##########
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) {

Review Comment:
   Why did you remove the final pass check? 



-- 
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: notifications-unsubscribe@grails.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org