Skip to content

Commit e6968ea

Browse files
committed
fix: handle || operator and remove & from X= lookahead
Address review feedback from gnodet on PR #12038: - Remove c == '&' from the >=/<= /==/!= lookahead (&= is not valid) - Add || tokenization symmetrically with && - Add regression tests for || with line breaks Fixes #11882 Signed-off-by: Blasius Patrick <blasius.patrick@gmail.com>
1 parent f64ecb7 commit e6968ea

2 files changed

Lines changed: 38 additions & 3 deletions

File tree

impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionParser.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,21 +129,33 @@ private List<String> tokenize(String expression) {
129129
}
130130
quoteType = c;
131131
sb.append(c);
132-
} else if (Character.isWhitespace(c) || c == '(' || c == ')' || c == ',' || c == '+'
133-
|| c == '>' || c == '<' || c == '=' || c == '!' || c == '&' || c == '|') {
132+
} else if (Character.isWhitespace(c)
133+
|| c == '('
134+
|| c == ')'
135+
|| c == ','
136+
|| c == '+'
137+
|| c == '>'
138+
|| c == '<'
139+
|| c == '='
140+
|| c == '!'
141+
|| c == '&'
142+
|| c == '|') {
134143
if (!sb.isEmpty()) {
135144
tokens.add(sb.toString());
136145
sb.setLength(0);
137146
}
138147
if (!Character.isWhitespace(c)) {
139-
if ((c == '>' || c == '<' || c == '=' || c == '!' || c == '&')
148+
if ((c == '>' || c == '<' || c == '=' || c == '!')
140149
&& i + 1 < expression.length()
141150
&& expression.charAt(i + 1) == '=') {
142151
tokens.add(c + "=");
143152
i++; // Skip the next character
144153
} else if (c == '&' && i + 1 < expression.length() && expression.charAt(i + 1) == '&') {
145154
tokens.add("&&");
146155
i++; // Skip the next character
156+
} else if (c == '|' && i + 1 < expression.length() && expression.charAt(i + 1) == '|') {
157+
tokens.add("||");
158+
i++; // Skip the next character
147159
} else {
148160
tokens.add(String.valueOf(c));
149161
}

impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ConditionParserTest.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,29 @@ void testAmpersandAmpersandTokenizerMultiline() {
287287
(Boolean) parser.parse("${os.arch} == 'amd64'\n&& ${os.name} == 'windows' && ${os.name} == 'windows'"));
288288
}
289289

290+
@Test
291+
void testPipePipeTokenizerMultiline() {
292+
// Regression test for https://github.com/apache/maven/issues/11882
293+
// The || operator was not being tokenized correctly when a line break appeared before it.
294+
// Uses ${os.name} which is set to 'windows' in the mock context.
295+
296+
// Case 1: Basic || without line breaks (baseline)
297+
assertTrue((Boolean) parser.parse("${os.arch} == 'amd64' || ${os.name} == 'windows'"));
298+
299+
// Case 2: Line break BEFORE ||
300+
assertTrue((Boolean) parser.parse("${os.arch} == 'amd64'\n|| ${os.name} == 'windows'"));
301+
302+
// Case 3: Line break AFTER ||
303+
assertTrue((Boolean) parser.parse("${os.arch} == 'amd64' ||\n${os.name} == 'windows'"));
304+
305+
// Case 4: Line breaks on both sides
306+
assertTrue((Boolean) parser.parse("${os.arch} == 'amd64'\n||\n${os.name} == 'windows'"));
307+
308+
// Case 5: Mixed && and || with line breaks
309+
assertTrue(
310+
(Boolean) parser.parse("${os.arch} == 'amd64'\n&& ${os.name} == 'windows' || ${os.name} == 'windows'"));
311+
}
312+
290313
@Test
291314
void testNestedPropertyAlias() {
292315
functions.put("property", args -> {

0 commit comments

Comments
 (0)