[
https://issues.apache.org/jira/browse/GROOVY-12755?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18142691#comment-18142691
]
ASF GitHub Bot commented on GROOVY-12755:
-----------------------------------------
codeconsole commented on PR #3286:
URL: https://github.com/apache/groovy/pull/3286#issuecomment-6784788096
@daniellansun thanks. I ran each program against javac and against the
branch. Three new commits on top of f2d84108eb:
1. **Simple name of an inner class.** `ResolveVisitor` now records the
enclosing type of such a name. `Inner` is `Outer<T>.Inner` within `Outer<T>`,
and `Outer<String>.Inner` within a subclass of `Outer<String>`. Both of your
programs are now errors, and the two tests are in `GenericsSTCTest` and,
against javac, in `GenericsJavaCompatibilityTest`:
```java
Class<? extends Inner> f() { return getClass(); } // inside
Outer<T>.Inner
Class<? extends Inner> x = c; // c is a Class<?
extends Outer.Inner>
```
The inherited name is covered too: in `Sub extends Outer<String>`,
`Class<? extends Inner> x = inner.getClass()` is an error. In the other
direction, two forms that the previous tip rejected and javac accepts now
compile, because the two names are the same type: `Class<? extends
Outer<T>.Inner> y = x` for a `Class<? extends Inner> x`, and `Class<? extends
Outer<String>.Inner> y = x` in `Sub`.
Left as they were, on purpose:
- Where the type variables are out of scope (a static method or field, or
under a method type parameter of the same name) the name stays raw. javac
rejects the name there; Groovy accepts it.
- A class literal, `instanceof Inner` and `new Inner[n]` still name the
class. javac rejects the last two for `Outer<T>.Inner`. Groovy accepts them
today, and I did not want this PR to break `equals` methods and array creation.
- A nested record, enum or interface is static, so its name has no
enclosing type. A generic inner class used without type arguments stays raw.
This is a resolver change, so it reaches further than the rest of the PR:
it applies to every such name, with or without static compilation. Outside the
type checker the visible effect is the `Signature` attribute. `Inner make()` in
`Outer<T>` now gets `()LOuter<TT;>.Inner;`, which is what javac writes; the
positive test compares the generic signatures with javac's.
2. **Diagnostic.** A use of an inner class with no enclosing type prints as
the raw type, so the message is `Cannot assign java.lang.Class<? extends
Outer.Inner> to: java.lang.Class<? extends Outer<T>.Inner>`. The declaration
itself still prints as `Outer<T>.Inner`.
3. **The three assignments.** The second and third are now errors. Within
the bounds of two wildcards, a type variable is taken as its bound, and a type
argument that is a raw type is not accepted for a parameterized one:
```java
Class<? extends List<List<String>>> g(Class<? extends List<List>> c) {
return c; }
<T extends List> Class<? extends List<String>> h(Class<? extends T> c) {
return c; }
```
The first one, `Class<ArrayList>` for `Class<? extends List<String>>`, is
unchanged. I tried it: treating a raw type argument like a raw bound rejects
your program and `Class<? extends List<String>> c = ArrayList.class`, but it
also rejects these, which javac accepts because it infers the element type from
the target:
```java
List<? extends List<String>> a = List.of(new ArrayList());
List<? extends List<String>> b = Arrays.asList(new ArrayList());
Map<String, ? extends List<String>> m = Map.of("k", new ArrayList());
```
and with them the Groovy forms `[new ArrayList()]`, `[k: new
ArrayList()]` and `Optional.of(new ArrayList())`. STC types all of those
bottom-up as `List<ArrayList>`, and the check cannot tell that from a declared
`List<ArrayList>`. So it needs target typing for those expressions first, or it
turns programs that javac accepts into errors. It is also not on the
`getClass()` path, which always gives a wildcard. I think it should be its own
issue.
4. **`instanceof`.** I left this one, and added a test that states it. In
the branch of `if (o instanceof String)`, STC gives `o` the type `String` for
every member: `o.substring(0)` and `String s = o` compile there, and javac
rejects both, just as it rejects `o.getClass()` for a `Class<? extends
String>`. `getClass()` takes the receiver type that any other call on `o` takes
at that point, which is the role the pattern variable has in Java, where the
two compilers agree. Using the declared type for `getClass()` only would make
it the one member that sees a different receiver. After the branch
`o.getClass()` is `Class<?>` again, and the test covers that too. If you still
want the declared type here, say so and I'll change it.
5. **Other differences** I found while checking these. None of them is on
the `getClass()` path:
- `Class<? extends List<String>> x = rawHolder.type`, for `class Holder<T
extends List> { Class<? extends T> type }`, is rejected since 7807d56395. javac
gives an unchecked warning. STC reads a member of a raw receiver with `T`
replaced by its bound, `Class<? extends List>`, where javac erases the member
type to `Class`.
- `List<Outer.Inner>` is accepted for `List<? extends Inner>` and for
`List<Inner>` (javac rejects both). That is a raw type argument again, as in 3.
- An inherited inner class named from a class nested in the subclass does
not resolve at all (`unable to resolve class Inner`). Master does the same.
The new negative tests fail on the previous tip, and so does the positive
simple-name test. The `instanceof` test passes there, since it records
behaviour that is not changed. The whole build's tests pass locally.
For GROOVY_6_0_X: the seven commits cherry-pick cleanly onto the current
branch, and the only failure in the core tests there is still the `inClosure()`
line of `testGetClassIsClassOfReceiverErasure2` (GROOVY-12576). Given the reach
of the resolver change, you may want to decide on that commit separately for
the port.
> STC: e.getClass() is typed Class<?>, so since GROOVY-12714 it is rejected
> where a Class<? extends T> is expected
> ----------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12755
> URL: https://issues.apache.org/jira/browse/GROOVY-12755
> Project: Groovy
> Issue Type: Bug
> Components: Static Type Checker
> Affects Versions: 7.0.0-alpha-1, 6.0.1
> Reporter: Scott Murphy Heiberg
> Priority: Major
>
> JLS 4.3.2 gives a call {{e.getClass()}} the type {{Class<? extends |T|>}},
> where {{|T|}} is the erasure of the static type of {{e}}. Static type
> checking types it with the declared return type of {{Object#getClass()}},
> {{Class<?>}}. Until GROOVY-12714 the wildcard check accepted {{Class<?>}}
> where a {{Class<? extends T>}} was expected. Since GROOVY-12714 (GROOVY_6_0_X
> and master) it is rejected:
> {code:groovy}
> @groovy.transform.CompileStatic
> class C {
> interface Store {}
> static <T> T with(Class<? extends Store> type, Closure<T> body) {
> body.call() }
> static class Adapter { Adapter(Class<? extends Store> type) {} }
> static Object viaMethod(Store store) { with(store.getClass()) { 1 } }
> static Object viaProperty(Store store) { with(store.class) { 1 } }
> static Object viaConstructor(Store store) { new Adapter(store.getClass())
> }
> static Class<? extends Store> viaReturn(Store store) { store.getClass() }
> }
> {code}
> {noformat}
> [Static type checking] - Cannot call <T> C#with(java.lang.Class<? extends
> C.Store>, groovy.lang.Closure<T>) with arguments [java.lang.Class<?>,
> groovy.lang.Closure]
> [Static type checking] - Cannot call <T> C#with(java.lang.Class<? extends
> C.Store>, groovy.lang.Closure<T>) with arguments [java.lang.Class<?>,
> groovy.lang.Closure]
> [Static type checking] - Cannot call C$Adapter#<init>(java.lang.Class<?
> extends C.Store>) with arguments [java.lang.Class<?>]
> [Static type checking] - Incompatible generic argument types. Cannot assign
> java.lang.Class<?> to: java.lang.Class<? extends C.Store>
> {noformat}
> javac accepts the equivalent Java. Grails no longer compiles against
> GROOVY_6_0_X: it passes {{datastore.getClass()}} to a {{Class<? extends
> Datastore>}} parameter and {{registrar.getClass()}} to a {{Class<? extends
> BeanRegistrar>}} one
> (https://github.com/apache/grails-core/actions/runs/42702095261).
> Typing {{e.getClass()}} and {{e.class}} as {{Class<? extends |T|>}} makes
> more calls pass an upper-bounded wildcard for a bounded type parameter, such
> as {{foo(list.class)}} for {{<T extends List<Integer>> foo(Class<T>)}}, which
> GROOVY-12754 rejected. So this depends on GROOVY-12754.
> The inferred type of {{list.class}} for a raw {{List list}} changes from
> {{Class<? extends Object>}} to {{Class<? extends List>}}, and the GROOVY-5415
> error for {{a.bar(this.getClass())}} now names {{Class<? extends ClassB>}};
> the two tests that assert those types are updated.
> PR: https://github.com/apache/groovy/pull/3286
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
