daniellansun commented on code in PR #3227:
URL: https://github.com/apache/groovy/pull/3227#discussion_r4550640697


##########
src/test/groovy/bugs/Groovy11916.groovy:
##########
@@ -0,0 +1,727 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package bugs
+
+import groovy.lang.GroovyClassLoader
+import org.codehaus.groovy.control.CompilationUnit
+import org.codehaus.groovy.control.CompilerConfiguration
+import org.codehaus.groovy.control.Phases
+import org.codehaus.groovy.control.customizers.ImportCustomizer
+import org.junit.jupiter.api.BeforeAll
+import org.junit.jupiter.api.Test
+
+import java.lang.reflect.Modifier
+
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
+
+/**
+ * Star imports, module imports, and default imports resolve only accessible
+ * types. A source import shadows those defaults, including {@code java.lang}.
+ * Two source star imports bind the first accessible type in source order.
+ * An explicit single-type import, a fully qualified name, and
+ * {@code Class.forName} still name a type that is not public.
+ */
+final class Groovy11916 {
+
+    private static GroovyClassLoader types
+
+    @BeforeAll
+    static void compilePackagePrivateTypes() {
+        types = compile(Groovy11916.classLoader, [
+            'p/Types': '''
+                package p
+
+                @groovy.transform.PackageScope
+                class Hidden {}
+
+                class Visible {}
+
+                @groovy.transform.PackageScope
+                class Outer {
+                    public static class Inner {}
+                }
+
+                class Protector {
+                    protected static class Secret {}
+                }
+
+                class Priv {
+                    private static class Secret {}
+                    public static class Shown {}
+                }
+
+                @groovy.transform.PackageScope
+                class Collision {}
+            '''
+        ])
+        assert !Modifier.isPublic(types.loadClass('p.Hidden').modifiers)
+        assert !Modifier.isPublic(types.loadClass('p.Collision').modifiers)
+        assert Modifier.isPublic(types.loadClass('p.Visible').modifiers)
+        assert !Modifier.isPublic(types.loadClass('p.Outer').modifiers)
+        assert Modifier.isPublic(types.loadClass('p.Outer$Inner').modifiers)
+        assert 
Modifier.isProtected(types.loadClass('p.Protector$Secret').modifiers)
+        assert Modifier.isPrivate(types.loadClass('p.Priv$Secret').modifiers)
+    }
+
+    private static GroovyClassLoader compile(ClassLoader parent, Map<String, 
String> sources, CompilerConfiguration config = null) {
+        def loader = new GroovyClassLoader(parent, config)
+        def unit = new CompilationUnit(config, null, loader)
+        sources.each { name, text -> unit.addSource(name + '.groovy', text) }
+        unit.compile(Phases.CLASS_GENERATION)
+        unit.classes.sort { it.name }.each { groovyClass ->
+            loader.defineClass(groovyClass.name, groovyClass.bytes)
+        }
+        loader
+    }
+
+    private static void putType(Map<String, String> sources, String pkg, 
String simpleName, String state) {
+        sources[pkg + '/Marker'] = "package ${pkg}\nclass Marker {}"
+        if (state == 'p') {
+            sources[pkg + '/' + simpleName] = "package ${pkg}\nclass 
${simpleName} {}"
+        } else if (state == 'k') {
+            sources[pkg + '/' + simpleName] = "package 
${pkg}\[email protected]\nclass ${simpleName} {}"
+        }
+    }
+
+    private static String useSource(String pkg, String imports, String 
simpleName) {
+        """\
+package ${pkg}
+${imports}
+class Use {
+    ${simpleName} field
+    static Class which() { ${simpleName} }
+}
+"""
+    }
+
+    /** First public type in source order. {@code p} public, {@code k} 
package-private, {@code m} missing. */
+    private static String firstStar(String a, String b, String c) {
+        if (a == 'p') return 'pa.X'
+        if (b == 'p') return 'pb.X'
+        if (c == 'p') return 'pc.X'
+        return 'unresolved'
+    }
+
+    private static String classifyUse(Map<String, String> sources, String 
className, CompilerConfiguration config = null) {
+        // Dependency types are compiled first so package-private modifiers are
+        // already on the bytecode when the importing compilation unit is 
resolved.
+        def deps = sources.findAll { name, text -> !name.endsWith('/Use') }
+        def use = sources.findAll { name, text -> name.endsWith('/Use') }
+        try {
+            def parent = deps ? compile(Groovy11916.classLoader, deps) : 
Groovy11916.classLoader
+            def loader = compile(parent, use, config)
+            return 
loader.loadClass(className).getDeclaredMethod('which').invoke(null).name
+        } catch (Exception e) {
+            String text = ''
+            for (Throwable t = e; t != null; t = t.cause) {
+                if (t.message != null) text += t.message + '\n'
+            }
+            if (text.contains('ambiguous')) return 'ambiguous'
+            if (text.contains('unable to resolve')) return 'unresolved'
+            if (text.contains('not public')) return 'not-public'
+            def snippet = text.readLines().collect { it.trim() }.findAll { it 
}.take(4).join(' | ')
+            return 'other:' + (snippet ?: e.getClass().simpleName)
+        }
+    }
+
+    @Test
+    void testStarImportDoesNotResolvePackagePrivateJdkType() {
+        assert !Modifier.isPublic(Class.forName('java.util.TimSort').modifiers)
+
+        def err = shouldFail '''
+            import java.util.*
+            def seen = null instanceof TimSort
+        '''
+        assert err.message.contains('unable to resolve class TimSort')
+
+        // A bare name in a script is a dynamic property when it is not a type.
+        // It must not bind the inaccessible class (that binding is what throws
+        // IllegalAccessError for a later instanceof / checkcast).
+        err = shouldFail '''
+            import java.util.*
+            assert TimSort.name == 'java.util.TimSort'
+        '''
+        assert err instanceof MissingPropertyException
+    }
+
+    @Test
+    void testDefaultImportDoesNotResolvePackagePrivateJdkType() {
+        def err = shouldFail '''
+            def seen = null instanceof TimSort
+        '''
+        assert err.message.contains('unable to resolve class TimSort')
+    }
+
+    @Test
+    void testModuleImportDoesNotResolvePackagePrivateJdkType() {
+        def err = shouldFail '''
+            import module java.base
+            def seen = null instanceof TimSort
+        '''
+        assert err.message.contains('unable to resolve class TimSort')
+    }
+
+    @Test
+    void testStarImportStillResolvesPublicTypes() {
+        assertScript '''
+            import java.util.*
+            assert ArrayList.name == 'java.util.ArrayList'
+            assert List.name == 'java.util.List'
+            assert new ArrayList() instanceof List
+        '''
+        assertScript '''
+            import java.util.Map.*
+            Entry e = [a:1].entrySet().first()
+            assert e.key == 'a'
+        '''
+        assertScript '''
+            import static java.util.Map.*
+            Entry e = [a:1].entrySet().first()
+            assert e instanceof Entry
+        '''
+        assertScript '''
+            import module java.base
+            assert ArrayList.name == 'java.util.ArrayList'
+            assert LocalDate.name == 'java.time.LocalDate'
+        '''
+    }
+
+    @Test
+    void testExplicitReferenceToPackagePrivateTypeStillResolves() {
+        assertScript '''
+            import java.util.TimSort
+            assert TimSort.name == 'java.util.TimSort'
+        '''
+        assertScript '''
+            assert java.util.TimSort.name == 'java.util.TimSort'
+        '''
+        assertScript '''
+            assert Class.forName('java.util.TimSort').name == 
'java.util.TimSort'
+        '''
+        compile(types, [
+            'q/Direct': '''
+                package q
+                import p.Hidden
+                class Direct {
+                    Hidden h
+                }
+            '''
+        ])
+    }
+
+    @Test
+    void testSamePackageStarImportSeesPackagePrivateType() {
+        compile(types, [
+            'p/Peer': '''
+                package p
+                import p.*
+                class Peer {
+                    Hidden h
+                    Visible v
+                }
+            '''
+        ])
+    }
+
+    @Test
+    void testOtherPackageStarImportDoesNotSeePackagePrivateType() {
+        def err = shouldFail {
+            compile(types, [
+                'q/User': '''
+                    package q
+                    import p.*
+                    class User {
+                        Hidden h
+                    }
+                '''
+            ])
+        }
+        assert err.message.contains('unable to resolve class Hidden')
+
+        compile(types, [
+            'q/PublicUser': '''
+                package q
+                import p.*
+                class PublicUser {
+                    Visible v
+                }
+            '''
+        ])
+    }
+
+    @Test
+    void testStarImportOfPublicMemberOfInaccessibleType() {
+        def err = shouldFail {
+            compile(types, [
+                'q/InnerUser': '''
+                    package q
+                    import p.Outer.*
+                    class InnerUser {
+                        Inner i
+                    }
+                '''
+            ])
+        }
+        assert err.message.contains('unable to resolve class Inner')
+
+        compile(types, [
+            'p/InnerPeer': '''
+                package p
+                import p.Outer.*
+                class InnerPeer {
+                    Inner i
+                }
+            '''
+        ])
+    }
+
+    @Test
+    void testProtectedMemberTypeFromSubclass() {
+        compile(types, [
+            'q/Child': '''
+                package q
+                import p.Protector
+                import p.Protector.*
+                class Child extends Protector {
+                    Secret s
+                }
+            '''
+        ])
+        def err = shouldFail {
+            compile(types, [
+                'q/Stranger': '''
+                    package q
+                    import p.Protector.*
+                    class Stranger {
+                        Secret s
+                    }
+                '''
+            ])
+        }
+        assert err.message.contains('unable to resolve class Secret')
+    }
+
+    @Test
+    void testPrivateMemberTypeStarImport() {
+        compile(types, [
+            'q/ShownUser': '''
+                package q
+                import p.Priv.*
+                class ShownUser {
+                    Shown s
+                }
+            '''
+        ])
+        compile(types, [
+            'q/ShownStatic': '''
+                package q
+                import static p.Priv.*
+                class ShownStatic {
+                    Shown s
+                }
+            '''
+        ])
+        def err = shouldFail {
+            compile(types, [
+                'q/SecretUser': '''
+                    package q
+                    import p.Priv.*
+                    class SecretUser {
+                        Secret s
+                    }
+                '''
+            ])
+        }
+        assert err.message.contains('unable to resolve class Secret')
+    }
+
+    @Test
+    void testInaccessibleStarImportDoesNotHideLaterPublicType() {
+        def loader = compile(types, [
+            'q/Collision': '''
+                package q
+                class Collision {}
+            ''',
+            'r/User': '''
+                package r
+                import p.*
+                import q.*
+                class User {
+                    static Class which() { Collision }
+                }
+            '''
+        ])
+        assert 
loader.loadClass('r.User').getDeclaredMethod('which').invoke(null).name == 
'q.Collision'
+    }
+
+    // GROOVY-11916
+    @Test
+    void testStarImportsBindFirstAccessibleType() {

Review Comment:
   Each method now says what it checks. 
`testStarImportsBindFirstAccessibleType`: `pa.X`, `pb.X`, and `pc.X` are 
missing, public, or package-private, and the use class imports the three stars 
in that order. The first public type wins. An inaccessible type does not hide a 
later public one. With no public type the name stays unresolved. A repeated 
star and the reversed list follow the same rule. 
`testSamePackageTypeShadowsOnDemandImports`: the use class is `pa.Use`. When 
`pa.X` exists it wins whether it is public or package-private. When it is 
missing, the first accessible star is used. 
`testDeclaredStarShadowsLanguageDefaults`: the use class names `String`. The 
first public `pb.String` or `pc.String` shadows `java.lang.String`. A 
package-private one is skipped. An explicit `import java.lang.*` is a source 
star too, so its position relative to `pb.*` decides the winner.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to