You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2019/07/02 05:05:11 UTC

[groovy] 01/02: GROOVY-9176: fix line and column of error for property-generated method (closes #959)

This is an automated email from the ASF dual-hosted git repository.

paulk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git

commit 218c4cdb245642d9a2d35a6ce2d60b59336b3c4c
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Jul 1 11:18:52 2019 -0500

    GROOVY-9176: fix line and column of error for property-generated method (closes #959)
---
 .../org/codehaus/groovy/classgen/Verifier.java     | 40 +++++++++++++++------
 src/test/groovy/bugs/Groovy9176.groovy             | 42 ++++++++++++++++++++++
 2 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
index dae1943..c9b4f37 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -26,6 +26,7 @@ import groovy.transform.Internal;
 import org.apache.groovy.ast.tools.ClassNodeUtils;
 import org.apache.groovy.util.BeanUtils;
 import org.codehaus.groovy.GroovyBugError;
+import org.codehaus.groovy.ast.ASTNode;
 import org.codehaus.groovy.ast.AnnotationNode;
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
@@ -89,6 +90,7 @@ import static java.lang.reflect.Modifier.isStatic;
 import static java.util.stream.Collectors.joining;
 import static org.apache.groovy.ast.tools.AnnotatedNodeUtils.markAsGenerated;
 import static org.apache.groovy.ast.tools.ExpressionUtils.transformInlineConstants;
+import static org.apache.groovy.ast.tools.MethodNodeUtils.getPropertyName;
 import static org.apache.groovy.ast.tools.MethodNodeUtils.methodDescriptorWithoutReturnType;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.callThisX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.castX;
@@ -98,6 +100,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
 import static org.codehaus.groovy.ast.tools.GenericsUtils.addMethodGenerics;
 import static org.codehaus.groovy.ast.tools.GenericsUtils.correctToGenericsSpec;
 import static org.codehaus.groovy.ast.tools.GenericsUtils.createGenericsSpec;
+import static org.codehaus.groovy.ast.tools.PropertyNodeUtils.adjustPropertyModifiersForMethod;
 
 /**
  * Verifies the AST node and adds any default AST code before bytecode generation occurs.
@@ -293,10 +296,10 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
             if (descriptors.contains(mySig)) {
                 if (mn.isScriptBody() || mySig.equals(scriptBodySignatureWithoutReturnType(cn))) {
                     throw new RuntimeParserException("The method " + mn.getText() +
-                            " is a duplicate of the one declared for this script's body code", mn);
+                            " is a duplicate of the one declared for this script's body code", sourceOf(mn));
                 } else {
                     throw new RuntimeParserException("The method " + mn.getText() +
-                            " duplicates another method of the same signature", mn);
+                            " duplicates another method of the same signature", sourceOf(mn));
                 }
             }
             descriptors.add(mySig);
@@ -725,7 +728,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         String getterName = "get" + capitalize(name);
         String setterName = "set" + capitalize(name);
 
-        int accessorModifiers = PropertyNodeUtils.adjustPropertyModifiersForMethod(node);
+        int accessorModifiers = adjustPropertyModifiersForMethod(node);
 
         Statement getterBlock = node.getGetterBlock();
         if (getterBlock == null) {
@@ -830,7 +833,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                         "The method with default parameters \"" + method.getTypeDescriptor() +
                                 "\" defines a method \"" + newMethod.getTypeDescriptor() +
                                 "\" that is already defined.",
-                        method);
+                        sourceOf(method));
             }
 
             List<AnnotationNode> annotations = method.getAnnotations();
@@ -953,7 +956,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                                     type.getNameWithoutPackage(),
                                     Arrays.stream(params).map(Parameter::getType).map(ClassNodeUtils::formatTypeName).collect(joining(",")),
                                     p.getName());
-                            throw new RuntimeParserException(error, method);
+                            throw new RuntimeParserException(error, sourceOf(method));
                         }
                     }
                 }
@@ -1314,7 +1317,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                     && !m.isPublic() && !m.isStaticConstructor()) {
                 throw new RuntimeParserException("The method " + m.getName() +
                         " should be public as it implements the corresponding method from interface " +
-                        intfMethod.getDeclaringClass(), m);
+                        intfMethod.getDeclaringClass(), sourceOf(m));
 
             }
         }
@@ -1404,7 +1407,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                             " in " + overridingMethod.getDeclaringClass().getName() +
                             " is incompatible with " + testmr.getName() +
                             " in " + oldMethod.getDeclaringClass().getName(),
-                    overridingMethod);
+                    sourceOf(overridingMethod));
         }
 
         if (equalReturnType && normalEqualParameters) return null;
@@ -1414,7 +1417,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                     "Cannot override final method " +
                             oldMethod.getTypeDescriptor() +
                             " in " + oldMethod.getDeclaringClass().getName(),
-                    overridingMethod);
+                    sourceOf(overridingMethod));
         }
         if (oldMethod.isStatic() != overridingMethod.isStatic()) {
             throw new RuntimeParserException(
@@ -1422,7 +1425,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                             oldMethod.getTypeDescriptor() +
                             " in " + oldMethod.getDeclaringClass().getName() +
                             " with disparate static modifier",
-                    overridingMethod);
+                    sourceOf(overridingMethod));
         }
         if (!equalReturnType) {
             boolean oldM = ClassHelper.isPrimitiveType(oldMethod.getReturnType());
@@ -1441,7 +1444,7 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
                                 oldMethod.getTypeDescriptor() +
                                 " in " + oldMethod.getDeclaringClass().getName() +
                                 message,
-                        overridingMethod);
+                        sourceOf(overridingMethod));
             }
         }
 
@@ -1588,6 +1591,23 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         return swapInitRequired;
     }
 
+    private static ASTNode sourceOf(MethodNode methodNode) {
+        if (methodNode.getLineNumber() < 1) {
+            ClassNode declaringClass = methodNode.getDeclaringClass();
+            if (methodNode.isSynthetic()) {
+                String propertyName = getPropertyName(methodNode);
+                if (propertyName != null) {
+                    PropertyNode propertyNode = declaringClass.getProperty(propertyName);
+                    if (propertyNode != null && propertyNode.getLineNumber() > 0) {
+                        return propertyNode;
+                    }
+                }
+            }
+            return declaringClass;
+        }
+        return methodNode;
+    }
+
     /**
      * When constant expressions are created, the value is always wrapped to a non primitive type.
      * Some constant expressions are optimized to return primitive types, but not all primitives are
diff --git a/src/test/groovy/bugs/Groovy9176.groovy b/src/test/groovy/bugs/Groovy9176.groovy
new file mode 100644
index 0000000..7d0a562
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy9176.groovy
@@ -0,0 +1,42 @@
+/*
+ * 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
+
+import groovy.transform.CompileStatic
+import org.codehaus.groovy.control.CompilationFailedException
+
+@CompileStatic
+final class Groovy9176 extends GroovyTestCase {
+
+    void testGroovyPropertyCovariantMethodCheck() {
+        def err = shouldFail CompilationFailedException, '''
+            class Pojo {
+                private String title;
+                public String getTitle() { return this.title; }
+                public void setTitle(String title) { this.title = title; }
+            }
+            class Test extends Pojo {
+                // property "title" with type that is incompatible with "String"
+                java.util.regex.Pattern title
+            }
+        '''
+
+        assert err =~ / The return type of java.util.regex.Pattern getTitle\(\) in Test is incompatible with java.lang.String in Pojo\n\. At \[9:17\] /
+    }
+}