This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-8659 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 99a19ebe79666ee5ae74bdda58065a95546f12a4 Author: Eric Milles <[email protected]> AuthorDate: Tue Sep 16 10:37:36 2025 -0500 GROOVY-8659: warning for inability to generate property access method --- .../org/codehaus/groovy/classgen/Verifier.java | 18 ++++- src/test/groovy/groovy/PropertyTest.groovy | 90 ++++++++++++++++++++++ 2 files changed, 105 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java index beac6f0125..0a37b824de 100644 --- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java +++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java @@ -86,6 +86,7 @@ import java.util.List; import java.util.ListIterator; import java.util.Map; import java.util.Set; +import java.util.function.Consumer; import java.util.function.IntFunction; import java.util.stream.Collectors; @@ -827,14 +828,16 @@ public class Verifier implements GroovyClassVisitor, Opcodes { if (getter == null && isPrimitiveBoolean(node.getType())) { getter = classNode.getGetterMethod("is" + capitalize(name), !node.isStatic()); } - if (methodNeedsReplacement(getter)) { + if (methodNeedsReplacement(getter, + (mn) -> cannotReplaceMethod(node, mn))) { getterBlock = createGetterBlock(node, field); } } Statement setterBlock = node.getSetterBlock(); if (setterBlock == null && !node.isPrivate() && !node.isFinal()) { MethodNode setter = classNode.getSetterMethod(setterName, false); // atypical: allow setter with non-void return type - if (methodNeedsReplacement(setter)) { + if (methodNeedsReplacement(setter, + (mn) -> cannotReplaceMethod(node, mn))) { setterBlock = createSetterBlock(node, field); } } @@ -859,13 +862,22 @@ public class Verifier implements GroovyClassVisitor, Opcodes { } } - private boolean methodNeedsReplacement(final MethodNode mn) { + private void cannotReplaceMethod(final PropertyNode pn, final MethodNode mn) { + String message = MethodNodeUtils.methodDescriptor(mn, true); + message = message.substring(message.indexOf(' ') + 1); // strip return type + message = String.format("Property %s cannot override final method %s of class %s", pn.getName(), message, mn.getDeclaringClass().getName()); + + classNode.getModule().getContext().addWarning(message, pn); + } + + private boolean methodNeedsReplacement(final MethodNode mn, final Consumer<MethodNode> cannotReplace) { if (mn != null) { ClassNode declaringClass = mn.getDeclaringClass(); // nothing to be done for method of current class if (declaringClass == classNode) return false; // cannot overwrite a non-private final method if (mn.isFinal() && !mn.isPrivate() && (!mn.isPackageScope() || inSamePackage(declaringClass, classNode))) { + cannotReplace.accept(mn); // GROOVY-8659 return false; } } diff --git a/src/test/groovy/groovy/PropertyTest.groovy b/src/test/groovy/groovy/PropertyTest.groovy index 88c3493600..2a9db0a400 100644 --- a/src/test/groovy/groovy/PropertyTest.groovy +++ b/src/test/groovy/groovy/PropertyTest.groovy @@ -18,6 +18,9 @@ */ package groovy +import org.codehaus.groovy.control.CompilationUnit +import org.codehaus.groovy.control.CompilerConfiguration +import org.codehaus.groovy.control.messages.WarningMessage import org.junit.jupiter.api.Test import static groovy.test.GroovyAssert.assertScript @@ -352,6 +355,93 @@ final class PropertyTest { ''' } + // GROOVY-8659 + @Test + void testPropertyCannotOverrideFinalGetter() { + File parentDir = File.createTempDir() + def config = new CompilerConfiguration() + config.targetDirectory = File.createTempDir() + config.warningLevel = WarningMessage.POSSIBLE_ERRORS + try { + def a = new File(parentDir, 'A.groovy') + a.write ''' + abstract class A { + final String getFoo() { 'A' } + } + ''' + def c = new File(parentDir, 'C.groovy') + c.write ''' + class C extends A { + final String foo = 'C' + } + ''' + def m = new File(parentDir, 'Main.groovy') + m.write ''' + def pogo = new C() + assert pogo.foo == 'A' + assert pogo.getFoo() == 'A' + ''' + + def loader = new GroovyClassLoader(this.class.classLoader) + def cu = new CompilationUnit(config, null, loader) + cu.addSources(a, c, m) + cu.compile() + + assert cu.errorCollector.warnings*.message == [ + 'Property foo cannot override final method getFoo() of class A' + ] + + loader.addClasspath(config.targetDirectory.absolutePath) + loader.loadClass('Main').main() + } finally { + parentDir.deleteDir() + config.targetDirectory.deleteDir() + } + } + + // GROOVY-8659 + @Test + void testPropertyCannotOverrideFinalSetter() { + File parentDir = File.createTempDir() + def config = new CompilerConfiguration() + config.targetDirectory = File.createTempDir() + config.warningLevel = WarningMessage.POSSIBLE_ERRORS + try { + def a = new File(parentDir, 'A.groovy') + a.write ''' + abstract class A { + final void setFoo(String foo) { } + } + ''' + def c = new File(parentDir, 'C.groovy') + c.write ''' + class C extends A { + String foo + } + ''' + def m = new File(parentDir, 'Main.groovy') + m.write ''' + def pogo = new C(foo: 'C') + assert pogo.foo == null + ''' + + def loader = new GroovyClassLoader(this.class.classLoader) + def cu = new CompilationUnit(config, null, loader) + cu.addSources(a, c, m) + cu.compile() + + assert cu.errorCollector.warnings*.message == [ + 'Property foo cannot override final method setFoo(java.lang.String) of class A' + ] + + loader.addClasspath(config.targetDirectory.absolutePath) + loader.loadClass('Main').main() + } finally { + parentDir.deleteDir() + config.targetDirectory.deleteDir() + } + } + @Test void testPropertyWithOverrideGetterAndSetter() { assertScript '''
