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

feat: Validate defaultValue for lists and arrays #996

Merged
merged 1 commit into from
Jul 11, 2023
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
@@ -0,0 +1,41 @@
package org.acme.config;

import javax.ws.rs.Path;
import org.eclipse.microprofile.config.inject.ConfigProperty;
import java.util.List;
import java.util.Set;

@Path("/greetings")
public class DefaultValueListResource {

@ConfigProperty(name = "listprop1", defaultValue="foo")
List<Integer> greeting1;

@ConfigProperty(name = "listprop2", defaultValue="12,13,14")
List<Integer> greeting2;

@ConfigProperty(name = "listprop3", defaultValue="12,13,14")
int[] greeting3;

@ConfigProperty(name = "listprop4", defaultValue="12,13\\,14")
int[] greeting4;

@ConfigProperty(name = "listprop5", defaultValue="1")
List<Boolean> greeting5;

@ConfigProperty(name = "listprop6", defaultValue=",,,,,,,,")
Set<Boolean> greeting6;

@ConfigProperty(name = "listprop7", defaultValue="1.0,2.0,3.0")
float[] greeting7;

@ConfigProperty(name = "listprop8", defaultValue="AB,CD")
char[] greeting8;

@ConfigProperty(name = "listprop9", defaultValue=",,,,")
char[] greeting9;

@ConfigProperty(name = "listprop10", defaultValue="")
List<String> greeting10;

}
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,6 @@ public class DefaultValueResource {
@ConfigProperty(name = "greeting9")
String greeting9;

@ConfigProperty(name = "greeting10", defaultValue="AB")
char greeting10;
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,9 @@

import com.google.gson.JsonObject;
import com.intellij.openapi.module.Module;
import com.intellij.psi.PsiAnnotation;
import com.intellij.psi.PsiAnnotationMemberValue;
import com.intellij.psi.PsiClass;
import com.intellij.psi.PsiField;
import com.intellij.psi.PsiLiteral;
import com.intellij.psi.PsiType;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.TypeConversionUtil;
import com.redhat.devtools.intellij.lsp4mp4ij.psi.core.MicroProfileConfigConstants;
import com.redhat.devtools.intellij.lsp4mp4ij.psi.core.java.diagnostics.JavaDiagnosticsContext;
import com.redhat.devtools.intellij.lsp4mp4ij.psi.core.java.validators.JavaASTValidator;
Expand All @@ -38,6 +34,7 @@
import java.text.MessageFormat;
import java.util.List;
import java.util.logging.Logger;
import java.util.regex.Pattern;

import static com.redhat.devtools.intellij.lsp4mp4ij.psi.core.MicroProfileConfigConstants.CONFIG_PROPERTIES_ANNOTATION;
import static com.redhat.devtools.intellij.lsp4mp4ij.psi.core.MicroProfileConfigConstants.CONFIG_PROPERTY_ANNOTATION;
Expand Down Expand Up @@ -65,8 +62,12 @@ public class MicroProfileConfigASTValidator extends JavaASTValidator {

private static final AntPathMatcher pathMatcher = new AntPathMatcher();

private static final Pattern ARRAY_SPLITTER = Pattern.compile("(?<!\\\\),");

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

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.";

private static final String EMPTY_KEY_ERROR_MESSAGE = "The member ''{0}'' can'''t be empty.";
Expand Down Expand Up @@ -144,10 +145,17 @@ private void validatePropertyDefaultValue(PsiAnnotation annotation, PsiAnnotatio
String defValue = (String) ((PsiLiteral) defaultValueExpr).getValue();
Module javaProject = getContext().getJavaProject();
PsiType fieldBinding = parent.getType();
if (fieldBinding != null && !isAssignable(fieldBinding, javaProject, defValue)) {
String message = MessageFormat.format(EXPECTED_TYPE_ERROR_MESSAGE, defValue, fieldBinding.getPresentableText());
super.addDiagnostic(message, MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE, defaultValueExpr,
MicroProfileConfigErrorCode.DEFAULT_VALUE_IS_WRONG_TYPE, DiagnosticSeverity.Error);
if (fieldBinding != null && defValue != null) {
if (isListLike(fieldBinding) && defValue.isEmpty()) {
String message = MessageFormat.format(EMPTY_LIST_LIKE_WARNING_MESSAGE, fieldBinding.getPresentableText());
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.getPresentableText());
super.addDiagnostic(message, MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE, defaultValueExpr,
MicroProfileConfigErrorCode.DEFAULT_VALUE_IS_WRONG_TYPE, DiagnosticSeverity.Error);
}
}
}
}
Expand Down Expand Up @@ -194,44 +202,86 @@ private boolean isPropertyIgnored(String propertyName) {
return false;
}

private boolean isAssignable(PsiType fieldBinding, Module javaProject, String defValue) {
String fqn = fieldBinding.getCanonicalText();
private static boolean isListLike(PsiType type) {
if (type instanceof PsiArrayType) {
return true;
}
PsiType erasedType = TypeConversionUtil.erasure(type);
String fqn = erasedType.getCanonicalText();
return "java.util.List".equals(fqn) || "java.util.Set".equals(fqn);
}


private static boolean isAssignable(PsiType fieldBinding, Module javaProject, String defValue) {
String fqn = TypeConversionUtil.erasure(fieldBinding).getCanonicalText();
// handle list-like types.
// MicroProfile config supports arrays, `java.util.List`, and `java.util.Set` by
// default. See:
// https://download.eclipse.org/microprofile/microprofile-config-2.0/microprofile-config-spec-2.0.html#_array_converters
if (isListLike(fieldBinding)) {
if (defValue.isEmpty()) {
// A different error is shown in this case
return true;
}
String itemsTypeFqn = "java.lang.Object";
if (fieldBinding instanceof PsiArrayType) {
itemsTypeFqn = TypeConversionUtil.erasure(((PsiArrayType)fieldBinding).getComponentType()).getCanonicalText();
} else if (fieldBinding instanceof PsiClassType) {
PsiClassType collection = (PsiClassType) fieldBinding;
if (collection.getParameterCount() < 1) {
return true;
}
itemsTypeFqn = TypeConversionUtil.erasure(collection.getParameters()[0]).getCanonicalText();
}
for (String listItemValue : ARRAY_SPLITTER.split(defValue, -1)) {
listItemValue = listItemValue.replace("\\,", ",");
if (!isAssignable(itemsTypeFqn, listItemValue, javaProject)) {
return false;
}
}
return true;
} else {
// Not a list-like type
return isAssignable(fqn, defValue, javaProject);
}
}

private static boolean isAssignable(String typeFqn, String value, Module javaProject) {
try {
if (fqn.startsWith("java.lang.Class")) {
return PsiTypeUtils.findType(javaProject, defValue) != null;
} else {
switch (fqn) {
case "boolean":
case "java.lang.Boolean":
return Boolean.valueOf(defValue) != null;
case "byte":
case "java.lang.Byte":
return Byte.valueOf(defValue) != null;
case "short":
case "java.lang.Short":
return Short.valueOf(defValue) != null;
case "int":
case "java.lang.Integer":
return Integer.valueOf(defValue) != null;
case "long":
case "java.lang.Long":
return Long.valueOf(defValue) != null;
case "float":
case "java.lang.Float":
return Float.valueOf(defValue) != null;
case "double":
case "java.lang.Double":
return Double.valueOf(defValue) != null;
case "char":
case "java.lang.Character":
return Character.valueOf(defValue.charAt(0)) != null;
case "java.lang.Class":
return PsiTypeUtils.findType(javaProject, defValue) != null;
case "java.lang.String":
return true;
default:
switch (typeFqn) {
case "boolean":
case "java.lang.Boolean":
return Boolean.valueOf(value) != null;
case "byte":
case "java.lang.Byte":
return Byte.valueOf(value) != null;
case "short":
case "java.lang.Short":
return Short.valueOf(value) != null;
case "int":
case "java.lang.Integer":
return Integer.valueOf(value) != null;
case "long":
case "java.lang.Long":
return Long.valueOf(value) != null;
case "float":
case "java.lang.Float":
return Float.valueOf(value) != null;
case "double":
case "java.lang.Double":
return Double.valueOf(value) != null;
case "char":
case "java.lang.Character":
if (value == null || value.length() != 1) {
return false;
}
}
return Character.valueOf(value.charAt(0)) != null;
case "java.lang.Class":
return PsiTypeUtils.findType(javaProject, value) != null;
case "java.lang.String":
return true;
default:
return false;
}
} catch (NumberFormatException e) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
*/
public enum MicroProfileConfigErrorCode implements IJavaErrorCode {

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

@Override
public String getCode() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,47 @@ public void testImproperDefaultValues() throws Exception {
MicroProfileConfigErrorCode.NO_VALUE_ASSIGNED_TO_PROPERTY);
setDataForUnassigned("greeting9", d4);

Diagnostic d5 = d(35, 54, 58, "'AB' does not match the expected type of 'char'.", DiagnosticSeverity.Error,
MicroProfileConfigConstants.MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE,
MicroProfileConfigErrorCode.DEFAULT_VALUE_IS_WRONG_TYPE);

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

@Test
public void testImproperDefaultValuesList() throws Exception {
Module javaProject = loadMavenProject(MicroProfileMavenProjectName.config_quickstart);
IPsiUtils utils = PsiUtilsLSImpl.getInstance(myProject);

MicroProfileJavaDiagnosticsParams diagnosticsParams = new MicroProfileJavaDiagnosticsParams();
System.err.println(new File(ModuleUtilCore.getModuleDirPath(javaProject), "src/main/java/org/acme/config/DefaultValueListResource.java"));
String javaFileUri = getFileUri("src/main/java/org/acme/config/DefaultValueListResource.java", javaProject);
diagnosticsParams.setUris(Arrays.asList(javaFileUri));
diagnosticsParams.setDocumentFormat(DocumentFormat.Markdown);

Diagnostic d1 = d(10, 53, 58, "'foo' does not match the expected type of 'List<Integer>'.", DiagnosticSeverity.Error,
MicroProfileConfigConstants.MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE,
MicroProfileConfigErrorCode.DEFAULT_VALUE_IS_WRONG_TYPE);

Diagnostic d2 = d(19, 53, 65, "'12,13\\,14' does not match the expected type of 'int[]'.", DiagnosticSeverity.Error,
MicroProfileConfigConstants.MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE,
MicroProfileConfigErrorCode.DEFAULT_VALUE_IS_WRONG_TYPE);

Diagnostic d3 = d(31, 53, 60, "'AB,CD' does not match the expected type of 'char[]'.", DiagnosticSeverity.Error,
MicroProfileConfigConstants.MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE,
MicroProfileConfigErrorCode.DEFAULT_VALUE_IS_WRONG_TYPE);

Diagnostic d4 = d(34, 53, 59, "',,,,' does not match the expected type of 'char[]'.", DiagnosticSeverity.Error,
MicroProfileConfigConstants.MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE,
MicroProfileConfigErrorCode.DEFAULT_VALUE_IS_WRONG_TYPE);

Diagnostic d5 = d(37, 54, 56, "'defaultValue=\"\"' will behave as if no default value is set, and will not be treated as an empty 'List<String>'.", DiagnosticSeverity.Warning,
MicroProfileConfigConstants.MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE,
MicroProfileConfigErrorCode.EMPTY_LIST_NOT_SUPPORTED);

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

@Test
Expand All @@ -107,8 +146,12 @@ public void testNoValueAssignedWithIgnore() throws Exception {
MicroProfileConfigConstants.MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE,
MicroProfileConfigErrorCode.DEFAULT_VALUE_IS_WRONG_TYPE);

Diagnostic d4 = d(35, 54, 58, "'AB' does not match the expected type of 'char'.", DiagnosticSeverity.Error,
MicroProfileConfigConstants.MICRO_PROFILE_CONFIG_DIAGNOSTIC_SOURCE,
MicroProfileConfigErrorCode.DEFAULT_VALUE_IS_WRONG_TYPE);

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

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public void testConfigItemIntBoolDefaultValueTest() throws Exception {
String intDefault = "0";

assertProperties(infoFromClasspath,
259 + 20 /* properties from Java sources with ConfigProperty */ + //
259 + 31 /* properties from Java sources with ConfigProperty */ + //
7 /* static properties from microprofile-context-propagation-api */ +
1 /* static property from microprofile config_ordinal */,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public void testConfigQuickstartFromClasspath() throws Exception {
assertNotNull("Test existing of quarkus-core-deployment*.jar", f);

assertProperties(infoFromClasspath, 257 /* properties from JAR */ + //
20 /* properties from Java sources with ConfigProperty */ + //
31 /* properties from Java sources with ConfigProperty */ + //
2 /* properties from Java sources with ConfigRoot */ + //
7 /* static properties from microprofile-context-propagation-api */ +
1 /* static property from microprofile config_ordinal */,
Expand Down Expand Up @@ -118,7 +118,7 @@ public void testConfigQuickstartFromJavaSources() throws Exception {
Module module = createMavenModule(new File("projects/lsp4mp/projects/maven/config-quickstart"));
MicroProfileProjectInfo infoFromJavaSources = PropertiesManager.getInstance().getMicroProfileProjectInfo(module, MicroProfilePropertiesScope.ONLY_SOURCES, ClasspathKind.SRC, PsiUtilsLSImpl.getInstance(myProject), DocumentFormat.PlainText);

assertProperties(infoFromJavaSources, 20 /* properties from Java sources with ConfigProperty */ + //
assertProperties(infoFromJavaSources, 31 /* properties from Java sources with ConfigProperty */ + //
2 /* properties from Java sources with ConfigRoot */,

// GreetingResource
Expand Down
Loading