Skip to content

Commit 2a2a966

Browse files
committed
GROOVY-11579: skip bridge method in final method override checking
4_0_X backport
1 parent fce85ec commit 2a2a966

File tree

3 files changed

+59
-30
lines changed

3 files changed

+59
-30
lines changed

src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java

+22-14
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
import static org.objectweb.asm.Opcodes.ACC_STATIC;
7575
import static org.objectweb.asm.Opcodes.ACC_STRICT;
7676
import static org.objectweb.asm.Opcodes.ACC_SYNCHRONIZED;
77+
import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
7778
import static org.objectweb.asm.Opcodes.ACC_TRANSIENT;
7879
import static org.objectweb.asm.Opcodes.ACC_VOLATILE;
7980

@@ -431,27 +432,33 @@ private void checkMethodsForWeakerAccess(final ClassNode cn) {
431432
}
432433

433434
private void checkMethodsForOverridingFinal(final ClassNode cn) {
435+
final int skips = ACC_SYNTHETIC | ACC_STATIC | ACC_PRIVATE;
434436
for (MethodNode method : cn.getMethods()) {
437+
if ((method.getModifiers() & skips) != 0) continue; // GROOVY-11579
438+
435439
Parameter[] params = method.getParameters();
436440
for (MethodNode superMethod : cn.getSuperClass().getMethods(method.getName())) {
437-
Parameter[] superParams = superMethod.getParameters();
438-
if (!ParameterUtils.parametersEqual(params, superParams)) continue;
439-
if (!superMethod.isFinal()) break;
440-
addInvalidUseOfFinalError(method, params, superMethod.getDeclaringClass());
441-
return;
441+
if ((superMethod.getModifiers() & skips + ACC_FINAL) == ACC_FINAL
442+
&& ParameterUtils.parametersEqual(params, superMethod.getParameters())) {
443+
StringBuilder sb = new StringBuilder();
444+
sb.append("You are not allowed to override the final method ");
445+
if (method.getName().contains(" ")) {
446+
sb.append('"').append(method.getName()).append('"');
447+
} else {
448+
sb.append(method.getName());
449+
}
450+
appendParamsDescription(params, sb);
451+
sb.append(" from ");
452+
sb.append(getDescription(superMethod.getDeclaringClass()));
453+
sb.append(".");
454+
455+
addError(sb.toString(), method.getLineNumber() > 0 ? method : cn);
456+
break;
457+
}
442458
}
443459
}
444460
}
445461

446-
private void addInvalidUseOfFinalError(final MethodNode method, final Parameter[] parameters, final ClassNode superCN) {
447-
StringBuilder msg = new StringBuilder();
448-
msg.append("You are not allowed to override the final method ").append(method.getName());
449-
appendParamsDescription(parameters, msg);
450-
msg.append(" from ").append(getDescription(superCN));
451-
msg.append(".");
452-
addError(msg.toString(), method);
453-
}
454-
455462
private void appendParamsDescription(final Parameter[] parameters, final StringBuilder msg) {
456463
msg.append('(');
457464
boolean needsComma = false;
@@ -478,6 +485,7 @@ private void addWeakerAccessError(final ClassNode cn, final MethodNode method, f
478485
msg.append(superMethod.getDeclaringClass().getName());
479486
msg.append("; attempting to assign weaker access privileges; was ");
480487
msg.append(superMethod.isPublic() ? "public" : (superMethod.isProtected() ? "protected" : "package-private"));
488+
481489
addError(msg.toString(), method);
482490
}
483491

src/main/java/org/codehaus/groovy/classgen/Verifier.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -1602,9 +1602,10 @@ private static Parameter[] cleanParameters(final Parameter[] parameters) {
16021602
}
16031603

16041604
private static ClassNode cleanType(final ClassNode type) {
1605-
// TODO: Should this be directly handled by getPlainNodeReference?
1606-
if (type.isArray()) return cleanType(type.getComponentType()).makeArray();
1607-
return type.getPlainNodeReference();
1605+
if (type.isArray()) {
1606+
return cleanType(type.getComponentType()).makeArray();
1607+
}
1608+
return type.redirect().getPlainNodeReference();
16081609
}
16091610

16101611
private void storeMissingCovariantMethods(final Iterable<MethodNode> methods, final MethodNode method, final Map<String, MethodNode> methodsToAdd, final Map<String, ClassNode> genericsSpec, final boolean ignoreError) {

src/test/groovy/OverrideTest.groovy

+33-13
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,8 @@ final class OverrideTest {
140140
assert err.message.contains("Method 'methodTakesObject' from class 'HasMethodWithBadArgType' does not override method from its superclass or interfaces but is annotated with @Override.")
141141
}
142142

143-
@Test // GROOVY-6654
143+
// GROOVY-6654
144+
@Test
144145
void testCovariantParameterType1() {
145146
assertScript '''
146147
class C<T> {
@@ -156,7 +157,8 @@ final class OverrideTest {
156157
'''
157158
}
158159

159-
@Test // GROOVY-10675
160+
// GROOVY-10675
161+
@Test
160162
void testCovariantParameterType2() {
161163
assertScript '''
162164
@FunctionalInterface
@@ -174,48 +176,66 @@ final class OverrideTest {
174176
'''
175177
}
176178

177-
@Test // GROOVY-7849
179+
// GROOVY-7849
180+
@Test
178181
void testCovariantArrayReturnType1() {
179182
assertScript '''
180-
interface Base {}
181-
182-
interface Derived extends Base {}
183-
183+
interface A {
184+
}
185+
interface B extends A {
186+
}
184187
interface I {
185-
Base[] foo()
188+
A[] foo()
186189
}
187-
188190
class C implements I {
189-
Derived[] foo() { null }
191+
B[] foo() { null }
190192
}
193+
191194
new C().foo()
192195
'''
193196
}
194197

195-
@Test // GROOVY-7185
198+
// GROOVY-7185
199+
@Test
196200
void testCovariantArrayReturnType2() {
197201
assertScript '''
198202
interface A<T> {
199203
T[] process();
200204
}
201-
202205
class B implements A<String> {
203206
@Override
204207
public String[] process() {
205208
['foo']
206209
}
207210
}
208-
209211
class C extends B {
210212
@Override
211213
String[] process() {
212214
super.process()
213215
}
214216
}
217+
215218
assert new C().process()[0] == 'foo'
216219
'''
217220
}
218221

222+
// GROOVY-11579
223+
@Test
224+
void testCovariantBridgeReturnType() {
225+
assertScript '''
226+
interface I<T> {
227+
T m()
228+
}
229+
abstract class A {
230+
final String m() { 'A' }
231+
}
232+
class C extends A implements I<String> {
233+
}
234+
235+
assert new C().m() == 'A'
236+
'''
237+
}
238+
219239
@Test
220240
void testOverrideOnMethodWithDefaultParameters() {
221241
assertScript '''

0 commit comments

Comments
 (0)