Skip to content

Commit beebaed

Browse files
committed
Prevent NPEs in completion tests
- Prevent NPE when completing name of the last parameter in a method declaration - This uses the existing "suggest a name for this variable" logic - Prevent various NPEs when completing the first parameter of a parameterized type - eg. ```java List<| asdf = new ArrayList<>(); ``` - move private check before deprecated check to avoid NPE trying to access the `IMethod` for a private JDK class member. (I think the root cause is that the private method doesn't exist in the stubbed jar, so we can't access the associated `IMethod`, but it does exist in javac's internal bindings, since javac has access to the real JDK class) - when completing throws clause, add null checks when accessing the return type and name to compute the scanning index - constructors don't have a return type node, and an incomplete method declaration might not have a name - replace a use of "Ljava/lang/Object;" with the constant from KeyUtils Should fix 5 cases, and switch several test errors over to failures. Signed-off-by: David Thompson <[email protected]>
1 parent 22784ff commit beebaed

File tree

5 files changed

+59
-12
lines changed

5 files changed

+59
-12
lines changed

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

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@
9696
import org.eclipse.jdt.core.dom.IfStatement;
9797
import org.eclipse.jdt.core.dom.ImportDeclaration;
9898
import org.eclipse.jdt.core.dom.InfixExpression;
99+
import org.eclipse.jdt.core.dom.InfixExpression.Operator;
99100
import org.eclipse.jdt.core.dom.Initializer;
100101
import org.eclipse.jdt.core.dom.InstanceofExpression;
101102
import org.eclipse.jdt.core.dom.Javadoc;
@@ -107,6 +108,7 @@
107108
import org.eclipse.jdt.core.dom.MethodInvocation;
108109
import org.eclipse.jdt.core.dom.MethodRef;
109110
import org.eclipse.jdt.core.dom.Modifier;
111+
import org.eclipse.jdt.core.dom.Modifier.ModifierKeyword;
110112
import org.eclipse.jdt.core.dom.ModuleDeclaration;
111113
import org.eclipse.jdt.core.dom.Name;
112114
import org.eclipse.jdt.core.dom.NodeFinder;
@@ -147,8 +149,6 @@
147149
import org.eclipse.jdt.core.dom.VariableDeclarationFragment;
148150
import org.eclipse.jdt.core.dom.VariableDeclarationStatement;
149151
import org.eclipse.jdt.core.dom.WhileStatement;
150-
import org.eclipse.jdt.core.dom.InfixExpression.Operator;
151-
import org.eclipse.jdt.core.dom.Modifier.ModifierKeyword;
152152
import org.eclipse.jdt.core.formatter.DefaultCodeFormatterConstants;
153153
import org.eclipse.jdt.core.search.IJavaSearchConstants;
154154
import org.eclipse.jdt.core.search.IJavaSearchScope;
@@ -1335,6 +1335,21 @@ public void complete(org.eclipse.jdt.internal.compiler.env.ICompilationUnit sour
13351335
suggestDefaultCompletions = false;
13361336
}
13371337
if (context instanceof MethodDeclaration methodDeclaration) {
1338+
int closingParenLocation = methodDeclaration.getName().getStartPosition() + methodDeclaration.getName().getLength();
1339+
boolean afterComma = true;
1340+
while (closingParenLocation < this.textContent.length() && this.textContent.charAt(closingParenLocation) != ')') {
1341+
if (afterComma) {
1342+
if (!Character.isWhitespace(this.textContent.charAt(closingParenLocation))) {
1343+
afterComma = false;
1344+
}
1345+
} else {
1346+
// TODO: handle \u002C
1347+
if (this.textContent.charAt(closingParenLocation) == ',') {
1348+
afterComma = true;
1349+
}
1350+
}
1351+
closingParenLocation++;
1352+
}
13381353
if (this.offset < methodDeclaration.getName().getStartPosition()) {
13391354
completeMethodModifiers(methodDeclaration);
13401355
// return type: suggest types from current CU
@@ -1347,9 +1362,22 @@ public void complete(org.eclipse.jdt.internal.compiler.env.ICompilationUnit sour
13471362
publishFromScope(specificCompletionBindings);
13481363
}
13491364
suggestDefaultCompletions = false;
1350-
} else if (methodDeclaration.getBody() == null || (methodDeclaration.getBody() != null && this.offset <= methodDeclaration.getBody().getStartPosition())) {
1365+
} else if (methodDeclaration.getBody() == null || (methodDeclaration.getBody() != null && this.offset <= methodDeclaration.getBody().getStartPosition()) && closingParenLocation < this.offset) {
13511366
completeThrowsClause(methodDeclaration, specificCompletionBindings);
13521367
suggestDefaultCompletions = false;
1368+
} else if (methodDeclaration.getName().getStartPosition() + methodDeclaration.getName().getLength() < this.offset && this.offset <= closingParenLocation && !afterComma) {
1369+
// void myMethod(Integer a, Boolean |) { ...
1370+
SingleVariableDeclaration svd = (SingleVariableDeclaration)methodDeclaration.parameters().get(methodDeclaration.parameters().size() - 1);
1371+
ITypeBinding typeBinding = svd.getType().resolveBinding();
1372+
Block block = methodDeclaration.getBody();
1373+
Set<String> alreadySuggestedNames = new HashSet<>();
1374+
if (block != null) {
1375+
suggestUndeclaredVariableNames(block, typeBinding, alreadySuggestedNames);
1376+
} else {
1377+
suggestUndeclaredVariableNames(typeBinding, alreadySuggestedNames);
1378+
}
1379+
suggestVariableNamesForType(typeBinding, alreadySuggestedNames);
1380+
suggestDefaultCompletions = false;
13531381
} else if (this.offset > methodDeclaration.getStartPosition() + methodDeclaration.getLength()) {
13541382
suggestModifierKeywords(0);
13551383
}
@@ -2705,7 +2733,9 @@ private boolean filterTypeBasedOnAccess(IType type, String currentPackage, IType
27052733
}
27062734

27072735
private boolean isParameterInNonParameterizedType(ASTNode context) {
2708-
if (DOMCompletionUtil.findParent(context, new int[] { ASTNode.PARAMETERIZED_TYPE }) != null) {
2736+
if (context instanceof ParameterizedType) {
2737+
return true;
2738+
} else if (DOMCompletionUtil.findParent(context, new int[] { ASTNode.PARAMETERIZED_TYPE }) != null) {
27092739
ASTNode cursor1 = context;
27102740
ASTNode cursor2 = context.getParent();
27112741
while (!(cursor2 instanceof ParameterizedType paramType)) {
@@ -3137,9 +3167,13 @@ private void completeJavadocInlineTags(TagElement tagNode) {
31373167

31383168
private void completeThrowsClause(MethodDeclaration methodDeclaration, Bindings scope) {
31393169
if (methodDeclaration.thrownExceptionTypes().size() == 0) {
3140-
int startScanIndex = Math.max(
3141-
methodDeclaration.getName().getStartPosition() + methodDeclaration.getName().getLength(),
3142-
methodDeclaration.getReturnType2().getStartPosition() + methodDeclaration.getReturnType2().getLength());
3170+
int startScanIndex = -1;
3171+
if (methodDeclaration.getReturnType2() != null) {
3172+
startScanIndex = methodDeclaration.getReturnType2().getStartPosition() + methodDeclaration.getReturnType2().getLength();
3173+
}
3174+
if (methodDeclaration.getName() != null) {
3175+
startScanIndex = methodDeclaration.getName().getStartPosition() + methodDeclaration.getName().getLength();
3176+
}
31433177
if (!methodDeclaration.parameters().isEmpty()) {
31443178
SingleVariableDeclaration lastParam = (SingleVariableDeclaration)methodDeclaration.parameters().get(methodDeclaration.parameters().size() - 1);
31453179
startScanIndex = lastParam.getName().getStartPosition() + lastParam.getName().getLength();
@@ -3470,9 +3504,9 @@ private void findOverridableMethods0(ITypeBinding currentType, ITypeBinding type
34703504
continue next;
34713505
}
34723506
if (method.isSynthetic() || method.isConstructor()
3507+
|| Modifier.isPrivate(method.getModifiers())
34733508
|| (this.assistOptions.checkDeprecation && isDeprecated(method.getJavaElement()))
34743509
|| Modifier.isStatic(method.getModifiers())
3475-
|| Modifier.isPrivate(method.getModifiers())
34763510
|| ((method.getModifiers() & (Modifier.PUBLIC | Modifier.PRIVATE | Modifier.PROTECTED)) == 0) && !typeBinding.getPackage().getKey().equals(originalPackageKey)) {
34773511
continue next;
34783512
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,16 @@ private void computeExpectedTypes(){
194194
}
195195
break;
196196
}
197+
if (parent2 instanceof ParameterizedType parameterizedType) {
198+
ITypeBinding typeBinding = parameterizedType.getType().resolveBinding().getTypeDeclaration();
199+
// TODO: does this ever happen for other entries?
200+
if (typeBinding.getTypeParameters()[0].isWildcardType() && typeBinding.getTypeParameters()[0].getBound() != null) {
201+
this.expectedTypes.add(typeBinding.getTypeParameters()[0].getBound());
202+
} else {
203+
this.expectedTypes.add(parent2.getAST().resolveWellKnownType(Object.class.getName()));
204+
}
205+
return;
206+
}
197207
if (parent2.getLocationInParent() == MemberValuePair.VALUE_PROPERTY && parent2.getParent() instanceof MemberValuePair mvp) {
198208
var binding = mvp.resolveMemberValuePairBinding();
199209
if (binding != null) {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
* Utility methods for manipulating binding keys.
1818
*/
1919
public class KeyUtils {
20+
21+
public static final String OBJECT_KEY = "Ljava/lang/Object;";
2022

2123
/**
2224
* Returns the given method key with the owner type and return type removed.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ static int computeRelevanceForExpectingType(ITypeBinding proposalType, ExpectedT
131131
relevance = CompletionEngine.R_EXPECTED_TYPE;
132132
}
133133
}
134-
if ("Ljava/lang/Object;".equals(expectedType.getKey()) && proposalType.isPrimitive()) {
134+
if (KeyUtils.OBJECT_KEY.equals(expectedType.getKey()) && proposalType.isPrimitive()) {
135135
// can be autoboxed
136136
relevance = CompletionEngine.R_EXPECTED_TYPE;
137137
}

org.eclipse.jdt.core.javac/src/org/eclipse/jdt/internal/javac/dom/JavacTypeBinding.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
import org.eclipse.jdt.core.dom.Modifier;
5959
import org.eclipse.jdt.core.dom.RecordDeclaration;
6060
import org.eclipse.jdt.core.dom.TypeDeclaration;
61+
import org.eclipse.jdt.internal.codeassist.KeyUtils;
6162
import org.eclipse.jdt.internal.compiler.codegen.ConstantPool;
6263
import org.eclipse.jdt.internal.core.BinaryType;
6364
import org.eclipse.jdt.internal.core.JavaElement;
@@ -314,10 +315,10 @@ public String getKey() {
314315
}
315316

316317
private String computeKey() {
317-
return getKeyWithPossibleGenerics(this.type, this.typeSymbol, ITypeBinding::getKey, true);
318+
return getKeyWithPossibleGenerics(this.type, this.typeSymbol, tb -> tb != null ? tb.getKey() : KeyUtils.OBJECT_KEY, true);
318319
}
319320
public String getKeyWithPossibleGenerics(Type t, TypeSymbol s) {
320-
return getKeyWithPossibleGenerics(t, s, ITypeBinding::getKey);
321+
return getKeyWithPossibleGenerics(t, s, tb -> tb != null ? tb.getKey() : KeyUtils.OBJECT_KEY);
321322
}
322323
private String getKeyWithPossibleGenerics(Type t, TypeSymbol s, Function<ITypeBinding, String> parameterizedCallback) {
323324
return getKeyWithPossibleGenerics(t, s, parameterizedCallback, false);
@@ -373,7 +374,7 @@ public String getGenericTypeSignature(Type t, TypeSymbol s, boolean useSlashes)
373374
return '[' + componentBinding.getGenericTypeSignature(useSlashes);
374375
}
375376
String ret = getKeyWithPossibleGenerics(t, s,
376-
x -> x instanceof JavacTypeBinding jtb ? jtb.getGenericTypeSignature(useSlashes) : x.getKey(),
377+
x -> x instanceof JavacTypeBinding jtb ? jtb.getGenericTypeSignature(useSlashes) : x != null ? x.getKey() : KeyUtils.OBJECT_KEY,
377378
useSlashes);
378379
return ret;
379380
}

0 commit comments

Comments
 (0)