You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2020/06/24 16:02:21 UTC
[groovy] branch master updated (7ecbaf5 -> 121fd51)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git.
from 7ecbaf5 TEST: show actual output when test fails
new a5e677f refactor out some duplication in "super.name" and "super.@name" handling
new 121fd51 minor fix-ups
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/main/java/groovy/lang/Script.java | 4 +-
.../groovy/classgen/AsmClassGenerator.java | 124 ++++++++++-----------
.../apache/groovy/parser/antlr4/AstBuilder.java | 89 ++++++---------
3 files changed, 95 insertions(+), 122 deletions(-)
[groovy] 02/02: minor fix-ups
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 121fd513ac00aa9f3adcabbbf2edd47ac18d9d33
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Jun 24 11:01:45 2020 -0500
minor fix-ups
---
src/main/java/groovy/lang/Script.java | 4 +-
.../apache/groovy/parser/antlr4/AstBuilder.java | 89 +++++++++-------------
2 files changed, 39 insertions(+), 54 deletions(-)
diff --git a/src/main/java/groovy/lang/Script.java b/src/main/java/groovy/lang/Script.java
index de29c9b..e36b64d 100644
--- a/src/main/java/groovy/lang/Script.java
+++ b/src/main/java/groovy/lang/Script.java
@@ -18,8 +18,8 @@
*/
package groovy.lang;
-import org.apache.groovy.util.BeanUtils;
import org.codehaus.groovy.ast.expr.ArgumentListExpression;
+import org.codehaus.groovy.ast.tools.GeneralUtils;
import org.codehaus.groovy.control.CompilationFailedException;
import org.codehaus.groovy.runtime.DefaultGroovyMethods;
import org.codehaus.groovy.runtime.InvokerHelper;
@@ -73,7 +73,7 @@ public abstract class Script extends GroovyObjectSupport {
}
private boolean hasSetterMethodFor(String property) {
- String setterName = "set" + BeanUtils.capitalize(property);
+ String setterName = GeneralUtils.getSetterName(property);
for (Method method : getClass().getDeclaredMethods()) {
if (method.getParameterCount() == 1
// TODO: Test modifiers or return type?
diff --git a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
index d5f80d1..86c4c9d 100644
--- a/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
+++ b/subprojects/parser-antlr4/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
@@ -431,35 +431,39 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
public ModuleNode visitCompilationUnit(CompilationUnitContext ctx) {
this.visit(ctx.packageDeclaration());
- this.visitScriptStatements(ctx.scriptStatements())
- .forEach(e -> {
- if (e instanceof DeclarationListStatement) { // local variable declaration
- ((DeclarationListStatement) e).getDeclarationStatements().forEach(moduleNode::addStatement);
- } else if (e instanceof Statement) {
- moduleNode.addStatement((Statement) e);
- } else if (e instanceof MethodNode) { // script method
- moduleNode.addMethod((MethodNode) e);
- }
- });
+ for (ASTNode node : this.visitScriptStatements(ctx.scriptStatements())) {
+ if (node instanceof DeclarationListStatement) { // local variable declaration(s)
+ for (Statement stmt: ((DeclarationListStatement) node).getDeclarationStatements()) {
+ this.moduleNode.addStatement(stmt);
+ }
+ } else if (node instanceof Statement) {
+ this.moduleNode.addStatement((Statement) node);
+ } else if (node instanceof MethodNode) {
+ this.moduleNode.addMethod((MethodNode) node);
+ }
+ }
- this.classNodeList.forEach(moduleNode::addClass);
+ for (ClassNode node : this.classNodeList) {
+ this.moduleNode.addClass(node);
+ }
if (this.isPackageInfoDeclaration()) {
- this.addPackageInfoClassNode();
- } else {
- // if groovy source file only contains blank(including EOF), add "return null" to the AST
- if (this.isBlankScript()) {
- this.addEmptyReturnStatement();
+ ClassNode packageInfo = ClassHelper.make(this.moduleNode.getPackageName() + PACKAGE_INFO);
+ if (!this.moduleNode.getClasses().contains(packageInfo)) {
+ this.moduleNode.addClass(packageInfo);
}
+ } else if (this.isBlankScript()) {
+ // add "return null" if script has no statements/methods/classes
+ this.moduleNode.addStatement(ReturnStatement.RETURN_NULL_OR_VOID);
}
this.configureScriptClassNode();
- if (null != this.numberFormatError) {
+ if (this.numberFormatError != null) {
throw createParsingFailedException(this.numberFormatError.getV2().getMessage(), this.numberFormatError.getV1());
}
- return moduleNode;
+ return this.moduleNode;
}
@Override
@@ -1801,7 +1805,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
}
private DeclarationListStatement createFieldDeclarationListStatement(VariableDeclarationContext ctx, ModifierManager modifierManager, ClassNode variableType, List<DeclarationExpression> declarationExpressionList, ClassNode classNode) {
- for (int i = 0, n = declarationExpressionList.size(); i < n; i++) {
+ for (int i = 0, n = declarationExpressionList.size(); i < n; i += 1) {
DeclarationExpression declarationExpression = declarationExpressionList.get(i);
VariableExpression variableExpression = (VariableExpression) declarationExpression.getLeftExpression();
@@ -2427,7 +2431,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
// e.g. m { return 1; }
MethodCallExpression methodCallExpression =
new MethodCallExpression(
- VariableExpression.THIS_EXPRESSION,
+ new VariableExpression("this"),
(baseExpr instanceof VariableExpression)
? this.createConstantExpression(baseExpr)
@@ -3092,14 +3096,14 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
for (Tuple3<Expression, List<AnnotationNode>, TerminalNode> dim : dimList) {
if (null == dim.getV1()) {
emptyDimList.add(dim);
- exprEmpty = true;
+ exprEmpty = Boolean.TRUE;
} else {
- if (null != exprEmpty && exprEmpty) {
+ if (Boolean.TRUE.equals(exprEmpty)) {
invalidDimLBrack = latestDim.getV3();
}
dimWithExprList.add(dim);
- exprEmpty = false;
+ exprEmpty = Boolean.FALSE;
}
latestDim = dim;
@@ -3574,7 +3578,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
}
@Override
- public Parameter[] visitStandardLambdaParameters(final StandardLambdaParametersContext ctx) {
+ public Parameter[] visitStandardLambdaParameters(StandardLambdaParametersContext ctx) {
if (asBoolean(ctx.variableDeclaratorId())) {
VariableExpression variable = this.visitVariableDeclaratorId(ctx.variableDeclaratorId());
Parameter parameter = new Parameter(ClassHelper.OBJECT_TYPE, variable.getName());
@@ -4206,8 +4210,8 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
// e.g. m(1, 2) or m 1, 2
private MethodCallExpression createMethodCallExpression(Expression baseExpr, Expression arguments) {
- return new MethodCallExpression(
- VariableExpression.THIS_EXPRESSION,
+ MethodCallExpression methodCallExpression = new MethodCallExpression(
+ new VariableExpression("this"),
(baseExpr instanceof VariableExpression)
? this.createConstantExpression(baseExpr)
@@ -4215,6 +4219,8 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
arguments
);
+
+ return methodCallExpression;
}
private Parameter processFormalParameter(GroovyParserRuleContext ctx,
@@ -4390,9 +4396,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
private boolean isPackageInfoDeclaration() {
String name = this.sourceUnit.getName();
-
- return null != name && name.endsWith(PACKAGE_INFO_FILE_NAME);
-
+ return name != null && name.endsWith(PACKAGE_INFO_FILE_NAME);
}
private boolean isBlankScript() {
@@ -4401,29 +4405,13 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
private boolean isInsideParentheses(NodeMetaDataHandler nodeMetaDataHandler) {
Integer insideParenLevel = nodeMetaDataHandler.getNodeMetaData(INSIDE_PARENTHESES_LEVEL);
-
- return null != insideParenLevel && insideParenLevel > 0;
-
- }
-
- private void addEmptyReturnStatement() {
- moduleNode.addStatement(ReturnStatement.RETURN_NULL_OR_VOID);
- }
-
- private void addPackageInfoClassNode() {
- List<ClassNode> classNodeList = moduleNode.getClasses();
- ClassNode packageInfoClassNode = ClassHelper.make(moduleNode.getPackageName() + PACKAGE_INFO);
-
- if (!classNodeList.contains(packageInfoClassNode)) {
- moduleNode.addClass(packageInfoClassNode);
- }
+ return insideParenLevel != null && insideParenLevel > 0;
}
private org.codehaus.groovy.syntax.Token createGroovyTokenByType(Token token, int type) {
- if (null == token) {
+ if (token == null) {
throw new IllegalArgumentException("token should not be null");
}
-
return new org.codehaus.groovy.syntax.Token(type, token.getText(), token.getLine(), token.getCharPositionInLine());
}
@@ -4444,7 +4432,7 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
}
/**
- * set the script source position
+ * Sets the script source position.
*/
private void configureScriptClassNode() {
ClassNode scriptClassNode = moduleNode.getScriptClassDummy();
@@ -4462,13 +4450,10 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
scriptClassNode.setLastColumnNumber(lastStatement.getLastColumnNumber());
scriptClassNode.setLastLineNumber(lastStatement.getLastLineNumber());
}
-
}
private String getOriginalText(ParserRuleContext context) {
- CharStream charStream = lexer.getInputStream();
- return charStream.getText(Interval.of(context.getStart().getStartIndex(), context.getStop().getStopIndex()));
-
+ return lexer.getInputStream().getText(Interval.of(context.getStart().getStartIndex(), context.getStop().getStopIndex()));
}
private boolean isTrue(NodeMetaDataHandler nodeMetaDataHandler, String key) {
[groovy] 01/02: refactor out some duplication in "super.name" and
"super.@name" handling
Posted by em...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit a5e677ff2feba7963d4aac8a120f55c980704b1e
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Jun 24 11:00:37 2020 -0500
refactor out some duplication in "super.name" and "super.@name" handling
---
.../groovy/classgen/AsmClassGenerator.java | 124 ++++++++++-----------
1 file changed, 56 insertions(+), 68 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index 3f0844d..b55d468 100644
--- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
+++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
@@ -898,10 +898,10 @@ public class AsmClassGenerator extends ClassGenerator {
return null;
}
- private void visitAttributeOrProperty(final PropertyExpression expression, final MethodCallerMultiAdapter adapter) {
+ private void visitAttributeOrProperty(final PropertyExpression pexp, final MethodCallerMultiAdapter adapter) {
ClassNode classNode = controller.getClassNode();
- String propertyName = expression.getPropertyAsString();
- Expression objectExpression = expression.getObjectExpression();
+ String propertyName = pexp.getPropertyAsString();
+ Expression objectExpression = pexp.getObjectExpression();
if (objectExpression instanceof ClassExpression && "this".equals(propertyName)) {
// we have something like A.B.this, and need to make it
@@ -912,7 +912,7 @@ public class AsmClassGenerator extends ClassGenerator {
// Outer.this in a special constructor call
ConstructorNode ctor = controller.getConstructorNode();
Expression receiver = !classNode.isStaticClass() ? new VariableExpression(ctor.getParameters()[0]) : new ClassExpression(type);
- receiver.setSourcePosition(expression);
+ receiver.setSourcePosition(pexp);
receiver.visit(this);
return;
}
@@ -951,40 +951,47 @@ public class AsmClassGenerator extends ClassGenerator {
if (propertyName != null) {
// TODO: spread safe should be handled inside
- if (adapter == getProperty && !expression.isSpreadSafe()) {
- controller.getCallSiteWriter().makeGetPropertySite(objectExpression, propertyName, expression.isSafe(), expression.isImplicitThis());
- } else if (adapter == getGroovyObjectProperty && !expression.isSpreadSafe()) {
- controller.getCallSiteWriter().makeGroovyObjectGetPropertySite(objectExpression, propertyName, expression.isSafe(), expression.isImplicitThis());
+ if (adapter == getProperty && !pexp.isSpreadSafe()) {
+ controller.getCallSiteWriter().makeGetPropertySite(objectExpression, propertyName, pexp.isSafe(), pexp.isImplicitThis());
+ } else if (adapter == getGroovyObjectProperty && !pexp.isSpreadSafe()) {
+ controller.getCallSiteWriter().makeGroovyObjectGetPropertySite(objectExpression, propertyName, pexp.isSafe(), pexp.isImplicitThis());
} else {
- controller.getCallSiteWriter().fallbackAttributeOrPropertySite(expression, objectExpression, propertyName, adapter);
+ controller.getCallSiteWriter().fallbackAttributeOrPropertySite(pexp, objectExpression, propertyName, adapter);
}
} else {
- controller.getCallSiteWriter().fallbackAttributeOrPropertySite(expression, objectExpression, null, adapter);
+ controller.getCallSiteWriter().fallbackAttributeOrPropertySite(pexp, objectExpression, null, adapter);
}
}
- private void setPropertyOfSuperClass(final ClassNode classNode, final PropertyExpression expression, final MethodVisitor mv) {
- String fieldName = expression.getPropertyAsString();
- FieldNode fieldNode = classNode.getSuperClass().getField(fieldName);
+ private boolean tryPropertyOfSuperClass(final PropertyExpression pexp, final String propertyName) {
+ ClassNode classNode = controller.getClassNode();
- if (null == fieldNode) {
- throw new RuntimeParserException("Failed to find field[" + fieldName + "] of " + classNode.getName() + "'s super class", expression);
+ if (!controller.getCompileStack().isLHS()) {
+ String methodName = "get" + capitalize(propertyName); // TODO: "is"
+ callX(pexp.getObjectExpression(), methodName).visit(this);
+ return true;
}
+ FieldNode fieldNode = classNode.getSuperClass().getField(propertyName);
+
+ if (fieldNode == null) {
+ throw new RuntimeParserException("Failed to find field[" + propertyName + "] of " + classNode.getName() + "'s super class", pexp);
+ }
if (fieldNode.isFinal()) {
- throw new RuntimeParserException("Cannot modify final field[" + fieldName + "] of " + classNode.getName() + "'s super class", expression);
+ throw new RuntimeParserException("Cannot modify final field[" + propertyName + "] of " + classNode.getName() + "'s super class", pexp);
}
- MethodNode setter = findSetterOfSuperClass(classNode, fieldNode);
- MethodNode getter = findGetterOfSuperClass(classNode, fieldNode);
+ MethodNode setter = classNode.getSuperClass().getSetterMethod(getSetterName(propertyName));
+ MethodNode getter = classNode.getSuperClass().getGetterMethod("get" + capitalize(propertyName));
- if (fieldNode.isPrivate() && !getterAndSetterExists(setter, getter)) {
- throw new RuntimeParserException("Cannot access private field[" + fieldName + "] of " + classNode.getName() + "'s super class", expression);
+ if (fieldNode.isPrivate() && (setter == null || getter == null || !setter.getDeclaringClass().equals(getter.getDeclaringClass()))) {
+ throw new RuntimeParserException("Cannot access private field[" + propertyName + "] of " + classNode.getName() + "'s super class", pexp);
}
OperandStack operandStack = controller.getOperandStack();
operandStack.doAsType(fieldNode.getType());
+ MethodVisitor mv = controller.getMethodVisitor();
mv.visitVarInsn(ALOAD, 0);
operandStack.push(classNode);
@@ -993,29 +1000,16 @@ public class AsmClassGenerator extends ClassGenerator {
String owner = BytecodeHelper.getClassInternalName(classNode.getSuperClass().getName());
String desc = BytecodeHelper.getTypeDescription(fieldNode.getType());
if (fieldNode.isPublic() || fieldNode.isProtected()) {
- mv.visitFieldInsn(PUTFIELD, owner, fieldName, desc);
+ mv.visitFieldInsn(PUTFIELD, owner, propertyName, desc);
} else {
mv.visitMethodInsn(INVOKESPECIAL, owner, setter.getName(), BytecodeHelper.getMethodDescriptor(setter), false);
}
+ return true;
}
- private static boolean getterAndSetterExists(final MethodNode setter, final MethodNode getter) {
- return setter != null && getter != null && setter.getDeclaringClass().equals(getter.getDeclaringClass());
- }
-
- private static MethodNode findSetterOfSuperClass(final ClassNode classNode, final FieldNode fieldNode) {
- String setterMethodName = getSetterName(fieldNode.getName());
- return classNode.getSuperClass().getSetterMethod(setterMethodName);
- }
-
- private static MethodNode findGetterOfSuperClass(final ClassNode classNode, final FieldNode fieldNode) {
- String getterMethodName = "get" + capitalize(fieldNode.getName());
- return classNode.getSuperClass().getGetterMethod(getterMethodName);
- }
-
- private boolean checkStaticOuterField(final PropertyExpression pexp, final String name) {
+ private boolean checkStaticOuterField(final PropertyExpression pexp, final String propertyName) {
for (final ClassNode outer : controller.getClassNode().getOuterClasses()) {
- FieldNode field = outer.getDeclaredField(name);
+ FieldNode field = outer.getDeclaredField(propertyName);
if (field != null) {
if (!field.isStatic()) break;
@@ -1028,7 +1022,7 @@ public class AsmClassGenerator extends ClassGenerator {
outerField.visit(this);
return true;
} else {
- field = outer.getField(name); // checks supers
+ field = outer.getField(propertyName); // checks supers
if (field != null && !field.isPrivate() && (field.isPublic() || field.isProtected()
|| Objects.equals(field.getDeclaringClass().getPackageName(), outer.getPackageName()))) {
if (!field.isStatic()) break;
@@ -1066,33 +1060,32 @@ public class AsmClassGenerator extends ClassGenerator {
if (isThisOrSuper(objectExpression)) {
String name = expression.getPropertyAsString();
if (name != null) {
- FieldNode field = null;
- boolean privateSuperField = false;
+ FieldNode fieldNode = null;
ClassNode classNode = controller.getClassNode();
- if (isSuperExpression(objectExpression)) {
- field = classNode.getSuperClass().getDeclaredField(name);
- privateSuperField = (field != null && field.isPrivate());
- } else if (controller.isInGeneratedFunction()) {
- if (expression.isImplicitThis())
- field = classNode.getDeclaredField(name); // params are stored as fields
+ if (isThisExpression(objectExpression)) {
+ if (controller.isInGeneratedFunction()) { // params are stored as fields
+ if (expression.isImplicitThis()) fieldNode = classNode.getDeclaredField(name);
+ } else {
+ fieldNode = classNode.getDeclaredField(name);
+
+ if (fieldNode == null && !isValidFieldNodeForByteCodeAccess(classNode.getField(name), classNode)) {
+ // GROOVY-9501, GROOVY-9569
+ if (checkStaticOuterField(expression, name)) return;
+ }
+ }
} else {
- field = classNode.getDeclaredField(name);
- if (field == null && !isValidFieldNodeForByteCodeAccess(classNode.getField(name), classNode)) {
- // GROOVY-9501, GROOVY-9569
- if (checkStaticOuterField(expression, name)) return;
+ fieldNode = classNode.getSuperClass().getDeclaredField(name);
+ // GROOVY-4497: do not visit super class field if it is private
+ if (fieldNode != null && fieldNode.isPrivate()) fieldNode = null;
+
+ if (fieldNode == null) {
+ visited = tryPropertyOfSuperClass(expression, name);
}
}
- if (field != null && !privateSuperField) { // GROOVY-4497: don't visit super field if it is private
- fieldX(field).visit(this);
- visited = true;
- } else if (isSuperExpression(objectExpression)) {
- if (controller.getCompileStack().isLHS()) {
- setPropertyOfSuperClass(classNode, expression, controller.getMethodVisitor());
- } else {
- callX(objectExpression, "get" + capitalize(name)).visit(this); // TODO: "is"
- }
+ if (fieldNode != null) {
+ fieldX(fieldNode).visit(this);
visited = true;
}
}
@@ -1129,17 +1122,12 @@ public class AsmClassGenerator extends ClassGenerator {
String name = expression.getPropertyAsString();
if (name != null) {
ClassNode classNode = controller.getClassNode();
- FieldNode field = getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(classNode, classNode, name, isSuperExpression(objectExpression));
- if (field != null) {
- visitFieldExpression(new FieldExpression(field));
+ FieldNode fieldNode = getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(classNode, classNode, name, isSuperExpression(objectExpression));
+ if (fieldNode != null) {
+ fieldX(fieldNode).visit(this);
visited = true;
} else if (isSuperExpression(objectExpression)) {
- if (controller.getCompileStack().isLHS()) {
- setPropertyOfSuperClass(classNode, expression, controller.getMethodVisitor());
- } else {
- visitMethodCallExpression(new MethodCallExpression(objectExpression, "get" + capitalize(name), MethodCallExpression.NO_ARGUMENTS));
- }
- visited = true;
+ visited = tryPropertyOfSuperClass(expression, name);
}
}
}