daniellansun commented on code in PR #3227:
URL: https://github.com/apache/groovy/pull/3227#discussion_r4544238289
##########
src/main/java/org/codehaus/groovy/control/ResolveVisitor.java:
##########
@@ -750,7 +750,7 @@ protected boolean resolveFromDefaultImports(final ClassNode
type, final String[]
// WARNING: This code does not expect a class that has a static
// inner class in DEFAULT_IMPORTS
ConstructedClassWithPackage tmp = new
ConstructedClassWithPackage(packagePrefix, typeName);
- if (resolve(tmp, false, false, false)) {
+ if (resolve(tmp, false, false, false) &&
isStarImportAccessible(tmp, null)) {
Review Comment:
Jochen,
You are right about both breaking changes, and the follow-up restores the
older rule.
A declared import shadows the default imports, including `java.lang`.
`import pb.*; class Use { String s }` with a public `pb.String` is `pb.String`
again. It is not ambiguous with `java.lang.String`, and it is not
`java.lang.String`. Making that a tie was a breaking change. The
program-structure page lists `java.lang.*` in the same default-import list as
`java.util.*`, `java.io.*`, `java.net.*`, `java.time.*`, `groovy.lang.*`,
`groovy.util.*`, `BigInteger`, and `BigDecimal`. Those are the imports Groovy
adds so the boilerplate can be left out. A source import shadows that whole
list. `java.lang` is not a special case that ties with a star the user wrote.
That is also why the extra defaults were kept below a source star in the
previous commit, and why they stay there. `import java.sql.*; Date` is
`java.sql.Date`, not `java.util.Date`. `import java.awt.*; List` is
`java.awt.List`. Under the same list, `java.lang` does not get a different rule
from `java.util`.
Two source star imports are first-match in source order again, not an
ambiguity error. `import java.util.*; import java.sql.*; Date` is
`java.util.Date`. The reverse order is `java.sql.Date`. The star-import
section's answer to a cluttered namespace is aliasing. The error the page
actually states is two simple imports of the same name (`import java.awt.List;
import java.util.List`), or a simple import and a top-level type of that name.
This is not the Generics section, so I am not applying the JLS on-demand tie
here.
An `ImportCustomizer` star sits below imports written in the source and
above that language list. A source star or a source single-type import hides
it. With no source import, `star 'java.sql'` still binds `Date` to
`java.sql.Date`, and a customizer star of a public `pb.String` binds
`pb.String`, without an ambiguity error against `java.lang` or `java.util`. It
still does not bind an inaccessible type. A simple import added by the
customizer stays a simple import: it still conflicts with another simple import
of the same name, which is the case `ImportTest` already covers for `Proxy`.
GROOVY-11916 itself stands. A star import, a module import, a default
import, and a customizer star bind only a type that is accessible from the
importing class. `import java.util.*` does not bind `java.util.TimSort`. An
inaccessible type is not a candidate, so it does not hide a later accessible
type and it is not reported as "not public" after being bound. A
package-private type in the same package, a protected member type used from a
subclass, and a private member type used from the enclosing nest stay visible.
An explicit single-type import, a fully qualified name, and `Class.forName`
still resolve `TimSort`. I am not adding an access diagnostic for those forms.
`import static pb.Holder.*` follows the same idea. It shadows the defaults,
so a public static member type `String` or `Date` is the member type, not
`java.lang.String` or `java.util.Date`. Two source static stars use source
order. A package-private static member type is not imported. The static-import
section talks about overloading and about an imported method taking precedence
when the signature matches. It does not make two stars an ambiguity error.
A module import is the star imports of the packages that module exports,
placed at that statement. `import module java.desktop` then `import
java.util.*` keeps `java.awt.List`, because the module import is earlier. The
reverse order keeps `java.util.List`. `import java.util.List` still wins over
the module, which is the priority the documentation states. If one module
exports two public types with the same simple name, the first accessible export
is used rather than a compile error; a single-type import is how you name the
other one. `import module java.base; String` remains `java.lang.String`,
because `java.lang` is already a language default.
The `NamedVariantTransformTest` and Guice tweaks were only there so those
programs would compile under the ambiguity rule. Both are reverted.
`NamedVariantTransformTest` is back to `star 'groovy.transform',
'org.codehaus.groovy.ast'`. `ASTTest` is `groovy.transform.ASTTest` again
because that star is configured first; the other candidate is the test class
`org.codehaus.groovy.ast.ASTTest`. The Guice example says `as Module` again:
`import com.google.inject.*` is a declared star, so `Module` is
`com.google.inject.Module`, not `java.lang.Module`.
The tests that were not on main are in the second commit on this PR,
`4e761587`: `Groovy11916.groovy`, the qualified Guice `Module`, and the extra
`ASTTest` import. This follow-up rewrites the cases that expected a javac-style
tie (a source star against `java.lang`, two source stars, and two static stars)
and restores those two test tweaks. The accessibility tests from the first
commit stay.
--
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]
