Skip to content

Tidy up ExpressionEvaluator #379

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fourls merged 2 commits into from
May 7, 2025
Merged

Tidy up ExpressionEvaluator #379

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2485d39
Use curly braces for if statements in `ExpressionEvaluator`
cirras May 7, 2025
f10d39f
Reflow comments to the line limit in `ExpressionEvaluator`
cirras May 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 55 additions & 27 deletions ...ontend/src/main/java/au/com/integradev/delphi/msbuild/expression/ExpressionEvaluator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,20 +126,25 @@ private String parseTopLevel(boolean stopAtQuote, boolean parseMetadata) {
}

private Optional<String> parseItem() {
if (nextChar() != '@') return Optional.empty();
if (nextChar() != '(') return Optional.empty();
if (nextChar() != '@' || nextChar() != '(') {
return Optional.empty();
}

skipWhitespace();
Optional<String> ident = parseIdentifier();
if (ident.isEmpty()) return Optional.empty();
if (ident.isEmpty()) {
return Optional.empty();
}
skipWhitespace();

Function<MSBuildItem, String> transform = MSBuildItem::getIdentity;
String separator = ";";

if (peekChar() == '-') {
var maybeTransform = parseItemTransform();
if (maybeTransform.isEmpty()) return Optional.empty();
if (maybeTransform.isEmpty()) {
return Optional.empty();
}
transform = maybeTransform.get();

skipWhitespace();
Expand All @@ -154,12 +159,16 @@ private Optional<String> 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));
}
Expand All @@ -170,8 +179,9 @@ private String concatItems(
}

private Optional<Function<MSBuildItem, String>> parseItemTransform() {
if (nextChar() != '-') return Optional.empty();
if (nextChar() != '>') return Optional.empty();
if (nextChar() != '-' || nextChar() != '>') {
return Optional.empty();
}

return parseItemTransformExpression();
}
Expand All @@ -182,9 +192,15 @@ private Optional<Function<MSBuildItem, String>> 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));
}
Expand All @@ -196,8 +212,10 @@ private String expandMetadataValues(MSBuildItem item, String expression) {
}

private Optional<String> 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();
Expand All @@ -215,32 +233,39 @@ private Optional<String> 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
// 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()));
}
}

private Optional<String> parseMetadata() {
if (nextChar() != '%') return Optional.empty();
if (nextChar() != '(') return Optional.empty();
if (nextChar() != '%' || nextChar() != '(') {
return Optional.empty();
}

var discard = skipWhitespace();
Optional<String> 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
// 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()));
Expand All @@ -258,7 +283,9 @@ private static boolean isSimpleStringChar(char character) {
}

private Optional<String> parseSimpleString() {
if (nextChar() != '\'') return Optional.empty();
if (nextChar() != '\'') {
return Optional.empty();
}

StringBuilder builder = new StringBuilder();

Expand All @@ -271,7 +298,9 @@ private Optional<String> parseSimpleString() {
}

private Optional<String> parseIdentifier() {
if (!isSimpleStringStart(peekChar())) return Optional.empty();
if (!isSimpleStringStart(peekChar())) {
return Optional.empty();
}

StringBuilder builder = new StringBuilder();

Expand All @@ -295,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);
}
}