This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_4_0_X by this push:
new cb145400f0 GROOVY-11356: STC: unlink variable expression from an
inaccessible field
cb145400f0 is described below
commit cb145400f060b06c2a72f51f5a2ced94e2fca220
Author: Eric Milles <eric.mil...@thomsonreuters.com>
AuthorDate: Sat Apr 6 13:09:17 2024 -0500
GROOVY-11356: STC: unlink variable expression from an inaccessible field
4_0_X backport
---
.../transform/stc/StaticTypeCheckingVisitor.java | 41 ++++++++++++++------
src/test/groovy/bugs/Groovy7165.groovy | 4 +-
src/test/groovy/bugs/Groovy9292.groovy | 15 +++++---
src/test/groovy/bugs/Groovy9293.groovy | 7 ++--
.../packageScope/DifferentPackageTest.groovy | 44 +++++++++++-----------
5 files changed, 66 insertions(+), 45 deletions(-)
diff --git
a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index b9107cae1d..a417fd1376 100644
---
a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++
b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -631,18 +631,22 @@ public class StaticTypeCheckingVisitor extends
ClassCodeVisitorSupport {
} else if (accessedVariable instanceof FieldNode) {
FieldNode accessedField = (FieldNode) accessedVariable;
ClassNode temporaryType = getInferredTypeFromTempInfo(vexp, null);
// GROOVY-9454
- if (enclosingClosure != null) {
- tryVariableExpressionAsProperty(vexp, name);
- } else if (getOutermost(accessedField.getDeclaringClass()) ==
getOutermost(typeCheckingContext.getEnclosingClassNode())
- || !tryVariableExpressionAsProperty(vexp, name)) { //
GROOVY-10981: check for property before super class field
- checkOrMarkPrivateAccess(vexp, accessedField,
typeCheckingContext.isTargetOfEnclosingAssignment(vexp));
- if (temporaryType == null) storeType(vexp, getType(vexp));
- }
- if (temporaryType != null && !isObjectType(temporaryType)) {
- vexp.putNodeMetaData(INFERRED_TYPE, temporaryType);
+ // GROOVY-10981: prior to Groovy 5, outer class field is preferred
over self-type property
+ boolean declaredInScope = enclosingClosure == null &&
getOutermost(accessedField.getDeclaringClass()) ==
getOutermost(typeCheckingContext.getEnclosingClassNode());
+ if (declaredInScope || tryVariableExpressionAsProperty(vexp,
name)) {
+ if (temporaryType == null) {
+ storeType(vexp, getType(vexp));
+ } else if (!isObjectType(temporaryType)) {
+ vexp.putNodeMetaData(INFERRED_TYPE, temporaryType);
+ }
+ if (declaredInScope) {
+ checkOrMarkPrivateAccess(vexp, accessedField,
typeCheckingContext.isTargetOfEnclosingAssignment(vexp));
+ }
+ } else if (!extension.handleUnresolvedVariableExpression(vexp)) {
+ addStaticTypeError("No such property: " + name + " for class:
" + prettyPrintTypeName(typeCheckingContext.getEnclosingClassNode()), vexp);
}
} else if (accessedVariable instanceof PropertyNode) {
- // we must be careful, because the property node may be of a wrong
type:
+ // we must be careful -- the property node may be of a wrong type:
// if a class contains a getter and a setter of different types or
// overloaded setters, the type of the property node is arbitrary!
if (tryVariableExpressionAsProperty(vexp, name)) {
@@ -1671,6 +1675,21 @@ public class StaticTypeCheckingVisitor extends
ClassCodeVisitorSupport {
return true;
}
+ if (pexp.isImplicitThis() && isThisExpression(objectExpression)) {
+ Iterator<ClassNode> iter = enclosingTypes.iterator(); // first
enclosing is "this" type
+ boolean staticOnly = Modifier.isStatic(iter.next().getModifiers())
|| staticOnlyAccess;
+ while (iter.hasNext()) {
+ ClassNode outer = iter.next();
+ // GROOVY-7994, GROOVY-11198: try "this.propertyName" as
"Outer.propertyName" or "Outer.this.propertyName"
+ PropertyExpression pe = propX(staticOnly ? classX(outer) :
propX(classX(outer), "this"), pexp.getProperty());
+ if (existsProperty(pe, readMode, visitor)) {
+ pexp.copyNodeMetaData(pe);
+ return true;
+ }
+ staticOnly = staticOnly ||
Modifier.isStatic(outer.getModifiers());
+ }
+ }
+
return foundGetterOrSetter;
}
@@ -1810,7 +1829,7 @@ public class StaticTypeCheckingVisitor extends
ClassCodeVisitorSupport {
if (!accessible) {
if (expressionToStoreOn instanceof AttributeExpression) {
addStaticTypeError("Cannot access field: " + field.getName() +
" of class: " + prettyPrintTypeName(field.getDeclaringClass()),
expressionToStoreOn.getProperty());
- } else if (field.isPrivate()) {
+ } else if (!field.isProtected()) { // private or package-private
return false;
}
}
diff --git a/src/test/groovy/bugs/Groovy7165.groovy
b/src/test/groovy/bugs/Groovy7165.groovy
index 5880057ad9..99663df647 100644
--- a/src/test/groovy/bugs/Groovy7165.groovy
+++ b/src/test/groovy/bugs/Groovy7165.groovy
@@ -73,8 +73,7 @@ final class Groovy7165 {
}
new B().test()
'''
-
- assert err =~ /Access to B#CONST is forbidden/
+ assert err =~ /No such property: CONST for class: B/
}
@Test
@@ -91,7 +90,6 @@ final class Groovy7165 {
}
assert false : 'compilation should fail'
'''
-
assert err =~ /Access to A#CONST is forbidden/
}
}
diff --git a/src/test/groovy/bugs/Groovy9292.groovy
b/src/test/groovy/bugs/Groovy9292.groovy
index 2dc077c246..6d74ffeed8 100644
--- a/src/test/groovy/bugs/Groovy9292.groovy
+++ b/src/test/groovy/bugs/Groovy9292.groovy
@@ -29,9 +29,9 @@ final class Groovy9292 {
ast(groovy.transform.CompileStatic)
}
- @Test
+ @Test // GROOVY-11356
void 'test accessing a private super class field inside a closure - same
module'() {
- shouldFail shell, MissingPropertyException, '''
+ def err = shouldFail shell, '''
package a
class A {
@@ -46,9 +46,10 @@ final class Groovy9292 {
new B().test()
'''
+ assert err =~ /No such property: superField for class: a.B/
}
- @Test
+ @Test // GROOVY-11356
void 'test accessing a private super class field inside a closure - same
package'() {
assertScript shell, '''
package a
@@ -59,7 +60,7 @@ final class Groovy9292 {
assert true
'''
- shouldFail shell, MissingPropertyException, '''
+ def err = shouldFail shell, '''
package a
class B extends A {
@@ -70,9 +71,10 @@ final class Groovy9292 {
new B().test()
'''
+ assert err =~ /No such property: superField for class: a.B/
}
- @Test
+ @Test // GROOVY-11356
void 'test accessing a private super class field inside a closure - diff
package'() {
assertScript shell, '''
package a
@@ -83,7 +85,7 @@ final class Groovy9292 {
assert true
'''
- shouldFail shell, MissingPropertyException, '''
+ def err = shouldFail shell, '''
package b
class B extends a.A {
@@ -94,6 +96,7 @@ final class Groovy9292 {
new B().test()
'''
+ assert err =~ /No such property: superField for class: b.B/
}
@Test
diff --git a/src/test/groovy/bugs/Groovy9293.groovy
b/src/test/groovy/bugs/Groovy9293.groovy
index 1a2ae0a028..344448fcea 100644
--- a/src/test/groovy/bugs/Groovy9293.groovy
+++ b/src/test/groovy/bugs/Groovy9293.groovy
@@ -51,7 +51,7 @@ final class Groovy9293 {
'''
}
- @Test
+ @Test // GROOVY-11356
void 'test accessing a package-private super class field inside a closure
- diff package'() {
assertScript shell, '''
package a
@@ -62,7 +62,7 @@ final class Groovy9293 {
assert true
'''
- shouldFail shell, MissingPropertyException, '''
+ def err = shouldFail shell, '''
package b
class B extends a.A {
@@ -73,6 +73,7 @@ final class Groovy9293 {
new B().test()
'''
+ assert err =~ /No such property: superField for class: b.B/
}
@Test
@@ -138,7 +139,7 @@ final class Groovy9293 {
'''
}
- @Test
+ @Test // GROOVY-9293
void 'test accessing a package-private super class field inside a closure
- diff package, this qualifier'() {
assertScript shell, '''
package a
diff --git
a/src/test/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy
b/src/test/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy
index f5650b3d19..e898bdb53b 100644
---
a/src/test/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy
+++
b/src/test/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy
@@ -192,20 +192,20 @@ final class DifferentPackageTest {
void testDifferentPackageShouldNotSeeInstanceProps() {
def err = shouldFail CompilationFailedException, {
addSources(
- One: P_DOT_ONE,
- Two: '''
- package q
-
- @groovy.transform.CompileStatic
- class Two extends p.One {
- int valueSize() {
- value.size() // not visible
- }
+ One: P_DOT_ONE,
+ Two: '''
+ package q
+
+ @groovy.transform.CompileStatic
+ class Two extends p.One {
+ int valueSize() {
+ value.size() // not visible
}
- ''')
+ }
+ ''')
}
- assert err.message =~ /Access to q.Two#value is forbidden/
+ assert err.message =~ /No such property: value for class: q.Two/
}
// GROOVY-9093
@@ -213,20 +213,20 @@ final class DifferentPackageTest {
void testDifferentPackageShouldNotSeeStaticProps1() {
def err = shouldFail CompilationFailedException, {
addSources(
- One: P_DOT_ONE,
- Two: '''
- package q
-
- @groovy.transform.CompileStatic
- class Two extends p.One {
- static def half() {
- (CONST / 2) // not visible
- }
+ One: P_DOT_ONE,
+ Two: '''
+ package q
+
+ @groovy.transform.CompileStatic
+ class Two extends p.One {
+ static def half() {
+ (CONST / 2) // not visible
}
- ''')
+ }
+ ''')
}
- assert err.message =~ /Access to q.Two#CONST is forbidden/
+ assert err.message =~ /No such property: CONST for class: q.Two/
}
@Test
