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 <[email protected]> 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\] / + } +}
