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
The following commit(s) were added to refs/heads/master by this push: new 6cd477b315 GROOVY-11356: STC: unlink variable expression from an inaccessible field 6cd477b315 is described below commit 6cd477b3157675f0ceee9fbd62e3efd0cc9bdb3e 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 --- .../transform/stc/StaticTypeCheckingVisitor.java | 22 +++++------ 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, 47 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 74a4990706..33845e0be0 100644 --- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java +++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java @@ -678,18 +678,18 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport { addStaticTypeError("The variable [" + name + "] is undeclared.", vexp); } } else if (accessedVariable instanceof FieldNode) { - FieldNode accessedField = (FieldNode) accessedVariable; - ClassNode temporaryType = getInferredTypeFromTempInfo(vexp, null); // GROOVY-9454 - boolean hasProperty = tryVariableExpressionAsProperty(vexp, name); - if (enclosingClosure == null && !hasProperty) { // GROOVY-10981: property selected 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); + if (tryVariableExpressionAsProperty(vexp, name)) { + ClassNode temporaryType = getInferredTypeFromTempInfo(vexp, null); // GROOVY-9454 + if (temporaryType == null) { + storeType(vexp, getType(vexp)); + } else if (!isObjectType(temporaryType)) { + vexp.putNodeMetaData(INFERRED_TYPE, temporaryType); + } + } else if (!extension.handleUnresolvedVariableExpression(vexp)) { // GROOVY-11356 + 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)) { @@ -1898,7 +1898,7 @@ out: if ((samParameterTypes.length == 1 && isOrImplements(samParameterTypes[0 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