gnodet-bot commented on code in PR #13144:
URL: https://github.com/apache/maven/pull/13144#discussion_r4016343413
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/profile/ConditionParser.java:
##########
@@ -379,15 +424,26 @@ private Object parseVariableOrUnknownFunction() {
/**
* Parses a list of arguments for a function call.
+ * For the {@code if} function, only the branch selected by the condition
is evaluated,
+ * the other one is skipped and passed as {@code null}.
*
+ * @param functionName the name of the called function
* @return a list of parsed arguments
* @throws RuntimeException if there's a mismatch in parentheses
*/
- private List<Object> parseArgumentList() {
+ private List<Object> parseArgumentList(String functionName) {
List<Object> args = new ArrayList<>();
current++; // Skip the opening parenthesis
while (current < tokens.size() && !tokens.get(current).equals(")")) {
- args.add(parseLogicalOr());
+ int index = args.size();
+ if (IF_FUNCTION.equals(functionName)
+ && (index == 1 || index == 2)
+ && toBoolean(args.get(0)) != (index == 1)) {
Review Comment:
💡 **Readability nit:** `toBoolean(args.get(0)) != (index == 1)` is correct
but takes a moment to decode. The intent is: skip the branch that was *not*
selected by the condition. An explicit form makes the invariant obvious:
```suggestion
if (IF_FUNCTION.equals(functionName)
&& (index == 1 || index == 2)
&& toBoolean(args.get(0)) == (index == 2)) {
```
Or equivalently:
```java
boolean condition = toBoolean(args.get(0));
if (IF_FUNCTION.equals(functionName) && (index == 1 || index == 2)
&& ((index == 1) ? !condition : condition)) {
```
Either form makes the if/else branch symmetry immediate to the reader. The
suggestion above (`== (index == 2)`) reads as: "skip this argument when it's
the branch the condition did *not* select."
--
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]
