New Rule: param-property-in-constructor

src/rules/noImproperConstructorParamUsageRule.ts

@@ -0,0 +1,96 @@
+/**
+ * @license
+ * Copyright 2014 Palantir Technologies, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as Lint from "../lint";
+import * as ts from "typescript";
+
+export class Rule extends Lint.Rules.AbstractRule {
+    /* tslint:disable:object-literal-sort-keys */
+    public static metadata: Lint.IRuleMetadata = {
+        ruleName: "no-improper-constructor-param-usage",
+        description: "Disallows constructor parameters to be accessed without the 'this.' prefix",
+        rationale: Lint.Utils.dedent`
+            This helps enforce consistancy.
+            When a constructor parameter is accessed without the 'this.' prefix, it is actually not acessing the same thing.
+            Example: 
+            class Foo {
+                constructor(public num: number) {
+                    this.num = 10;
+                    num = 5;  //this should be disallowed!
+                }
+            }
+            const f = new Foo();
+            alert(f.num);  // will display 10`,
+        optionsDescription: "Not configurable.",
+        options: null,
+        optionExamples: ["true"],
+        type: "functionality",
+    };
+    /* tslint:enable:object-literal-sort-keys */
+
+    public static FAILURE_STRING_FACTORY = (paramName: string) => {
+        return `The constructor parameter '${paramName}' should only be accessed with the 'this' keyword: 'this.${paramName}'`;
+    }
+
+    public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
+        const languageService = Lint.createLanguageService(sourceFile.fileName, sourceFile.getFullText());
+        return this.applyWithWalker(new NoImproperCtorParamUsageWalker(sourceFile, this.getOptions(), languageService));
+    }
+}
+
+export class NoImproperCtorParamUsageWalker extends Lint.RuleWalker {
+
+    private languageService: ts.LanguageService;
+
+    constructor(sourceFile: ts.SourceFile, options: Lint.IOptions, languageService: ts.LanguageService) {
+        super(sourceFile, options);
+        this.languageService = languageService;
+    }
+
+    public visitConstructorDeclaration(node: ts.ConstructorDeclaration) {
+        if (node.parameters && node.parameters.length > 0) {
+            const fileName = this.getSourceFile().fileName;
+            node.parameters.forEach((param: ts.ParameterDeclaration) => {
+                this.validateParam(param, fileName);
+            });
+        }
+        super.visitConstructorDeclaration(node);
+    }
+
+    private validateParam(param: ts.ParameterDeclaration, fileName: string) {
+        const highlights = this.languageService.getDocumentHighlights(fileName, param.name.getStart(), [fileName]);
+
+        if ((highlights !== null && highlights[0].highlightSpans.length > 0)) {
+            const paramName = param.name.getText();
+            highlights[0].highlightSpans.forEach((span: ts.HighlightSpan, idx: number) => {
+                // Ignore the 1st reference which is the actual parameter definition
+                if (idx !== 0 && !this.spanHasThisUsage(span, fileName)) {
+                    const msg = Rule.FAILURE_STRING_FACTORY(paramName);
+                    this.addFailure(this.createFailure(span.textSpan.start, span.textSpan.length, msg));
+                }
+            });
+        }
+    }
+
+    private spanHasThisUsage(span: ts.HighlightSpan, fileName: string) {
+        const endPos = span.textSpan.start + span.textSpan.length;
+        const nameSpanInfo = this.languageService.getNameOrDottedNameSpan(fileName, span.textSpan.start, endPos);
+
+        // If the difference between these two is 5 characters, then that account for the missing `this.`!
+        return nameSpanInfo.length - span.textSpan.length === 5;
+    }
+}

src/tsconfig.json

@@ -98,6 +98,7 @@
         "rules/noDuplicateVariableRule.ts",
         "rules/noEmptyRule.ts",
         "rules/noEvalRule.ts",
+        "rules/noImproperConstructorParamUsageRule.ts",
         "rules/noInferrableTypesRule.ts",
         "rules/noInternalModuleRule.ts",
         "rules/noInvalidThisRule.ts",

test/rules/no-improper-constructor-param-usage/test.ts.lint

@@ -0,0 +1,48 @@
+class FailingExample1 {
+    constructor(fruit: string, public num: number, private animal: string) {
+        this.num = 10;
+        num = 5;
+        ~~~ [The constructor parameter 'num' should only be accessed with the 'this' keyword: 'this.num']
+
+        animal = "aardvark";
+        ~~~~~~ [The constructor parameter 'animal' should only be accessed with the 'this' keyword: 'this.animal']
+        this.animal = "skunk";
+
+        fruit = "banana";
+        ~~~~~ [The constructor parameter 'fruit' should only be accessed with the 'this' keyword: 'this.fruit']
+        this.fruit = "persimmon";
+    }
+}
+
+class FailingExample2 {
+    constructor(thing: Object) {
+        const wow=thing.doTheThing()
+                  ~~~~~ [The constructor parameter 'thing' should only be accessed with the 'this' keyword: 'this.thing']
+        console.log(wow,thing.amazingDvdCollection);
+                        ~~~~~ [The constructor parameter 'thing' should only be accessed with the 'this' keyword: 'this.thing']
+
+        const pasta = thing.nested.makePasta();
+                      ~~~~~ [The constructor parameter 'thing' should only be accessed with the 'this' keyword: 'this.thing']
+
+        thing = {};
+        ~~~~~ [The constructor parameter 'thing' should only be accessed with the 'this' keyword: 'this.thing']
+
+        this.thing.doSomethingElse();
+    }
+}
+
+class PassingExample1 {
+    constructor(a: string, private b: string, public c: string) {
+        this.a = "aaa";
+        this.b = "bbb";
+        this.c = "ccc";
+    }
+}
+
+class PassingExample2 {
+    constructor(a: Object, private b: Object, public c: Object) {
+        this.a.doThing();
+        this.b.doOtherThing();
+        this.c.doLastThing();
+    }
+}

test/rules/no-improper-constructor-param-usage/tslint.json

@@ -0,0 +1,5 @@
+{
+  "rules": {
+    "no-improper-constructor-param-usage": true
+  }
+}

test/tsconfig.json

@@ -94,12 +94,12 @@
         "../src/rules/noDuplicateVariableRule.ts",
         "../src/rules/noEmptyRule.ts",
         "../src/rules/noEvalRule.ts",
+        "../src/rules/noImproperConstructorParamUsageRule.ts",
         "../src/rules/noInferrableTypesRule.ts",
         "../src/rules/noInternalModuleRule.ts",
         "../src/rules/noInvalidThisRule.ts",
         "../src/rules/noMergeableNamespaceRule.ts",
         "../src/rules/noNamespaceRule.ts",
-        "../src/rules/noUnusedNewRule.ts",
         "../src/rules/noNullKeywordRule.ts",
         "../src/rules/noReferenceRule.ts",
         "../src/rules/noRequireImportsRule.ts",
@@ -109,6 +109,7 @@
         "../src/rules/noTrailingWhitespaceRule.ts",
         "../src/rules/noUnreachableRule.ts",
         "../src/rules/noUnusedExpressionRule.ts",
+        "../src/rules/noUnusedNewRule.ts",
         "../src/rules/noUnusedVariableRule.ts",
         "../src/rules/noUseBeforeDeclareRule.ts",
         "../src/rules/noVarKeywordRule.ts",