Skip to content

Commit

Permalink
Fix for #439: add local variables to JDT model for conflict detection
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Dec 12, 2018
1 parent d57b996 commit 6384411
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 2 deletions.
15 changes: 15 additions & 0 deletions ...y.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/ErrorRecoveryTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public void testParsingRecovery_BasicBlock() {
" public void bar() {\n" +
" }\n" +
" public java.lang.Object baz() {\n" +
" java.lang.Object good;\n" +
" }\n" +
"}\n");
}
Expand All @@ -92,6 +93,7 @@ public void testParsingRecovery_IncompleteAssignment1() {
" public static void main(java.lang.String... args) {\n" +
" }\n" +
" public @java.lang.Override java.lang.Object run() {\n" +
" int err;\n" +
" }\n" +
"}\n");
}
Expand Down Expand Up @@ -119,6 +121,7 @@ public void testParsingRecovery_IncompleteAssignment1a() {
" public static void main(java.lang.String... args) {\n" +
" }\n" +
" public @java.lang.Override java.lang.Object run() {\n" +
" int err;\n" +
" }\n" +
"}\n");
}
Expand Down Expand Up @@ -146,6 +149,7 @@ public void testParsingRecovery_IncompleteAssignment1b() {
" public static void main(java.lang.String... args) {\n" +
" }\n" +
" public @java.lang.Override java.lang.Object run() {\n" +
" int err;\n" +
" }\n" +
"}\n");
}
Expand Down Expand Up @@ -173,6 +177,7 @@ public void testParsingRecovery_IncompleteAssignment1c() {
" public static void main(java.lang.String... args) {\n" +
" }\n" +
" public @java.lang.Override java.lang.Object run() {\n" +
" int err;\n" +
" }\n" +
"}\n");
}
Expand Down Expand Up @@ -252,8 +257,10 @@ public void testParsingRecovery_IncompleteAssignment3() {
" public X() {\n" +
" }\n" +
" public void bar() {\n" +
" int err;\n" +
" }\n" +
" public java.lang.Object baz() {\n" +
" java.lang.Object good;\n" +
" }\n" +
"}\n");
}
Expand Down Expand Up @@ -565,6 +572,7 @@ public void testParsingRecovery_IncompleteRangeExpression9() {
" public X() {\n" +
" }\n" +
" public java.lang.Object y() {\n" +
" java.lang.Object range;\n" +
" }\n" +
"}\n");
}
Expand Down Expand Up @@ -872,6 +880,7 @@ public void testParsingRecovery_GRE1107_1() {
" public X() {\n" +
" }\n" +
" public void foo() {\n" +
" java.lang.Object blah;\n" +
" }\n" +
"}\n");
}
Expand Down Expand Up @@ -906,6 +915,7 @@ public void testParsingRecovery_GRE1107_2() {
" public X() {\n" +
" }\n" +
" public void foo() {\n" +
" java.lang.Object blah;\n" +
" }\n" +
"}\n");
}
Expand Down Expand Up @@ -998,6 +1008,7 @@ public void testParsingRecovery_GRE468_3() {
" public X() {\n" +
" }\n" +
" public void m() {\n" +
" java.lang.Object a;\n" +
" }\n" +
"}\n");
}
Expand Down Expand Up @@ -1025,6 +1036,7 @@ public void testParsingRecovery_GRE468_4() {
" public static void main(java.lang.String... args) {\n" +
" }\n" +
" public @java.lang.Override java.lang.Object run() {\n" +
" java.lang.Object a;\n" +
" }\n" +
"}\n");
}
Expand Down Expand Up @@ -1085,6 +1097,7 @@ public void testParsingRecovery_GRE468_5() {
" public X() {\n" +
" }\n" +
" public void m() {\n" +
" java.lang.Object leppard;\n" +
" }\n" +
"}\n");
}
Expand Down Expand Up @@ -1112,6 +1125,7 @@ public void testParsingRecovery_GRE468_6() {
" public X() {\n" +
" }\n" +
" <clinit>() {\n" +
" java.lang.Object a;\n" +
" }\n" +
"}\n");
}
Expand Down Expand Up @@ -1187,6 +1201,7 @@ public void testParsingRecovery_GRE468_9() {
" public static void main(java.lang.String... args) {\n" +
" }\n" +
" public @java.lang.Override java.lang.Object run() {\n" +
" HTML h;\n" +
" }\n" +
"}\n");
}
Expand Down
10 changes: 8 additions & 2 deletions ...vy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/basic/GroovySimpleTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1655,12 +1655,16 @@ public void testCrashingOnBadCode_GRE290_2() {
"1. ERROR in Moo.groovy (at line 4)\n" +
"\tfinal moo = processMoo(moo)\n" +
"\t ^^^\n" +
"Duplicate local variable moo\n" +
"----------\n" +
"2. ERROR in Moo.groovy (at line 4)\n" +
"\tfinal moo = processMoo(moo)\n" +
"\t ^^^\n" +
"Groovy:The current scope already contains a variable of the name moo\n" +
"----------\n");
}

// 'this' by itself isn't an error
@Test
@Test // 'this' by itself isn't an error
public void testCrashingOnBadCode_GRE290_3() {
runNegativeTest(new String[] {
"Moo.groovy",
Expand Down Expand Up @@ -4669,6 +4673,7 @@ public void testAnonymousInnerClass1() {
" public static void main(java.lang.String... args) {\n" +
" }\n" +
" public @java.lang.Override java.lang.Object run() {\n" +
" java.lang.Object foo;\n" +
" new Runnable() {\n" +
" x() {\n" +
" super();\n" +
Expand Down Expand Up @@ -4914,6 +4919,7 @@ public void testAnonymousInnerClass9() {
" public A() {\n" +
" }\n" +
" <clinit>() {\n" +
" java.lang.Object foo;\n" +
" new Runnable() {\n" +
" x() {\n" +
" super();\n" +
Expand Down
51 changes: 51 additions & 0 deletions ...e/src/org/codehaus/jdt/groovy/internal/compiler/ast/GroovyCompilationUnitDeclaration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import org.codehaus.groovy.ast.expr.ClosureExpression;
import org.codehaus.groovy.ast.expr.ConstantExpression;
import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
import org.codehaus.groovy.ast.expr.DeclarationExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.ListExpression;
import org.codehaus.groovy.ast.expr.MethodCall;
Expand Down Expand Up @@ -121,6 +122,7 @@
import org.eclipse.jdt.internal.compiler.ast.IntLiteral;
import org.eclipse.jdt.internal.compiler.ast.Javadoc;
import org.eclipse.jdt.internal.compiler.ast.Literal;
import org.eclipse.jdt.internal.compiler.ast.LocalDeclaration;
import org.eclipse.jdt.internal.compiler.ast.LongLiteral;
import org.eclipse.jdt.internal.compiler.ast.MarkerAnnotation;
import org.eclipse.jdt.internal.compiler.ast.MethodDeclaration;
Expand Down Expand Up @@ -1343,6 +1345,11 @@ private void createConstructorDeclarations(ClassNode classNode, boolean isEnum,

methodDeclarations.add(constructorDecl);

if (constructorNode.getCode() != null) {
List<VariableExpression> variables = new ArrayList<>();
constructorNode.getCode().visit(new LocalVariableFinder(variables));
if (!variables.isEmpty()) constructorDecl.statements = createStatements(variables);
}
if (constructorNode.hasDefaultValue()) {
for (Argument[] variantArgs : getVariantsAllowingForDefaulting(constructorNode.getParameters(), constructorDecl.arguments)) {
ConstructorDeclaration variantDecl = new ConstructorDeclaration(unitDeclaration.compilationResult);
Expand Down Expand Up @@ -1391,6 +1398,10 @@ private void createMethodDeclarations(ClassNode classNode, boolean isEnum, Groov

if (methodNode.isAbstract()) {
typeDeclaration.bits |= ASTNode.HasAbstractMethods;
} else if (methodNode.getCode() != null) {
List<VariableExpression> variables = new ArrayList<>();
methodNode.getCode().visit(new LocalVariableFinder(variables));
if (!variables.isEmpty()) methodDecl.statements = createStatements(variables);
}
if (methodNode.hasDefaultValue()) {
for (Argument[] variantArgs : getVariantsAllowingForDefaulting(methodNode.getParameters(), methodDecl.arguments)) {
Expand Down Expand Up @@ -1696,6 +1707,23 @@ private Argument[] createArguments(Parameter[] ps) {
return arguments;
}

private org.eclipse.jdt.internal.compiler.ast.Statement[] createStatements(List<VariableExpression> expressions) {
final int n = expressions.size();
org.eclipse.jdt.internal.compiler.ast.Statement[] statements = new org.eclipse.jdt.internal.compiler.ast.Statement[n];

for (int i = 0; i < n; i += 1) {
VariableExpression variableExpression = expressions.get(i);

LocalDeclaration variableDeclaration = new LocalDeclaration(variableExpression.getName().toCharArray(), variableExpression.getStart(), variableExpression.getEnd() - 1);
variableDeclaration.type = createTypeReferenceForClassNode(variableExpression.getOriginType());
variableDeclaration.bits |= (variableDeclaration.type.bits & ASTNode.HasTypeAnnotations);

statements[i] = variableDeclaration;
}

return statements;
}

/**
* Creates JDT TypeReference that represents the given array ClassNode.
* The name of the node is expected to be like 'java.lang.String[][]'.
Expand Down Expand Up @@ -2616,6 +2644,29 @@ public void visitConstructorCallExpression(ConstructorCallExpression call) {
super.visitConstructorCallExpression(call);
}
}

private static class LocalVariableFinder extends CodeVisitorSupport {
private LocalVariableFinder(List<VariableExpression> variables) {
this.variables = variables;
}
private final List<VariableExpression> variables;

@Override
public void visitConstructorCallExpression(ConstructorCallExpression expression) {
// don't visit anon. inner types
}

@Override
@SuppressWarnings({"cast", "rawtypes", "unchecked"})
public void visitDeclarationExpression(DeclarationExpression expression) {
if (!expression.isMultipleAssignmentDeclaration()) {
variables.add(expression.getVariableExpression());
} else {
variables.addAll((List<VariableExpression>) (List) expression.getTupleExpression().getExpressions());
}
super.visitDeclarationExpression(expression);
}
}
}

/**
Expand Down

0 comments on commit 6384411

Please sign in to comment.