You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2020/09/12 16:26:19 UTC
[groovy] 06/19: minor refactor
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit c77a608050090addad7af91802937a6395a4b807
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Aug 22 10:30:01 2020 +0800
minor refactor
(cherry picked from commit 6a02298d75f6597f9dc83ab5b4b4e79ce978b651)
---
.../asm/sc/StaticPropertyAccessHelper.java | 22 +-
.../transformers/BinaryExpressionTransformer.java | 2 +-
.../sc/FieldsAndPropertiesStaticCompileTest.groovy | 299 +++++++++++----------
3 files changed, 164 insertions(+), 159 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticPropertyAccessHelper.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticPropertyAccessHelper.java
index 72499b5..ac30d6d 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticPropertyAccessHelper.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticPropertyAccessHelper.java
@@ -39,29 +39,29 @@ public abstract class StaticPropertyAccessHelper {
public static Expression transformToSetterCall(
final Expression receiver,
final MethodNode setterMethod,
- final Expression argument,
+ final Expression valueExpression,
final boolean implicitThis,
final boolean safe,
final boolean spreadSafe,
- final boolean requiresReturnValue,
- final Expression propertyExpression) {
- if (requiresReturnValue) {
- TemporaryVariableExpression tmp = new TemporaryVariableExpression(argument);
+ final boolean returnValue,
+ final Expression sourceExpression) {
+ if (returnValue) {
+ TemporaryVariableExpression tmp = new TemporaryVariableExpression(valueExpression);
PoppingMethodCallExpression call = new PoppingMethodCallExpression(receiver, setterMethod, tmp);
call.setSafe(safe);
call.setSpreadSafe(spreadSafe);
call.setImplicitThis(implicitThis);
- call.setSourcePosition(propertyExpression);
+ call.setSourcePosition(sourceExpression);
PoppingListOfExpressionsExpression list = new PoppingListOfExpressionsExpression(tmp, call);
- list.setSourcePosition(propertyExpression);
+ list.setSourcePosition(sourceExpression);
return list;
} else {
- MethodCallExpression call = new MethodCallExpression(receiver, setterMethod.getName(), argument);
+ MethodCallExpression call = new MethodCallExpression(receiver, setterMethod.getName(), valueExpression);
call.setSafe(safe);
call.setSpreadSafe(spreadSafe);
call.setImplicitThis(implicitThis);
call.setMethodTarget(setterMethod);
- call.setSourcePosition(propertyExpression);
+ call.setSourcePosition(sourceExpression);
return call;
}
}
@@ -79,7 +79,7 @@ public abstract class StaticPropertyAccessHelper {
@Override
public Expression transformExpression(final ExpressionTransformer transformer) {
- PoppingMethodCallExpression call = (PoppingMethodCallExpression) this.call.transformExpression(transformer);
+ PoppingMethodCallExpression call = (PoppingMethodCallExpression) transformer.transform(this.call);
return new PoppingListOfExpressionsExpression(call.tmp, call);
}
@@ -104,7 +104,7 @@ public abstract class StaticPropertyAccessHelper {
@Override
public Expression transformExpression(final ExpressionTransformer transformer) {
- PoppingMethodCallExpression call = new PoppingMethodCallExpression(getObjectExpression().transformExpression(transformer), getMethodTarget(), (TemporaryVariableExpression) tmp.transformExpression(transformer));
+ PoppingMethodCallExpression call = new PoppingMethodCallExpression(transformer.transform(getObjectExpression()), getMethodTarget(), (TemporaryVariableExpression) transformer.transform(tmp));
call.copyNodeMetaData(this);
call.setSourcePosition(this);
call.setSafe(isSafe());
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
index 8941398..31df52a 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/transformers/BinaryExpressionTransformer.java
@@ -194,7 +194,7 @@ public class BinaryExpressionTransformer {
expr.setSourcePosition(bin);
return expr;
}
- if (operationType == Types.EQUAL && leftExpression instanceof TupleExpression && rightExpression instanceof ListExpression) {
+ if (operationType == Types.ASSIGN && leftExpression instanceof TupleExpression && rightExpression instanceof ListExpression) {
// multiple assignment
ListOfExpressionsExpression cle = new ListOfExpressionsExpression();
boolean isDeclaration = (bin instanceof DeclarationExpression);
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
index a488532..8a0a33d 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/FieldsAndPropertiesStaticCompileTest.groovy
@@ -44,29 +44,29 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
// GROOVY-5561
void testShouldNotThrowAccessForbidden() {
assertScript '''
- class Test {
- def foo() {
- def bar = createBar()
- bar.foo
- }
+ class Test {
+ def foo() {
+ def bar = createBar()
+ bar.foo
+ }
- Bar createBar() { new Bar() }
- }
- class Bar {
- List<String> foo = ['1','2']
- }
- assert new Test().foo() == ['1','2']
+ Bar createBar() { new Bar() }
+ }
+ class Bar {
+ List<String> foo = ['1','2']
+ }
+ assert new Test().foo() == ['1','2']
'''
}
// GROOVY-5579
void testUseSetterAndNotSetProperty() {
assertScript '''
- Date d = new Date()
- d.time = 1
+ Date d = new Date()
+ d.time = 1
- assert d.time == 1
- '''
+ assert d.time == 1
+ '''
assert astTrees.values().any {
it.toString().contains 'INVOKEVIRTUAL java/util/Date.setTime (J)V'
}
@@ -116,38 +116,38 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
void testUseDirectWriteFieldAccess() {
assertScript '''
- class A {
- boolean setterCalled = false
+ class A {
+ boolean setterCalled
- protected int x
- public void setX(int a) {
- setterCalled = true
- x = a
- }
+ protected int x
+ public void setX(int a) {
+ setterCalled = true
+ x = a
}
- class B extends A {
- void directAccess() {
- this.@x = 2
- }
+ }
+ class B extends A {
+ void directAccess() {
+ this.@x = 2
}
- B b = new B()
- b.directAccess()
- assert b.isSetterCalled() == false
- assert b.x == 2
- '''
+ }
+ B b = new B()
+ b.directAccess()
+ assert b.isSetterCalled() == false
+ assert b.x == 2
+ '''
assert astTrees['B'][1].contains('PUTFIELD A.x')
}
void testUseDirectWriteStaticFieldAccess() {
assertScript '''
class A {
- static boolean setterCalled = false
+ static boolean setterCalled
- static protected int x
- public static void setX(int a) {
- setterCalled = true
- x = a
- }
+ static protected int x
+ public static void setX(int a) {
+ setterCalled = true
+ x = a
+ }
}
class B extends A {
static void directAccess() {
@@ -157,31 +157,31 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
B.directAccess()
assert B.isSetterCalled() == false
assert B.x == 2
- '''
+ '''
assert astTrees['B'][1].contains('PUTSTATIC A.x')
}
void testUseSetterFieldAccess() {
assertScript '''
- class A {
- boolean setterCalled = false
+ class A {
+ boolean setterCalled
- protected int x
- public void setX(int a) {
- setterCalled = true
- x = a
- }
+ protected int x
+ public void setX(int a) {
+ setterCalled = true
+ x = a
}
- class B extends A {
- void setterAccess() {
- this.x = 2
- }
+ }
+ class B extends A {
+ void setterAccess() {
+ this.x = 2
}
- B b = new B()
- b.setterAccess()
- assert b.isSetterCalled() == true
- assert b.x == 2
- '''
+ }
+ B b = new B()
+ b.setterAccess()
+ assert b.isSetterCalled() == true
+ assert b.x == 2
+ '''
assert astTrees['B'][1].contains('INVOKEVIRTUAL B.setX')
}
@@ -251,7 +251,6 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
A a = new A(1)
assert b.sameAs(a)
'''
-
}
void testDirectReadFieldFromSameClass() {
@@ -288,54 +287,55 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
void testUseDirectReadFieldAccess() {
assertScript '''
- class A {
- boolean getterCalled = false
+ class A {
+ boolean getterCalled
- protected int x
- public int getX() {
- getterCalled = true
- x
- }
+ protected int x
+ public int getX() {
+ getterCalled = true
+ x
}
- class B extends A {
- void m() {
- this.@x
- }
+ }
+ class B extends A {
+ void m() {
+ this.@x
}
- B b = new B()
- b.m()
- assert b.isGetterCalled() == false
- '''
+ }
+ B b = new B()
+ b.m()
+ assert b.isGetterCalled() == false
+ '''
assert astTrees['B'][1].contains('GETFIELD A.x')
}
void testUseGetterFieldAccess() {
assertScript '''
- class A {
- boolean getterCalled = false
-
- protected int x
- public int getX() {
- getterCalled = true
- x
- }
- }
- class B extends A {
- void usingGetter() {
- this.x
- }
- }
- B b = new B()
- b.usingGetter()
- assert b.isGetterCalled() == true
- '''
+ class A {
+ boolean getterCalled
+
+ protected int x
+ public int getX() {
+ getterCalled = true
+ x
+ }
+ }
+ class B extends A {
+ void usingGetter() {
+ this.x
+ }
+ }
+ B b = new B()
+ b.usingGetter()
+ assert b.isGetterCalled() == true
+ '''
assert astTrees['B'][1].contains('INVOKEVIRTUAL B.getX')
}
void testUseAttributeExternal() {
assertScript '''
class A {
- boolean setterCalled = false
+ boolean setterCalled
+
public int x
void setX(int a) {
setterCalled = true
@@ -352,7 +352,8 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
void testUseAttributeExternalSafe() {
assertScript '''
class A {
- boolean setterCalled = false
+ boolean setterCalled
+
public int x
void setX(int a) {
setterCalled = true
@@ -369,7 +370,8 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
void testUseAttributeExternalSafeWithNull() {
assertScript '''
class A {
- boolean setterCalled = false
+ boolean setterCalled
+
public int x
void setX(int a) {
setterCalled = true
@@ -384,7 +386,8 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
void testUseGetterExternal() {
assertScript '''
class A {
- boolean setterCalled = false
+ boolean setterCalled
+
public int x
void setX(int a) {
setterCalled = true
@@ -401,7 +404,8 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
void testUseAttributeExternalSpread() {
assertScript '''
class A {
- boolean setterCalled = false
+ boolean setterCalled
+
public int x
void setX(int a) {
setterCalled = true
@@ -418,7 +422,8 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
void testUseAttributeExternalSpreadSafeWithNull() {
assertScript '''
class A {
- boolean setterCalled = false
+ boolean setterCalled
+
public int x
void setX(int a) {
setterCalled = true
@@ -436,7 +441,8 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
void testUseAttributeExternalSpreadUsingSetter() {
assertScript '''
class A {
- boolean setterCalled = false
+ boolean setterCalled
+
public int x
void setX(int a) {
setterCalled = true
@@ -453,7 +459,8 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
void testUseAttributeExternalSpreadSafeWithNullUsingSetter() {
assertScript '''
class A {
- boolean setterCalled = false
+ boolean setterCalled
+
public int x
void setX(int a) {
setterCalled = true
@@ -471,45 +478,43 @@ final class FieldsAndPropertiesStaticCompileTest extends FieldsAndPropertiesSTCT
// GROOVY-5649
void testShouldNotThrowStackOverflowUsingThis() {
new GroovyShell().evaluate '''
- class HaveOption {
-
- private String helpOption;
+ class HaveOption {
+ private String helpOption
- public void setHelpOption(String helpOption) {
- this.helpOption = helpOption
- }
-
- }
- def o = new HaveOption()
- o.setHelpOption 'foo'
- assert o.helpOption
+ public void setHelpOption(String helpOption) {
+ this.helpOption = helpOption
+ }
+ }
+ def o = new HaveOption()
+ o.setHelpOption 'foo'
+ assert o.helpOption
'''
}
void testShouldNotThrowStackOverflow() {
new GroovyShell().evaluate '''
- class HaveOption {
-
- private String helpOption;
-
- public void setHelpOption(String ho) {
- helpOption = ho
- }
+ class HaveOption {
+ private String helpOption
- }
- def o = new HaveOption()
- o.setHelpOption 'foo'
- assert o.helpOption
+ public void setHelpOption(String ho) {
+ helpOption = ho
+ }
+ }
+ def o = new HaveOption()
+ o.setHelpOption 'foo'
+ assert o.helpOption
'''
}
@Override
void testPropertyWithMultipleSetters() {
// we need to override the test because the AST is going to be changed
- assertScript '''import org.codehaus.groovy.ast.expr.BinaryExpression
-import org.codehaus.groovy.ast.expr.BooleanExpression
-import org.codehaus.groovy.ast.stmt.AssertStatement
-import org.codehaus.groovy.transform.sc.ListOfExpressionsExpression
+ assertScript '''
+ import org.codehaus.groovy.ast.expr.BinaryExpression
+ import org.codehaus.groovy.ast.expr.BooleanExpression
+ import org.codehaus.groovy.ast.stmt.AssertStatement
+ import org.codehaus.groovy.transform.sc.ListOfExpressionsExpression
+
class A {
private field
void setX(Integer a) {field=a}
@@ -537,24 +542,24 @@ import org.codehaus.groovy.transform.sc.ListOfExpressionsExpression
assert a.x == "3"
}
testBody()
- '''
+ '''
}
void testCallSetterAsPropertyWithinFinallyBlockShouldNotThrowVerifyError() {
try {
assertScript '''
- class Multi {
- void setOut(int a) {}
- }
+ class Multi {
+ void setOut(int a) {}
+ }
- void foo() {
- def m = new Multi()
- try {
- } finally {
- m.out = 1
- }
- }
- foo()
+ void foo() {
+ def m = new Multi()
+ try {
+ } finally {
+ m.out = 1
+ }
+ }
+ foo()
'''
} finally {
assert astTrees.values().any {
@@ -566,20 +571,20 @@ import org.codehaus.groovy.transform.sc.ListOfExpressionsExpression
void testCallMultiSetterAsPropertyWithinFinallyBlockShouldNotThrowVerifyError() {
try {
assertScript '''
- class Multi {
- void setOut(int a) {}
- void setOut(String a) {}
- }
+ class Multi {
+ void setOut(int a) {}
+ void setOut(String a) {}
+ }
- void foo() {
- def m = new Multi()
- try {
- } finally {
- m.out = 1
- m.out = 'foo'
- }
- }
- foo()
+ void foo() {
+ def m = new Multi()
+ try {
+ } finally {
+ m.out = 1
+ m.out = 'foo'
+ }
+ }
+ foo()
'''
} finally {
assert astTrees.values().any {