Skip to content

Commit

Permalink
Reference only property declared in *.properties file in property
Browse files Browse the repository at this point in the history 
expresion

Fixes #205

Signed-off-by: azerr <azerr@redhat.com>
  • Loading branch information
@angelozerr
angelozerr committed Oct 29, 2021
1 parent 5f98c0c commit 79281b1
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 51 deletions.
13 changes: 8 additions & 5 deletions ...mp.ls/src/main/java/org/eclipse/lsp4mp/services/properties/PropertiesFileCompletions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,13 +410,16 @@ private static void collectPropertyValueExpressionSuggestions(Node node, Propert
for (String independentProperty : independentProperties) {
list.getItems().add(getPropertyCompletionItem(independentProperty, (PropertyValueExpression) node, model));
}

// Add all properties not referenced in the properties file as completion
// options
// options only the property has no default value
for (ItemMetadata candidateCompletion : projectInfo.getProperties()) {
String candidateCompletionName = candidateCompletion.getName();
if (!graph.hasNode(candidateCompletionName)) {
list.getItems()
.add(getPropertyCompletionItem(candidateCompletionName, (PropertyValueExpression) node, model));
if (candidateCompletion.getDefaultValue() == null) {
String candidateCompletionName = candidateCompletion.getName();
if (!graph.hasNode(candidateCompletionName)) {
list.getItems().add(
getPropertyCompletionItem(candidateCompletionName, (PropertyValueExpression) node, model));
}
}
}
}
Expand Down
55 changes: 36 additions & 19 deletions ...p4mp.ls/src/main/java/org/eclipse/lsp4mp/services/properties/PropertiesFileValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import java.math.BigInteger;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -58,15 +57,18 @@ class PropertiesFileValidator {

private final MicroProfileValidationSettings validationSettings;
private final Map<String, List<Property>> existingProperties;
private Set<String> allProperties;
private Set<String> allPropertiesFromFile;
private Set<String> allPropertiesFromJava;

public PropertiesFileValidator(MicroProfileProjectInfo projectInfo, List<Diagnostic> diagnostics,
MicroProfileValidationSettings validationSettings) {
this.projectInfo = projectInfo;
this.diagnostics = diagnostics;
this.validationSettings = validationSettings;
this.existingProperties = new HashMap<String, List<Property>>();
this.allProperties = null; // to be lazily init
// to be lazily init
this.allPropertiesFromFile = null;
this.allPropertiesFromJava = null;
}

public void validate(PropertiesModel document, CancelChecker cancelChecker) {
Expand Down Expand Up @@ -178,9 +180,7 @@ private void validatePropertyValue(String propertyName, ItemMetadata metadata, P
* Checks if the property expression is closed, and if the referenced property
* exists.
*
* @param property The property to validate
* @param allProperties A list of all the properties defined in the property
* file and in the project info
* @param property The property to validate
*/
private void validatePropertyValueExpressions(Property property) {
if (property.getValue() == null) {
Expand All @@ -197,26 +197,43 @@ private void validatePropertyValueExpressions(Property property) {
if (child != null && child.getNodeType() == NodeType.PROPERTY_VALUE_EXPRESSION) {
PropertyValueExpression propValExpr = (PropertyValueExpression) child;
if (expressionSeverity != null) {
if (allProperties == null) {
if (allPropertiesFromFile == null) {
// Collect names of all properties defined in the configuration file and the
// project information
allProperties = new HashSet<>();
allProperties.addAll(projectInfo.getProperties().stream().map((ItemMetadata info) -> {
return info.getName();
}).collect(Collectors.toList()));
allProperties.addAll(property.getOwnerModel().getChildren().stream().filter(n -> {
allPropertiesFromFile = property.getOwnerModel().getChildren().stream().filter(n -> {
return n.getNodeType() == NodeType.PROPERTY;
}).map(prop -> {
return ((Property) prop).getPropertyNameWithProfile();
}).collect(Collectors.toList()));
}).collect(Collectors.toSet());
allPropertiesFromJava = projectInfo.getProperties().stream().map((ItemMetadata info) -> {
return info.getName();
}).collect(Collectors.toSet());
}
String refdProp = propValExpr.getReferencedPropertyName();
if (!allProperties.contains(refdProp)) {
// Check if expression has default value (ex : ${DBUSER:sa}) otherwise the error
// is reported
if (!propValExpr.hasDefaultValue()) {
Range range = PositionUtils.createAdjustedRange(propValExpr, 2,
propValExpr.isClosed() ? -1 : 0);
if (!allPropertiesFromFile.contains(refdProp)) {
// The referenced property name doesn't reference a property inside the file
if (allPropertiesFromJava.contains(refdProp)) {
Range range = PositionUtils.createRange(propValExpr.getReferenceStartOffset(),
propValExpr.getReferenceEndOffset(), propValExpr.getDocument());
if (range != null) {
ItemMetadata referencedProperty = PropertiesFileUtils.getProperty(refdProp,
projectInfo);
if (referencedProperty.getDefaultValue() != null) {
// The referenced property has a default value.
addDiagnostic("Cannot reference the property '" + refdProp
+ "'. A default value defined via annotation like ConfigProperty is not eligible to be expanded since multiple candidates may be available.",
range, expressionSeverity, ValidationType.expression.name());
} else if (!propValExpr.hasDefaultValue()) {
// The referenced property and the property expression have not a default value.
addDiagnostic("The referenced property '" + refdProp + "' has no default value.",
range, expressionSeverity, ValidationType.expression.name());
}
}
} else if (!propValExpr.hasDefaultValue()) {
// The expression has default value (ex : ${DBUSER:sa}) otherwise the error
// is reported
Range range = PositionUtils.createRange(propValExpr.getReferenceStartOffset(),
propValExpr.getReferenceEndOffset(), propValExpr.getDocument());
if (range != null) {
addDiagnostic("Unknown referenced property '" + refdProp + "'", range,
expressionSeverity, ValidationType.expression.name());
Expand Down
18 changes: 0 additions & 18 deletions ...rofile.ls/org.eclipse.lsp4mp.ls/src/main/java/org/eclipse/lsp4mp/utils/PositionUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,22 +32,4 @@ public static Range createRange(Node node) {
return PositionUtils.createRange(node.getStart(), node.getEnd(), node.getDocument());
}

/**
* Create a range for the given node and adjust the beginning and ending by the indicated amount of characters
*
* @param beginAdjust Amount of characters to move the start of the range forward by (negative values supported for going backwards)
* @param endAdjust Amount of characters to move the end of the range forward by (negative values supported for going backwards)
* @return A range for the given node where the beginning and ending of the range are adjusted by the given amount of characters
*/
public static Range createAdjustedRange(Node node, int beginAdjust, int endAdjust) {
Range range = createRange(node);
TextDocument doc = node.getDocument();
try {
return createRange(doc.offsetAt(range.getStart()) + beginAdjust, doc.offsetAt(range.getEnd()) + endAdjust,
doc);
} catch (BadLocationException e) {
return null;
}
}

}
43 changes: 39 additions & 4 deletions ...s/src/test/java/org/eclipse/lsp4mp/services/properties/PropertiesFileDiagnosticsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,10 @@ public void undefinedPropertyInPropertyExpression() {
public void validateMicroProfilePropertyInPropertyExpression() {
String value = "test.property = ${mp.opentracing.server.skip-pattern}";
testDiagnosticsFor(value, //
d(0, 0, 13, "Unknown property 'test.property'", DiagnosticSeverity.Warning, ValidationType.unknown));
d(0, 0, 13, "Unknown property 'test.property'", DiagnosticSeverity.Warning, ValidationType.unknown), //
d(0, 18, 52,
"The referenced property 'mp.opentracing.server.skip-pattern' has no default value.",
DiagnosticSeverity.Error, ValidationType.expression));
}

@Test
Expand Down Expand Up @@ -846,9 +849,41 @@ public void validateMultipleInvalidPropertyExpressions() {
public void ignoreErrorForPropertyExpressionWithDefaultValue() {
String value = "quarkus.datasource.username = ${DBUSER:sa}";
testDiagnosticsFor(value);

value = "quarkus.datasource.username = ${DBUSER:}";
testDiagnosticsFor(value,//
d(0, 32, 39, "Unknown referenced property 'DBUSER'", DiagnosticSeverity.Error, ValidationType.expression));
testDiagnosticsFor(value, //
d(0, 32, 38, "Unknown referenced property 'DBUSER'", DiagnosticSeverity.Error,
ValidationType.expression));
}

@Test
public void referencePropertyFromJavaWithoutDefaultValue() {

// "name": "quarkus.application.name",
String value = "quarkus.datasource.username = ${quarkus.application.name}";
testDiagnosticsFor(value, //
d(0, 32, 56,
"The referenced property 'quarkus.application.name' has no default value.",
DiagnosticSeverity.Error, ValidationType.expression));

value = "quarkus.datasource.username = ${quarkus.application.name:sa}";
testDiagnosticsFor(value);
}

@Test
public void referencePropertyFromJavaWithDefaultValue() {
// "name": "quarkus.platform.group-id",
// "defaultValue": "io.quarkus",
String value = "quarkus.datasource.username = ${quarkus.platform.group-id}";
testDiagnosticsFor(value, //
d(0, 32, 57,
"Cannot reference the property 'quarkus.platform.group-id'. A default value defined via annotation like ConfigProperty is not eligible to be expanded since multiple candidates may be available.",
DiagnosticSeverity.Error, ValidationType.expression));

value = "quarkus.datasource.username = ${quarkus.platform.group-id:sa}";
testDiagnosticsFor(value, //
d(0, 32, 57,
"Cannot reference the property 'quarkus.platform.group-id'. A default value defined via annotation like ConfigProperty is not eligible to be expanded since multiple candidates may be available.",
DiagnosticSeverity.Error, ValidationType.expression));
}
}
32 changes: 27 additions & 5 deletions ...t/java/org/eclipse/lsp4mp/services/properties/PropertiesFileExpressionCompletionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void noCloseBraceNoNewline() throws BadLocationException {
"test.property = hello\n" + //
"other.test.property = ${|";
testCompletionFor(text, generateInfoFor("test.property", "other.test.property"),
c("${test.property}", r(1, 22, 24)));
c("${test.property}", r(1, 22, 24)));
}

@Test
Expand Down Expand Up @@ -205,15 +205,17 @@ public void hasCommentInIt() throws BadLocationException {
public void manyUndefinedProperties() throws BadLocationException {
String text = "test.property = ${|}\n";
testCompletionFor(text, generateInfoFor("test.property", "http.port", "http.ip"),
c("${http.port}", r(0, 16, 19)), c("${http.ip}", r(0, 16, 19)));
c("${http.port}", r(0, 16, 19)), //
c("${http.ip}", r(0, 16, 19)));
}

@Test
public void nonExistingProperties() throws BadLocationException {
String text = "test.property = hi\n" + //
"other.property = hello\n" + //
"yet.another.property = ${|";
testCompletionFor(text, generateInfoFor(), c("${test.property}", r(2, 23, 25)),
testCompletionFor(text, generateInfoFor(), //
c("${test.property}", r(2, 23, 25)), //
c("${other.property}", r(2, 23, 25)));
}

Expand Down Expand Up @@ -266,8 +268,9 @@ public void complexDependencies2() throws BadLocationException {
"D=hi\n" + //
"E=${D}${C}\n" + //
"F=${D}";
testCompletionFor(text, generateInfoFor(),
c("${D}", r(2, 2, 4)), c("${F}", r(2, 2, 4)));
testCompletionFor(text, generateInfoFor(), //
c("${D}", r(2, 2, 4)), //
c("${F}", r(2, 2, 4)));
}

@Test
Expand All @@ -285,6 +288,25 @@ public void selfLoopsDontPreventCompletion() throws BadLocationException {
testCompletionFor(text, generateInfoFor(), c("${a}", r(1, 4, 5)));
}

@Test
public void referencedJavadPropertyWithoutDefaultValue() throws BadLocationException {
String text = "a = ${|}\n" + //
"b = c";
testCompletionFor(text, generateInfoFor("quarkus.http.port"), //
c("${b}", r(0, 4, 7)), //
c("${quarkus.http.port}", r(0, 4, 7)));
}

@Test
public void referencedJavadPropertyWithDefaultValue() throws BadLocationException {
String text = "a = ${|}\n" + //
"b = c";
MicroProfileProjectInfo info = generateInfoFor("quarkus.http.port");
info.getProperties().get(0).setDefaultValue("8080");
testCompletionFor(text, info, //
c("${b}", r(0, 4, 7)));
}

// Utility functions

private static MicroProfileProjectInfo generateInfoFor(String... properties) {
Expand Down

0 comments on commit 79281b1

Please sign in to comment.