davsclaus commented on code in PR #26307:
URL: https://github.com/apache/camel/pull/26307#discussion_r3989798119
##########
components/camel-joor/src/main/java/org/apache/camel/language/joor/JoorCompiler.java:
##########
@@ -38,29 +38,37 @@
public class JoorCompiler extends ServiceSupport implements StaticService {
+ // quoted name: 'foo' or "foo"
+ private static final String QUOTED_NAME = "(['\"][A-Za-z0-9.$]*['\"])";
+ // default value: anything but parentheses, or a single level of balanced
parentheses (eg new Date(), '(none)'),
+ // so a default value can never span from one headerAs/exchangePropertyAs
call into the next one on the same line
+ private static final String DEFAULT_VALUE = "((?:[^()]|\\([^()]*\\))+?)";
Review Comment:
The comment documents the rule precisely ("anything but parentheses, or a
single level of balanced parentheses"), which I appreciate — but it is worth
being explicit that this is *narrower* than the `(.+)` it replaces, so two
shapes that `main` rewrote now fall through and become a javac error instead:
```
headerAs('a', foo(bar(1)), Integer) old: rewritten new: no match ->
compile error
headerAs('x', "a)b", String) old: rewritten new: no match ->
compile error
```
Nested calls more than one level deep, and an unbalanced parenthesis inside
a string literal. Both are narrow, and the greedy pattern's "correctness" on
them was luck rather than design, so I would not chase them — a regex cannot
balance parentheses in general anyway, and pushing further would just move the
cliff.
What I would like is for the limit to be a stated contract rather than an
accident:
- a sentence in the `Default value` part of `joor-language.adoc` saying a
default value may contain at most one level of parentheses, and to assign to a
local first for anything more complex; and
- a test asserting the current behaviour for `headerAs('a', foo(bar(1)),
Integer)`, so the next person to touch these patterns finds out from a test
what the intended boundary is.
Also worth a line in the PR description that this narrowing exists, since it
is the one way the change is not strictly a superset of the old behaviour.
--
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]
