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

The signature of certain functions do not match the signature of the implementation #55010

Closed
NilsIrl opened this issue Jul 13, 2023 · 5 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@NilsIrl
Copy link
Contributor

NilsIrl commented Jul 13, 2023

Bug Report

Functions such as isArrayType() and isTupleType have a different signature in src/compiler/types.ts and in the implementation in src/checker/types.ts.

In src/compiler/types.ts:

isArrayType(type: Type): boolean;
isTupleType(type: Type): boolean;

In src/compiler/checker.ts:

isTupleType(type: Type): type is TupleTypeReference;
isArrayType(type: Type): type is TypeReference;

This means that the post-condition of these functions cannot be made use of in external code.

🔎 Search Terms

  • isArrayType, isTupleType
  • Signature, TypeChecker

🕗 Version & Regression Information

This was only an internal issue until these functions were exposed in #52467

💻 Code

if (typeChecker.isArrayType(type)) {
    // complains that type is not TypeReference
    typeChecker.getTypeArguments(type);
}

🙁 Actual behavior

TS2345 [ERROR]: Argument of type 'Type' is not assignable to parameter of type 'TypeReference'.
  Type 'Type' is missing the following properties from type 'TypeReference': target, objectFlags
                const typeArgument = checker.getTypeArguments(type);

🙂 Expected behavior

The code example given compiles without issues.

PS: I have a PR ready to be submitted to fix it for these particular functions

@RyanCavanaugh
Copy link
Member

The internal signature is wrong, dangerous, and convenient, because they make negative implications that aren't correct (a thing that is not an ArrayType is not necessarily not a TypeReference). The external signature is correct, safe, and less-convenient.

We think this is a good trade-off for us, but not for people who aren't as familiar with TS as people who are working on TS itself.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jul 13, 2023
@NilsIrl
Copy link
Contributor Author

NilsIrl commented Jul 14, 2023

Could adding an ArrayType which extends TypeReference fix the problem?

@fatcerberus
Copy link

@NilsIrl That won’t fix anything. The issue is that, if you say the return type is type is TypeReference and the function returns false, TS then assumes that type is not a TypeReference and narrows accordingly. Which is wrong and a potential footgun because it might still be a TypeReference even if it’s not an ArrayType.

This is the same reason functions like Number.isFinite() don’t return x is number. See #15048.

@NilsIrl
Copy link
Contributor Author

NilsIrl commented Jul 14, 2023

The idea would then be to then the signature to this:

interface ArrayType extends TypeReference {}

isArrayType(type: Type): type is ArrayType;

Which means that if the function then returns false, it doesn't mean type is not a TypeReference

@typescript-bot
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

4 participants