You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by paulk-asert <gi...@git.apache.org> on 2018/11/04 10:13:13 UTC
[GitHub] groovy pull request #819: GROOVY-7975/GROOVY-3278/GROOVY-7854: improved acce...
GitHub user paulk-asert opened a pull request:
https://github.com/apache/groovy/pull/819
GROOVY-7975/GROOVY-3278/GROOVY-7854: improved accessing of constants …
…for annotation attributes
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/paulk-asert/groovy annotationConstantFixes
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/groovy/pull/819.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #819
----
commit 236e4d5c06cec60840209e0b3c83512a90badfda
Author: Paul King <pa...@...>
Date: 2018-11-04T10:10:43Z
GROOVY-7975/GROOVY-3278/GROOVY-7854: improved accessing of constants for annotation attributes
----
---
[GitHub] groovy pull request #819: GROOVY-7975/GROOVY-3278/GROOVY-7854: improved acce...
Posted by paulk-asert <gi...@git.apache.org>.
Github user paulk-asert commented on a diff in the pull request:
https://github.com/apache/groovy/pull/819#discussion_r230627859
--- Diff: src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java ---
@@ -18,20 +18,205 @@
*/
package org.apache.groovy.ast.tools;
+import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.FieldNode;
+import org.codehaus.groovy.ast.expr.BinaryExpression;
import org.codehaus.groovy.ast.expr.ClassExpression;
import org.codehaus.groovy.ast.expr.ConstantExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.ListExpression;
import org.codehaus.groovy.ast.expr.PropertyExpression;
+import org.codehaus.groovy.ast.expr.VariableExpression;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+
+import static org.codehaus.groovy.syntax.Types.DIVIDE;
+import static org.codehaus.groovy.syntax.Types.MINUS;
+import static org.codehaus.groovy.syntax.Types.MULTIPLY;
+import static org.codehaus.groovy.syntax.Types.PLUS;
public class ExpressionUtils {
+ private static ArrayList<Integer> handledTypes = new ArrayList<Integer>();
+
private ExpressionUtils() {
}
- // resolve constant-looking expressions statically (do here as gets transformed away later)
+ static {
+ handledTypes.add(PLUS);
+ handledTypes.add(MINUS);
+ handledTypes.add(MULTIPLY);
+ handledTypes.add(DIVIDE);
+ }
+
+ public static ConstantExpression transformBinaryConstantExpression(BinaryExpression be, ClassNode targetType) {
+ if (isTypeOrArrayOfType(targetType, ClassHelper.STRING_TYPE, false)) {
+ if (be.getOperation().getType() == PLUS) {
+ Expression left = transformInlineConstants(be.getLeftExpression(), targetType);
+ Expression right = transformInlineConstants(be.getRightExpression(), targetType);
+ if (left instanceof ConstantExpression && right instanceof ConstantExpression) {
+ ConstantExpression newExp = new ConstantExpression((String) ((ConstantExpression) left).getValue() +
+ ((ConstantExpression) right).getValue());
+ newExp.setSourcePosition(be);
+ return newExp;
+ }
+ }
+ } else if (isTypeOrArrayOfType(targetType, ClassHelper.Integer_TYPE, false) || isTypeOrArrayOfType(targetType, ClassHelper.int_TYPE, false)) {
+ int type = be.getOperation().getType();
+ if (handledTypes.contains(type)) {
+ Expression left = transformInlineConstants(be.getLeftExpression(), targetType);
+ Expression right = transformInlineConstants(be.getRightExpression(), targetType);
+ if (left instanceof ConstantExpression && right instanceof ConstantExpression) {
+ Integer newVal = null;
+ switch(type) {
+ case PLUS:
+ newVal = (Integer) ((ConstantExpression) left).getValue() +
+ (Integer) ((ConstantExpression) right).getValue();
+ break;
+ case MINUS:
+ newVal = (Integer) ((ConstantExpression) left).getValue() -
+ (Integer) ((ConstantExpression) right).getValue();
+ break;
+ case MULTIPLY:
+ newVal = (Integer) ((ConstantExpression) left).getValue() *
+ (Integer) ((ConstantExpression) right).getValue();
+ break;
+ case DIVIDE:
+ newVal = (Integer) ((ConstantExpression) left).getValue() /
+ (Integer) ((ConstantExpression) right).getValue();
+ break;
+ }
+ if (newVal != null) {
+ ConstantExpression newExp = new ConstantExpression(newVal, true);
+ newExp.setSourcePosition(be);
+ return newExp;
+ }
+ }
+ }
+ } else if (isTypeOrArrayOfType(targetType, ClassHelper.Double_TYPE, false) || isTypeOrArrayOfType(targetType, ClassHelper.double_TYPE, false)) {
--- End diff --
Already refactored - thanks for the suggestion.
---
[GitHub] groovy pull request #819: GROOVY-7975/GROOVY-3278/GROOVY-7854: improved acce...
Posted by danielsun1106 <gi...@git.apache.org>.
Github user danielsun1106 commented on a diff in the pull request:
https://github.com/apache/groovy/pull/819#discussion_r230589376
--- Diff: src/main/java/org/apache/groovy/ast/tools/ExpressionUtils.java ---
@@ -18,20 +18,205 @@
*/
package org.apache.groovy.ast.tools;
+import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.FieldNode;
+import org.codehaus.groovy.ast.expr.BinaryExpression;
import org.codehaus.groovy.ast.expr.ClassExpression;
import org.codehaus.groovy.ast.expr.ConstantExpression;
import org.codehaus.groovy.ast.expr.Expression;
import org.codehaus.groovy.ast.expr.ListExpression;
import org.codehaus.groovy.ast.expr.PropertyExpression;
+import org.codehaus.groovy.ast.expr.VariableExpression;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
+import java.util.ArrayList;
+
+import static org.codehaus.groovy.syntax.Types.DIVIDE;
+import static org.codehaus.groovy.syntax.Types.MINUS;
+import static org.codehaus.groovy.syntax.Types.MULTIPLY;
+import static org.codehaus.groovy.syntax.Types.PLUS;
public class ExpressionUtils {
+ private static ArrayList<Integer> handledTypes = new ArrayList<Integer>();
+
private ExpressionUtils() {
}
- // resolve constant-looking expressions statically (do here as gets transformed away later)
+ static {
+ handledTypes.add(PLUS);
+ handledTypes.add(MINUS);
+ handledTypes.add(MULTIPLY);
+ handledTypes.add(DIVIDE);
+ }
+
+ public static ConstantExpression transformBinaryConstantExpression(BinaryExpression be, ClassNode targetType) {
+ if (isTypeOrArrayOfType(targetType, ClassHelper.STRING_TYPE, false)) {
+ if (be.getOperation().getType() == PLUS) {
+ Expression left = transformInlineConstants(be.getLeftExpression(), targetType);
+ Expression right = transformInlineConstants(be.getRightExpression(), targetType);
+ if (left instanceof ConstantExpression && right instanceof ConstantExpression) {
+ ConstantExpression newExp = new ConstantExpression((String) ((ConstantExpression) left).getValue() +
+ ((ConstantExpression) right).getValue());
+ newExp.setSourcePosition(be);
+ return newExp;
+ }
+ }
+ } else if (isTypeOrArrayOfType(targetType, ClassHelper.Integer_TYPE, false) || isTypeOrArrayOfType(targetType, ClassHelper.int_TYPE, false)) {
+ int type = be.getOperation().getType();
+ if (handledTypes.contains(type)) {
+ Expression left = transformInlineConstants(be.getLeftExpression(), targetType);
+ Expression right = transformInlineConstants(be.getRightExpression(), targetType);
+ if (left instanceof ConstantExpression && right instanceof ConstantExpression) {
+ Integer newVal = null;
+ switch(type) {
+ case PLUS:
+ newVal = (Integer) ((ConstantExpression) left).getValue() +
+ (Integer) ((ConstantExpression) right).getValue();
+ break;
+ case MINUS:
+ newVal = (Integer) ((ConstantExpression) left).getValue() -
+ (Integer) ((ConstantExpression) right).getValue();
+ break;
+ case MULTIPLY:
+ newVal = (Integer) ((ConstantExpression) left).getValue() *
+ (Integer) ((ConstantExpression) right).getValue();
+ break;
+ case DIVIDE:
+ newVal = (Integer) ((ConstantExpression) left).getValue() /
+ (Integer) ((ConstantExpression) right).getValue();
+ break;
+ }
+ if (newVal != null) {
+ ConstantExpression newExp = new ConstantExpression(newVal, true);
+ newExp.setSourcePosition(be);
+ return newExp;
+ }
+ }
+ }
+ } else if (isTypeOrArrayOfType(targetType, ClassHelper.Double_TYPE, false) || isTypeOrArrayOfType(targetType, ClassHelper.double_TYPE, false)) {
--- End diff --
looks like some template code, which is duplicated now. It's better to refactor it IMO
---
[GitHub] groovy pull request #819: GROOVY-7975/GROOVY-3278/GROOVY-7854: improved acce...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/groovy/pull/819
---