Repository navigation
GROOVY-12514, GROOVY-12517: qualified this and super in nested classes under static type checking and compilation #3049
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
68811e5
ecac373
e920fba
68a832c
07f4caa
18c7fd8
d091397
cdb685d
0d282e1
da0bf45
585caf6
2fd3cbe
1b7a08c
4936199
f774694
7b7060c
b48fdf2
fd90bd4
412c44a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -143,6 +143,7 @@ | |
| import static org.apache.groovy.ast.tools.ClassNodeUtils.getField; | ||
| import static org.apache.groovy.ast.tools.ClassNodeUtils.getNestHost; | ||
| import static org.apache.groovy.ast.tools.ExpressionUtils.isNullConstant; | ||
| import static org.apache.groovy.ast.tools.ExpressionUtils.isOuterSuperExpression; | ||
| import static org.apache.groovy.ast.tools.ExpressionUtils.isSuperExpression; | ||
| import static org.codehaus.groovy.ast.ClassHelper.isClassType; | ||
| import static org.codehaus.groovy.ast.ClassHelper.isObjectType; | ||
|
|
@@ -1391,6 +1392,14 @@ public static FieldNode getDeclaredFieldOfCurrentClassOrAccessibleFieldOfSuper(f | |
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Determines if an expression is "super" or "Outer.super" for a class: a | ||
| * property of it is a property of the superclass of the instance. | ||
| */ | ||
| private static boolean isSuperOrOuterSuper(final Expression expression) { | ||
| return isSuperExpression(expression) || isOuterSuperExpression(expression); | ||
| } | ||
|
|
||
| private void visitAttributeOrProperty(final PropertyExpression pexp, final MethodCallerMultiAdapter adapter) { | ||
| ClassNode classNode = controller.getClassNode(); | ||
| String propertyName = pexp.getPropertyAsString(); | ||
|
|
@@ -1534,9 +1543,9 @@ public void visitPropertyExpression(final PropertyExpression expression) { | |
| if (!visited) { | ||
| MethodCallerMultiAdapter adapter; | ||
| if (controller.getCompileStack().isLHS()) { | ||
| adapter = isSuperExpression(objectExpression) ? setPropertyOnSuper : isGroovyObject(expression) ? setGroovyObjectProperty : setProperty; | ||
| adapter = isSuperOrOuterSuper(objectExpression) ? setPropertyOnSuper : isGroovyObject(expression) ? setGroovyObjectProperty : setProperty; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Property form now goes through isSuperOrOuterSuper → get/setPropertyOnSuper, but the attribute adapters in visitAttributeExpression (tip ~1550) still key only on isSuperExpression. Outer.super.@q will load Outer.this and then getField without useSuper, i.e. Outer's own field — the same bug shape this PR fixes for Outer.super.q. Please mirror isSuperOrOuterSuper there (and cover Outer.super.@x / N.super.@x in InnerClassTest), or open a follow-up ticket and call it out in the PR so the @ form is not left silently wrong. |
||
| } else { | ||
| adapter = isSuperExpression(objectExpression) ? getPropertyOnSuper : isGroovyObject(expression) ? getGroovyObjectProperty : getProperty; | ||
| adapter = isSuperOrOuterSuper(objectExpression) ? getPropertyOnSuper : isGroovyObject(expression) ? getGroovyObjectProperty : getProperty; | ||
| } | ||
| visitAttributeOrProperty(expression, adapter); | ||
| } | ||
|
|
@@ -1589,9 +1598,9 @@ public void visitAttributeExpression(final AttributeExpression expression) { | |
| if (!visited) { | ||
| MethodCallerMultiAdapter adapter; | ||
| if (controller.getCompileStack().isLHS()) { | ||
| adapter = isSuperExpression(objectExpression) ? setFieldOnSuper : isGroovyObject(expression) ? setGroovyObjectField : setField; | ||
| adapter = isSuperOrOuterSuper(objectExpression) ? setFieldOnSuper : isGroovyObject(expression) ? setGroovyObjectField : setField; | ||
| } else { | ||
| adapter = isSuperExpression(objectExpression) ? getFieldOnSuper : isGroovyObject(expression) ? getGroovyObjectField : getField; | ||
| adapter = isSuperOrOuterSuper(objectExpression) ? getFieldOnSuper : isGroovyObject(expression) ? getGroovyObjectField : getField; | ||
| } | ||
| visitAttributeOrProperty(expression, adapter); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isSuperOrOuterSuper matches InvocationWriter.isOuterClassSuper / isClassWithSuper (GROOVY-11611). Fine as a local copy for this fix, but consider one shared ExpressionUtils (or package) helper so the !isInterface() guard stays aligned with GROOVY-12526 (#3071).