Add no-unnecessary-qualifier rule

docs/_data/rules.json

@@ -968,6 +968,19 @@
     "type": "maintainability",
     "typescriptOnly": false
   },
+  {
+    "ruleName": "no-unnecessary-qualifier",
+    "description": "Warns when a namespace qualifier (`A.x`) is unnecessary.",
+    "hasFix": true,
+    "optionsDescription": "Not configurable.",
+    "options": null,
+    "optionExamples": [
+      "true"
+    ],
+    "type": "style",
+    "typescriptOnly": true,
+    "requiresTypeInfo": true
+  },
   {
     "ruleName": "no-unsafe-finally",
     "description": "\nDisallows control flow statements, such as `return`, `continue`,\n`break` and `throws` in finally blocks.",
@@ -1708,7 +1721,7 @@
     "ruleName": "whitespace",
     "description": "Enforces whitespace style conventions.",
     "rationale": "Helps maintain a readable, consistent style in your codebase.",
-    "optionsDescription": "\nSeven arguments may be optionally provided:\n\n* `\"check-branch\"` checks branching statements (`if`/`else`/`for`/`while`) are followed by whitespace.\n* `\"check-decl\"`checks that variable declarations have whitespace around the equals token.\n* `\"check-operator\"` checks for whitespace around operator tokens.\n* `\"check-module\"` checks for whitespace in import & export statements.\n* `\"check-separator\"` checks for whitespace after separator tokens (`,`/`;`).\n* `\"check-type\"` checks for whitespace before a variable type specification.\n* `\"check-typecast\"` checks for whitespace between a typecast and its target.",
+    "optionsDescription": "\nSeven arguments may be optionally provided:\n\n* `\"check-branch\"` checks branching statements (`if`/`else`/`for`/`while`) are followed by whitespace.\n* `\"check-decl\"`checks that variable declarations have whitespace around the equals token.\n* `\"check-operator\"` checks for whitespace around operator tokens.\n* `\"check-module\"` checks for whitespace in import & export statements.\n* `\"check-separator\"` checks for whitespace after separator tokens (`,`/`;`).\n* `\"check-type\"` checks for whitespace before a variable type specification.\n* `\"check-typecast\"` checks for whitespace between a typecast and its target.\n* `\"check-preblock\"` checks for whitespace before the opening brace of a block",
     "options": {
       "type": "array",
       "items": {
@@ -1720,7 +1733,8 @@
           "check-module",
           "check-separator",
           "check-type",
-          "check-typecast"
+          "check-typecast",
+          "check-preblock"
         ]
       },
       "minLength": 0,

docs/rules/no-unnecessary-qualifier/index.html

@@ -0,0 +1,15 @@
+---
+ruleName: no-unnecessary-qualifier
+description: Warns when a namespace qualifier (`A.x`) is unnecessary.
+hasFix: true
+optionsDescription: Not configurable.
+options: null
+optionExamples:
+  - 'true'
+type: style
+typescriptOnly: true
+requiresTypeInfo: true
+layout: rule
+title: 'Rule: no-unnecessary-qualifier'
+optionsJSON: 'null'
+---

docs/rules/whitespace/index.html

@@ -13,6 +13,7 @@
   * `"check-separator"` checks for whitespace after separator tokens (`,`/`;`).
   * `"check-type"` checks for whitespace before a variable type specification.
   * `"check-typecast"` checks for whitespace between a typecast and its target.
+  * `"check-preblock"` checks for whitespace before the opening brace of a block
 options:
   type: array
   items:
@@ -25,6 +26,7 @@
       - check-separator
       - check-type
       - check-typecast
+      - check-preblock
   minLength: 0
   maxLength: 7
 optionExamples:
@@ -45,7 +47,8 @@
         "check-module",
         "check-separator",
         "check-type",
-        "check-typecast"
+        "check-typecast",
+        "check-preblock"
       ]
     },
     "minLength": 0,

src/language/utils.ts

@@ -146,46 +146,49 @@ export function isAssignment(node: ts.Node) {
  * Bitwise check for node flags.
  */
 export function isNodeFlagSet(node: ts.Node, flagToCheck: ts.NodeFlags): boolean {
-    /* tslint:disable:no-bitwise */
+    // tslint:disable-next-line:no-bitwise
     return (node.flags & flagToCheck) !== 0;
-    /* tslint:enable:no-bitwise */
 }
 
 /**
  * Bitwise check for combined node flags.
  */
 export function isCombinedNodeFlagSet(node: ts.Node, flagToCheck: ts.NodeFlags): boolean {
-    /* tslint:disable:no-bitwise */
+    // tslint:disable-next-line:no-bitwise
     return (ts.getCombinedNodeFlags(node) & flagToCheck) !== 0;
-    /* tslint:enable:no-bitwise */
 }
 
 /**
  * Bitwise check for combined modifier flags.
  */
 export function isCombinedModifierFlagSet(node: ts.Node, flagToCheck: ts.ModifierFlags): boolean {
-    /* tslint:disable:no-bitwise */
+    // tslint:disable-next-line:no-bitwise
     return (ts.getCombinedModifierFlags(node) & flagToCheck) !== 0;
-    /* tslint:enable:no-bitwise */
 }
 
 /**
  * Bitwise check for type flags.
  */
 export function isTypeFlagSet(type: ts.Type, flagToCheck: ts.TypeFlags): boolean {
-    /* tslint:disable:no-bitwise */
+    // tslint:disable-next-line:no-bitwise
     return (type.flags & flagToCheck) !== 0;
-    /* tslint:enable:no-bitwise */
+}
+
+/**
+ * Bitwise check for symbol flags.
+ */
+export function isSymbolFlagSet(symbol: ts.Symbol, flagToCheck: ts.SymbolFlags): boolean {
+    // tslint:disable-next-line:no-bitwise
+    return (symbol.flags & flagToCheck) !== 0;
 }
 
 /**
  * Bitwise check for object flags.
  * Does not work with TypeScript 2.0.x
  */
 export function isObjectFlagSet(objectType: ts.ObjectType, flagToCheck: ts.ObjectFlags): boolean {
-    /* tslint:disable:no-bitwise */
+    // tslint:disable-next-line:no-bitwise
     return (objectType.objectFlags & flagToCheck) !== 0;
-    /* tslint:enable:no-bitwise */
 }
 
 /**

src/rules/arrowReturnShorthandRule.ts

@@ -85,11 +85,11 @@ class Walker extends Lint.RuleWalker {
                 this.appendText(expr.getEnd(), ")"),
             ] : []),
             // " {"
-            deleteFromTo(arrow.end, openBrace.end),
+            this.deleteFromTo(arrow.end, openBrace.end),
             // "return "
-            deleteFromTo(statement.getStart(), expr.getStart()),
+            this.deleteFromTo(statement.getStart(), expr.getStart()),
             // " }" (may include semicolon)
-            deleteFromTo(expr.end, closeBrace.end),
+            this.deleteFromTo(expr.end, closeBrace.end),
         );
 
         function hasComments(node: ts.Node): boolean {
@@ -104,7 +104,3 @@ function getSimpleReturnExpression(block: ts.Block): ts.Expression | undefined {
         ? (block.statements[0] as ts.ReturnStatement).expression
         : undefined;
 }
-
-function deleteFromTo(start: number, end: number): Lint.Replacement {
-    return new Lint.Replacement(start, end - start, "");
-}

src/rules/noUnnecessaryQualifierRule.ts

@@ -0,0 +1,142 @@
+/**
+ * @license
+ * Copyright 2017 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 ts from "typescript";
+
+import * as Lint from "../index";
+import { arraysAreEqual } from "../utils";
+
+export class Rule extends Lint.Rules.TypedRule {
+    /* tslint:disable:object-literal-sort-keys */
+    public static metadata: Lint.IRuleMetadata = {
+        ruleName: "no-unnecessary-qualifier",
+        description: "Warns when a namespace qualifier (`A.x`) is unnecessary.",
+        hasFix: true,
+        optionsDescription: "Not configurable.",
+        options: null,
+        optionExamples: ["true"],
+        type: "style",
+        typescriptOnly: true,
+        requiresTypeInfo: true,
+    };
+    /* tslint:enable:object-literal-sort-keys */
+
+    public static FAILURE_STRING(name: string) {
+        return `Qualifier is unnecessary since '${name}' is in scope.`;
+    }
+
+    public applyWithProgram(sourceFile: ts.SourceFile, langSvc: ts.LanguageService): Lint.RuleFailure[] {
+        return this.applyWithWalker(new Walker(sourceFile, this.getOptions(), langSvc.getProgram()));
+    }
+}
+
+class Walker extends Lint.ProgramAwareRuleWalker {
+    private namespacesInScope: Array<ts.ModuleDeclaration | ts.EnumDeclaration> = [];
+
+    public visitModuleDeclaration(node: ts.ModuleDeclaration) {
+        this.namespacesInScope.push(node);
+        super.visitModuleDeclaration(node);
+        this.namespacesInScope.pop();
+    }
+
+    public visitEnumDeclaration(node: ts.EnumDeclaration) {
+        this.namespacesInScope.push(node);
+        super.visitEnumDeclaration(node);
+        this.namespacesInScope.pop();
+    }
+
+    public visitNode(node: ts.Node) {
+        switch (node.kind) {
+            case ts.SyntaxKind.QualifiedName:
+                const { left, right } = node as ts.QualifiedName;
+                this.visitNamespaceAccess(node, left, right);
+                break;
+            case ts.SyntaxKind.PropertyAccessExpression:
+                const { expression, name } = node as ts.PropertyAccessExpression;
+                if (isEntityNameExpression(expression)) {
+                    this.visitNamespaceAccess(node, expression, name);
+                    break;
+                }
+                // fall through
+            default:
+                super.visitNode(node);
+        }
+    }
+
+    private visitNamespaceAccess(node: ts.Node, qualifier: ts.EntityNameOrEntityNameExpression, name: ts.Identifier) {
+        if (this.qualifierIsUnnecessary(qualifier, name)) {
+            const fix = this.createFix(this.deleteFromTo(qualifier.getStart(), name.getStart()));
+            this.addFailureAtNode(qualifier, Rule.FAILURE_STRING(qualifier.getText()), fix);
+        } else {
+            // Only look for nested qualifier errors if we didn't already fail on the outer qualifier.
+            super.visitNode(node);
+        }
+    }
+
+    private qualifierIsUnnecessary(qualifier: ts.EntityNameOrEntityNameExpression, name: ts.Identifier): boolean {
+        const namespaceSymbol = this.symbolAtLocation(qualifier);
+        if (namespaceSymbol === undefined || !this.symbolIsNamespaceInScope(namespaceSymbol)) {
+            return false;
+        }
+
+        const accessedSymbol = this.symbolAtLocation(name);
+        if (accessedSymbol === undefined) {
+            return false;
+        }
+
+        // If the symbol in scope is different, the qualifier is necessary.
+        const fromScope = this.getSymbolInScope(qualifier, accessedSymbol.flags, name.text);
+        return fromScope === undefined || symbolsAreEqual(fromScope, accessedSymbol);
+    }
+
+    private getSymbolInScope(node: ts.Node, flags: ts.SymbolFlags, name: string): ts.Symbol | undefined {
+        // TODO:PERF `getSymbolsInScope` gets a long list. Is there a better way?
+        const scope = this.getTypeChecker().getSymbolsInScope(node, flags);
+        return scope.find((scopeSymbol) => scopeSymbol.name === name);
+    }
+
+    private symbolAtLocation(node: ts.Node): ts.Symbol | undefined {
+        return this.getTypeChecker().getSymbolAtLocation(node);
+    }
+
+    private symbolIsNamespaceInScope(symbol: ts.Symbol): boolean {
+        if (symbol.getDeclarations().some((decl) => this.namespacesInScope.some((ns) => nodesAreEqual(ns, decl)))) {
+            return true;
+        }
+        const alias = this.tryGetAliasedSymbol(symbol);
+        return alias !== undefined && this.symbolIsNamespaceInScope(alias);
+    }
+
+    private tryGetAliasedSymbol(symbol: ts.Symbol): ts.Symbol | undefined {
+        return Lint.isSymbolFlagSet(symbol, ts.SymbolFlags.Alias) ? this.getTypeChecker().getAliasedSymbol(symbol) : undefined;
+    }
+}
+
+function isEntityNameExpression(expr: ts.Expression): expr is ts.EntityNameExpression {
+    return expr.kind === ts.SyntaxKind.Identifier ||
+        expr.kind === ts.SyntaxKind.PropertyAccessExpression && isEntityNameExpression((expr as ts.PropertyAccessExpression).expression);
+}
+
+// TODO: Should just be `===`. See https://github.com/palantir/tslint/issues/1969
+function nodesAreEqual(a: ts.Node, b: ts.Node) {
+    return a.pos === b.pos;
+}
+
+// Only needed in global files. Likely due to https://github.com/palantir/tslint/issues/1969. See `test.global.ts.lint`.
+function symbolsAreEqual(a: ts.Symbol, b: ts.Symbol): boolean {
+    return arraysAreEqual(a.declarations, b.declarations, nodesAreEqual);
+}

src/rules/unifiedSignaturesRule.ts

@@ -18,6 +18,7 @@
 import * as ts from "typescript";
 
 import * as Lint from "../index";
+import { arraysAreEqual, Equal } from "../utils";
 
 import { getOverloadKey, isSignatureDeclaration } from "./adjacentOverloadSignaturesRule";
 
@@ -202,9 +203,6 @@ type GetOverload<T> = (node: T) => { signature: ts.SignatureDeclaration, key: st
  */
 type IsTypeParameter = (typeName: string) => boolean;
 
-/** Return true if both parameters are equal. */
-type Equal<T> = (a: T, b: T) => boolean;
-
 /** Given type parameters, returns a function to test whether a type is one of those parameters. */
 function getIsTypeParameter(typeParameters?: ts.TypeParameterDeclaration[]): IsTypeParameter {
     if (!typeParameters) {
@@ -297,7 +295,3 @@ function forEachPair<T>(values: T[], action: (a: T, b: T) => void): void {
         }
     }
 }
-
-function arraysAreEqual<T>(a: T[] | undefined, b: T[] | undefined, eq: Equal<T>): boolean {
-    return a === b || !!a && !!b && a.length === b.length && a.every((x, idx) => eq(x, b[idx]));
-}

src/utils.ts

@@ -107,3 +107,10 @@ export function stripComments(content: string): string {
 export function escapeRegExp(re: string): string {
     return re.replace(/[.+*?|^$[\]{}()\\]/g, "\\$&");
 }
+
+/** Return true if both parameters are equal. */
+export type Equal<T> = (a: T, b: T) => boolean;
+
+export function arraysAreEqual<T>(a: T[] | undefined, b: T[] | undefined, eq: Equal<T>): boolean {
+    return a === b || !!a && !!b && a.length === b.length && a.every((x, idx) => eq(x, b[idx]));
+}

test/rules/no-unnecessary-qualifier/b.ts

@@ -0,0 +1 @@
+export type T = number;

test/rules/no-unnecessary-qualifier/test-global.ts.fix

@@ -0,0 +1,5 @@
+// We need `symbolsAreEqual` in global files.
+namespace N {
+    export type T = number;
+    export const x: T = 0;
+}

test/rules/no-unnecessary-qualifier/test-global.ts.lint

@@ -0,0 +1,6 @@
+// We need `symbolsAreEqual` in global files.
+namespace N {
+    export type T = number;
+    export const x: N.T = 0;
+                    ~ [Qualifier is unnecessary since 'N' is in scope.]
+}