Repository: groovy
Updated Branches:
  refs/heads/GROOVY_2_4_X f3d031014 -> 036c14fa0


GROOVY-7620: No error if abstract getter is not implemented but static field 
exists (closes #342)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/df63a38d
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/df63a38d
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/df63a38d

Branch: refs/heads/GROOVY_2_4_X
Commit: df63a38da748fc82d2c703059ca6af3c4dcfb36c
Parents: f3d0310
Author: paulk <pa...@asert.com.au>
Authored: Tue May 31 21:14:54 2016 +1000
Committer: paulk <pa...@asert.com.au>
Committed: Wed Jun 1 13:44:15 2016 +1000

----------------------------------------------------------------------
 src/main/org/codehaus/groovy/ast/ClassNode.java | 23 ++++---
 .../classgen/ClassCompletionVerifier.java       | 71 +++++++++++---------
 src/test/groovy/bugs/Groovy7081Bug.groovy       |  2 +-
 src/test/groovy/bugs/Groovy7620Bug.groovy       | 59 ++++++++++++++++
 4 files changed, 114 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/df63a38d/src/main/org/codehaus/groovy/ast/ClassNode.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/ast/ClassNode.java 
b/src/main/org/codehaus/groovy/ast/ClassNode.java
index c011241..faf4570 100644
--- a/src/main/org/codehaus/groovy/ast/ClassNode.java
+++ b/src/main/org/codehaus/groovy/ast/ClassNode.java
@@ -429,30 +429,33 @@ public class ClassNode extends AnnotatedNode implements 
Opcodes {
     public Map<String, MethodNode> getDeclaredMethodsMap() {
         // Start off with the methods from the superclass.
         ClassNode parent = getSuperClass();
-        Map<String, MethodNode> result = null;
+        Map<String, MethodNode> result;
         if (parent != null) {
             result = parent.getDeclaredMethodsMap();
         } else {
             result = new HashMap<String, MethodNode>();
         }
+        addInterfaceMethods(result);
 
+        // And add in the methods implemented in this class.
+        for (MethodNode method : getMethods()) {
+            String sig = method.getTypeDescriptor();
+            result.put(sig, method);
+        }
+        return result;
+    }
+
+    public void addInterfaceMethods(Map<String, MethodNode> methodsMap) {
         // add in unimplemented abstract methods from the interfaces
         for (ClassNode iface : getInterfaces()) {
             Map<String, MethodNode> ifaceMethodsMap = 
iface.getDeclaredMethodsMap();
             for (String methSig : ifaceMethodsMap.keySet()) {
-                if (!result.containsKey(methSig)) {
+                if (!methodsMap.containsKey(methSig)) {
                     MethodNode methNode = ifaceMethodsMap.get(methSig);
-                    result.put(methSig, methNode);
+                    methodsMap.put(methSig, methNode);
                 }
             }
         }
-
-        // And add in the methods implemented in this class.
-        for (MethodNode method : getMethods()) {
-            String sig = method.getTypeDescriptor();
-            result.put(sig, method);
-        }
-        return result;
     }
 
     public String getName() {

http://git-wip-us.apache.org/repos/asf/groovy/blob/df63a38d/src/main/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/classgen/ClassCompletionVerifier.java 
b/src/main/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
index 47e3175..c29c705 100644
--- a/src/main/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
+++ b/src/main/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
@@ -84,33 +84,48 @@ public class ClassCompletionVerifier extends 
ClassCodeVisitorSupport {
     }
 
     private void checkNoStaticMethodWithSameSignatureAsNonStatic(final 
ClassNode node) {
-        Map<String, MethodNode> result = new HashMap<String, MethodNode>();
-        // add in unimplemented abstract methods from the interfaces
-        for (ClassNode iface : node.getInterfaces()) {
-            Map<String, MethodNode> ifaceMethodsMap = 
iface.getDeclaredMethodsMap();
-            for (String methSig : ifaceMethodsMap.keySet()) {
-                if (!result.containsKey(methSig)) {
-                    MethodNode methNode = ifaceMethodsMap.get(methSig);
-                    result.put(methSig, methNode);
-                }
-            }
+        ClassNode parent = node.getSuperClass();
+        Map<String, MethodNode> result;
+        // start with methods from the parent if any
+        if (parent != null) {
+            result = parent.getDeclaredMethodsMap();
+        } else {
+            result = new HashMap<String, MethodNode>();
         }
+        // add in unimplemented abstract methods from the interfaces
+        node.addInterfaceMethods(result);
         for (MethodNode methodNode : node.getMethods()) {
             MethodNode mn = result.get(methodNode.getTypeDescriptor());
-            if (mn!=null && methodNode.isStatic() && 
!methodNode.isStaticConstructor()) {
+            if (mn != null && (mn.isStatic() ^ methodNode.isStatic()) && 
!methodNode.isStaticConstructor()) {
+                if (!mn.isAbstract()) continue;
                 ClassNode declaringClass = mn.getDeclaringClass();
                 ClassNode cn = declaringClass.getOuterClass();
-                if (cn==null && declaringClass.isResolved()) {
+                if (cn == null && declaringClass.isResolved()) {
                     // in case of a precompiled class, the outerclass is 
unknown
                     Class typeClass = declaringClass.getTypeClass();
                     typeClass = typeClass.getEnclosingClass();
-                    if (typeClass!=null) {
+                    if (typeClass != null) {
                         cn = ClassHelper.make(typeClass);
                     }
                 }
-                if (cn==null || !Traits.isTrait(cn)) {
-                    addError("Method '" + mn.getName() + "' is already defined 
in " + getDescription(node) + ". You cannot have " +
-                            "both a static and a non static method with the 
same signature", methodNode);
+                if (cn == null || !Traits.isTrait(cn)) {
+                    ASTNode errorNode = methodNode;
+                    String name = mn.getName();
+                    if (errorNode.getLineNumber() == -1) {
+                        // try to get a better error message location based on 
the property
+                        for (PropertyNode propertyNode : node.getProperties()) 
{
+                            if (name.startsWith("set") || 
name.startsWith("get") || name.startsWith("is")) {
+                                String propName = 
Verifier.capitalize(propertyNode.getField().getName());
+                                String shortName = 
name.substring(name.startsWith("is") ? 2 : 3);
+                                if (propName.equals(shortName)) {
+                                    errorNode = propertyNode;
+                                    break;
+                                }
+                            }
+                        }
+                    }
+                    addError("The " + getDescription(methodNode) + " is 
already defined in " + getDescription(node) +
+                            ". You cannot have both a static and an instance 
method with the same signature", errorNode);
                 }
             }
             result.put(methodNode.getTypeDescriptor(), methodNode);
@@ -307,6 +322,13 @@ public class ClassCompletionVerifier extends 
ClassCodeVisitorSupport {
     private void addInvalidUseOfFinalError(MethodNode method, Parameter[] 
parameters, ClassNode superCN) {
         StringBuilder msg = new StringBuilder();
         msg.append("You are not allowed to override the final method 
").append(method.getName());
+        appendParamsDescription(parameters, msg);
+        msg.append(" from ").append(getDescription(superCN));
+        msg.append(".");
+        addError(msg.toString(), method);
+    }
+
+    private void appendParamsDescription(Parameter[] parameters, StringBuilder 
msg) {
         msg.append("(");
         boolean needsComma = false;
         for (Parameter parameter : parameters) {
@@ -317,25 +339,14 @@ public class ClassCompletionVerifier extends 
ClassCodeVisitorSupport {
             }
             msg.append(parameter.getType());
         }
-        msg.append(") from ").append(getDescription(superCN));
-        msg.append(".");
-        addError(msg.toString(), method);
+        msg.append(")");
     }
 
     private void addWeakerAccessError(ClassNode cn, MethodNode method, 
Parameter[] parameters, MethodNode superMethod) {
         StringBuilder msg = new StringBuilder();
         msg.append(method.getName());
-        msg.append("(");
-        boolean needsComma = false;
-        for (Parameter parameter : parameters) {
-            if (needsComma) {
-                msg.append(",");
-            } else {
-                needsComma = true;
-            }
-            msg.append(parameter.getType());
-        }
-        msg.append(") in ");
+        appendParamsDescription(parameters, msg);
+        msg.append(" in ");
         msg.append(cn.getName());
         msg.append(" cannot override ");
         msg.append(superMethod.getName());

http://git-wip-us.apache.org/repos/asf/groovy/blob/df63a38d/src/test/groovy/bugs/Groovy7081Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy7081Bug.groovy 
b/src/test/groovy/bugs/Groovy7081Bug.groovy
index 85411d8..441e3a6 100644
--- a/src/test/groovy/bugs/Groovy7081Bug.groovy
+++ b/src/test/groovy/bugs/Groovy7081Bug.groovy
@@ -34,7 +34,7 @@ class Groovy7081Bug extends GroovyTestCase {
             new Foo().name
             '''
 
-        assert msg.contains("Method 'getName' is already defined in class 
'Foo'")
+        assert msg.contains("The method 'java.lang.String getName()' is 
already defined in class 'Foo'")
     }
 
     void testShouldSeeConflictInTypeSignature() {

http://git-wip-us.apache.org/repos/asf/groovy/blob/df63a38d/src/test/groovy/bugs/Groovy7620Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy7620Bug.groovy 
b/src/test/groovy/bugs/Groovy7620Bug.groovy
new file mode 100644
index 0000000..44525ef
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy7620Bug.groovy
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+
+package groovy.bugs
+
+class Groovy7620Bug extends GroovyTestCase {
+    void testShouldSeeThatMethodIsNotImplemented() {
+        def msg = shouldFail '''
+            abstract class A {
+               abstract Object getFoo()
+
+               void test() {
+                   println getFoo()
+               }
+            }
+
+            class B extends A {
+               static Object foo
+            }
+
+            new B().test()
+            '''
+
+        assert msg.contains("The method 'java.lang.Object getFoo()' is already 
defined in class 'B'")
+    }
+
+    void testShouldSeeConflictInTypeSignature() {
+        def msg = shouldFail '''
+            interface C {
+               Object getFoo()
+            }
+
+            class D implements C {
+               static Object foo
+            }
+
+            new B().test()
+            '''
+
+        assert msg.contains("The method 'java.lang.Object getFoo()' is already 
defined in class 'D'")
+    }
+}

Reply via email to