Skip to content

Commit

Permalink
missing pieces in the AST: parser seems to skip broken annotation alt…
Browse files Browse the repository at this point in the history 
…ogether when on method parameters (#3159)

recover incomplete annotation on argument, while making sure that:
+ no "real" fields are converted into arguments.
+ we can merge existing arguments with one recovered from a field

expect better recovery also in some existing tests

fixes #3155
  • Loading branch information
@stephan-herrmann
stephan-herrmann authored Oct 25, 2024
1 parent 25d4b54 commit a0b0601
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 9 deletions.
7 changes: 7 additions & 0 deletions org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/Parser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6668,6 +6668,13 @@ protected void consumeZeroTypeAnnotations() {
// Name ::= SimpleName
// TypeAnnotationsopt ::= $empty
pushOnTypeAnnotationLengthStack(0); // signal absence of @308 annotations.
if (this.currentElement instanceof RecoveredAnnotation ann
&& ann.parent instanceof RecoveredMethod meth
&& !meth.foundOpeningBrace
&& this.currentToken == TokenNameRPAREN) {
// take note of an incomplete annotation "@Ann(v=)":
meth.incompleteParameterAnnotationSeen = true;
}
}
// BEGIN_AUTOGENERATED_REGION_CONSUME_RULE
// This method is part of an automatic generation : do NOT edit-modify
Expand Down
4 changes: 4 additions & 0 deletions ...core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/RecoveredAnnotation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,10 @@ public void setKind(int kind) {
this.kind = kind;
}

public int sourceStart() {
return this.sourceStart;
}

@Override
public int sourceEnd() {
if (this.annotation == null) {
Expand Down
43 changes: 43 additions & 0 deletions ...jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/RecoveredMethod.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*******************************************************************************/
package org.eclipse.jdt.internal.compiler.parser;

import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
import org.eclipse.jdt.core.compiler.CharOperation;
Expand Down Expand Up @@ -48,6 +49,8 @@ public class RecoveredMethod extends RecoveredElement implements TerminalTokens
RecoveredAnnotation[] pendingAnnotations;
int pendingAnnotationCount;

public boolean incompleteParameterAnnotationSeen = false;

public RecoveredMethod(AbstractMethodDeclaration methodDeclaration, RecoveredElement parent, int bracketBalance, Parser parser){
super(parent, bracketBalance, parser);
this.methodDeclaration = methodDeclaration;
Expand Down Expand Up @@ -97,6 +100,10 @@ public RecoveredElement add(Block nestedBlockDeclaration, int bracketBalanceValu
*/
@Override
public RecoveredElement add(FieldDeclaration fieldDeclaration, int bracketBalanceValue) {
if (recoverAsArgument(fieldDeclaration)) {
resetPendingModifiers();
return this;
}
resetPendingModifiers();

/* local variables inside method can only be final and non void */
Expand Down Expand Up @@ -133,6 +140,37 @@ public RecoveredElement add(FieldDeclaration fieldDeclaration, int bracketBalanc
// still inside method, treat as local variable
return this; // ignore
}

private boolean recoverAsArgument(FieldDeclaration fieldDeclaration) {
if (!this.foundOpeningBrace
&& this.methodDeclaration.declarationSourceEnd == 0
&& this.incompleteParameterAnnotationSeen) { // misparsed parameter?
long position = ((long) fieldDeclaration.sourceStart << 32)+fieldDeclaration.sourceEnd;
Argument arg = new Argument(fieldDeclaration.name, position, fieldDeclaration.type, fieldDeclaration.modifiers);
if (this.methodDeclaration.arguments == null) {
this.methodDeclaration.arguments = new Argument[] { arg };
} else {
int len = this.methodDeclaration.arguments.length;
this.methodDeclaration.arguments = Arrays.copyOf(this.methodDeclaration.arguments, len+1);
this.methodDeclaration.arguments[len] = arg;
}
int annotCount = this.pendingAnnotationCount;
if (this.pendingAnnotations != null) {
int end = 0;
arg.annotations = new Annotation[annotCount];
for (int i = 0; i < this.pendingAnnotationCount; i++) {
arg.annotations[i] = this.pendingAnnotations[i].annotation;
if (i == 0)
arg.declarationSourceStart = arg.annotations[i].sourceStart;
end = arg.sourceEnd;
}
if (end > 0)
this.methodDeclaration.bodyStart = end + 1;
}
return true;
}
return false;
}
/*
* Record a local declaration - regular method should have been created a block body
*/
Expand Down Expand Up @@ -606,6 +644,11 @@ public RecoveredElement addAnnotationName(int identifierPtr, int identifierLengt
this.pendingAnnotations = new RecoveredAnnotation[5];
this.pendingAnnotationCount = 0;
} else {
if (this.pendingAnnotationCount > 0) {
RecoveredAnnotation lastAnnot = this.pendingAnnotations[this.pendingAnnotationCount-1];
if (lastAnnot.sourceStart() == annotationStart)
return this;
}
if (this.pendingAnnotationCount == this.pendingAnnotations.length) {
System.arraycopy(
this.pendingAnnotations,
Expand Down
40 changes: 31 additions & 9 deletions ...s.compiler/src/org/eclipse/jdt/core/tests/compiler/parser/AnnotationDietRecoveryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ public class AnnotationDietRecoveryTest extends AbstractCompilerTest {
private static final boolean CHECK_ALL_PARSE = true;
public static boolean optimizeStringLiterals = false;

static {
// TESTS_NAMES = new String[] { "test0025" };
}

public AnnotationDietRecoveryTest(String testName){
super(testName);
}
Expand Down Expand Up @@ -1210,7 +1214,7 @@ public void test0024() {
"public class X {\n" +
" public X() {\n" +
" }\n" +
" void foo(int param1) {\n" +
" void foo(int param1, @AnAnnotation(name) int param2) {\n" +
" }\n" +
"}\n";

Expand All @@ -1220,7 +1224,7 @@ public void test0024() {
" public X() {\n" +
" super();\n" +
" }\n" +
" void foo(int param1) {\n" +
" void foo(int param1, @AnAnnotation(name) int param2) {\n" +
" }\n" +
"}\n";

Expand All @@ -1229,7 +1233,13 @@ public void test0024() {
expectedDietUnitToString;

String expectedCompletionDietUnitToString =
expectedDietUnitToString;
"package a;\n" +
"public class X {\n" +
" public X() {\n" +
" }\n" +
" void foo(int param1) {\n" +
" }\n" +
"}\n";

String testName = "<generic type recovery>";
checkParse(
Expand All @@ -1254,7 +1264,7 @@ public void test0025() {
"public class X {\n" +
" public X() {\n" +
" }\n" +
" void foo(int param1) {\n" +
" void foo(int param1, @AnAnnotation(name = $missing$) int param2) {\n" +
" }\n" +
"}\n";

Expand All @@ -1264,15 +1274,21 @@ public void test0025() {
" public X() {\n" +
" super();\n" +
" }\n" +
" void foo(int param1) {\n" +
" void foo(int param1, @AnAnnotation(name = $missing$) int param2) {\n" +
" }\n" +
"}\n";

String expectedFullUnitToString =
expectedDietUnitToString;

String expectedCompletionDietUnitToString =
expectedDietUnitToString;
"package a;\n" +
"public class X {\n" +
" public X() {\n" +
" }\n" +
" void foo(int param1) {\n" +
" }\n" +
"}\n";

String testName = "<generic type recovery>";
checkParse(
Expand All @@ -1297,7 +1313,7 @@ public void test0026() {
"public class X {\n" +
" public X() {\n" +
" }\n" +
" void foo(int param1) {\n" +
" void foo(int param1, @AnAnnotation @AnAnnotation1(name1 = \"a\",name2 = $missing$) int param2) {\n" +
" }\n" +
"}\n";

Expand All @@ -1307,15 +1323,21 @@ public void test0026() {
" public X() {\n" +
" super();\n" +
" }\n" +
" void foo(int param1) {\n" +
" void foo(int param1, @AnAnnotation @AnAnnotation1(name1 = \"a\",name2 = $missing$) int param2) {\n" +
" }\n" +
"}\n";

String expectedFullUnitToString =
expectedDietUnitToString;

String expectedCompletionDietUnitToString =
expectedDietUnitToString;
"package a;\n" +
"public class X {\n" +
" public X() {\n" +
" }\n" +
" void foo(int param1) {\n" +
" }\n" +
"}\n";

String testName = "<generic type recovery>";
checkParse(
Expand Down
35 changes: 35 additions & 0 deletions ...pse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/dom/ASTConverterRecoveryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1085,4 +1085,39 @@ public void test0021() throws JavaModelException {
List statements = block.statements();
assertEquals("wrong size", 0, statements.size()); //$NON-NLS-1$
}

public void testGH3155() throws JavaModelException {
this.workingCopies = new ICompilationUnit[1];
this.workingCopies[0] = getWorkingCopy(
"/Converter18/src/test/TestDependsOnClass.java",
"""
package test;
public class TestDependsOnClass {
public TestDependsOnClass(@Qualifier(value= ) Object mybean) { }
}
""");
ASTNode result = runConversion(getJLS8(), this.workingCopies[0], true, true);

assertASTNodeEquals(
"""
package test;
public class TestDependsOnClass {
public TestDependsOnClass( @Qualifier(value=$missing$) Object mybean){
}
}
""",
result);

ASTNode node = getASTNode((CompilationUnit) result, 0, 0);
assertNotNull(node);
assertTrue("Not a method declaration", node.getNodeType() == ASTNode.METHOD_DECLARATION); //$NON-NLS-1$
MethodDeclaration methodDeclaration = (MethodDeclaration) node;
assertTrue(methodDeclaration.isConstructor());
assertEquals(1, methodDeclaration.parameters().size());
SingleVariableDeclaration param = (SingleVariableDeclaration) methodDeclaration.parameters().get(0);
assertEquals(1, param.modifiers().size());
IExtendedModifier mod = (IExtendedModifier) param.modifiers().get(0);
assertTrue(mod.isAnnotation());
assertEquals("Qualifier", ((Annotation) mod).getTypeName().toString());
}
}

0 comments on commit a0b0601

Please sign in to comment.