Skip to content

Make Number and String literal types immutable. #28344

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 5 commits into from

Conversation

collin5
Copy link
Contributor

@collin5 collin5 commented Nov 5, 2018

Fixes #14745

Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

You're got binary and postfixUnary expressions covered; but it looks like you're missing prefixUnary, like ++a.

@@ -21820,10 +21829,18 @@ namespace ts {

function checkBinaryLikeExpression(left: Expression, operatorToken: Node, right: Expression, checkMode?: CheckMode, errorNode?: Node): Type {
const operator = operatorToken.kind;
let leftType: Type;

const symbol = (<Identifier>left).escapedText ? getResolvedSymbol(left as Identifier) : undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Use isEntityNameExpression and not left.escapedText (since checking a.b += 1 is also desireable. Also, a test for a.b += 1 would be good.

@@ -21465,7 +21465,16 @@ namespace ts {
}

function checkPostfixUnaryExpression(node: PostfixUnaryExpression): Type {
const operandType = checkExpression(node.operand);
let operandType: Type;
const symbol = (<Identifier>node.operand).escapedText ? getResolvedSymbol(node.operand as Identifier) : undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Use isEntityNameExpression and not left.operand.escapedText (since checking a.b++ is also desirable. Also, a test for a.b++ would be good.

@collin5 collin5 force-pushed the b14745 branch 2 times, most recently from a1dd0ae to 56817c8 Compare November 6, 2018 22:08
@collin5
Copy link
Contributor Author

collin5 commented Nov 6, 2018

@weswigham Thank you! Addressed the feedback. Also added more tests for both prefix and postfix unary expressions.

@weswigham weswigham added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Nov 7, 2018
@weswigham weswigham self-assigned this Nov 7, 2018
@weswigham
Copy link
Member

We closed the original issue as Won't Fix in Feb. after further consideration, sorry 😿

@weswigham weswigham closed this Mar 12, 2019
@collin5
Copy link
Contributor Author

collin5 commented Mar 12, 2019

No worries. Thanks for the update 🙂

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants