Skip to content
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.

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

Reference only property declared in *.properties file in property expression #206

Merged
merged 1 commit into from
Oct 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
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;
}
}

}
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));
}
}
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