feat: Validate defaultValue for lists and arrays

Port of eclipse/lsp4mp@a95a3f7 Signed-off-by: Fred Bricon <[email protected]>
redhat-developer · Jul 10, 2023 · 9f66dc7 · 9f66dc7
1 parent 0f081b9
commit 9f66dc7
Show file tree

Hide file tree

Showing 7 changed files with 188 additions and 52 deletions.
diff --git a/...jects/maven/config-quickstart/src/main/java/org/acme/config/DefaultValueListResource.java b/...jects/maven/config-quickstart/src/main/java/org/acme/config/DefaultValueListResource.java
@@ -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;
+
+}
diff --git a/.../projects/maven/config-quickstart/src/main/java/org/acme/config/DefaultValueResource.java b/.../projects/maven/config-quickstart/src/main/java/org/acme/config/DefaultValueResource.java
@@ -33,4 +33,6 @@ public class DefaultValueResource {
     @ConfigProperty(name = "greeting9")
     String greeting9;
 
+    @ConfigProperty(name = "greeting10", defaultValue="AB")
+    char greeting10;
 }
diff --git a/.../devtools/intellij/lsp4mp4ij/psi/internal/config/java/MicroProfileConfigASTValidator.java b/.../devtools/intellij/lsp4mp4ij/psi/internal/config/java/MicroProfileConfigASTValidator.java
@@ -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;
@@ -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;
@@ -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.";
@@ -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);
+				}
 			}
 		}
 	}
@@ -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;

diff --git a/...hat/devtools/intellij/lsp4mp4ij/psi/internal/config/java/MicroProfileConfigErrorCode.java b/...hat/devtools/intellij/lsp4mp4ij/psi/internal/config/java/MicroProfileConfigErrorCode.java
@@ -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() {

diff --git a/...vtools/intellij/lsp4mp4ij/psi/core/config/java/MicroProfileConfigJavaDiagnosticsTest.java b/...vtools/intellij/lsp4mp4ij/psi/core/config/java/MicroProfileConfigJavaDiagnosticsTest.java
@@ -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
@@ -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

diff --git a/...ools/intellij/lsp4mp4ij/psi/core/config/properties/ConfigItemIntBoolDefaultValueTest.java b/...ools/intellij/lsp4mp4ij/psi/core/config/properties/ConfigItemIntBoolDefaultValueTest.java
@@ -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 */,
 

diff --git a/...evtools/intellij/lsp4mp4ij/psi/core/config/properties/MicroProfileConfigPropertyTest.java b/...evtools/intellij/lsp4mp4ij/psi/core/config/properties/MicroProfileConfigPropertyTest.java
@@ -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 */,
@@ -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