From 3b82478e0643c6f3187de092cc2d3257a35885fd Mon Sep 17 00:00:00 2001 From: Paul King Date: Mon, 6 Aug 2018 12:51:14 +1000 Subject: [PATCH] GROOVY-8722: final modifier for non-abstract methods in traits is ignored (closes #780) --- .../trait/TraitASTTransformation.java | 2 +- src/spec/doc/core-traits.adoc | 15 +++++- .../traitx/TraitASTTransformationTest.groovy | 54 +++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java index 3fa15aed8cb..1dbbe96e743 100644 --- a/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java @@ -543,7 +543,7 @@ private MethodNode processMethod(ClassNode traitClass, ClassNode traitHelperClas Parameter[] newParams = new Parameter[initialParams.length + 1]; newParams[0] = createSelfParameter(traitClass, methodNode.isStatic()); System.arraycopy(initialParams, 0, newParams, 1, initialParams.length); - final int mod = methodNode.isPrivate()?ACC_PRIVATE:ACC_PUBLIC; + final int mod = methodNode.isPrivate() ? ACC_PRIVATE : ACC_PUBLIC | (methodNode.isFinal() ? ACC_FINAL : 0); MethodNode mNode = new MethodNode( methodNode.getName(), mod | ACC_STATIC, diff --git a/src/spec/doc/core-traits.adoc b/src/spec/doc/core-traits.adoc index 2f5b9a1773c..e74ad980c4d 100644 --- a/src/spec/doc/core-traits.adoc +++ b/src/spec/doc/core-traits.adoc @@ -100,6 +100,18 @@ include::{projectdir}/src/spec/test/TraitsSpecificationTest.groovy[tags=private_ WARNING: Traits only support `public` and `private` methods. Neither `protected` nor `package private` scopes are supported. +=== Final methods + +If we have a class implementing a trait, conceptually implementations from the trait methods +are "inherited" into the class. But, in reality, there is no base class containing such +implementations. Rather, they are woven directly into the class. A final modifier on a method +just indicates what the modifier will be for the woven method. While it would likely be +considered bad style to inherit and override or multiply inherit methods with the same +signature but a mix of final and non-final variants, Groovy doesn't prohibit this scenario. +Normal method selection applies and the modifier used will be determined from the resulting method. +You might consider creating a base class which implements the desired trait(s) if you +want trait implementation methods that can't be overridden. + == The meaning of this `this` represents the implementing instance. Think of a trait as a superclass. This means that when you write: @@ -639,7 +651,7 @@ runtime mixins, not the @Mixin annotation which is deprecated in favour of trait First of all, methods defined in a trait are visible in bytecode: -* internally, the trait is represented as an interface (without default methods) and several helper classes +* internally, the trait is represented as an interface (without default or static methods) and several helper classes * this means that an object implementing a trait effectively implements an _interface_ * those methods are visible from Java * they are compatible with type checking and static compilation @@ -671,6 +683,7 @@ It is possible to define static methods in a trait, but it comes with numerous l * Traits with static methods cannot be compiled statically or type checked. All static methods, properties and field are accessed dynamically (it's a limitation from the JVM). +* Static methods do not appear within the generated interfaces for each trait. * The trait is interpreted as a _template_ for the implementing class, which means that each implementing class will get its own static methods, properties and fields. So a static member declared on a trait doesn't belong to the `Trait`, but to it's implementing class. diff --git a/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy b/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy index 97f96559b07..ca38d168981 100644 --- a/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy +++ b/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy @@ -2553,4 +2553,58 @@ assert c.b() == 2 ''' } + //GROOVY-8722 + void testFinalModifierSupport() { + assertScript ''' + import static java.lang.reflect.Modifier.isFinal + + trait Foo { + final int bar() { 2 } + final int baz() { 4 } + } + + trait Foo2 { + int baz() { 6 } + } + + class FooFoo2 implements Foo, Foo2 { } + + class Foo2Foo implements Foo2, Foo { + int bar() { 8 } + } + + def isFinal(Class k, String methodName) { + isFinal(k.getMethod(methodName, [] as Class[]).modifiers) + } + + new Foo2Foo().with { + assert bar() == 8 + assert baz() == 4 + assert !isFinal(Foo2Foo, 'bar') + assert isFinal(Foo2Foo, 'baz') + } + + new FooFoo2().with { + assert bar() == 2 + assert baz() == 6 + assert isFinal(FooFoo2, 'bar') + assert !isFinal(FooFoo2, 'baz') + } + ''' + assertScript ''' + trait Startable { + final int start() { doStart() * 2 } + abstract int doStart() { } + } + + abstract class Base implements Startable { } + + class Application extends Base { + int doStart() { 21 } + } + + assert new Application().start() == 42 + ''' + } + }