-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Variance Annotations do not impact "infer" correctly #55920
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
Comments
Your type Duplicate of #40796. |
Since variance annotations only affect relations between two direct instantiations of the same generic (there are issues about this), if you need to enforce variance even at the structural level, youβll need to keep doing it that way. |
See #53798 and #48240 (comment) |
This has less to do with variance annotations than it has to do with our approximations in inference, and so I feel the best label is "not a defect". interface X<T> {}
declare function f<T>(x: X<T>): T;
let x = f({} as X<number>);
// ^?
let y = f({} as X<number> & {});
// ^?
let z = f({} as X<number> & { yadda: string });
// ^? When you can infer between two identical type references or aliases, our inference algorithm will avoid any sort of structural checks because it is unnecessary; however, introducing an object type with any contents means that inference must consider the holistic structure. @MartinJohns and @fatcerberus have given the context around there being nothing to infer from in the underlying structure |
Here is a repro without any unused type parameters or empty types: interface Brand<in out T> {
_dummy?: T
}
// This is "true", which seems wrong. Adding {unrelatedField: number} causes the type checking to ignore the variance annotation as infer covariant.
type C = (Brand<"A"> & {unrelatedField: number}) extends Brand<string> ? true : false; Based on this it seems like the issue is with "extends", and not the infer part. |
I'd suggest that if this is expected behavior, we should include a disclaimer in the official docs about variance annotations, but I can't find any such docs other than the 4.7 release notes. Is there any official documentation explaining how variance annotations are expected to work with "extends" and "infer"? If the type system just discard them whenever it has to do a non-trivial type comparison (and thats expected behavior), that seems like something we should make clear. |
I'm surprised this isn't already documented, given Ryan's comment at #48240 (comment):
Which I agree with, it really does need to be documented, quite clearly, that |
π Hi, I'm the Repro bot. I can help narrow down and track compiler bugs across releases! This comment reflects the current state of this repro running against the nightly TypeScript.
Historical Information
|
"Why don't unused type parameters infer" is in the FAQ. Expecting variance annotations to change that behavior, when the documentation doesn't say they do, implies that the docs have to explain everything that you might expect to happen but doesn't -- but I have no idea how to come up with that list. |
DraftVariance Annotations
TypeScript automatically infers the variance of generic types using its normal type comparison algorithm. You can optionally annotate type parameters of some types with the variance annotations // Correct covariant annotation
interface Producer<out T> {
create(): T;
}
// Incorrect covariant annotation
interface ProducerWrongAnnotated<in T> {
create(): T;
} If you don't know what those terms means, it's best to simply avoid this feature, as it's almost never strictly needed. Variance annotations are an advanced feature designed to specifically address some very rare situations, and have some important limitations to keep in mind. Because the purpose of variance annotations is to correct situations where TypeScript produced the wrong result, they aren't strictly validated, so it's critical to only write a variance annotation if you're confident that you're matching the correct structural variance. Do not suppress errors about incorrect variance errors with Note that variance annotations do not change the actual variance of a type parameter as it occurs in an instantiated type. Variance annotations are only used during nominal inference between two types, not structural inference. |
Usage suggestions are good, but we need a spec. What do variance annotations do? Do they do nothing by add compile errors if they disagree with that the compiler infers? Can they only restrict/constrain access types or can they replace the inferred variance entirely, allowing both restricting and relaxations of use of the type? Is their purpose to be an error at the declaration site if the annotation is wrong, or is getting it wrong undefined behavior? Also, to follow this usage guideline, I have to know "the variance that would be structurally measured by TypeScript (modulo limitations)". How do they interact with mapped types? Is that interaction with mapped types a TypeScript working as intended or one of the "limitations"? How do they works with extends clauses and is that a "limitation" or not? Without those answers, following that usage guidance isn't possible. That means I need a hard answer on which of the behaviors around variance are limitations and which are not. I do not think this is clearly documented. Personally I find the variance rules TypeScript uses to be frequently incorrect (I wrote and use a library to mitigate this since I kept having real issues with it). Thats using soundness as my definition of correct: Its clearly wrong without strict function checks, and even with them its unsound for properties that are functions (when it infers bivariant), its unsound when it doesn't have enough information (Types are used in whats TS doesn't see as members, like by other Types with infer) its unsound when it deletes private fields etc. Since these seem to be by design its not clear what "would be structurally measured by TypeScript (modulo limitations)." What I want to use variance annotations for is three things:
To me how it apparently works sounds like its the opposite of #1? Since to use it you have to confidently know the variance TypeScript (modulo limitations) would have gotten and anyone changing types in the codebase has to ensure they update all variance annotations they might have transiently impacted)? Thus it seems like adding the annotations adds additional risk for bad behavior to crop up later when the types are modified if someone who does so doesn't know to update the variance annotation. If thats really whats going on here thats very unlike other similar languages (ex: C#'s use of them), and unlike most other typing in TypeScript (this makes the code maintaince less likely to cause runtime issues that go unnoticed by the compiler).
Since these cases apparently differ in behavior in important ways, is when each of these happens, and any other ways they differ documented anywhere? Also are these difference included in the "limitations" above (and which version is the limited one? I'm guessing the structural case?) |
Generally if at compile type TypeScript generates a type in a way that is undefined behavior (there is no document rule for what the type should be), I'd like that to be a compile error rather than the compiler just throwing out an "unknown" (or some other type) from nowhere. Alternatively, can I have a --noImplicitUnknown for when the compiler makes up a type from no where which might be unsound? We have noImplicitAny, and in cases which are not covariant, "unknown" is plenty unsound to warrant wanting to opt out of. |
The thing I'm trying to make as clear as possible in this writeup is that if you haven't already been told to write a variance annotation, by someone who absolutely knows what's going on, to fix a specific situation where an identifiable problem has occurred, you almost certainly shouldn't be writing variance annotations. It seems like that applies in your situation -- none of the three goals you have there match those criteria. I guess what I'm hearing is that the writeup needs additional language to dissuade people from writing variance annotations. 99.9% of people should not write variance annotations, because they don't know the answers to the questions you posted either, and they need to know the answers from their own experience in order to write correct annotations. If you're at Microsoft you should stop by TypeScript Office Hours sometime. |
Is this referring to the details in 2.6 release notes + the stuff in the 4.7 release notes + the FAQ entry on the wiki about unused type parameters? As far as I'm aware those three things compose the official docs for variance? While I have those version numbers memorized, it's hard for new developers to know that's where you have to look, and it's hard to know how the rules from the 2.6 release notes are supposed to be interpreted in the context of the 4.7 feature. I clearly got that wrong and I think having some more unified up to date coverage of it (like a language spec) would help with things like this. |
Variance is a computer science concept; the docs are not intended to replace general knowledge. "the writeup" refers to the blurb I posted above |
I know that. I know variance as a general concept very well. Assuming general ideas about variance knowledge applied here is the mistake I made which you pointed out. My understanding of type theory doesn't tell me how to use a language full of unsoundness and rules like use the "same variance that would be structurally measured by TypeScript (modulo limitations)." I can trick TypeScript into giving me nominal types where variance annotations mostly make sense, then add the annotations to me code, assume the variance will be treated as I annotated and work like it does in type theory, then the compiler gives "unknown" for an invariant string parameter reasons that depend on if something is a "direct instantiations of the same generic", so I minimize a repro and come here to report the bug since that isn't what variance from type theory taught me to expect ("unknown" is not compatible with an invariant parameter thats a string, only string is). Now I'm directed to not use the language feature and we don't need to document it because its already defined by type theory? Edit: I think that came across more confrontational than I really intended. I'm glad you are making a new writeup to help clarify this :) |
v2 Handbook writeup inside the fold
## Variance Annotations (`in` and `out`)
TypeScript automatically infers the variance of generic types using its normal type comparison algorithm. TypeScript's variance inference system produces correct results the vast majority of the time, and writing an incorrect variance annotation will cause surprising results down the road. Again, variance annotations are not intended for everday use. You can optionally annotate type parameters of some types with the variance annotations // Correct covariant annotation
interface Producer<out T> {
create(): T;
}
// Correct contravariant annotation
interface Consumer<in T> {
accept: (in: T) => void;
}
// Correct invariant annotation
interface Prosumer<in out T> {
create(): T;
accept: (in: T) => void;
} If you don't know what those terms means, it's best to simply avoid this feature, as it's almost never strictly needed. Variance annotations are an advanced feature designed to specifically address some very rare situations, and have some important limitations to keep in mind. Because the purpose of variance annotations is to correct situations where TypeScript produced the wrong result, they aren't strictly validated, so it's critical to only write a variance annotation if you're confident that you're matching the correct structural variance. // Incorrect invariant annotation (type is actually covariant); does not error.
interface ProducerWrong<in out T> {
create(): T;
} This creates a confusing situation, because nominal inference and comparisions on Do not suppress errors about incorrect variance errors with Note that variance annotations will not change the actual variance of a type parameter as it occurs in an instantiated type. type MaybeThisRemovesBivariance<in T> = {
method(x: T): void;
}
declare const obj: {
alsoMethod(x: string): void;
}
// No error; variance annotations do not change structural operations
let a: MaybeThisRemovesBivariance<string | number>["method"] = obj.alsoMethod; Variance annotations are only used during nominal inference between two types, not structural inference. |
I think this is potentially misleading FWIW; if variance measurement produces the wrong type relation, an explicit annotation will only correct it in a narrow subset of cases. I wouldn't expect all the exceptions to such a rule to be explicitly enumerated, but in the absence of text to the contrary I think it's reasonable to assume such a "correction" is comprehensive, so it should be noted, at least, that exceptions exist. |
Can you elaborate (maybe with an example) ? |
I still don't think it covers what variance annotations really do. I think the main value is to use them to trigger TS 2636 letting you know the variance you want is not possible for your type. From this thread it wasn't clear they even did this so I had to check in the playground: the nice error produced is way better than the "surprising or wrong results in many cases." I was starting to fear. Also it should note that "Variance annotations are only supported in type aliases for object, function, constructor, and mapped types" (source: error 2637) I think starting out with something like this would help:
|
@RyanCavanaugh Any case where a structural check is done (i.e. not directly comparing two instances of the same generic type alias) |
This kind of language, and the similar implication throughout, is what worries me. The whole reason we're here is that you 1) started with an unobserved type parameter, 2) got something that appeared to be working by marking it with a VA, and then 3) got surprised when a structural inference didn't capture the behavior of the VA'd type parameter. We need to be encouraging people to not do step 2, otherwise they'll come here at step 3 and say that they were just following instructions and that the behavior at step 3 is a "bug" when it's really an incorrect use of a VA. |
looks at original documentation for |
This issue has been marked as "Not a Defect" and has seen no recent activity. It has been automatically closed for house-keeping purposes. |
π Search Terms
Variance Annotations, infer, extends, type
π Version & Regression Information
β― Playground Link
https://www.typescriptlang.org/play?#code/JYOwLgpgTgZghgYwgAgEJTiAJgHlMgewFcxkAVAPmQG8BfAKDAE8AHFAJQgGciAbUgLzIAFNSIgoEXnEhYAYsClYAXMhBEAtgCNotZADI0GbDi5gooAOYUAlMggAPSNi5HMuUDGjkqAfnLIqiAQAG7QANxAA
π» Code
π Actual behavior
Result
isunknown
This is incorrect since the inferred type parameter is "string" not "unknown". Additionally, "unknown" is not referred to anywhere in this code, so it's very strange for it to get inferred.
π Expected behavior
Result
isstring
since the type extendsBrand<string>
.This is how the example behaves if the "unrelatedField" is removed. Since that field is unrelated to the extends clause, it should have no impact on the inferred type. This is further evidence that "string" is the right result, but this case is messing that up somehow.
Additional information about the issue
Another workaround for this is to explicitly reference the type parameter in dummy fields. This is the pattern taken in my TypeCheck library to enable it to control variance before TypeScript 4.7. I found this issue when updating usages of that library to use the built-in variance annotation feature, and it broke some usages of infer.
The text was updated successfully, but these errors were encountered: