Fix ConditionParser to handle newlines before && operator

[blaspat]

Fix for #11882

The tokenize() method only treated space as whitespace, so newlines became standalone tokens. When a line break appeared before &&, the operator was parsed as two separate & tokens instead of one &&.

Added \n, \r, \t to whitespace handling and properly tokenize && as a single operator token regardless of surrounding whitespace.

[Copilot]

Pull request overview

This PR fixes profile activation condition parsing so formatted multiline conditions can place a line break around the && operator without breaking tokenization.

Changes:

• Treats newline, carriage return, and tab characters as whitespace in ConditionParser.
• Tokenizes && as a single logical AND operator when encountered directly.
• Adds regression coverage for multiline && expressions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionParser.java	Updates tokenizer whitespace/operator handling for multiline logical AND conditions.
impl/maven-impl/src/test/java/org/apache/maven/impl/model/profile/ConditionParserTest.java	Adds regression tests covering && with line breaks before, after, and around the operator.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gnodet]

Thanks for the fix, @blaspat! The root cause analysis is spot-on — whitespace and & delimiter handling in the tokenizer were the issue. Here are a few improvements to make the fix more robust before merging:

1. Use Character.isWhitespace(c) instead of enumerating ' ', '\n', '\r', '\t' — cleaner and handles edge cases
2. Handle || symmetrically with && — || has the same tokenization gap (not in the operator char list)
3. Remove & from the X= lookahead — &= is not a valid operator in this language

See inline comments for concrete suggestions.

For the remaining arithmetic operators (-, *, /) that also are not handled as delimiter characters, I have filed #12052 as a separate follow-up since those need more careful design (e.g., - in property names).

Claude Code on behalf of Guillaume Nodet

[gnodet]

Two issues:

1. & in the X= group: & was added to the >=/<=/==/!= lookahead, which means &= would be tokenized as a valid two-char operator. But &= is not valid in this language — remove || c == '&' from this condition.

2. || not handled: The || operator has the exact same tokenization gap that && had. The | character is not in the operator list, so || only works when surrounded by spaces. Handle it symmetrically with &&.

Suggested change

	if ((c == '>' || c == '<' || c == '=' || c == '!' || c == '&')
	&& i + 1 < expression.length()
	&& expression.charAt(i + 1) == '=') {
	tokens.add(c + "=");
	i++; // Skip the next character
	} else if (c == '&' && i + 1 < expression.length() && expression.charAt(i + 1) == '&') {
	tokens.add("&&");
	i++; // Skip the next character
	if ((c == '>' || c == '<' || c == '=' || c == '!')
	&& i + 1 < expression.length()
	&& expression.charAt(i + 1) == '=') {
	tokens.add(c + "=");
	i++; // Skip the next character
	} else if (c == '&' && i + 1 < expression.length() && expression.charAt(i + 1) == '&') {
	tokens.add("&&");
	i++; // Skip the next character
	} else if (c == '|' && i + 1 < expression.length() && expression.charAt(i + 1) == '|') {
	tokens.add("||");
	i++; // Skip the next character

Please also add corresponding test cases for || with newlines, similar to the && tests.

[gnodet]

Please also add a test for || with newlines, similar to testAmpersandAmpersandTokenizerMultiline. For example:

@Test
void testPipePipeTokenizerMultiline() {
    // || with newlines should work the same as &&
    assertTrue((Boolean) parser.parse("${os.arch} == 'amd64' || ${os.name} == 'linux'"));
    assertTrue((Boolean) parser.parse("${os.arch} == 'amd64'\n|| ${os.name} == 'linux'"));
    assertTrue((Boolean) parser.parse("${os.arch} == 'amd64' ||\n${os.name} == 'linux'"));
    assertTrue((Boolean) parser.parse("${os.arch} == 'amd64'\n||\n${os.name} == 'linux'"));
}

[gnodet]

@blaspat Thanks for applying the suggestions! Two items still need to be addressed before this can be merged:

1. & is still in the X= lookahead (line 139)

|| c == '&' needs to be removed from the >=/<=/==/!= condition. Currently &= would be tokenized as a valid two-char operator, but it isn't one. The && handling in the else if below is correct — it's only the X= group that needs the fix.

Current:

if ((c == '>' || c == '<' || c == '=' || c == '!' || c == '&')
        && i + 1 < expression.length()
        && expression.charAt(i + 1) == '=') {

Should be:

if ((c == '>' || c == '<' || c == '=' || c == '!')
        && i + 1 < expression.length()
        && expression.charAt(i + 1) == '=') {

2. || tokenization is missing — this is now a regression

| was added to the delimiter list (good), but there's no branch to combine two consecutive | chars into a single || token. This means a || b now tokenizes as ["a", "|", "|", "b"] instead of ["a", "||", "b"], which breaks || entirely — even with spaces, which worked before this PR.

Please add a || branch after the && one:

} else if (c == '&' && i + 1 < expression.length() && expression.charAt(i + 1) == '&') {
    tokens.add("&&");
    i++; // Skip the next character
} else if (c == '|' && i + 1 < expression.length() && expression.charAt(i + 1) == '|') {
    tokens.add("||");
    i++; // Skip the next character
} else {

3. Add || multiline tests (as mentioned in the earlier review comment)

Claude Code on behalf of Guillaume Nodet