Merge pull request microsoft#5709 from Microsoft/fixPreferConstRule

mhegazy · mhegazy · commit 7389922eb659 · 2015-11-18T08:54:24.000-08:00
Fix prefer const rule
diff --git a/scripts/tslint/preferConstRule.ts b/scripts/tslint/preferConstRule.ts
@@ -85,12 +85,12 @@ class PreferConstWalker extends Lint.RuleWalker {
 
     visitBinaryExpression(node: ts.BinaryExpression) {
         if (isAssignmentOperator(node.operatorToken.kind)) {
-            this.visitLHSExpressions(node.left);
+            this.visitLeftHandSideExpression(node.left);
         }
         super.visitBinaryExpression(node);
     }
 
-    private visitLHSExpressions(node: ts.Expression) {
+    private visitLeftHandSideExpression(node: ts.Expression) {
         while (node.kind === ts.SyntaxKind.ParenthesizedExpression) {
             node = (node as ts.ParenthesizedExpression).expression;
         }
@@ -106,18 +106,23 @@ class PreferConstWalker extends Lint.RuleWalker {
         if (node.kind === ts.SyntaxKind.ObjectLiteralExpression) {
             const pattern = node as ts.ObjectLiteralExpression;
             for (const element of pattern.properties) {
-                if (element.name.kind === ts.SyntaxKind.Identifier) {
-                    this.markAssignment(element.name as ts.Identifier);
+                const kind = element.kind;
+
+                if (kind === ts.SyntaxKind.ShorthandPropertyAssignment) {
+                    this.markAssignment((element as ts.ShorthandPropertyAssignment).name);
+                }
+                else if (kind === ts.SyntaxKind.PropertyAssignment) {
+                    this.visitLeftHandSideExpression((element as ts.PropertyAssignment).initializer);
                 }
-                else if (isBindingPattern(element.name)) {
-                    this.visitBindingPatternIdentifiers(element.name as ts.BindingPattern);
+                else {
+                    // Should we throw an exception?
                 }
             }
         }
         else if (node.kind === ts.SyntaxKind.ArrayLiteralExpression) {
             const pattern = node as ts.ArrayLiteralExpression;
             for (const element of pattern.elements) {
-                this.visitLHSExpressions(element);
+                this.visitLeftHandSideExpression(element);
             }
         }
     }
@@ -145,7 +150,7 @@ class PreferConstWalker extends Lint.RuleWalker {
 
     private visitAnyUnaryExpression(node: ts.PrefixUnaryExpression | ts.PostfixUnaryExpression) {
         if (node.operator === ts.SyntaxKind.PlusPlusToken || node.operator === ts.SyntaxKind.MinusMinusToken) {
-            this.visitLHSExpressions(node.operand);
+            this.visitLeftHandSideExpression(node.operand);
         }
     }
 
@@ -211,12 +216,12 @@ class PreferConstWalker extends Lint.RuleWalker {
         }
     }
 
-    private collectNameIdentifiers(value: ts.VariableDeclaration, node: ts.Identifier | ts.BindingPattern, table: ts.Map<DeclarationUsages>) {
+    private collectNameIdentifiers(declaration: ts.VariableDeclaration, node: ts.Identifier | ts.BindingPattern, table: ts.Map<DeclarationUsages>) {
         if (node.kind === ts.SyntaxKind.Identifier) {
-            table[(node as ts.Identifier).text] = {declaration: value, usages: 0};
+            table[(node as ts.Identifier).text] = { declaration, usages: 0 };
         }
         else {
-            this.collectBindingPatternIdentifiers(value, node as ts.BindingPattern, table);
+            this.collectBindingPatternIdentifiers(declaration, node as ts.BindingPattern, table);
         }
     }
 

Original file line number	Diff line number	Diff line change
`@@ -85,12 +85,12 @@ class PreferConstWalker extends Lint.RuleWalker {`
`85`	`85`
`86`	`86`	`visitBinaryExpression(node: ts.BinaryExpression) {`
`87`	`87`	`if (isAssignmentOperator(node.operatorToken.kind)) {`
`88`		`- this.visitLHSExpressions(node.left);`
	`88`	`+ this.visitLeftHandSideExpression(node.left);`
`89`	`89`	`}`
`90`	`90`	`super.visitBinaryExpression(node);`
`91`	`91`	`}`
`92`	`92`
`93`		`- private visitLHSExpressions(node: ts.Expression) {`
	`93`	`+ private visitLeftHandSideExpression(node: ts.Expression) {`
`94`	`94`	`while (node.kind === ts.SyntaxKind.ParenthesizedExpression) {`
`95`	`95`	`node = (node as ts.ParenthesizedExpression).expression;`
`96`	`96`	`}`
`@@ -106,18 +106,23 @@ class PreferConstWalker extends Lint.RuleWalker {`
`106`	`106`	`if (node.kind === ts.SyntaxKind.ObjectLiteralExpression) {`
`107`	`107`	`const pattern = node as ts.ObjectLiteralExpression;`
`108`	`108`	`for (const element of pattern.properties) {`
`109`		`- if (element.name.kind === ts.SyntaxKind.Identifier) {`
`110`		`- this.markAssignment(element.name as ts.Identifier);`
	`109`	`+ const kind = element.kind;`
	`110`	`+`
	`111`	`+ if (kind === ts.SyntaxKind.ShorthandPropertyAssignment) {`
	`112`	`+ this.markAssignment((element as ts.ShorthandPropertyAssignment).name);`
	`113`	`+ }`
	`114`	`+ else if (kind === ts.SyntaxKind.PropertyAssignment) {`
	`115`	`+ this.visitLeftHandSideExpression((element as ts.PropertyAssignment).initializer);`
`111`	`116`	`}`
`112`		`- else if (isBindingPattern(element.name)) {`
`113`		`- this.visitBindingPatternIdentifiers(element.name as ts.BindingPattern);`
	`117`	`+ else {`
	`118`	`+ // Should we throw an exception?`
`114`	`119`	`}`
`115`	`120`	`}`
`116`	`121`	`}`
`117`	`122`	`else if (node.kind === ts.SyntaxKind.ArrayLiteralExpression) {`
`118`	`123`	`const pattern = node as ts.ArrayLiteralExpression;`
`119`	`124`	`for (const element of pattern.elements) {`
`120`		`- this.visitLHSExpressions(element);`
	`125`	`+ this.visitLeftHandSideExpression(element);`
`121`	`126`	`}`
`122`	`127`	`}`
`123`	`128`	`}`
`@@ -145,7 +150,7 @@ class PreferConstWalker extends Lint.RuleWalker {`
`145`	`150`
`146`	`151`	`private visitAnyUnaryExpression(node: ts.PrefixUnaryExpression \| ts.PostfixUnaryExpression) {`
`147`	`152`	`if (node.operator === ts.SyntaxKind.PlusPlusToken \|\| node.operator === ts.SyntaxKind.MinusMinusToken) {`
`148`		`- this.visitLHSExpressions(node.operand);`
	`153`	`+ this.visitLeftHandSideExpression(node.operand);`
`149`	`154`	`}`
`150`	`155`	`}`
`151`	`156`
`@@ -211,12 +216,12 @@ class PreferConstWalker extends Lint.RuleWalker {`
`211`	`216`	`}`
`212`	`217`	`}`
`213`	`218`
`214`		`- private collectNameIdentifiers(value: ts.VariableDeclaration, node: ts.Identifier \| ts.BindingPattern, table: ts.Map<DeclarationUsages>) {`
	`219`	`+ private collectNameIdentifiers(declaration: ts.VariableDeclaration, node: ts.Identifier \| ts.BindingPattern, table: ts.Map<DeclarationUsages>) {`
`215`	`220`	`if (node.kind === ts.SyntaxKind.Identifier) {`
`216`		`- table[(node as ts.Identifier).text] = {declaration: value, usages: 0};`
	`221`	`+ table[(node as ts.Identifier).text] = { declaration, usages: 0 };`
`217`	`222`	`}`
`218`	`223`	`else {`
`219`		`- this.collectBindingPatternIdentifiers(value, node as ts.BindingPattern, table);`
	`224`	`+ this.collectBindingPatternIdentifiers(declaration, node as ts.BindingPattern, table);`
`220`	`225`	`}`
`221`	`226`	`}`
`222`	`227`