Skip to content

Commit 403f3fc

Browse files
committed
GROOVY-11046
1 parent ccec657 commit 403f3fc

File tree

10 files changed

+718
-20
lines changed

10 files changed

+718
-20
lines changed

base-test/org.eclipse.jdt.groovy.core.tests.compiler/src/org/eclipse/jdt/groovy/core/tests/xform/GrabTests.java

+21-13
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,15 @@ public void testGrab() {
3737
"Test.groovy",
3838
"@Grab('joda-time:joda-time:2.12.5;transitive=false')\n" +
3939
"import org.joda.time.DateTime\n" +
40-
"void printDate() {\n" +
41-
" def now = new DateTime()\n" +
42-
"}\n" +
43-
"printDate()\n",
40+
"def now = new DateTime()\n",
4441
};
4542
//@formatter:on
4643

4744
runNegativeTest(sources, "");
4845
}
4946

5047
/**
51-
* This program has a broken grab. Without changes we get a 'general error'
52-
* recorded on the first line of the source file:
48+
* This program has a broken grab. Without changes we get a 'general error' recorded on the first line of the source file:
5349
* <tt>General error during conversion: Error grabbing Grapes -- [unresolved dependency: org.aspectj#aspectjweaver;1.x: not found]</tt>
5450
* <p>
5551
* With grab improvements we get two errors: the missing dependency and the missing type (which is at the right version of that dependency!)
@@ -58,13 +54,6 @@ public void testGrab() {
5854
public void testGrabError() {
5955
//@formatter:off
6056
String[] sources = {
61-
"Main.java",
62-
"public class Main {\n" +
63-
" public static void main(String[] args) {\n" +
64-
// not concerned with execution
65-
" }\n" +
66-
"}\n",
67-
6857
"Test.groovy",
6958
"@Grapes([\n" +
7059
" @Grab('joda-time:joda-time:2.12.5;transitive=false'),\n" +
@@ -94,6 +83,25 @@ public void testGrabError() {
9483
"----------\n");
9584
}
9685

86+
@Test // GROOVY-11046
87+
public void testGrabError2() {
88+
//@formatter:off
89+
String[] sources = {
90+
"Main.groovy",
91+
"@Grab('org.apache.logging.log4j:log4j-core:2.22.0')\n" +
92+
"org.apache.logging.log4j.core.async.AsyncLogger log\n",
93+
};
94+
//@formatter:on
95+
96+
runNegativeTest(sources,
97+
"----------\n" +
98+
"1. ERROR in Main.groovy (at line 2)\n" +
99+
"\torg.apache.logging.log4j.core.async.AsyncLogger log\n" +
100+
"\t^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" +
101+
"Groovy:unable to define class org.apache.logging.log4j.core.async.AsyncLogger : com/lmax/disruptor/EventTranslatorVararg\n" +
102+
"----------\n");
103+
}
104+
97105
@Test // GRECLIPSE-1432
98106
public void testGrabConfig() {
99107
//@formatter:off

base/org.codehaus.groovy30/.checkstyle

+1
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
<file-match-pattern match-pattern="groovy/control/CompilationUnit.java" include-pattern="false" />
5151
<file-match-pattern match-pattern="groovy/control/CompilerConfiguration.java" include-pattern="false" />
5252
<file-match-pattern match-pattern="groovy/control/ErrorCollector.java" include-pattern="false" />
53+
<file-match-pattern match-pattern="groovy/control/GenericsVisitor.java" include-pattern="false" />
5354
<file-match-pattern match-pattern="groovy/control/ProcessingUnit.java" include-pattern="false" />
5455
<file-match-pattern match-pattern="groovy/control/ResolveVisitor.java" include-pattern="false" />
5556
<file-match-pattern match-pattern="groovy/control/SourceUnit.java" include-pattern="false" />
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.codehaus.groovy.control;
20+
21+
import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
22+
import org.codehaus.groovy.ast.ClassNode;
23+
import org.codehaus.groovy.ast.FieldNode;
24+
import org.codehaus.groovy.ast.GenericsType;
25+
import org.codehaus.groovy.ast.InnerClassNode;
26+
import org.codehaus.groovy.ast.MethodNode;
27+
import org.codehaus.groovy.ast.Parameter;
28+
import org.codehaus.groovy.ast.expr.ArrayExpression;
29+
import org.codehaus.groovy.ast.expr.CastExpression;
30+
import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
31+
import org.codehaus.groovy.ast.expr.DeclarationExpression;
32+
import org.codehaus.groovy.ast.expr.Expression;
33+
import org.codehaus.groovy.ast.expr.VariableExpression;
34+
import org.codehaus.groovy.transform.trait.Traits;
35+
36+
import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.isUnboundedWildcard;
37+
38+
/**
39+
* Verify correct usage of generics.
40+
* This includes:
41+
* <ul>
42+
* <li>class header (class and superclass declaration)</li>
43+
* <li>arity of type parameters for fields, parameters, local variables</li>
44+
* <li>invalid diamond {@code <>} usage</li>
45+
* </ul>
46+
*/
47+
public class GenericsVisitor extends ClassCodeVisitorSupport {
48+
49+
private final SourceUnit source;
50+
51+
@Override
52+
protected SourceUnit getSourceUnit() {
53+
return source;
54+
}
55+
56+
public GenericsVisitor(final SourceUnit source) {
57+
this.source = source;
58+
}
59+
60+
//--------------------------------------------------------------------------
61+
62+
@Override
63+
public void visitClass(final ClassNode node) {
64+
ClassNode sn = node.getUnresolvedSuperClass(false);
65+
if (checkWildcard(sn)) return;
66+
67+
boolean isAIC = node instanceof InnerClassNode && ((InnerClassNode) node).isAnonymous();
68+
checkGenericsUsage(sn, node.getSuperClass(), isAIC ? Boolean.TRUE : null);
69+
for (ClassNode face : node.getInterfaces()) {
70+
checkGenericsUsage(face);
71+
}
72+
73+
visitObjectInitializerStatements(node);
74+
node.visitContents(this);
75+
}
76+
77+
@Override
78+
public void visitField(final FieldNode node) {
79+
checkGenericsUsage(node.getType());
80+
81+
super.visitField(node);
82+
}
83+
84+
@Override
85+
protected void visitConstructorOrMethod(final MethodNode node, final boolean isConstructor) {
86+
for (Parameter p : node.getParameters()) {
87+
checkGenericsUsage(p.getType());
88+
}
89+
if (!isConstructor) {
90+
checkGenericsUsage(node.getReturnType());
91+
}
92+
93+
super.visitConstructorOrMethod(node, isConstructor);
94+
}
95+
96+
@Override
97+
public void visitConstructorCallExpression(final ConstructorCallExpression expression) {
98+
ClassNode type = expression.getType();
99+
boolean isAIC = type instanceof InnerClassNode
100+
&& ((InnerClassNode) type).isAnonymous();
101+
checkGenericsUsage(type, type.redirect(), isAIC);
102+
103+
super.visitConstructorCallExpression(expression);
104+
}
105+
106+
@Override
107+
public void visitDeclarationExpression(final DeclarationExpression expression) {
108+
if (expression.isMultipleAssignmentDeclaration()) {
109+
for (Expression e : expression.getTupleExpression()) {
110+
checkGenericsUsage(((VariableExpression) e).getOriginType());
111+
}
112+
} else {
113+
checkGenericsUsage(expression.getVariableExpression().getOriginType());
114+
}
115+
116+
super.visitDeclarationExpression(expression);
117+
}
118+
119+
@Override
120+
public void visitArrayExpression(final ArrayExpression expression) {
121+
checkGenericsUsage(expression.getType());
122+
123+
super.visitArrayExpression(expression);
124+
}
125+
126+
@Override
127+
public void visitCastExpression(final CastExpression expression) {
128+
checkGenericsUsage(expression.getType());
129+
130+
super.visitCastExpression(expression);
131+
}
132+
133+
//--------------------------------------------------------------------------
134+
135+
private boolean checkWildcard(final ClassNode sn) {
136+
boolean wildcard = false;
137+
if (sn.getGenericsTypes() != null) {
138+
for (GenericsType gt : sn.getGenericsTypes()) {
139+
if (gt.isWildcard()) {
140+
addError("A supertype may not specify a wildcard type", sn);
141+
wildcard = true;
142+
}
143+
}
144+
}
145+
return wildcard;
146+
}
147+
148+
private void checkGenericsUsage(ClassNode cn) {
149+
while (cn.isArray())
150+
cn = cn.getComponentType();
151+
checkGenericsUsage(cn, cn.redirect(), null);
152+
}
153+
154+
private void checkGenericsUsage(final ClassNode cn, final ClassNode rn, final Boolean isAIC) {
155+
if (cn.isGenericsPlaceHolder()) return;
156+
GenericsType[] cnTypes = cn.getGenericsTypes();
157+
// raw type usage is always allowed
158+
if (cnTypes == null) return;
159+
GenericsType[] rnTypes = rn.getGenericsTypes();
160+
// you can't parameterize a non-generified type
161+
if (rnTypes == null) {
162+
String message = "The class " + cn.toString(false) + " (supplied with " + plural("type parameter", cnTypes.length) +
163+
") refers to the class " + rn.toString(false) + " which takes no parameters";
164+
if (cnTypes.length == 0) {
165+
message += " (invalid Diamond <> usage?)";
166+
}
167+
addError(message, cn);
168+
return;
169+
}
170+
// parameterize a type by using all of the parameters only
171+
if (cnTypes.length != rnTypes.length) {
172+
if (Boolean.FALSE.equals(isAIC) && cnTypes.length == 0) {
173+
return; // allow Diamond for non-AIC cases from CCE
174+
}
175+
String message;
176+
if (Boolean.TRUE.equals(isAIC) && cnTypes.length == 0) {
177+
message = "Cannot use diamond <> with anonymous inner classes";
178+
} else {
179+
message = "The class " + cn.toString(false) + " (supplied with " + plural("type parameter", cnTypes.length) +
180+
") refers to the class " + rn.toString(false) + " which takes " + plural("parameter", rnTypes.length);
181+
if (cnTypes.length == 0) {
182+
message += " (invalid Diamond <> usage?)";
183+
}
184+
}
185+
addError(message, cn);
186+
return;
187+
}
188+
for (int i = 0; i < cnTypes.length; i++) {
189+
ClassNode cnType = cnTypes[i].getType();
190+
ClassNode rnType = rnTypes[i].getType();
191+
// check nested type parameters
192+
checkGenericsUsage(cnType);
193+
// check bounds: unbounded wildcard (aka "?") is universal substitute
194+
if (!isUnboundedWildcard(cnTypes[i])) {
195+
// check upper bound(s)
196+
ClassNode[] bounds = rnTypes[i].getUpperBounds();
197+
198+
// first can be class or interface
199+
boolean valid = cnType.isDerivedFrom(rnType) || ((rnType.isInterface() || Traits.isTrait(rnType)) && cnType.implementsInterface(rnType));
200+
201+
// subsequent bounds if present can be interfaces
202+
if (valid && bounds != null && bounds.length > 1) {
203+
for (int j = 1; j < bounds.length; j++) {
204+
ClassNode bound = bounds[j];
205+
if (!cnType.implementsInterface(bound)) {
206+
valid = false;
207+
break;
208+
}
209+
}
210+
}
211+
212+
if (!valid) {
213+
addError("The type " + cnTypes[i].getName() + " is not a valid substitute for the bounded parameter <" + rnTypes[i] + ">", cnTypes[i]);
214+
}
215+
}
216+
}
217+
}
218+
219+
private static String plural(final String string, final int count) {
220+
return "" + count + " " + (count == 1 ? string : string + "s");
221+
}
222+
}

base/org.codehaus.groovy30/src/org/codehaus/groovy/control/ResolveVisitor.java

+11-4
Original file line numberDiff line numberDiff line change
@@ -83,13 +83,13 @@
8383
import static org.codehaus.groovy.ast.tools.ClosureUtils.getParametersSafe;
8484

8585
/**
86-
* Visitor to resolve Types and convert VariableExpression to
86+
* Visitor to resolve types and convert VariableExpression to
8787
* ClassExpressions if needed. The ResolveVisitor will try to
8888
* find the Class for a ClassExpression and prints an error if
8989
* it fails to do so. Constructions like C[], foo as C, (C) foo
9090
* will force creation of a ClassExpression for C
9191
* <p>
92-
* Note: the method to start the resolving is startResolving(ClassNode, SourceUnit).
92+
* Note: the method to start the resolving is {@link #startResolving(ClassNode,SourceUnit)}.
9393
*/
9494
public class ResolveVisitor extends ClassCodeExpressionTransformer {
9595
// note: BigInteger and BigDecimal are also imported by default
@@ -347,12 +347,12 @@ private void resolveOrFail(final ClassNode type, final String msg, final ASTNode
347347
}
348348

349349
private void resolveOrFail(final ClassNode type, final String msg, final ASTNode node, final boolean preferImports) {
350-
if (preferImports) {
350+
if (preferImports && !type.isResolved() && !type.isPrimaryClassNode()) {
351351
resolveGenericsTypes(type.getGenericsTypes());
352352
if (resolveAliasFromModule(type)) return;
353353
}
354-
if (resolve(type)) return;
355354
/* GRECLIPSE edit
355+
if (resolve(type)) return;
356356
if (resolveToInner(type)) return;
357357
if (resolveToOuterNested(type)) return;
358358
@@ -362,6 +362,13 @@ private void resolveOrFail(final ClassNode type, final String msg, final ASTNode
362362
while (temp.isArray())//GROOVY-8715
363363
temp = temp.getComponentType();
364364
final String name = temp.getName();
365+
try {
366+
if (resolve(type)) return;
367+
} catch (LinkageError e) {
368+
String message = "unable to define class " + name + msg + " : " + e.getLocalizedMessage();
369+
addError(message, temp.getEnd() > 0 ? temp : node);
370+
return;
371+
}
365372
String nameAsType = name.replace('.', '$');
366373
ModuleNode module = currentClass.getModule();
367374
if (!name.equals(nameAsType) && module.hasPackageName()) {

base/org.codehaus.groovy40/.checkstyle

+1
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
<file-match-pattern match-pattern="groovy/control/CompilationUnit.java" include-pattern="false" />
4848
<file-match-pattern match-pattern="groovy/control/CompilerConfiguration.java" include-pattern="false" />
4949
<file-match-pattern match-pattern="groovy/control/ErrorCollector.java" include-pattern="false" />
50+
<file-match-pattern match-pattern="groovy/control/GenericsVisitor.java" include-pattern="false" />
5051
<file-match-pattern match-pattern="groovy/control/ProcessingUnit.java" include-pattern="false" />
5152
<file-match-pattern match-pattern="groovy/control/ResolveVisitor.java" include-pattern="false" />
5253
<file-match-pattern match-pattern="groovy/control/SourceUnit.java" include-pattern="false" />

0 commit comments

Comments
 (0)