Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@In/@Out annotations for declaration-site variance -- kind of #72

Open
kevinb9n opened this issue Oct 8, 2019 · 13 comments
Open

@In/@Out annotations for declaration-site variance -- kind of #72

kevinb9n opened this issue Oct 8, 2019 · 13 comments
Labels
design An issue that is resolved by making a decision, about whether and how something should work. new-feature Adding something entirely new for users, not improving what's there
Milestone

Comments

@kevinb9n
Copy link
Collaborator

kevinb9n commented Oct 8, 2019

These annotations would be used like:

interface Function<@In F, @Out T> {}

This represents the interface's promise that it will not use F as an out-type, nor use T as an in-type (hasty definitions at bottom).

Tools could presumably use this information in ways including the following:

Effect no. 1: Validating that "promise" in the type itself (including inherited members). Using class Foo<@Out T> as example:

  • All non-private method return types and field types involving T must be T or involve T in an "out"/covariant sense
  • All non-private method/constructor parameter types involving T must involve T in the "in"/contravariant direction
  • All non-private fields involving T in any way must be final.

(It's a bit harder than this...)

Effect no. 2: type usages of Foo could be validated, enforcing proper use of wildcards as given in Effective Java. (We have no way to help users whose wish is to avoid wildcards.)

Effect no. 3: screening out unchecked warnings from the compiler for the con(ntra)variant cast idiom. [EDIT: this needs explanation. Google, internally, is quite selective about which javac unchecked warnings we allow to be surfaced to users. The warning for a cast like (Iterator<Object>) (Iterator) stringIterator could be hidden if we had knowledge that Iterator type is covariant.]

Effect no. 4: nullness analysis could exploit this information as discussed in #40.

Effect no. 5: callers from kotlin should benefit as if the API had real DSV. (Andrey thinks feasible)

In addition: if JEP 300 lands and DSV comes to Java, this would leave us very well-positioned to migrate to it - not to mention that we would have the best possible data to support how it should be designed.

Iterable would be Iterable<@Out T> but Collection would drop the @Out. Compiling a class like Collection in this example should produce a warning. That warning will often be suppressed, but then I think any downcast of Iterable to Collection or below should also produce a similar warning at that site as well... where it may also be suppressed but what can you do.

Interestingly ImmutableCollection would re-add the @Out again, and would thus need to suppress a warning on all the mutation methods.


out-type = root type in a return type or final field; the type after an “extends” wildcard under an out-type; the type after a “super” wildcard under an in-type

in-type = root type of a parameter; the type after an “extends” wildcard of an in-type; the type after a “super” wildcard of an out-type

@kevinb9n kevinb9n changed the title @In / @Out annotations for declaration-site variance -- kind of @In/@Out annotations for declaration-site variance -- kind of Oct 8, 2019
@kevinb9n
Copy link
Collaborator Author

re: Effect no. 2: actually, we can do this anyway, for method bodies that we can observe are only calling producer methods or only consumer methods. But sometimes the method wouldn't be able to change because it overrides an abstract method. What the annotations would allow is for us to validate the abstract methods, at least in cases where the type really is variant. Still can't do much about an interface method accepting a wildcardless List.

@kevinb9n kevinb9n added this to the Post-1.0 milestone Aug 18, 2021
@kevinb9n
Copy link
Collaborator Author

(for no particular reason I discussed this feature at length on a reddit post.)

@kevinb9n kevinb9n added design An issue that is resolved by making a decision, about whether and how something should work. new-feature Adding something entirely new for users, not improving what's there and removed requirements design An issue that is resolved by making a decision, about whether and how something should work. labels Nov 30, 2022
@vlsi
Copy link

vlsi commented Jan 10, 2023

Is there a workable checker that can ensure users have java.util.function.Predicate<? super T> rather than Predicate<T> in their method parameters?

In other words, since java.util.function.Predicate is known to have java.util.function.Predicate<@In T> variance, almost every usage of Predicate in parameters should be Predicate<? super T>.
The same goes for Function.
It would be nice if there were tools that could detect invariant usages and advice users to add use-site variance as it is required by Java.

@kevinb9n
Copy link
Collaborator Author

I agree -- in the meantime until this issue is resolved, it would be nice if the tools went ahead and caught those problems using hardcoded lists or other means.

I'm surprised we never did it in Error Prone! Thanks for filing the issue there; we'll see what Liam thinks.

@vlsi
Copy link

vlsi commented Jan 10, 2023

It looks like one more annotation is needed: @Invariant to declare use-sites that are intentionally invariant.

@Kroppeb
Copy link

Kroppeb commented Jan 10, 2023

It looks like one more annotation is needed: @Invariant to declare use-sites that are intentionally invariant.

What advantages does this produce over not annotating at all?
I could see perhaps an extension (more like opposite) of effect no. 3 but instead resulting in errors when such casts happen?

@vlsi
Copy link

vlsi commented Jan 10, 2023

Suppose the user wants to have Predicate<Number> instead of Predicate<? super Number> because of backward compatibility or whatever else reasons.

If they keep the type unannotated, then checkers should complain the variance is missing.
Predicate<@Invariant Number> would suppress the checker and inform the readers that variance is absent here.

@kevinb9n
Copy link
Collaborator Author

I think that would be false information though; it would be better to see a suppression there.

We can explore use-site annotations in a separate issue, assuming this one gets any traction. At declaration-site, it does seem that any type parameter not marked as In or Out should be assumed invariant.

@jkschneider
Copy link

Are there also a series of types for which we wouldn't add variance? For example: Function<String, Car> should be Function<String, ? extends Car> not Function<? super String, ? extends Car> right?

@vlsi
Copy link

vlsi commented Jan 10, 2023

How do you suggest implementing a suppression?

Predicate<@Suppress("wildcards") Number>?

@Kroppeb
Copy link

Kroppeb commented Jan 10, 2023

Are there also a series of types for which we wouldn't add variance? For example: Function<String, Car> should be Function<String, ? extends Car> not Function<? super String, ? extends Car> right?

Why wouldn't it be the latter?

@kevinb9n
Copy link
Collaborator Author

How do you suggest implementing a suppression?

Predicate<@Suppress("wildcards") Number>?

Assumption is to let @SuppressWarnings handle it. Through JSpecify we intend to standardize suppression strings too (#55)

It's too bad that Java doesn't support finer-grained scopes for it, but solving that would be a separate problem (and there are at least 2 ways to go about it).

@kevinb9n kevinb9n added the design An issue that is resolved by making a decision, about whether and how something should work. label Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design An issue that is resolved by making a decision, about whether and how something should work. new-feature Adding something entirely new for users, not improving what's there
Projects
None yet
Development

No branches or pull requests

4 participants