Skip to content

Expose getStringLiteralType and getNumberLiteralType on the TypeChecker, plus remove /** @internal */ from several useful methods. #52473

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Mar 7, 2023
Merged

Expose getStringLiteralType and getNumberLiteralType on the TypeChecker, plus remove /** @internal */ from several useful methods. #52473

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aab7cb9
Expose getStringType, getStringLiteralType, getNumberType, getNumberL…
sstchur-msft Jan 27, 2023
59650df
Merge branch 'main' into sstchur/expose-various-helpers-on-checker
andrewbranch Feb 3, 2023
7d6158f
Update API baseline
andrewbranch Feb 3, 2023
d3cdc58
Don’t expose `fresh` parameter, add `getBigIntType`, add JSDoc warnings
andrewbranch Mar 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1701,7 +1701,9 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
createIndexInfo,
getAnyType: () => anyType,
getStringType: () => stringType,
getStringLiteralType,
getNumberType: () => numberType,
getNumberLiteralType,
createPromiseType,
createArrayType,
getElementTypeOfArrayType,
Expand Down
24 changes: 13 additions & 11 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5099,17 +5099,19 @@ export interface TypeChecker {
getBaseConstraintOfType(type: Type): Type | undefined;
getDefaultFromTypeParameter(type: Type): Type | undefined;

/** @internal */ getAnyType(): Type;
/** @internal */ getStringType(): Type;
/** @internal */ getNumberType(): Type;
/** @internal */ getBooleanType(): Type;
/** @internal */ getFalseType(fresh?: boolean): Type;
/** @internal */ getTrueType(fresh?: boolean): Type;
/** @internal */ getVoidType(): Type;
/** @internal */ getUndefinedType(): Type;
/** @internal */ getNullType(): Type;
/** @internal */ getESSymbolType(): Type;
/** @internal */ getNeverType(): Type;
getAnyType(): Type;
getStringType(): Type;
getStringLiteralType(value: string): StringLiteralType;
getNumberType(): Type;
getNumberLiteralType(value: number): NumberLiteralType;
getBooleanType(): Type;
getFalseType(fresh?: boolean): Type;
getTrueType(fresh?: boolean): Type;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rereading this again, do we want to expose the fresh parameter? Or should we maybe overload it and only expose one without parameters publicly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the fresh param do? In my personal needs using this, I'm just invoking getFalseType(). How likely is it that people using this would need/want to pass in the fresh arg?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Literal types have fresh and regular variants. When assigning from a fresh literal to a mutable location, the resulting type is widened to the base type:

const freshFalse = false;
const regularFalse: false = false;

let widenedToBoolean = freshFalse;
let stillFalse = regularFalse;

from an API perspective, this means it’s important to note that

checker.getFalseType(true) !== checker.getFalseType(false)

but that a false type will always be one of these. Within the checker, we would probably never do an equality comparison to determine if a type is false; we’d call getTypeFacts and look at those, but that’s not exported. Soooo, whether to expose it, 🤷‍♂️?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of places appear to do things like foo === falseType || foo === freshFalseType too. I guess it depends, but it might be important if you are trying to do something with these functions...

Alternatively what people want externally is "is this assignable to false" but obviously that API is a different issue so...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. So in my case, I have checker.getFalseType(false) (since I don't pass anything in). And I'm later using that, but not with ===. Rather, I'm using it with isTypeAssignableTo, because what I desire to know is if some type I have could accept the value false.

For example:

const foo: false | number = getRandomNumberOrMaybeFalse();

Later, I want to know if foo would accept false. I don't want or need to know if foo would accept boolean, only if it will accept false.

But I have to admit that I don't know which of the two: checker.getFalseType(true) or checker.getFalseType(false) I want in this case, or if it even matters?

And the whole reason I mention my use-case at all is to try to add insight that might help decide if we need to expose the freshType arg or not? I can't purport to know what others might be doing with getFalseType or if their use-cases are at all like mine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fresh/regular shouldn’t matter for assignability, so it sounds like in your case it doesn’t matter.

Noticing that a couple of our codefixes grab a type, check its flags for BooleanLiteral, and then compare to checker.getFalseType(true) and checker.getFalseType(false), I’m inclined to say it’s fine to leave it exposed. Codemods are a good example of a reasonable usage of our API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Obviously the value of my input is limited, since I don't have the same depth on knowledge about TS that you guys have, but my intuition would be to leave it exposed. As a developer, I prefer having flexibility available to me, even if I don't use it, so long as that flexibility doesn't make my life difficult. And in this case, it doesn't seem to, so I say leave it.

getVoidType(): Type;
getUndefinedType(): Type;
getNullType(): Type;
getESSymbolType(): Type;
getNeverType(): Type;
/** @internal */ getOptionalType(): Type;
/** @internal */ getUnionType(types: Type[], subtypeReduction?: UnionReduction): Type;
/** @internal */ createArrayType(elementType: Type): Type;
Expand Down
13 changes: 13 additions & 0 deletions tests/baselines/reference/api/tsserverlibrary.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6400,6 +6400,19 @@ declare namespace ts {
getApparentType(type: Type): Type;
getBaseConstraintOfType(type: Type): Type | undefined;
getDefaultFromTypeParameter(type: Type): Type | undefined;
getAnyType(): Type;
getStringType(): Type;
getStringLiteralType(value: string): StringLiteralType;
getNumberType(): Type;
getNumberLiteralType(value: number): NumberLiteralType;
getBooleanType(): Type;
getFalseType(fresh?: boolean): Type;
getTrueType(fresh?: boolean): Type;
getVoidType(): Type;
getUndefinedType(): Type;
getNullType(): Type;
getESSymbolType(): Type;
getNeverType(): Type;
getTypePredicateOfSignature(signature: Signature): TypePredicate | undefined;
/**
* Depending on the operation performed, it may be appropriate to throw away the checker
Expand Down
13 changes: 13 additions & 0 deletions tests/baselines/reference/api/typescript.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2425,6 +2425,19 @@ declare namespace ts {
getApparentType(type: Type): Type;
getBaseConstraintOfType(type: Type): Type | undefined;
getDefaultFromTypeParameter(type: Type): Type | undefined;
getAnyType(): Type;
getStringType(): Type;
getStringLiteralType(value: string): StringLiteralType;
getNumberType(): Type;
getNumberLiteralType(value: number): NumberLiteralType;
getBooleanType(): Type;
getFalseType(fresh?: boolean): Type;
getTrueType(fresh?: boolean): Type;
getVoidType(): Type;
getUndefinedType(): Type;
getNullType(): Type;
getESSymbolType(): Type;
getNeverType(): Type;
getTypePredicateOfSignature(signature: Signature): TypePredicate | undefined;
/**
* Depending on the operation performed, it may be appropriate to throw away the checker
Expand Down