Skip to content

Conversation

@ahejlsberg
Copy link
Member

Fixes #8851.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jun 8, 2016

👍 (if you don't care about compound assignment)

return getReferenceFromExpression((<ParenthesizedExpression>node).expression);
case SyntaxKind.BinaryExpression:
switch ((<BinaryExpression>node).operatorToken.kind) {
case SyntaxKind.EqualsToken:
Copy link
Member

Choose a reason for hiding this comment

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

Technically you should consider the compound assignment operators here as well (e.g. +=, -= etc)

Copy link
Member Author

Choose a reason for hiding this comment

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

Compound operators aren't included in control flow analysis in our current design, so that would be a separate issue. But as I see it they wouldn't affect the type because they both a read and a write operation (you're basically storing a value of the same type back into the variable). Did you have something in mind?

Copy link
Member

@DanielRosenwasser DanielRosenwasser Jun 8, 2016

Choose a reason for hiding this comment

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

Nothing other than odd cases like this:

var x: number | string = getNumberOrString();
x += "hello";
x; // currently has type 'string | number'

whereas in this case you get a different result

var x: number | string = getNumberOrString();
x = x + "hello";
x; // currently has type 'string'

I don't know how often something like that comes up in practice though.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 8, 2016

👍

@ahejlsberg ahejlsberg merged commit b3c5c1d into master Jun 8, 2016
@ahejlsberg ahejlsberg deleted the typeGuardNestedAssignment branch June 8, 2016 23:26
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants