[GitHub] groovy pull request #819: GROOVY-7975/GROOVY-3278/GROOVY-7854: improved acce...

2018-11-05 Thread asfgit 
Github user asfgit closed the pull request at:

https://github.com/apache/groovy/pull/819


---

[GitHub] groovy pull request #819: GROOVY-7975/GROOVY-3278/GROOVY-7854: improved acce...

2018-11-04 Thread paulk-asert 
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 handledTypes = new 
ArrayList();
+
 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...

2018-11-04 Thread danielsun1106 
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 handledTypes = new 
ArrayList();
+
 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...

2018-11-04 Thread paulk-asert 
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 
Date:   2018-11-04T10:10:43Z

GROOVY-7975/GROOVY-3278/GROOVY-7854: improved accessing of constants for 
annotation attributes




---