Skip to content

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

Closed
CraigMacomber opened this issue Sep 29, 2023 · 24 comments
Closed

Variance Annotations do not impact "infer" correctly #55920

CraigMacomber opened this issue Sep 29, 2023 · 24 comments
Assignees
Labels
Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros Not a Defect This behavior is one of several equally-correct options

Comments

@CraigMacomber
Copy link

πŸ”Ž Search Terms

Variance Annotations, infer, extends, type

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried (4.7 +), and I reviewed the FAQ for entries about "bugs-that-arent-bugs"

⏯ Playground Link

https://www.typescriptlang.org/play?#code/JYOwLgpgTgZghgYwgAgEJTiAJgHlMgewFcxkAVAPmQG8BfAKDAE8AHFAJQgGciAbUgLzIAFNSIgoEXnEhYAYsClYAXMhBEAtgCNotZADI0GbDi5gooAOYUAlMggAPSNi5HMuUDGjkqAfnLIqiAQAG7QANxAA

πŸ’» Code

interface Brand<in out T> {}
type Result = ({unrelatedField: number} & Brand<string>) extends Brand<infer T> ? T : never;

πŸ™ Actual behavior

Result is unknown

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 is string since the type extends Brand<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.

@MartinJohns
Copy link
Contributor

MartinJohns commented Sep 29, 2023

https://github.com/microsoft/TypeScript/wiki/FAQ#why-doesnt-type-inference-work-on-this-interface-interface-foot--

Your type Brand is empty, there's no member to infer T from. You shouldn't have empty types or unused type arguments.

Duplicate of #40796.

@fatcerberus
Copy link

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.

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.

@DanielRosenwasser DanielRosenwasser added the Not a Defect This behavior is one of several equally-correct options label Sep 29, 2023
@fatcerberus
Copy link

variance annotations only affect relations between two direct instantiations of the same generic

See #53798 and #48240 (comment)

@DanielRosenwasser
Copy link
Member

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".

See this example:

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

@typescript-bot typescript-bot added the Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros label Sep 29, 2023
@CraigMacomber
Copy link
Author

CraigMacomber commented Sep 29, 2023

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;

playground

Based on this it seems like the issue is with "extends", and not the infer part.

@CraigMacomber
Copy link
Author

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.

@fatcerberus
Copy link

fatcerberus commented Sep 30, 2023

I'm surprised this isn't already documented, given Ryan's comment at #48240 (comment):

We'll be putting this in <h1><blink>....</blink></h1> tags in the blog post and docs: you must write the same variance that would normally be measured. This is not a mechanism for changing the variance of a generic type.

Which I agree with, it really does need to be documented, quite clearly, that in/out annotations only exist as an optimization mechanism for cases where the compiler's automatic variance measurement is a performance issue, and if you lie to the compiler about what it already knows, things will likely go sideways.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 30, 2023

πŸ‘‹ 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.


Comment by @DanielRosenwasser

⚠️ Assertions:

  • let x: number
  • let y: number
  • let z: unknown

Historical Information
Version Reproduction Outputs
4.8.2, 4.9.3, 5.0.2, 5.1.3, 5.2.2

⚠️ Assertions:

  • let x: number
  • let y: number
  • let z: unknown

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Oct 2, 2023

"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.

@RyanCavanaugh
Copy link
Member

Draft

Variance Annotations

Variance annotations are not common, and you should feel free to skip this section

TypeScript automatically infers the variance of generic types using its normal type comparison algorithm.
In extremely rare cases, this inference process can produce results that are incorrect, or might take longer to measure than is ideal.
If that happens, you can use a variance annotation to either correct or speed up the process.

You can optionally annotate type parameters of some types with the variance annotations in (contravariant), out (covariant), or in out (invariant):

// 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.
You should always write the same variance that would be structurally measured by TypeScript (modulo limitations).
If you're not sure what the correct variance of a type is, then it's not a good situation to write a variance annotation.

Do not suppress errors about incorrect variance errors with ts-ignore or similar constructs.
This will lead to surprising or incorrect results elsewhere.
Trying to change the variance of a type parameter using a variance annotation will not succeed, because this is not what they are designed for.

Note that variance annotations do not change the actual variance of a type parameter as it occurs in an instantiated type.
Trying to change the variance of a particular position (i.e. making a function argument covariant) will not succeed.

Variance annotations are only used during nominal inference between two types, not structural inference.
This means that writing an incorrect variance annotation will generate surprising or wrong results in many cases.
Again, to produce consistent and correct results, you must write a variance annotation that would match the structurally-measured variance.
Variance annotations which don't match the structural variance will produce surprising results in many cases.

@RyanCavanaugh RyanCavanaugh self-assigned this Oct 2, 2023
@CraigMacomber
Copy link
Author

CraigMacomber commented Oct 2, 2023

You should always write the same variance that would be structurally measured by TypeScript (modulo limitations).

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:

  1. To give me errors/warnings if the TypeScript won't/can't make my type have the desired variance. (My assumption is that they worked equivalent to adding an invisible field of the proper type to cause the variance, and would compile error if this didn't result in the variance requested due to other fields being more restrictive" apparently I was wrong?).
  2. Communicate the variance to other developers for maintainability and API usability purposes.
  3. Give errors to users of the types if they try to use them in a way that disagrees with the actual desired variance.

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).

nominal inference between two types, not structural inference.

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?)

@CraigMacomber
Copy link
Author

"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.

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.

@RyanCavanaugh
Copy link
Member

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.

@CraigMacomber
Copy link
Author

...the writeup...

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.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Oct 2, 2023

As far as I'm aware those three things compose the official docs for variance

Variance is a computer science concept; the docs are not intended to replace general knowledge. "the writeup" refers to the blurb I posted above

@CraigMacomber
Copy link
Author

CraigMacomber commented Oct 2, 2023

As far as I'm aware those three things compose the official docs for variance

Variance is a computer science concept; the docs are not intended to replace general knowledge

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 :)

@RyanCavanaugh
Copy link
Member

v2 Handbook writeup inside the fold

## Variance Annotations (`in` and `out`)

Variance annotations are not common, and you should feel free to skip this section. They are not intended for everyday coding, and are documented here for reference only.

TypeScript automatically infers the variance of generic types using its normal type comparison algorithm.
In extremely rare cases (usually involving circularity), this inference process can produce results that are incorrect, or might take longer to measure than is ideal.
If that happens, you can use a variance annotation to either correct or speed up the process.

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.
If you're not in a situation where an identifiably wrong variance inference has occurred, and aren't trying to improve typechecking performance in a complex situation, it's best to avoid writing variance annotations.
You should have high confidence that you are addressing a specific problem, and have computed the correct variance yourself, before writing a variance annotation.

Again, variance annotations are not intended for everday use.
The only time a variance annotation should appear in your code is to fix very specific and rare problems that most people will not ever encounter.
They are not intended as a means of documentation, a way to change the behavior of types, or as additional validation.

You can optionally annotate type parameters of some types with the variance annotations in (contravariant), out (covariant), or in out (invariant).

// 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.
An example of an incorrect variance annotation is:

// 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 ProducerWrong<T> will produce different results from structural inferences or comparisons.
You should always write the exact same variance that would be structurally measured by TypeScript (modulo limitations).
If you're not confident what the correct variance of a type is, then it's not a good situation to write a variance annotation.

Do not suppress errors about incorrect variance errors with ts-ignore or similar constructs.
This will lead to surprising or incorrect results elsewhere.
Trying to change the variance of a type parameter using a variance annotation will not succeed, because this is not what they are designed for.

Note that variance annotations will not change the actual variance of a type parameter as it occurs in an instantiated type.
Trying to change the variance of a particular position (i.e. making a function argument covariant) will not succeed:

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.
Whether any particular type operation is done nominally or structurally is implementation-defined behavior that is not trivial to predict.
This means that writing an incorrect variance annotation will generate surprising or wrong results in many cases.
Again, to produce consistent and correct results, you must write a variance annotation that would match the structurally-measured variance.

@fatcerberus
Copy link

fatcerberus commented Oct 2, 2023

If that happens, you can use a variance annotation to either correct or speed up the process.

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.

@RyanCavanaugh
Copy link
Member

if variance measurement produces the wrong type relation, an explicit annotation will only correct it in a narrow subset of cases

Can you elaborate (maybe with an example) ?

@CraigMacomber
Copy link
Author

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:

Variance annotations can be added to type aliases for object, function, constructor, and mapped types.
Adding a variance annotation is like adding a type annotation: It replaces a compiler inferred value with an explicit one.
Just like with types, this has four main effects:

  1. if the provided variance is incompatible with what was inferred, a compile error will be produced.
  2. if the provided variance is more constraining for users of the type (for example specifying (in out when only out was inferred), some usages of the type (ones which do not treat it structurally) will give compile errors if they violate this more strict variance.
  3. it can help when the compiler is unable to do inference efficiently or accurately including some complex or recursive types.
  4. the desired semantics are explicitly captured in the code for both readers/users of the code and future maintainers of it (who may benefit from the first three aspects).

@fatcerberus
Copy link

@RyanCavanaugh Any case where a structural check is done (i.e. not directly comparing two instances of the same generic type alias)

@RyanCavanaugh
Copy link
Member

it can help when the compiler is unable to do inference efficiently or accurately

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.

@fatcerberus
Copy link

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 baseUrl and paths saying you can use them to simplify your import specifiers πŸ˜‰

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Not a Defect" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros Not a Defect This behavior is one of several equally-correct options
Projects
None yet
Development

No branches or pull requests

6 participants