Improve variableName proposals for collections

Author: mickaelistria

org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/codeassist/DOMCompletionContext.java

@@ -71,28 +71,38 @@ class DOMCompletionContext extends CompletionContext {
 		this.modelUnit = modelUnit;
 		this.textContent = textContent;
 		this.offset = offset;
-		// Use the raw text to walk back the offset to the first non-whitespace spot
 		int adjustedOffset = this.offset;
+		if (adjustedOffset > 0 && Character.isJavaIdentifierPart(textContent.charAt(adjustedOffset - 1))) {
+			// workaround for cases where right node is empty and reported (wrongly) as starting at same offset
+			adjustedOffset--;
+		}
+		ASTNode currentNode = NodeFinder.perform(domUnit, adjustedOffset, 0);
+		// Use the raw text to walk back the offset to the first non-whitespace spot
+		adjustedOffset = this.offset;
 		if (adjustedOffset >= textContent.length()) {
 			adjustedOffset = textContent.length() - 1;
 		}
+		if (adjustedOffset > 0 && Character.isJavaIdentifierPart(textContent.charAt(adjustedOffset - 1))) {
+			// workaround for cases where right node is empty and reported (wrongly) as starting at same offset
+			adjustedOffset--;
+		}
 		if (adjustedOffset + 1 >= textContent.length()
 				|| !Character.isJavaIdentifierStart(textContent.charAt(adjustedOffset))) {
 			while (adjustedOffset > 0 && Character.isWhitespace(textContent.charAt(adjustedOffset - 1)) ) {
 				adjustedOffset--;
 			}
 		}
 		ASTNode previousNodeBeforeWhitespaces = NodeFinder.perform(domUnit, adjustedOffset, 0);
-//		if (cuBuffer.getChar(adjustedOffset - 1) == ',' && Character.isWhitespace(cuBuffer.getChar(adjustedOffset))) {
-//			// probably an empty parameter
-//			adjustedOffset = this.offset;
-//			while (adjustedOffset < cuBuffer.getLength() && Character.isWhitespace(cuBuffer.getChar(adjustedOffset))) {
-//				adjustedOffset++;
-//			}
-//		}
-		this.node = previousNodeBeforeWhitespaces instanceof SimpleName || previousNodeBeforeWhitespaces instanceof StringLiteral || previousNodeBeforeWhitespaces instanceof CharacterLiteral || previousNodeBeforeWhitespaces instanceof NumberLiteral
-			?  NodeFinder.perform(domUnit, this.offset, 0) // keep default node from initial offset
-			: previousNodeBeforeWhitespaces; // use previous node
+		adjustedOffset = this.offset;
+		if (adjustedOffset < textContent.length() - 1 && Character.isWhitespace(textContent.charAt(adjustedOffset)) ) {
+			adjustedOffset++;
+		}
+		ASTNode nextNodeAfterWhitespaces = NodeFinder.perform(domUnit, adjustedOffset, 0);
+		this.node = (nextNodeAfterWhitespaces.getLength() == 0 && nextNodeAfterWhitespaces.getParent() == currentNode)
+			? nextNodeAfterWhitespaces
+			: (previousNodeBeforeWhitespaces instanceof SimpleName || previousNodeBeforeWhitespaces instanceof StringLiteral || previousNodeBeforeWhitespaces instanceof CharacterLiteral || previousNodeBeforeWhitespaces instanceof NumberLiteral)
+				? currentNode
+				: previousNodeBeforeWhitespaces;
 		this.expectedTypes = new ExpectedTypes(assistOptions, this.node, offset);
 		this.token = tokenBefore(this.textContent).toCharArray();
 		this.bindingsAcquirer = bindings::all;
@@ -329,6 +339,9 @@ public int getTokenEnd() {
 		if (this.isJustAfterStringLiteral) {
 			return -1;
 		}
+		if (this.node.getLength() == 0) { // recovered
+			return this.offset - 1;
+		}
 		if (this.node instanceof SimpleName || this.node instanceof StringLiteral) {
 			return this.node.getStartPosition() + this.node.getLength() - 1;
 		}

org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/codeassist/DOMCompletionEngine.java

@@ -20,6 +20,7 @@
 import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -1786,7 +1787,8 @@ public void complete(org.eclipse.jdt.internal.compiler.env.ICompilationUnit sour
 				}
 			}
 			if (context instanceof VariableDeclarationFragment vdf) {
-				if (this.toComplete.equals(vdf.getName())) {
+				if (this.toComplete.equals(vdf.getName()) 
+					|| this.toComplete.getLength() == 0 /* recovered */) {
 					ITypeBinding typeBinding = null;
 					if (vdf.getParent() instanceof VariableDeclarationStatement vds) {
 						typeBinding = vds.getType().resolveBinding();
@@ -2211,16 +2213,35 @@ private void suggestUndeclaredVariableNames(Block block, ITypeBinding typeBindin
 		}
 	}
 
+	private static boolean isCollection(ITypeBinding type) {
+		if (type == null) {
+			return false;
+		}
+		if (Collection.class.getName().equals(type.getErasure().getQualifiedName())) {
+			return true;
+		}
+		return isCollection(type.getSuperclass()) || Arrays.stream(type.getInterfaces()).anyMatch(DOMCompletionEngine::isCollection);
+	}
+
 	private void suggestVariableNamesForType(ITypeBinding typeBinding, Set<String> alreadySuggestedNames) {
 		if (typeBinding.isPrimitive() || typeBinding.isRecovered()) {
 			return;
 		}
+		Set<String> possibleNames = new LinkedHashSet<>();
 
-		String simpleName;
+		String simpleName = "";
+		boolean multiple = false;
 		if (typeBinding.isArray()) {
 			simpleName = typeBinding.getElementType().getName();
+			multiple = true;
+		} else if (typeBinding.isParameterizedType() && isCollection(typeBinding)) {
+			possibleNames.add(typeBinding.getErasure().getName().toLowerCase());
+			multiple = true;
+			if (typeBinding.getTypeArguments().length > 0) {
+				simpleName = typeBinding.getTypeArguments()[0].getErasure().getName();
+			}
 		} else {
-			simpleName = typeBinding.getName();
+			simpleName = typeBinding.getErasure().getName();
 		}
 
 		List<String> nameSegments = Stream.of(simpleName.split("(?=_)|(?<=_)")).flatMap(nameSegment -> Stream.of(nameSegment.split("(?<=[a-z0-9])(?=[A-Z])"))).filter(str -> !str.isEmpty()).toList();
@@ -2241,7 +2262,7 @@ private void suggestVariableNamesForType(ITypeBinding typeBinding, Set<String> a
 			for (int j = i + 1; j < nameSegments.size(); j++) {
 				variablePortionOfName.append(nameSegments.get(j));
 			}
-			if (typeBinding.isArray()) {
+			if (multiple) {
 				if (variablePortionOfName.toString().endsWith("s")) {
 					variablePortionOfName.append("es");
 				} else {
@@ -2265,7 +2286,6 @@ private void suggestVariableNamesForType(ITypeBinding typeBinding, Set<String> a
 		}
 		// there is always the implicit suffix of ""
 		suffixes.add("");
-		Set<String> possibleNames = new HashSet<>();
 		Map<String, Integer> additionalRelevances = new HashMap<>();
 		boolean firstPrefix = true;
 		boolean firstSuffix = true;
@@ -2374,7 +2394,7 @@ private void suggestVariableNamesForType(ITypeBinding typeBinding, Set<String> a
 				alreadySuggestedNames.add(possibleName);
 				CompletionProposal res = createProposal(CompletionProposal.VARIABLE_DECLARATION);
 
-				int additionalRelevance = additionalRelevances.get(possibleName);
+				int additionalRelevance = additionalRelevances.getOrDefault(possibleName, 0);
 				if (SourceVersion.isKeyword(possibleName)) {
 					possibleName += "1";
 				}
@@ -4075,14 +4095,6 @@ private boolean staticOnly() {
 		return false;
 	}
 
-	private String qualifiedTypeName(ITypeBinding typeBinding) {
-		if (typeBinding.isTypeVariable()) {
-			return typeBinding.getName();
-		} else {
-			return typeBinding.getQualifiedName();
-		}
-	}
-	
 	private CompletionProposal toProposal(TypeNameMatch typeNameMatch) {
 		return toProposal(typeNameMatch.getType(), typeNameMatch.getAccessibility());
 	}

org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/codeassist/ExpectedTypes.java

@@ -749,7 +749,7 @@ private LinkedHashSet<IMethodBinding> avaiableMethods(ITypeBinding typeBinding)
 	public List<ITypeBinding> getExpectedTypes() {
 		if (!this.isReady) {
 			computeExpectedTypes();
-			this.expectedTypes.removeIf(binding -> binding.isNullType());
+			this.expectedTypes.removeIf(ITypeBinding::isNullType);
 			this.isReady = true;
 		}
 		return new ArrayList<>(this.expectedTypes);