From 2485d3924cba92497ee8865025282e087e510cc4 Mon Sep 17 00:00:00 2001 From: Jonah Jeleniewski Date: Wed, 7 May 2025 11:24:52 +1000 Subject: [PATCH 1/2] Use curly braces for if statements in `ExpressionEvaluator` --- .../expression/ExpressionEvaluator.java | 67 +++++++++++++------ 1 file changed, 48 insertions(+), 19 deletions(-) diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/expression/ExpressionEvaluator.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/expression/ExpressionEvaluator.java index db42bc7d4..e13692fd5 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/expression/ExpressionEvaluator.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/expression/ExpressionEvaluator.java @@ -126,12 +126,15 @@ private String parseTopLevel(boolean stopAtQuote, boolean parseMetadata) { } private Optional parseItem() { - if (nextChar() != '@') return Optional.empty(); - if (nextChar() != '(') return Optional.empty(); + if (nextChar() != '@' || nextChar() != '(') { + return Optional.empty(); + } skipWhitespace(); Optional ident = parseIdentifier(); - if (ident.isEmpty()) return Optional.empty(); + if (ident.isEmpty()) { + return Optional.empty(); + } skipWhitespace(); Function transform = MSBuildItem::getIdentity; @@ -139,7 +142,9 @@ private Optional parseItem() { if (peekChar() == '-') { var maybeTransform = parseItemTransform(); - if (maybeTransform.isEmpty()) return Optional.empty(); + if (maybeTransform.isEmpty()) { + return Optional.empty(); + } transform = maybeTransform.get(); skipWhitespace(); @@ -154,12 +159,16 @@ private Optional parseItem() { nextChar(); skipWhitespace(); var maybeSeparator = parseSimpleString(); - if (maybeSeparator.isEmpty()) return Optional.empty(); + if (maybeSeparator.isEmpty()) { + return Optional.empty(); + } separator = maybeSeparator.get(); skipWhitespace(); } - if (nextChar() != ')') return Optional.empty(); + if (nextChar() != ')') { + return Optional.empty(); + } return Optional.of(concatItems(ident.get(), transform, separator)); } @@ -170,8 +179,9 @@ private String concatItems( } private Optional> parseItemTransform() { - if (nextChar() != '-') return Optional.empty(); - if (nextChar() != '>') return Optional.empty(); + if (nextChar() != '-' || nextChar() != '>') { + return Optional.empty(); + } return parseItemTransformExpression(); } @@ -182,9 +192,15 @@ private Optional> parseItemTransformExpression() { return Optional.empty(); } - if (nextChar() != '\'') return Optional.empty(); + if (nextChar() != '\'') { + return Optional.empty(); + } + String expression = parseTopLevel(true, false); - if (nextChar() != '\'') return Optional.empty(); + + if (nextChar() != '\'') { + return Optional.empty(); + } return Optional.of(item -> expandMetadataValues(item, expression)); } @@ -196,8 +212,10 @@ private String expandMetadataValues(MSBuildItem item, String expression) { } private Optional parseProperty() { - if (nextChar() != '$') return Optional.empty(); - if (nextChar() != '(') return Optional.empty(); + if (nextChar() != '$' || nextChar() != '(') { + return Optional.empty(); + } + if (peekChar() == '[') { unsupportedFeature("static property function"); return Optional.empty(); @@ -215,7 +233,9 @@ private Optional parseProperty() { } discard = skipWhitespace() || discard; - if (nextChar() != ')') return Optional.empty(); + if (nextChar() != ')') { + return Optional.empty(); + } if (discard) { // Whitespace is significant inside property expressions, but it's impossible to @@ -227,16 +247,21 @@ private Optional parseProperty() { } private Optional parseMetadata() { - if (nextChar() != '%') return Optional.empty(); - if (nextChar() != '(') return Optional.empty(); + if (nextChar() != '%' || nextChar() != '(') { + return Optional.empty(); + } var discard = skipWhitespace(); Optional ident = parseIdentifier(); - if (ident.isEmpty()) return Optional.empty(); + if (ident.isEmpty()) { + return Optional.empty(); + } discard = skipWhitespace() || discard; - if (nextChar() != ')') return Optional.empty(); + if (nextChar() != ')') { + return Optional.empty(); + } if (discard) { // Whitespace is significant inside metadata expressions, but it's impossible to @@ -258,7 +283,9 @@ private static boolean isSimpleStringChar(char character) { } private Optional parseSimpleString() { - if (nextChar() != '\'') return Optional.empty(); + if (nextChar() != '\'') { + return Optional.empty(); + } StringBuilder builder = new StringBuilder(); @@ -271,7 +298,9 @@ private Optional parseSimpleString() { } private Optional parseIdentifier() { - if (!isSimpleStringStart(peekChar())) return Optional.empty(); + if (!isSimpleStringStart(peekChar())) { + return Optional.empty(); + } StringBuilder builder = new StringBuilder(); From f10d39f53b1a48477efe0a903da7e5654f9c889a Mon Sep 17 00:00:00 2001 From: Jonah Jeleniewski Date: Wed, 7 May 2025 11:29:31 +1000 Subject: [PATCH 2/2] Reflow comments to the line limit in `ExpressionEvaluator` --- .../msbuild/expression/ExpressionEvaluator.java | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/expression/ExpressionEvaluator.java b/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/expression/ExpressionEvaluator.java index e13692fd5..2957cd46b 100644 --- a/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/expression/ExpressionEvaluator.java +++ b/delphi-frontend/src/main/java/au/com/integradev/delphi/msbuild/expression/ExpressionEvaluator.java @@ -238,8 +238,8 @@ private Optional parseProperty() { } if (discard) { - // Whitespace is significant inside property expressions, but it's impossible to - // define properties with spaces, so expressions with spaces always evaluate to "". + // Whitespace is significant inside property expressions, but it's impossible to define + // properties with spaces, so expressions with spaces always evaluate to "". return Optional.of(""); } else { return Optional.of(state.getProperty(ident.get())); @@ -264,8 +264,8 @@ private Optional parseMetadata() { } if (discard) { - // Whitespace is significant inside metadata expressions, but it's impossible to - // define metadata with spaces, so expressions with spaces always evaluate to "". + // Whitespace is significant inside metadata expressions, but it's impossible to define + // metadata with spaces, so expressions with spaces always evaluate to "". return Optional.of(""); } else if (metadataProvider != null) { return Optional.of(metadataProvider.apply(ident.get())); @@ -324,10 +324,9 @@ private boolean skipWhitespace() { private void unsupportedFeature(String feature) { // We don't know the relative importance of this expression - it could be an expression that - // SonarDelphi - // never needs to read. Making this a warning despite having a well-defined behaviour for when - // an unsupported - // feature is encountered (to interpret it literally) would probably be overkill. + // SonarDelphi never needs to read. Making this a warning despite having a well-defined + // behaviour for when an unsupported feature is encountered (to interpret it literally) would + // probably be overkill. LOG.debug("Unsupported MSBuild feature '{}' ignored in expression: {}", feature, text); } }