deprecation: Rewrite and enhance (#2748)

Rewrite to walker function.
Use TypeScript's JsDoc parser
[enhancement] `deprecation`: error message includes deprecation text if available

src/rules/deprecationRule.ts

@@ -15,20 +15,19 @@
  * limitations under the License.
  */
 
+import { isIdentifier } from "tsutils";
 import * as ts from "typescript";
 import * as Lint from "../index";
 
 export class Rule extends Lint.Rules.TypedRule {
-
     /* tslint:disable:object-literal-sort-keys */
     public static metadata: Lint.IRuleMetadata = {
         ruleName: "deprecation",
         description: "Warns when deprecated APIs are used.",
         descriptionDetails: Lint.Utils.dedent`Any usage of an identifier
             with the @deprecated JSDoc annotation will trigger a warning.
             See http://usejsdoc.org/tags-deprecated.html`,
-        rationale: Lint.Utils.dedent`
-            Deprecated APIs should be avoided, and usage updated.`,
+        rationale: "Deprecated APIs should be avoided, and usage updated.",
         optionsDescription: "",
         options: null,
         optionExamples: [],
@@ -37,60 +36,80 @@ export class Rule extends Lint.Rules.TypedRule {
         requiresTypeInfo: true,
     };
     /* tslint:enable:object-literal-sort-keys */
+
+    public static FAILURE_STRING(name: string, message: string) {
+        return `${name} is deprecated${message === "" ? "." : `: ${message.trim()}`}`;
+    }
+
     public applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): Lint.RuleFailure[] {
-        return this.applyWithWalker(new Walker(sourceFile, this.getOptions(), program));
+        return this.applyWithFunction(sourceFile, (ctx: Lint.WalkContext<void>) => walk(ctx, program.getTypeChecker()));
     }
 }
 
-class Walker extends Lint.ProgramAwareRuleWalker {
-  // Implementation inspired by angular/tsickle:
-  // https://github.com/angular/tsickle/blob/cad7c180a2155db6f6fb8d22c44151d7e8a9149f/src/decorator-annotator.ts#L42
-  protected visitIdentifier(node: ts.Identifier) {
-    let decSym = this.getTypeChecker().getSymbolAtLocation(node);
+function walk(ctx: Lint.WalkContext<void>, tc: ts.TypeChecker) {
+    return ts.forEachChild(ctx.sourceFile, function cb(node): void {
+        if (isIdentifier(node)) {
+            if (!isDeclaration(node)) {
+                const deprecation = getDeprecation(node, tc);
+                if (deprecation !== undefined) {
+                    ctx.addFailureAtNode(node, Rule.FAILURE_STRING(node.text, deprecation));
+                }
+            }
+        } else {
+            return ts.forEachChild(node, cb);
+        }
+    });
+}
 
-    if (decSym !== undefined && Lint.isSymbolFlagSet(decSym, ts.SymbolFlags.Alias)) {
-        decSym = this.getTypeChecker().getAliasedSymbol(decSym);
-    }
-    const declarations = decSym === undefined ? undefined : decSym.getDeclarations() as ts.Node[] | undefined;
-    if (declarations === undefined) {
-        super.visitIdentifier(node);
-        return;
+function isDeclaration(identifier: ts.Identifier): boolean {
+    const parent = identifier.parent!;
+    switch (parent.kind) {
+        case ts.SyntaxKind.ClassDeclaration:
+        case ts.SyntaxKind.ClassExpression:
+        case ts.SyntaxKind.InterfaceDeclaration:
+        case ts.SyntaxKind.TypeParameter:
+        case ts.SyntaxKind.FunctionExpression:
+        case ts.SyntaxKind.FunctionDeclaration:
+        case ts.SyntaxKind.LabeledStatement:
+        case ts.SyntaxKind.JsxAttribute:
+        case ts.SyntaxKind.MethodDeclaration:
+        case ts.SyntaxKind.MethodSignature:
+        case ts.SyntaxKind.PropertySignature:
+        case ts.SyntaxKind.TypeAliasDeclaration:
+        case ts.SyntaxKind.GetAccessor:
+        case ts.SyntaxKind.SetAccessor:
+        case ts.SyntaxKind.EnumDeclaration:
+            return true;
+        case ts.SyntaxKind.VariableDeclaration:
+        case ts.SyntaxKind.TypeAliasDeclaration:
+        case ts.SyntaxKind.Parameter:
+        case ts.SyntaxKind.ModuleDeclaration:
+        case ts.SyntaxKind.PropertyDeclaration:
+        case ts.SyntaxKind.PropertyAssignment:
+        case ts.SyntaxKind.EnumMember:
+            return (parent as ts.Declaration).name === identifier;
+        case ts.SyntaxKind.BindingElement:
+        case ts.SyntaxKind.ExportSpecifier:
+        case ts.SyntaxKind.ImportSpecifier:
+            // return true for `b` in `import {a as b} from "foo"`
+            return (parent as ts.ImportOrExportSpecifier | ts.BindingElement).name === identifier &&
+                (parent as ts.ImportOrExportSpecifier | ts.BindingElement).propertyName !== undefined;
+        default:
+            return false;
     }
+}
 
-    for (let commentNode of declarations) {
-      // Switch to the TS JSDoc parser in the future to avoid false positives here.
-      // For example using '@deprecated' in a true comment.
-      // However, a new TS API would be needed, track at
-      // https://github.com/Microsoft/TypeScript/issues/7393.
-
-      if (commentNode.kind === ts.SyntaxKind.VariableDeclaration) {
-          commentNode = commentNode.parent!;
-      }
-
-      // Go up one more level to VariableDeclarationStatement, where usually
-      // the comment lives. If the declaration has an 'export', the
-      // VDList.getFullText will not contain the comment.
-      if (commentNode.kind === ts.SyntaxKind.VariableDeclarationList) {
-        commentNode = commentNode.parent!;
-      }
-
-      // Don't warn on the declaration of the @deprecated symbol.
-      if (commentNode.pos <= node.pos
-          && node.getEnd() <= commentNode.getEnd()
-          && commentNode.getSourceFile() === this.getSourceFile()) {
-          continue;
-      }
-
-      const range = ts.getLeadingCommentRanges(commentNode.getFullText(), 0);
-      if (range === undefined) { continue; }
-      for (const {pos, end} of range) {
-        const jsDocText = commentNode.getFullText().substring(pos, end);
-        if (jsDocText.includes("@deprecated")) {
-            this.addFailureAtNode(node, `${node.text} is deprecated.`);
+function getDeprecation(node: ts.Identifier, tc: ts.TypeChecker): string | undefined {
+    let symbol = tc.getSymbolAtLocation(node);
+    if (symbol !== undefined && Lint.isSymbolFlagSet(symbol, ts.SymbolFlags.Alias)) {
+        symbol = tc.getAliasedSymbol(symbol);
+    }
+    if (symbol !== undefined) {
+        for (const tag of symbol.getJsDocTags()) {
+            if (tag.name === "deprecated") {
+                return tag.text;
+            }
         }
-      }
     }
-
-    super.visitIdentifier(node);
-  }
+    return undefined;
 }

test/rules/deprecation/other.test.ts

@@ -0,0 +1,14 @@
+/** @deprecated reason */
+export function other() {}
+
+/** @deprecated */
+export let other2: Function;
+
+/** This one has @deprecated somewhere in it's jsdoc */
+export let notDeprecated: any;
+/* @deprecated but it's no JsDoc */
+export let notDeprecated2: any;
+
+/** @deprecated deprecated default export */
+let def = "";
+export default def;

test/rules/deprecation/other2.test.ts

@@ -0,0 +1,3 @@
+/** @deprecated */
+let x = "";
+export = x;

test/rules/deprecation/test.ts.lint

@@ -1,16 +1,21 @@
-import {other, other2} from './other';
-        ~~~~~          [other is deprecated.]
-               ~~~~~~  [other2 is deprecated.]
+import def, {other, other2 as foobar, notDeprecated, notDeprecated2} from './other.test';
+       ~~~ [errmsg % ('def', 'deprecated default export')]
+             ~~~~~          [errmsg % ('other', 'reason')]
+                    ~~~~~~  [err % ('other2')]
+import * as other2 from './other2.test';
+            ~~~~~~ [err % ('other2')]
 other();
-~~~~~ [other is deprecated.]
+~~~~~ [errmsg % ('other', 'reason')]
+foobar();
+~~~~~~ [err % ('foobar')]
 
 declare interface D {
     /** @deprecated */ m: () => void;
 }
 
 declare let d: D;
 d.m();
-  ~ [m is deprecated.]
+  ~ [err % ('m')]
 
 /**
  * Some text
@@ -20,22 +25,38 @@ d.m();
  */
 export class P {
     /** @deprecated */ f: string;
-    g, /** @deprecated */ h: number;
+    g,
+    /** @deprecated Use g instead.*/ h: number;
 }
 
 let p = new P();
-            ~ [P is deprecated.]
-p.f;
-  ~ [f is deprecated.]
+            ~ [errmsg % ('P', 'reason is this')]
+let pf = p.f;
+           ~ [err % ('f')]
+pf; // that indirection is allowed
 p.g;
 p.h;
-  ~ [h is deprecated.]
+  ~ [errmsg % ('h', 'Use g instead.')]
+
+interface I extends P {}
+                    ~ [errmsg % ('P', 'reason is this')]
+declare var i: I;
+i.f;
+  ~ [err % ('f')]
+i.g;
 
 /** @deprecated */
 const A = 1, B = 2;
 
 A + B;
-~     [A is deprecated.]
-    ~ [B is deprecated.]
+~     [err % ('A')]
+    ~ [err % ('B')]
 
 declarationIsMissing();
+
+// TODO: those should be an error
+let {f, g, h} = p;
+(function ({f, g}: I) {})
+
+[err]: %s is deprecated.
+[errmsg]: %s is deprecated: %s