Skip to content
Open
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 @@ -76,6 +76,8 @@ public class MicroProfileConfigASTValidator extends JavaASTValidator {

private static final String EXPECTED_TYPE_ERROR_MESSAGE = "''{0}'' does not match the expected type of ''{1}''.";

private static final String EXPECTED_TYPE_UNSUPPORTED_MESSAGE = "Validation of default values for type ''{0}'' is not supported.";

private static final String EMPTY_LIST_LIKE_WARNING_MESSAGE = "''defaultValue=\"\"'' will behave as if no default value is set, and will not be treated as an empty ''{0}''.";

private static final String NO_VALUE_ERROR_MESSAGE = "The property ''{0}'' is not assigned a value in any config file, and must be assigned at runtime.";
Expand Down Expand Up @@ -180,11 +182,21 @@ private void validatePropertyDefaultValue(Annotation annotation, Expression defa
String message = MessageFormat.format(EMPTY_LIST_LIKE_WARNING_MESSAGE, fieldBinding.getName());
super.addDiagnostic(message, MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE, defaultValueExpr,
MicroProfileConfigErrorCode.EMPTY_LIST_NOT_SUPPORTED, DiagnosticSeverity.Warning);
}
if (!isAssignable(fieldBinding, javaProject, defValue)) {
String message = MessageFormat.format(EXPECTED_TYPE_ERROR_MESSAGE, defValue, fieldBinding.getName());
super.addDiagnostic(message, MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE, defaultValueExpr,
MicroProfileConfigErrorCode.DEFAULT_VALUE_IS_WRONG_TYPE, DiagnosticSeverity.Error);
} else {
switch (validatePropertyDefaultValue(fieldBinding, javaProject, defValue)) {
case INVALID:
String message1 = MessageFormat.format(EXPECTED_TYPE_ERROR_MESSAGE, defValue, fieldBinding.getName());
super.addDiagnostic(message1, MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE, defaultValueExpr,
MicroProfileConfigErrorCode.DEFAULT_VALUE_IS_WRONG_TYPE, DiagnosticSeverity.Error);
break;
case UNSUPPORTED:
String message2 = MessageFormat.format(EXPECTED_TYPE_UNSUPPORTED_MESSAGE, fieldBinding.getName());
super.addDiagnostic(message2, MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE, defaultValueExpr,
MicroProfileConfigErrorCode.VALIDATION_FOR_TYPE_UNSUPPORTED, DiagnosticSeverity.Information);
break;
case VALID:
break;
}
}
}
}
Expand Down Expand Up @@ -244,7 +256,7 @@ private static boolean isListLike(ITypeBinding type) {
return "java.util.List".equals(fqn) || "java.util.Set".equals(fqn);
}

private static boolean isAssignable(ITypeBinding fieldBinding, IJavaProject javaProject, String defValue) {
private static ValidationResult validatePropertyDefaultValue(ITypeBinding fieldBinding, IJavaProject javaProject, String defValue) {
String fqn = Signature.getTypeErasure(fieldBinding.getQualifiedName());
// handle list-like types.
// MicroProfile config supports arrays, `java.util.List`, and `java.util.Set` by
Expand All @@ -253,7 +265,7 @@ private static boolean isAssignable(ITypeBinding fieldBinding, IJavaProject java
if (isListLike(fieldBinding)) {
if (defValue.isEmpty()) {
// A different error is shown in this case
return true;
return ValidationResult.VALID;
}
String itemsTypeFqn;
if (fieldBinding.isArray()) {
Expand All @@ -263,56 +275,68 @@ private static boolean isAssignable(ITypeBinding fieldBinding, IJavaProject java
}
for (String listItemValue : ARRAY_SPLITTER.split(defValue, -1)) {
listItemValue = listItemValue.replace("\\,", ",");
if (!isAssignable(itemsTypeFqn, listItemValue, javaProject)) {
return false;
ValidationResult r = validateItemValue(itemsTypeFqn, listItemValue, javaProject);
if (r != ValidationResult.VALID) {
return r;
}
}
return true;
return ValidationResult.VALID;
} else {
// Not a list-like type
return isAssignable(fqn, defValue, javaProject);
return validateItemValue(fqn, defValue, javaProject);
}
}

private static boolean isAssignable(String typeFqn, String value, IJavaProject javaProject) {
private static ValidationResult validateItemValue(String typeFqn, String value, IJavaProject javaProject) {
try {
switch (typeFqn) {
case "boolean":
case "java.lang.Boolean":
return Boolean.valueOf(value) != null;
Boolean.parseBoolean(value);
break;
case "byte":
case "java.lang.Byte":
return Byte.valueOf(value.trim()) != null;
Byte.parseByte(value.trim());
break;
case "short":
case "java.lang.Short":
return Short.valueOf(value.trim()) != null;
Short.parseShort(value.trim());
break;
case "int":
case "java.lang.Integer":
return Integer.valueOf(value.trim()) != null;
Integer.parseInt(value.trim());
break;
case "long":
case "java.lang.Long":
return Long.valueOf(value.trim()) != null;
Long.parseLong(value.trim());
break;
case "float":
case "java.lang.Float":
return Float.valueOf(value.trim()) != null;
Float.parseFloat(value.trim());
break;
case "double":
case "java.lang.Double":
return Double.valueOf(value.trim()) != null;
Double.parseDouble(value.trim());
break;
case "char":
case "java.lang.Character":
if (value == null || value.length() != 1) {
return false;
return ValidationResult.INVALID;
}
return Character.valueOf(value.charAt(0)) != null;
break;
case "java.lang.Class":
return JDTTypeUtils.findType(javaProject, value.trim()) != null;
if(JDTTypeUtils.findType(javaProject, value.trim()) == null) {
return ValidationResult.UNSUPPORTED;
}
break;
case "java.lang.String":
return true;
break;
default:
return false;
return ValidationResult.UNSUPPORTED;
}
return ValidationResult.VALID;
} catch (NumberFormatException e) {
return false;
return ValidationResult.INVALID;
}
}

Expand All @@ -333,4 +357,8 @@ public static void setDataForUnassigned(String name, Diagnostic diagnostic) {
data.addProperty(DIAGNOSTIC_DATA_NAME, name);
diagnostic.setData(data);
}

private static enum ValidationResult {
VALID, INVALID, UNSUPPORTED
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*/
public enum MicroProfileConfigErrorCode implements IJavaErrorCode {

NO_VALUE_ASSIGNED_TO_PROPERTY, DEFAULT_VALUE_IS_WRONG_TYPE, EMPTY_LIST_NOT_SUPPORTED, EMPTY_KEY;
NO_VALUE_ASSIGNED_TO_PROPERTY, DEFAULT_VALUE_IS_WRONG_TYPE, EMPTY_LIST_NOT_SUPPORTED, EMPTY_KEY, VALIDATION_FOR_TYPE_UNSUPPORTED;

@Override
public String getCode() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package org.acme;

import org.eclipse.microprofile.config.inject.ConfigProperty;
import java.util.Optional;

public class DefaultValuesValidation {
@ConfigProperty(defaultValue = "a string")
public String string;

@ConfigProperty(defaultValue = "yes")
public boolean bool;

@ConfigProperty(defaultValue = "4")
public int integer;

@ConfigProperty(defaultValue = "x")
public int invalidInteger;

@ConfigProperty(defaultValue = "a string")
public Custom custom;

@ConfigProperty(defaultValue = "/foo/bar")
public java.nio.file.Path path;

@ConfigProperty(defaultValue = "http://localhost/bar")
public java.net.URI uri;

@ConfigProperty(defaultValue = "2000-12-31T01:23:45Z")
public java.time.Instant instant;

@ConfigProperty(defaultValue = "PT2S")
public java.time.Duration duration;

@ConfigProperty(defaultValue = "boo")
public boolean boolUnexpectedFalse;

/**
* This type has a public Constructor with a single String parameter and as such
* represents a valid implicit converter as per Microprofile-Config
* specification (See {@link org.eclipse.microprofile.config.spi.Converter}).
*/
public static class Custom {
private final String value;

public Custom(String value) {
this.value = value;
}

public String getValue() {
return value;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,19 @@
*******************************************************************************/
package org.eclipse.lsp4mp.jdt.core.config.java;

import static java.lang.String.format;
import static org.eclipse.lsp4j.DiagnosticSeverity.Error;
import static org.eclipse.lsp4j.DiagnosticSeverity.Information;
import static org.eclipse.lsp4mp.jdt.core.MicroProfileConfigConstants.MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE;
import static org.eclipse.lsp4mp.jdt.core.MicroProfileForJavaAssert.assertJavaCodeAction;
import static org.eclipse.lsp4mp.jdt.core.MicroProfileForJavaAssert.assertJavaDiagnostics;
import static org.eclipse.lsp4mp.jdt.core.MicroProfileForJavaAssert.ca;
import static org.eclipse.lsp4mp.jdt.core.MicroProfileForJavaAssert.createCodeActionParams;
import static org.eclipse.lsp4mp.jdt.core.MicroProfileForJavaAssert.d;
import static org.eclipse.lsp4mp.jdt.core.MicroProfileForJavaAssert.te;
import static org.eclipse.lsp4mp.jdt.internal.config.java.MicroProfileConfigASTValidator.setDataForUnassigned;
import static org.eclipse.lsp4mp.jdt.internal.config.java.MicroProfileConfigErrorCode.DEFAULT_VALUE_IS_WRONG_TYPE;
import static org.eclipse.lsp4mp.jdt.internal.config.java.MicroProfileConfigErrorCode.VALIDATION_FOR_TYPE_UNSUPPORTED;

import java.util.Arrays;

Expand Down Expand Up @@ -126,6 +132,37 @@ public void improperDefaultValuesList() throws Exception {
d1, d2, d3, d4, d5);
}

@Test
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test (and others that are built similar) is unstable for me (and it seems gh CI) - sometimes it doesn't report any diagnostics.

I'm assuming there is issue resulting from an async result taking just a tiny bit too long, I don't know how to avoid it other than changing the assertion helper method to use busy waiting like awaitility does. That would however only work reliably with at least one diagnostic, and also cause higher CPU load during tests (tests are already being aborted by CI due to taking too long)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I dug a bit (a lot) deeper into this issue yesterday, and it seems it cannot be resolved by retrying.

I was able to narrow it down to "somewhere before" MicroProfileConfigASTValidator#isAdaptedForDiagnostics, because when this method returns false (meaning when JDTTypeUtils.findType(javaProject, CONFIG_PROPERTY_ANNOTATION) returns null), it will always do so for the test run. This leads me to believe it's related to project loading (but the project data seems fine)

At that point, I'm out - I hope you can agree that this is well out of scope for this PR and stability issues of the test have nothing to do with my changes.

public void unsupportedDefaultValues() throws Exception {
String src = MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE;
IJavaProject javaProject = ProjectUtils.getJavaProject(MicroProfileMavenProjectName.microprofile_configproperties);
IJDTUtils utils = JDT_UTILS;

IFile javaFile = javaProject.getProject().getFile(new Path("src/main/java/org/acme/DefaultValues.java"));
MicroProfileJavaDiagnosticsParams diagnosticsParams = new MicroProfileJavaDiagnosticsParams();
diagnosticsParams.setUris(Arrays.asList(javaFile.getLocation().toFile().toURI().toString()));
diagnosticsParams.setDocumentFormat(DocumentFormat.Markdown);

String invalidType = "'%s' does not match the expected type of '%s'.";
String unsupported = "Validation of default values for type '%s' is not supported.";

// Line 16: default value 'x' for 'int' property is an error
Diagnostic d1 = d(15, 32, 35, format(invalidType, "x", "int"), Error, src, DEFAULT_VALUE_IS_WRONG_TYPE);
// Line 19: Custom types (types not in JDK) are not supported
Diagnostic d2 = d(18, 32, 42, format(unsupported, "Custom"), Information, src, VALIDATION_FOR_TYPE_UNSUPPORTED);
// Line 22: type java.nio.file.Path is not supported (but can be in the future!)
Diagnostic d3 = d(21, 32, 42, format(unsupported, "Path"), Information, src, VALIDATION_FOR_TYPE_UNSUPPORTED);
// Line 25: type java.net.URI is not supported (but can be in the future!)
Diagnostic d4 = d(24, 32, 54, format(unsupported, "URI"), Information, src, VALIDATION_FOR_TYPE_UNSUPPORTED);
// Line 28: type java.time.Instant is not supported (but can be in the future!)
Diagnostic d5 = d(27, 32, 54, format(unsupported, "Instant"), Information, src, VALIDATION_FOR_TYPE_UNSUPPORTED);
// Line 31: type java.time.Duration is not supported (but can be in the future!)
Diagnostic d6 = d(30, 32, 38, format(unsupported, "Duration"), Information, src, VALIDATION_FOR_TYPE_UNSUPPORTED);
// Line 34: Does not cause diagnostic, but maybe should show warning

assertJavaDiagnostics(diagnosticsParams, utils, d1, d2, d3, d4, d5, d6);
}

@Test
public void noValueAssignedWithIgnore() throws Exception {
IJavaProject javaProject = ProjectUtils.getJavaProject(MicroProfileMavenProjectName.config_quickstart);
Expand Down