JEXL-455: ignore whitespaces when creating embedded expressions (interpolations, templates);#397
Conversation
…rpolations, templates);
There was a problem hiding this comment.
Pull request overview
This PR addresses JEXL-455 by modifying the template expression parser to ignore whitespace characters (newlines, tabs, carriage returns, form feeds, and backspaces) when they appear inside embedded expressions. The goal is to allow multi-line expressions in templates without breaking the expression parsing.
Changes:
- Added
isIgnorable()helper method to identify whitespace characters that should be ignored during parsing - Modified IMMEDIATE1 state parsing to skip ignorable characters when building expressions
- Added three test cases verifying the new behavior with newlines and tabs in immediate expressions
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/test/java/org/apache/commons/jexl3/Issues400Test.java | Adds StringWriter import and three test methods (testIssue455a, testIssue455b, testIssue455) to verify whitespace is properly ignored in immediate expressions and templates |
| src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java | Implements isIgnorable() method and applies whitespace filtering in IMMEDIATE1 parsing state; reformats DEFERRED1 switch statement for better readability |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java
Outdated
Show resolved
Hide resolved
- fixed lurking bugs involving resolution of local variables in template expressions; - added tests;
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/org/apache/commons/jexl3/internal/TemplateInterpreter.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/org/apache/commons/jexl3/internal/TemplateInterpreter.java
Show resolved
Hide resolved
- default method in interface not meant to be called drops coverage by 1%;
- hardening code again, class loader can never be null
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/main/java/org/apache/commons/jexl3/internal/introspection/Introspector.java:160
- The Objects.requireNonNull check will throw a NullPointerException if loader is null, but this NPE will not be caught by the ClassNotFoundException catch block at line 158. This means the method will throw an unchecked exception instead of returning null as the JavaDoc suggests. Consider either: (1) ensuring loader is never null in the constructor, (2) catching NullPointerException and returning null, or (3) updating the JavaDoc to indicate that a NullPointerException may be thrown if the class loader is null.
public Class<?> getClassByName(final String className) {
try {
final ClassLoader classLoader = Objects.requireNonNull(loader, "class loader should not be null");
final Class<?> clazz = Class.forName(className, false, classLoader);
return permissions.allow(clazz)? clazz : null;
} catch (final ClassNotFoundException xignore) {
return null;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
….java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.