Skip to content

Commit

Permalink
GROOVY-9344
Browse files Browse the repository at this point in the history
  • Loading branch information
eric-milles committed Jun 3, 2020
1 parent 2a5701c commit 45e9554
Show file tree
Hide file tree
Showing 10 changed files with 363 additions and 14 deletions.
Expand Up @@ -74,8 +74,8 @@ public void testCompileStatic1() {
}

/**
* Testing the code in the StaticTypeCheckingSupport.checkCompatibleAssignmentTypes.
*
* Test case for {@link org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport#checkCompatibleAssignmentTypes(ClassNode,ClassNode,Expression,boolean) checkCompatibleAssignmentTypes}.
* <p>
* That method does a lot of == testing against ClassNode constants, which may not work so well for us.
*/
@Test
Expand Down Expand Up @@ -481,6 +481,34 @@ public void testCompileStatic18() {
runConformTest(sources, "123");
}

/**
* Test case for {@link org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor#performSecondPass() performSecondPass}.
*/
@Test
public void testCompileStatic19() {
//@formatter:off
String[] sources = {
"Script.groovy",
"@groovy.transform.CompileStatic\n" +
"void test() {\n" +
" def x = 'xyz';\n" +
" { -> x = 1 }\n" +
" x.charAt(0)\n" +
"}\n",
};
//@formatter:on

runNegativeTest(sources,
"----------\n" +
"1. ERROR in Script.groovy (at line 5)\n" +
"\tx.charAt(0)\n" +
"\t^^^^^^^^^^^\n" +
"Groovy:[Static type checking] - A closure shared variable [x] has been assigned with various types" +
" and the method [charAt(int)] does not exist in the lowest upper bound of those types: [java.io.Serializable <? extends java.lang.Object>]." +
" In general, this is a bad practice (variable reuse) because the compiler cannot determine safely what is the type of the variable at the moment of the call in a multithreaded context.\n" +
"----------\n");
}

@Test
public void testCompileStatic1505() {
//@formatter:off
Expand Down Expand Up @@ -598,6 +626,29 @@ public void testCompileStatic6095() {
runConformTest(sources, "123.0");
}

@Test
public void testCompileStatic6921() {
//@formatter:off
String[] sources = {
"Script.groovy",
"@groovy.transform.CompileStatic\n" +
"def test(List<List<?>> list) {\n" +
" list.collectMany { pair ->\n" +
" (1..pair[1]).collect {\n" +
" \"${pair[0]} supports $it\".toString()\n" +
" }\n" +
" }\n" +
"}\n" +
"print test([\n" +
" ['x', 1],\n" +
" ['y', 2],\n" +
"])\n",
};
//@formatter:on

runConformTest(sources, "[x supports 1, y supports 1, y supports 2]");
}

@Test
public void testCompileStatic7300() {
//@formatter:off
Expand Down Expand Up @@ -3797,6 +3848,54 @@ public void testCompileStatic9342() {
runConformTest(sources, "6");
}

@Test
public void testCompileStatic9344() {
//@formatter:off
String[] sources = {
"Script.groovy",
"class A {}\n" +
"class B {}\n" +
"@groovy.transform.CompileStatic\n" +
"def test() {\n" +
" def var\n" +
" var = new A()\n" +
" def c = { ->\n" +
" var = new B()\n" + // Cannot cast object 'B@4e234c52' with class 'B' to class 'A'
" print var.class.simpleName\n" +
" }\n" +
" c.call()\n" +
"}\n" +
"test()\n",
};
//@formatter:on

runConformTest(sources, "B");
}

@Test @Ignore("https://issues.apache.org/jira/browse/GROOVY-9344")
public void testCompileStatic9344a() {
//@formatter:off
String[] sources = {
"Script.groovy",
"class A {}\n" +
"class B {}\n" +
"@groovy.transform.CompileStatic\n" +
"def test() {\n" +
" def var\n" +
" var = new A()\n" +
" def c = { ->\n" +
" var = new B()\n" + // Cannot cast object 'B@4e234c52' with class 'B' to class 'A'
" }\n" +
" c.call()\n" +
" print var.class.simpleName\n" +
"}\n" +
"test()\n",
};
//@formatter:on

runConformTest(sources, "B");
}

@Test
public void testCompileStatic9347() {
assumeTrue(isAtLeastJava(JDK8) && isParrotParser());
Expand Down
1 change: 1 addition & 0 deletions base/org.codehaus.groovy24/.checkstyle
Expand Up @@ -54,6 +54,7 @@
<file-match-pattern match-pattern="groovy/classgen/asm/sc/StaticInvocationWriter.java" include-pattern="false" />
<file-match-pattern match-pattern="groovy/classgen/asm/sc/StaticPropertyAccessHelper.java" include-pattern="false" />
<file-match-pattern match-pattern="groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java" include-pattern="false" />
<file-match-pattern match-pattern="groovy/classgen/asm/sc/StaticTypesTypeChooser.java" include-pattern="false" />
<file-match-pattern match-pattern="groovy/control/CompilationUnit.java" include-pattern="false" />
<file-match-pattern match-pattern="groovy/control/CompilerConfiguration.java" include-pattern="false" />
<file-match-pattern match-pattern="groovy/control/ErrorCollector.java" include-pattern="false" />
Expand Down
@@ -0,0 +1,77 @@
/*
* 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 org.codehaus.groovy.classgen.asm.sc;

import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.VariableExpression;
import org.codehaus.groovy.classgen.asm.StatementMetaTypeChooser;
import org.codehaus.groovy.transform.stc.StaticTypesMarker;

/**
* A {@link org.codehaus.groovy.classgen.asm.TypeChooser} which reads type information from node metadata
* generated by the {@link groovy.transform.CompileStatic} annotation.
*/
public class StaticTypesTypeChooser extends StatementMetaTypeChooser {
@Override
public ClassNode resolveType(final Expression exp, final ClassNode current) {
/* GRECLIPSE edit
ASTNode target = exp instanceof VariableExpression ? getTarget((VariableExpression) exp) : exp;
*/
Expression target = exp instanceof VariableExpression && !((VariableExpression) exp).isClosureSharedVariable() ? getTarget((VariableExpression) exp) : exp;
// GRECLIPSE end
ClassNode inferredType = target.getNodeMetaData(StaticTypesMarker.DECLARATION_INFERRED_TYPE);
if (inferredType == null) {
inferredType = target.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
/* GRECLIPSE edit
if (inferredType == null && target instanceof VariableExpression && ((VariableExpression) target).getAccessedVariable() instanceof Parameter) {
target = (Parameter) ((VariableExpression) target).getAccessedVariable();
inferredType = ((Parameter) target).getOriginType();
}
*/
}
if (inferredType != null) {
if (ClassHelper.VOID_TYPE == inferredType) {
// we are in a case of a type inference failure, probably because code was generated
// it is better to avoid using this
inferredType = super.resolveType(exp, current);
}
return inferredType;
}
if (target instanceof VariableExpression && ((VariableExpression) target).isThisExpression()) {
// AsmClassGenerator may create "this" expressions that the type checker knows nothing about
return current;
}
return super.resolveType(exp, current);
}

/**
* The inferred type, in case of a variable expression, can be set on the accessed variable, so we take it instead
* of the facade one.
*
* @param ve the variable expression for which to return the target expression
* @return the target variable expression
*/
private static VariableExpression getTarget(VariableExpression ve) {
if (ve.getAccessedVariable() == null || ve.getAccessedVariable() == ve || (!(ve.getAccessedVariable() instanceof VariableExpression)))
return ve;
return getTarget((VariableExpression) ve.getAccessedVariable());
}
}
Expand Up @@ -167,7 +167,6 @@
import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.castX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
import static org.codehaus.groovy.ast.tools.WideningCategories.LowestUpperBoundClassNode;
import static org.codehaus.groovy.ast.tools.WideningCategories.isBigDecCategory;
import static org.codehaus.groovy.ast.tools.WideningCategories.isBigIntCategory;
import static org.codehaus.groovy.ast.tools.WideningCategories.isDouble;
Expand Down Expand Up @@ -2370,7 +2369,7 @@ public void visitClosureExpression(final ClosureExpression expression) {
// perform visit
typeCheckingContext.pushEnclosingClosureExpression(expression);
DelegationMetadata dmd = getDelegationMetadata(expression);
if (dmd ==null) {
if (dmd == null) {
typeCheckingContext.delegationMetadata = new DelegationMetadata(
typeCheckingContext.getEnclosingClassNode(), Closure.OWNER_FIRST, typeCheckingContext.delegationMetadata
);
Expand Down Expand Up @@ -2439,6 +2438,12 @@ protected void saveVariableExpressionMetadata(final Set<VariableExpression> clos
// GROOVY-6921: We must force a call to getType in order to update closure shared variable whose
// types are inferred thanks to closure parameter type inference
getType(ve);
// GRECLIPSE add -- GROOVY-9344
Variable v;
while ((v = ve.getAccessedVariable()) != ve && v instanceof VariableExpression) {
ve = (VariableExpression) v;
}
// GRECLIPSE end
ListHashMap<StaticTypesMarker, Object> metadata = new ListHashMap<StaticTypesMarker, Object>();
for (StaticTypesMarker marker : StaticTypesMarker.values()) {
Object value = ve.getNodeMetaData(marker);
Expand All @@ -2447,10 +2452,12 @@ protected void saveVariableExpressionMetadata(final Set<VariableExpression> clos
}
}
typesBeforeVisit.put(ve, metadata);
/* GRECLIPSE edit
Variable accessedVariable = ve.getAccessedVariable();
if (accessedVariable != ve && accessedVariable instanceof VariableExpression) {
saveVariableExpressionMetadata(Collections.singleton((VariableExpression) accessedVariable), typesBeforeVisit);
}
*/
}
}

Expand Down Expand Up @@ -2630,8 +2637,6 @@ public void visitStaticMethodCallExpression(final StaticMethodCallExpression cal

/**
* @deprecated this method is unused, replaced with {@link DelegatesTo} inference.
* @param callArguments
* @param receiver
*/
@Deprecated
protected void checkClosureParameters(final Expression callArguments, final ClassNode receiver) {
Expand Down Expand Up @@ -3514,7 +3519,7 @@ private static ClassNode adjustWithTraits(final MethodNode directMethodCallCandi
nodes.add(arg);
}
}
return new LowestUpperBoundClassNode(returnType.getName()+"Composed", OBJECT_TYPE, nodes.toArray(new ClassNode[nodes.size()]));
return new WideningCategories.LowestUpperBoundClassNode(returnType.getName()+"Composed", OBJECT_TYPE, nodes.toArray(new ClassNode[nodes.size()]));
}
}
return returnType;
Expand Down
1 change: 1 addition & 0 deletions base/org.codehaus.groovy25/.checkstyle
Expand Up @@ -50,6 +50,7 @@
<file-match-pattern match-pattern="groovy/classgen/asm/StatementWriter.java" include-pattern="false" />
<file-match-pattern match-pattern="groovy/classgen/asm/sc/StaticPropertyAccessHelper.java" include-pattern="false" />
<file-match-pattern match-pattern="groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java" include-pattern="false" />
<file-match-pattern match-pattern="groovy/classgen/asm/sc/StaticTypesTypeChooser.java" include-pattern="false" />
<file-match-pattern match-pattern="groovy/control/CompilationUnit.java" include-pattern="false" />
<file-match-pattern match-pattern="groovy/control/CompilerConfiguration.java" include-pattern="false" />
<file-match-pattern match-pattern="groovy/control/ErrorCollector.java" include-pattern="false" />
Expand Down
@@ -0,0 +1,77 @@
/*
* 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 org.codehaus.groovy.classgen.asm.sc;

import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.VariableExpression;
import org.codehaus.groovy.classgen.asm.StatementMetaTypeChooser;
import org.codehaus.groovy.transform.stc.StaticTypesMarker;

/**
* A {@link org.codehaus.groovy.classgen.asm.TypeChooser} which reads type information from node metadata
* generated by the {@link groovy.transform.CompileStatic} annotation.
*/
public class StaticTypesTypeChooser extends StatementMetaTypeChooser {
@Override
public ClassNode resolveType(final Expression exp, final ClassNode current) {
/* GRECLIPSE edit
ASTNode target = exp instanceof VariableExpression ? getTarget((VariableExpression) exp) : exp;
*/
Expression target = exp instanceof VariableExpression && !((VariableExpression) exp).isClosureSharedVariable() ? getTarget((VariableExpression) exp) : exp;
// GRECLIPSE end
ClassNode inferredType = target.getNodeMetaData(StaticTypesMarker.DECLARATION_INFERRED_TYPE);
if (inferredType == null) {
inferredType = target.getNodeMetaData(StaticTypesMarker.INFERRED_TYPE);
/* GRECLIPSE edit
if (inferredType == null && target instanceof VariableExpression && ((VariableExpression) target).getAccessedVariable() instanceof Parameter) {
target = (Parameter) ((VariableExpression) target).getAccessedVariable();
inferredType = ((Parameter) target).getOriginType();
}
*/
}
if (inferredType != null) {
if (ClassHelper.VOID_TYPE == inferredType) {
// we are in a case of a type inference failure, probably because code was generated
// it is better to avoid using this
inferredType = super.resolveType(exp, current);
}
return inferredType;
}
if (target instanceof VariableExpression && ((VariableExpression) target).isThisExpression()) {
// AsmClassGenerator may create "this" expressions that the type checker knows nothing about
return current;
}
return super.resolveType(exp, current);
}

/**
* The inferred type, in case of a variable expression, can be set on the accessed variable, so we take it instead
* of the facade one.
*
* @param ve the variable expression for which to return the target expression
* @return the target variable expression
*/
private static VariableExpression getTarget(VariableExpression ve) {
if (ve.getAccessedVariable() == null || ve.getAccessedVariable() == ve || (!(ve.getAccessedVariable() instanceof VariableExpression)))
return ve;
return getTarget((VariableExpression) ve.getAccessedVariable());
}
}

0 comments on commit 45e9554

Please sign in to comment.