This is an automated email from the ASF dual-hosted git repository. sunlan pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/groovy.git
commit ce8605726451742084967fab45b270715ee60e06 Author: Eric Milles <[email protected]> AuthorDate: Mon Jun 10 14:18:30 2019 -0500 GROOVY-9151: check for references to parameters that have been removed Constructor with multiple defaults (case 1): Type(String s1 = 'x', String s2 = s1) { ... } VariableExpression s1 can be replaced by ConstantExpression 'x' Constructor with multiple defaults (case 2): Type(String s1 = generate(), String s2 = s1) { ... } VariableExpression s1 cannot be replaced without possibility of side- effect. Compiler error is recorded. Constructor with multiple defaults (case 3): Type(String s1 = 'xyz', String s2 = s1.substring(1)) { ... } VariableExpression s1 cannot be replaced without transforming default expression. Compiler error is recorded. --- .../org/codehaus/groovy/classgen/Verifier.java | 37 ++++++++++++++++++++++ src/test/gls/invocation/DefaultParamTest.groovy | 17 +++++++++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java index 77642cd..a8fe634 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 static java.lang.reflect.Modifier.isFinal; import static java.lang.reflect.Modifier.isPrivate; import static java.lang.reflect.Modifier.isPublic; 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.methodDescriptorWithoutReturnType; @@ -923,6 +924,42 @@ public class Verifier implements GroovyClassVisitor, Opcodes { protected void addDefaultParameterConstructors(ClassNode type) { List<ConstructorNode> constructors = new ArrayList<>(type.getDeclaredConstructors()); addDefaultParameters(constructors, (arguments, params, method) -> { + // GROOVY-9151: check for references to parameters that have been removed + for (ListIterator<Expression> it = arguments.getExpressions().listIterator(); it.hasNext();) { + Expression argument = it.next(); + if (argument instanceof CastExpression) { + argument = ((CastExpression) argument).getExpression(); + } + if (argument instanceof VariableExpression) { + VariableExpression v = (VariableExpression) argument; + if (v.getAccessedVariable() instanceof Parameter) { + Parameter p = (Parameter) v.getAccessedVariable(); + if (p.hasInitialExpression() && !Arrays.asList(params).contains(p) + && p.getInitialExpression() instanceof ConstantExpression) { + // replace argument "(Type) param" with "(Type) <param's default>" for simple default value + it.set(castX(method.getParameters()[it.nextIndex() - 1].getType(), p.getInitialExpression())); + } + } + } + } + GroovyCodeVisitor visitor = new CodeVisitorSupport() { + @Override + public void visitVariableExpression(VariableExpression e) { + if (e.getAccessedVariable() instanceof Parameter) { + Parameter p = (Parameter) e.getAccessedVariable(); + if (p.hasInitialExpression() && !Arrays.asList(params).contains(p)) { + String error = String.format( + "The generated constructor \"%s(%s)\" references parameter '%s' which has been replaced by a default value expression.", + type.getNameWithoutPackage(), + Arrays.stream(params).map(Parameter::getType).map(ClassNodeUtils::formatTypeName).collect(joining(",")), + p.getName()); + throw new RuntimeParserException(error, method); + } + } + } + }; + visitor.visitArgumentlistExpression(arguments); + // delegate to original constructor using arguments derived from defaults Statement code = new ExpressionStatement(new ConstructorCallExpression(ClassNode.THIS, arguments)); addConstructor(params, (ConstructorNode) method, code, type); diff --git a/src/test/gls/invocation/DefaultParamTest.groovy b/src/test/gls/invocation/DefaultParamTest.groovy index 465af04..d7a501f 100644 --- a/src/test/gls/invocation/DefaultParamTest.groovy +++ b/src/test/gls/invocation/DefaultParamTest.groovy @@ -139,9 +139,22 @@ final class DefaultParamTest extends GroovyTestCase { } // GROOVY-9151 - void _FIXME_testConstructorWithAllParametersDefaulted() { + void testConstructorWithAllParametersDefaulted() { assertScript ''' class Greeting { + Greeting(Object o = 'world', String s = o) { + this.text = "hello $s" + } + String text + } + assert new Greeting().text == 'hello world' + ''' + } + + // GROOVY-9151 + void testConstructorWithAllParametersDefaulted2() { + def err = shouldFail ''' + class Greeting { Greeting(Object o = 'world', String s = o.toString()) { this.text = "hello $s" } @@ -149,6 +162,8 @@ final class DefaultParamTest extends GroovyTestCase { } assert new Greeting().text == 'hello world' ''' + + assert err =~ /The generated constructor "Greeting\(\)" references parameter 'o' which has been replaced by a default value expression./ } // GROOVY-5632
