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.

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

Allow intersections to be used as valid types for template literal placeholders #54188

Merged
merged 3 commits into from
Aug 10, 2023
Merged

Allow intersections to be used as valid types for template literal placeholders #54188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2877ff5
Allow intersections to be used as valid types for template literal pl…
Andarist May 8, 2023
3458d37
Merge remote-tracking branch 'origin/main' into fix/intersections-as-…
Andarist Jul 28, 2023
f5f1403
add fourslash test and fix the reduction issue
Andarist Jul 28, 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
22 changes: 17 additions & 5 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16439,7 +16439,11 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function removeStringLiteralsMatchedByTemplateLiterals(types: Type[]) {
const templates = filter(types, t => !!(t.flags & TypeFlags.TemplateLiteral) && isPatternLiteralType(t)) as TemplateLiteralType[];
const templates = filter(types, t =>
!!(t.flags & TypeFlags.TemplateLiteral) &&
isPatternLiteralType(t) &&
(t as TemplateLiteralType).types.every(t => !(t.flags & TypeFlags.Intersection) || !areIntersectedTypesAvoidingPrimitiveReduction((t as IntersectionType).types))
) as TemplateLiteralType[];
if (templates.length) {
let i = types.length;
while (i > 0) {
Expand Down Expand Up @@ -16948,16 +16952,20 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return reduceLeft(types, (n, t) => n + getConstituentCount(t), 0);
}

function areIntersectedTypesAvoidingPrimitiveReduction(t1: Type, t2: Type) {
return !!(t1.flags & (TypeFlags.String | TypeFlags.Number | TypeFlags.BigInt)) && t2 === emptyTypeLiteralType;
function areIntersectedTypesAvoidingPrimitiveReduction(types: Type[], primitiveFlags = TypeFlags.String | TypeFlags.Number | TypeFlags.BigInt): boolean {
if (types.length !== 2) {
return false;
}
const [t1, t2] = types;
return !!(t1.flags & primitiveFlags) && t2 === emptyTypeLiteralType || !!(t2.flags & primitiveFlags) && t1 === emptyTypeLiteralType;
}

function getTypeFromIntersectionTypeNode(node: IntersectionTypeNode): Type {
const links = getNodeLinks(node);
if (!links.resolvedType) {
const aliasSymbol = getAliasSymbolForTypeNode(node);
const types = map(node.types, getTypeFromTypeNode);
const noSupertypeReduction = types.length === 2 && (areIntersectedTypesAvoidingPrimitiveReduction(types[0], types[1]) || areIntersectedTypesAvoidingPrimitiveReduction(types[1], types[0]));
const noSupertypeReduction = areIntersectedTypesAvoidingPrimitiveReduction(types);
links.resolvedType = getIntersectionType(types, aliasSymbol, getTypeArgumentsForAliasSymbol(aliasSymbol), noSupertypeReduction);
}
return links.resolvedType;
Expand Down Expand Up @@ -24312,12 +24320,16 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (source === target || target.flags & (TypeFlags.Any | TypeFlags.String)) {
return true;
}
if (target.flags & TypeFlags.Intersection) {
return every((target as IntersectionType).types, t => t === emptyTypeLiteralType || isValidTypeForTemplateLiteralPlaceholder(source, t));
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to have t === emptyTypeLiteralType here? Couldn't source not be assignable to {}?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

emptyTypeLiteralType isn't a valid type for a template literal placeholder. Without this, we run into problems with what this PR is fixing:

test failures 
diff --git a/tests/baselines/reference/templateLiteralTypesPatterns.errors.txt b/tests/baselines/reference/templateLiteralTypesPatterns.errors.txt
index cd82afc839..7bd15a6f82 100644
--- a/tests/baselines/reference/templateLiteralTypesPatterns.errors.txt
+++ b/tests/baselines/reference/templateLiteralTypesPatterns.errors.txt
@@ -55,10 +55,12 @@ templateLiteralTypesPatterns.ts(129,9): error TS2345: Argument of type '"1.1e-10
 templateLiteralTypesPatterns.ts(140,1): error TS2322: Type '`a${string}`' is not assignable to type '`a${number}`'.
 templateLiteralTypesPatterns.ts(141,1): error TS2322: Type '"bno"' is not assignable to type '`a${any}`'.
 templateLiteralTypesPatterns.ts(160,7): error TS2322: Type '"anything"' is not assignable to type '`${number} ${number}`'.
+templateLiteralTypesPatterns.ts(205,16): error TS2345: Argument of type '"testDowncast"' is not assignable to parameter of type '`${string & {}}Downcast` | "downcast" | "dataDowncast" | "editingDowncast"'.
+templateLiteralTypesPatterns.ts(207,17): error TS2345: Argument of type '"testDowncast"' is not assignable to parameter of type '"downcast" | "dataDowncast" | "editingDowncast" | `${{} & string}Downcast`'.
 templateLiteralTypesPatterns.ts(211,5): error TS2345: Argument of type '"abcTest"' is not assignable to parameter of type '`${`a${string}` & `${string}a`}Test`'.
 
 
-==== templateLiteralTypesPatterns.ts (58 errors) ====
+==== templateLiteralTypesPatterns.ts (60 errors) ====
     type RequiresLeadingSlash = `/${string}`;
     
     // ok
@@ -378,8 +380,12 @@ templateLiteralTypesPatterns.ts(211,5): error TS2345: Argument of type '"abcTest
     // repro from https://github.com/microsoft/TypeScript/issues/54177#issuecomment-1538436654
     function conversionTest(groupName: | "downcast" | "dataDowncast" | "editingDowncast" | `${string & {}}Downcast`) {}
     conversionTest("testDowncast");
+                   ~~~~~~~~~~~~~~
+!!! error TS2345: Argument of type '"testDowncast"' is not assignable to parameter of type '`${string & {}}Downcast` | "downcast" | "dataDowncast" | "editingDowncast"'.
     function conversionTest2(groupName: | "downcast" | "dataDowncast" | "editingDowncast" | `${{} & string}Downcast`) {}
     conversionTest2("testDowncast");
+                    ~~~~~~~~~~~~~~
+!!! error TS2345: Argument of type '"testDowncast"' is not assignable to parameter of type '"downcast" | "dataDowncast" | "editingDowncast" | `${{} & string}Downcast`'.
     
     function foo(str: `${`a${string}` & `${string}a`}Test`) {}
     foo("abaTest"); // ok

This was really meant to be a "faster"/more concise version of:

        if (target.flags & TypeFlags.Intersection) {
            if (areIntersectedTypesAvoidingPrimitiveReduction((target as IntersectionType).types)) {
                const primitive = (target as IntersectionType).types[0] === emptyTypeLiteralType ? (target as IntersectionType).types[1] : (target as IntersectionType).types[0];
                return isValidTypeForTemplateLiteralPlaceholder(source, primitive);
            }
            return every((target as IntersectionType).types, t => isValidTypeForTemplateLiteralPlaceholder(source, t));
        }

Couldn't source not be assignable to {}?

This is a good question. I've tried to end up with such a source and I couldn't figure out a way to hit this. The whole existing test suite also doesn't hit such a case. I won't swear that it's impossible though so maybe it's better to use this alternative version that I posted above?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I looked at the code some more and I think this is actually ok (but like you said, I can't prove it's impossible).

}
if (source.flags & TypeFlags.StringLiteral) {
const value = (source as StringLiteralType).value;
return !!(target.flags & TypeFlags.Number && isValidNumberString(value, /*roundTripOnly*/ false) ||
target.flags & TypeFlags.BigInt && isValidBigIntString(value, /*roundTripOnly*/ false) ||
target.flags & (TypeFlags.BooleanLiteral | TypeFlags.Nullable) && value === (target as IntrinsicType).intrinsicName ||
target.flags & TypeFlags.StringMapping && isMemberOfStringMapping(getStringLiteralType(value), target));
target.flags & TypeFlags.StringMapping && isMemberOfStringMapping(getStringLiteralType(value), target) ||
target.flags & TypeFlags.TemplateLiteral && isTypeMatchedByTemplateLiteralType(source, target as TemplateLiteralType));
}
if (source.flags & TypeFlags.TemplateLiteral) {
const texts = (source as TemplateLiteralType).texts;
Expand Down
16 changes: 14 additions & 2 deletions tests/baselines/reference/templateLiteralTypesPatterns.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ templateLiteralTypesPatterns.ts(129,9): error TS2345: Argument of type '"1.1e-10
templateLiteralTypesPatterns.ts(140,1): error TS2322: Type '`a${string}`' is not assignable to type '`a${number}`'.
templateLiteralTypesPatterns.ts(141,1): error TS2322: Type '"bno"' is not assignable to type '`a${any}`'.
templateLiteralTypesPatterns.ts(160,7): error TS2322: Type '"anything"' is not assignable to type '`${number} ${number}`'.
templateLiteralTypesPatterns.ts(211,5): error TS2345: Argument of type '"abcTest"' is not assignable to parameter of type '`${`a${string}` & `${string}a`}Test`'.


==== templateLiteralTypesPatterns.ts (57 errors) ====
==== templateLiteralTypesPatterns.ts (58 errors) ====
type RequiresLeadingSlash = `/${string}`;

// ok
Expand Down Expand Up @@ -373,4 +374,15 @@ templateLiteralTypesPatterns.ts(160,7): error TS2322: Type '"anything"' is not a
this.get(id!);
}
}


// repro from https://github.com/microsoft/TypeScript/issues/54177#issuecomment-1538436654
function conversionTest(groupName: | "downcast" | "dataDowncast" | "editingDowncast" | `${string & {}}Downcast`) {}
conversionTest("testDowncast");
function conversionTest2(groupName: | "downcast" | "dataDowncast" | "editingDowncast" | `${{} & string}Downcast`) {}
conversionTest2("testDowncast");

function foo(str: `${`a${string}` & `${string}a`}Test`) {}
foo("abaTest"); // ok
foo("abcTest"); // error
~~~~~~~~~
!!! error TS2345: Argument of type '"abcTest"' is not assignable to parameter of type '`${`a${string}` & `${string}a`}Test`'.
19 changes: 18 additions & 1 deletion tests/baselines/reference/templateLiteralTypesPatterns.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,16 @@ export abstract class BB {
this.get(id!);
}
}


// repro from https://github.com/microsoft/TypeScript/issues/54177#issuecomment-1538436654
function conversionTest(groupName: | "downcast" | "dataDowncast" | "editingDowncast" | `${string & {}}Downcast`) {}
conversionTest("testDowncast");
function conversionTest2(groupName: | "downcast" | "dataDowncast" | "editingDowncast" | `${{} & string}Downcast`) {}
conversionTest2("testDowncast");

function foo(str: `${`a${string}` & `${string}a`}Test`) {}
foo("abaTest"); // ok
foo("abcTest"); // error

//// [templateLiteralTypesPatterns.js]
"use strict";
Expand Down Expand Up @@ -353,3 +362,11 @@ var BB = /** @class */ (function () {
return BB;
}());
exports.BB = BB;
// repro from https://github.com/microsoft/TypeScript/issues/54177#issuecomment-1538436654
function conversionTest(groupName) { }
conversionTest("testDowncast");
function conversionTest2(groupName) { }
conversionTest2("testDowncast");
function foo(str) { }
foo("abaTest"); // ok
foo("abcTest"); // error
25 changes: 25 additions & 0 deletions tests/baselines/reference/templateLiteralTypesPatterns.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,3 +487,28 @@ export abstract class BB {
}
}

// repro from https://github.com/microsoft/TypeScript/issues/54177#issuecomment-1538436654
function conversionTest(groupName: | "downcast" | "dataDowncast" | "editingDowncast" | `${string & {}}Downcast`) {}
>conversionTest : Symbol(conversionTest, Decl(templateLiteralTypesPatterns.ts, 200, 1))
>groupName : Symbol(groupName, Decl(templateLiteralTypesPatterns.ts, 203, 24))

conversionTest("testDowncast");
>conversionTest : Symbol(conversionTest, Decl(templateLiteralTypesPatterns.ts, 200, 1))

function conversionTest2(groupName: | "downcast" | "dataDowncast" | "editingDowncast" | `${{} & string}Downcast`) {}
>conversionTest2 : Symbol(conversionTest2, Decl(templateLiteralTypesPatterns.ts, 204, 31))
>groupName : Symbol(groupName, Decl(templateLiteralTypesPatterns.ts, 205, 25))

conversionTest2("testDowncast");
>conversionTest2 : Symbol(conversionTest2, Decl(templateLiteralTypesPatterns.ts, 204, 31))

function foo(str: `${`a${string}` & `${string}a`}Test`) {}
>foo : Symbol(foo, Decl(templateLiteralTypesPatterns.ts, 206, 32))
>str : Symbol(str, Decl(templateLiteralTypesPatterns.ts, 208, 13))

foo("abaTest"); // ok
>foo : Symbol(foo, Decl(templateLiteralTypesPatterns.ts, 206, 32))

foo("abcTest"); // error
>foo : Symbol(foo, Decl(templateLiteralTypesPatterns.ts, 206, 32))

33 changes: 33 additions & 0 deletions tests/baselines/reference/templateLiteralTypesPatterns.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,3 +635,36 @@ export abstract class BB {
}
}

// repro from https://github.com/microsoft/TypeScript/issues/54177#issuecomment-1538436654
function conversionTest(groupName: | "downcast" | "dataDowncast" | "editingDowncast" | `${string & {}}Downcast`) {}
>conversionTest : (groupName: "downcast" | "dataDowncast" | "editingDowncast" | `${string & {}}Downcast`) => void
>groupName : `${string & {}}Downcast` | "downcast" | "dataDowncast" | "editingDowncast"

conversionTest("testDowncast");
>conversionTest("testDowncast") : void
>conversionTest : (groupName: `${string & {}}Downcast` | "downcast" | "dataDowncast" | "editingDowncast") => void
>"testDowncast" : "testDowncast"

function conversionTest2(groupName: | "downcast" | "dataDowncast" | "editingDowncast" | `${{} & string}Downcast`) {}
>conversionTest2 : (groupName: "downcast" | "dataDowncast" | "editingDowncast" | `${{} & string}Downcast`) => void
>groupName : "downcast" | "dataDowncast" | "editingDowncast" | `${{} & string}Downcast`

conversionTest2("testDowncast");
>conversionTest2("testDowncast") : void
>conversionTest2 : (groupName: "downcast" | "dataDowncast" | "editingDowncast" | `${{} & string}Downcast`) => void
>"testDowncast" : "testDowncast"

function foo(str: `${`a${string}` & `${string}a`}Test`) {}
>foo : (str: `${`a${string}` & `${string}a`}Test`) => void
>str : `${`a${string}` & `${string}a`}Test`

foo("abaTest"); // ok
>foo("abaTest") : void
>foo : (str: `${`a${string}` & `${string}a`}Test`) => void
>"abaTest" : "abaTest"

foo("abcTest"); // error
>foo("abcTest") : void
>foo : (str: `${`a${string}` & `${string}a`}Test`) => void
>"abcTest" : "abcTest"

10 changes: 10 additions & 0 deletions tests/cases/conformance/types/literal/templateLiteralTypesPatterns.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,3 +200,13 @@ export abstract class BB {
this.get(id!);
}
}

// repro from https://github.com/microsoft/TypeScript/issues/54177#issuecomment-1538436654
function conversionTest(groupName: | "downcast" | "dataDowncast" | "editingDowncast" | `${string & {}}Downcast`) {}
conversionTest("testDowncast");
function conversionTest2(groupName: | "downcast" | "dataDowncast" | "editingDowncast" | `${{} & string}Downcast`) {}
conversionTest2("testDowncast");

function foo(str: `${`a${string}` & `${string}a`}Test`) {}
foo("abaTest"); // ok
foo("abcTest"); // error
6 changes: 6 additions & 0 deletions tests/cases/fourslash/stringLiteralCompletionsForOpenEndedTemplateLiteralType.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/// <reference path="fourslash.ts" />

//// function conversionTest(groupName: | "downcast" | "dataDowncast" | "editingDowncast" | `${string & {}}Downcast`) {}
//// conversionTest("/**/");

verify.completions({ marker: "", exact: ["downcast", "dataDowncast", "editingDowncast"] });