Skip to content

Infer string literals at comparison locations#6196

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

Closed
Daniel Rosenwasser (DanielRosenwasser) wants to merge 31 commits into from
Closed

Infer string literals at comparison locations #6196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
0d2fb26
Added tests.
DanielRosenwasser Dec 21, 2015
639d9bf
Accepted baselines.
DanielRosenwasser Dec 22, 2015
13ec5d7
Infer string literal types at comparison locations.
DanielRosenwasser Dec 22, 2015
e109b17
Accepted baselines.
DanielRosenwasser Dec 22, 2015
de9789a
Removed ad-hoc string-like checking for type assertions, switch/cases…
DanielRosenwasser Dec 22, 2015
d0a8573
Accepted baselines.
DanielRosenwasser Dec 22, 2015
e452955
Amended tests.
DanielRosenwasser Dec 22, 2015
ced65ac
Accepted baselines.
DanielRosenwasser Dec 22, 2015
881b52d
Separated logic out to 'shouldAcquireLiteralType'.
DanielRosenwasser Dec 22, 2015
8365410
Amended comment.
DanielRosenwasser Dec 22, 2015
58580ed
Rewrote loop, amended comment.
DanielRosenwasser Dec 22, 2015
069ff33
Add conditional expressions, be more conservative in checks for '||'.
DanielRosenwasser Dec 23, 2015
2874156
Added more to test.
DanielRosenwasser Dec 23, 2015
e2ddb29
Accepted baselines.
DanielRosenwasser Dec 23, 2015
f7e9135
Merge branch 'master' into literalTypeLocations
DanielRosenwasser Jan 5, 2016
e6bd7ad
Added test.
DanielRosenwasser Jan 5, 2016
16fe01b
Accepted baselines.
DanielRosenwasser Jan 5, 2016
bc34ebb
Added RHS operands of '&&' and ',' as match locations.
DanielRosenwasser Jan 5, 2016
4eced90
Accepted baselines.
DanielRosenwasser Jan 5, 2016
01cc2f1
Added tests for type assertions in match locations.
DanielRosenwasser Jan 8, 2016
259a3cf
Accepted baselines.
DanielRosenwasser Jan 8, 2016
cc2ab55
Factored out operand detection logic and made it recursive.
DanielRosenwasser Jan 8, 2016
5b9e5d1
Included type assertions in operand detection logic.
DanielRosenwasser Jan 8, 2016
fdd7fde
Accepted baselines.
DanielRosenwasser Jan 8, 2016
09d1762
Fix lint issues.
DanielRosenwasser Jan 8, 2016
dec70f1
Added scenario to tests.
DanielRosenwasser Jan 8, 2016
1dd43fa
Accepted baselines.
DanielRosenwasser Jan 8, 2016
ab25584
Merge branch 'master' into literalTypeLocations
DanielRosenwasser Jan 8, 2016
9673152
Merge branch 'master' into literalTypeLocations
DanielRosenwasser Apr 5, 2016
6b8bac3
Accepted other baselines.
DanielRosenwasser Apr 5, 2016
740792c
Merge branch 'master' into literalTypeLocations
DanielRosenwasser Jun 10, 2016
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
78 changes: 75 additions & 3 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -8820,11 +8820,13 @@ namespace ts {
function getContextualTypeForBinaryOperand(node: Expression): Type {
const binaryExpression = <BinaryExpression>node.parent;
const operator = binaryExpression.operatorToken.kind;
if (operator >= SyntaxKind.FirstAssignment && operator <= SyntaxKind.LastAssignment) {

if (SyntaxKind.FirstAssignment <= operator && operator <= SyntaxKind.LastAssignment) {

@ahejlsbergAnders Hejlsberg (ahejlsberg)Jan 7, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Purely stylistic, but everywhere else in the compiler we always put the constant on the right hand side (as the code was originally written). I'd prefer to keep things consistent.

// In an assignment expression, the right operand is contextually typed by the type of the left operand.
if (node === binaryExpression.right) {
return checkExpression(binaryExpression.left);
}
return undefined;

@JsonFreemanJason Freeman (JsonFreeman)Dec 22, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably don't need this

}
else if (operator === SyntaxKind.BarBarToken) {
// When an || expression has a contextual type, the operands are contextually typed by that type. When an ||
Expand DownExpand Up@@ -8991,6 +8993,7 @@ namespace ts {
* @returns the contextual type of an expression.
*/
function getContextualType(node: Expression): Type {

if (isInsideWithStatementBody(node)) {
// We cannot answer semantic questions within a with block, do not proceed any further
return undefined;
Expand DownExpand Up@@ -9039,6 +9042,75 @@ namespace ts {
return undefined;
}

function shouldAcquireLiteralType(literalNode: LiteralExpression) {
if (isEqualityComparisonOperand(literalNode)) {
return true;
}

@JsonFreemanJason Freeman (JsonFreeman)Dec 22, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove whitespace

@sandersnNathan Shively-Sanders (sandersn)Dec 22, 2015

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And elsewhere in this file.

@DanielRosenwasserDaniel Rosenwasser (DanielRosenwasser)Dec 22, 2015

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The whitespace here and below is intentional and it's for readability so that everything doesn't look like one big clump of code.

const contextualType = getContextualType(literalNode);
return !!contextualType && contextualTypeIsStringLiteralType(contextualType);
}

/**
* Returns true if an expression might be evaluated as part of an equality comparison.
* This includes inequality (e.g. '!==') and 'switch'/'case' equality.
*/
function isEqualityComparisonOperand(expression: Expression): boolean {
const parent = expression.parent;

switch (parent.kind) {
// The operand of a 'switch' should get a literal type.
case SyntaxKind.SwitchStatement:
return expression === (parent as SwitchStatement).expression;

// The tested expression of a 'case' clause should get a literal type.
case SyntaxKind.CaseClause:
return expression === (parent as CaseClause).expression;

case SyntaxKind.BinaryExpression:
const binaryExpr = parent as BinaryExpression;
switch (binaryExpr.operatorToken.kind) {
// Either operand of an equality/inequality comparison
// should get a literal type.
case SyntaxKind.EqualsEqualsEqualsToken:
case SyntaxKind.ExclamationEqualsEqualsToken:
case SyntaxKind.EqualsEqualsToken:
case SyntaxKind.ExclamationEqualsToken:
return true;

case SyntaxKind.AmpersandAmpersandToken:
case SyntaxKind.CommaToken:
if (expression === binaryExpr.right) {
return isEqualityComparisonOperand(binaryExpr);
}
return false;

case SyntaxKind.BarBarToken:
return isEqualityComparisonOperand(binaryExpr);
}

// No binary operators apply.
return false;

case SyntaxKind.ConditionalExpression:
const conditional = parent as ConditionalExpression;
if (expression === conditional.whenTrue || expression === conditional.whenFalse) {
return isEqualityComparisonOperand(conditional);
}
return false;

case SyntaxKind.ParenthesizedExpression:
return isEqualityComparisonOperand(parent as ParenthesizedExpression);

case SyntaxKind.TypeAssertionExpression:
case SyntaxKind.AsExpression:
return isEqualityComparisonOperand(parent as TypeAssertion);
}

return false;
}


// If the given type is an object or union type, if that type has a single signature, and if
// that signature is non-generic, return the signature. Otherwise return undefined.
function getNonGenericSignature(type: Type): Signature {
Expand DownExpand Up@@ -11472,6 +11544,7 @@ namespace ts {
const targetType = getTypeFromTypeNode(node.type);
if (produceDiagnostics && targetType !== unknownType) {
const widenedType = getWidenedType(exprType);

if (!isTypeComparableTo(targetType, widenedType)) {
checkTypeComparableTo(exprType, targetType, node, Diagnostics.Type_0_cannot_be_converted_to_type_1);
}
Expand DownExpand Up@@ -12546,8 +12619,7 @@ namespace ts {
}

function checkStringLiteralExpression(node: StringLiteral): Type {
const contextualType = getContextualType(node);
if (contextualType && contextualTypeIsStringLiteralType(contextualType)) {
if (shouldAcquireLiteralType(node)) {
return getStringLiteralTypeForText(node.text);
}

Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/TypeGuardWithEnumUnion.types
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,7 +14,7 @@ function f1(x: Color | string) {
>typeof x === "number" : boolean
>typeof x : string
>x : Color | string
>"number" : string
>"number" : "number"

var y = x;
>y : Color
Expand DownExpand Up@@ -43,7 +43,7 @@ function f2(x: Color | string | string[]) {
>typeof x === "object" : boolean
>typeof x : string
>x : Color | string | string[]
>"object" : string
>"object" : "object"

var y = x;
>y : string[]
Expand All@@ -56,7 +56,7 @@ function f2(x: Color | string | string[]) {
>typeof x === "number" : boolean
>typeof x : string
>x : string[] | Color | string
>"number" : string
>"number" : "number"

var z = x;
>z : Color
Expand All@@ -78,7 +78,7 @@ function f2(x: Color | string | string[]) {
>typeof x === "string" : boolean
>typeof x : string
>x : Color | string[] | string
>"string" : string
>"string" : "string"

var a = x;
>a : string
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/anonymousClassExpression1.types
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,5 +6,5 @@ function f() {
>typeof class {} === "function" : boolean
>typeof class {} : string
>class {} : typeof (Anonymous class)
>"function" : string
>"function" : "function"
}
4 changes: 2 additions & 2 deletions tests/baselines/reference/capturedLetConstInLoop5.types
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -74,7 +74,7 @@ function foo00(x) {
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

return;
}
Expand DownExpand Up@@ -549,7 +549,7 @@ function foo00_c(x) {
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

return;
}
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/capturedLetConstInLoop5_ES6.types
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -75,7 +75,7 @@ function foo00(x) {
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

return;
}
Expand DownExpand Up@@ -550,7 +550,7 @@ function foo00_c(x) {
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

return;
}
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/capturedLetConstInLoop6.types
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,14 +47,14 @@ for (let x in []) {
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

break;
}
if (x == "2") {
>x == "2" : boolean
>x : string
>"2" : string
>"2" : "2"

continue;
}
Expand DownExpand Up@@ -412,14 +412,14 @@ for (const x in []) {
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

break;
}
if (x == "2") {
>x == "2" : boolean
>x : string
>"2" : string
>"2" : "2"

continue;
}
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/capturedLetConstInLoop6_ES6.types
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,14 +47,14 @@ for (let x in []) {
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

break;
}
if (x == "2") {
>x == "2" : boolean
>x : string
>"2" : string
>"2" : "2"

continue;
}
Expand DownExpand Up@@ -412,14 +412,14 @@ for (const x in []) {
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

break;
}
if (x == "2") {
>x == "2" : boolean
>x : string
>"2" : string
>"2" : "2"

continue;
}
Expand Down
16 changes: 8 additions & 8 deletions tests/baselines/reference/capturedLetConstInLoop7.types
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,29 +69,29 @@ for (let x in []) {
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

break;
}
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

break l00;
>l00 : any
}
if (x == "2") {
>x == "2" : boolean
>x : string
>"2" : string
>"2" : "2"

continue;
}
if (x == "2") {
>x == "2" : boolean
>x : string
>"2" : string
>"2" : "2"

continue l00;
>l00 : any
Expand DownExpand Up@@ -623,29 +623,29 @@ for (const x in []) {
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

break;
}
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

break l00_c;
>l00_c : any
}
if (x == "2") {
>x == "2" : boolean
>x : string
>"2" : string
>"2" : "2"

continue;
}
if (x == "2") {
>x == "2" : boolean
>x : string
>"2" : string
>"2" : "2"

continue l00_c;
>l00_c : any
Expand Down
16 changes: 8 additions & 8 deletions tests/baselines/reference/capturedLetConstInLoop7_ES6.types
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -69,29 +69,29 @@ for (let x in []) {
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

break;
}
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

break l00;
>l00 : any
}
if (x == "2") {
>x == "2" : boolean
>x : string
>"2" : string
>"2" : "2"

continue;
}
if (x == "2") {
>x == "2" : boolean
>x : string
>"2" : string
>"2" : "2"

continue l00;
>l00 : any
Expand DownExpand Up@@ -623,29 +623,29 @@ for (const x in []) {
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

break;
}
if (x == "1") {
>x == "1" : boolean
>x : string
>"1" : string
>"1" : "1"

break l00_c;
>l00_c : any
}
if (x == "2") {
>x == "2" : boolean
>x : string
>"2" : string
>"2" : "2"

continue;
}
if (x == "2") {
>x == "2" : boolean
>x : string
>"2" : string
>"2" : "2"

continue l00_c;
>l00_c : any
Expand Down
Loading