Print stack trace of exceptions (palantir#2890)

HyphnKnight · Apr 9, 2018 · 3fadee7 · 3fadee7
1 parent d81e72a
commit 3fadee7
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 14 deletions.
diff --git a/src/language/rule/typedRule.ts b/src/language/rule/typedRule.ts
@@ -17,14 +17,16 @@
 
 import * as ts from "typescript";
 
+import { showWarningOnce } from "../../error";
 import {AbstractRule} from "./abstractRule";
 import {ITypedRule, RuleFailure} from "./rule";
 
 export abstract class TypedRule extends AbstractRule implements ITypedRule {
 
     public apply(): RuleFailure[] {
-        // if no program is given to the linter, throw an error
-        throw new Error(`The '${this.ruleName}' rule requires type checking`);
+        // if no program is given to the linter, show an error
+        showWarningOnce(`Warning: The '${this.ruleName}' rule requires type infomation.`);
+        return [];
     }
 
     public abstract applyWithProgram(sourceFile: ts.SourceFile, program: ts.Program): RuleFailure[];

diff --git a/src/linter.ts b/src/linter.ts
@@ -200,10 +200,10 @@ class Linter {
                 return rule.apply(sourceFile);
             }
         } catch (error) {
-            if (isError(error)) {
-                showWarningOnce(`Warning: ${error.message}`);
+            if (isError(error) && error.stack !== undefined) {
+                showWarningOnce(error.stack);
             } else {
-                console.warn(`Warning: ${error}`);
+                showWarningOnce(String(error));
             }
             return [];
         }

diff --git a/src/rules/memberAccessRule.ts b/src/rules/memberAccessRule.ts
@@ -15,15 +15,22 @@
  * limitations under the License.
  */
 
-import { getChildOfKind, isClassLikeDeclaration, isConstructorDeclaration, isIdentifier } from "tsutils";
+import { getChildOfKind, isClassLikeDeclaration } from "tsutils";
 import * as ts from "typescript";
 
+import { showWarningOnce } from "../error";
 import * as Lint from "../index";
 
 const OPTION_NO_PUBLIC = "no-public";
 const OPTION_CHECK_ACCESSOR = "check-accessor";
 const OPTION_CHECK_CONSTRUCTOR = "check-constructor";
 
+interface Options {
+    noPublic: boolean;
+    checkAccessor: boolean;
+    checkConstructor: boolean;
+}
+
 export class Rule extends Lint.Rules.AbstractRule {
     /* tslint:disable:object-literal-sort-keys */
     public static metadata: Lint.IRuleMetadata = {
@@ -65,15 +72,21 @@ export class Rule extends Lint.Rules.AbstractRule {
         let checkConstructor = options.indexOf(OPTION_CHECK_CONSTRUCTOR) !== -1;
         if (noPublic) {
             if (checkAccessor || checkConstructor) {
-                throw new Error("If 'no-public' is present, it should be the only option.");
+                showWarningOnce(`Warning: ${this.ruleName} - If 'no-public' is present, it should be the only option.`);
+                return [];
             }
             checkAccessor = checkConstructor = true;
         }
-        return this.applyWithFunction(sourceFile, (ctx) => walk(ctx, noPublic, checkAccessor, checkConstructor));
+        return this.applyWithFunction(sourceFile, walk, {
+            checkAccessor,
+            checkConstructor,
+            noPublic,
+        });
     }
 }
 
-function walk(ctx: Lint.WalkContext<void>, noPublic: boolean, checkAccessor: boolean, checkConstructor: boolean) {
+function walk(ctx: Lint.WalkContext<Options>) {
+    const {noPublic, checkAccessor, checkConstructor} = ctx.options;
     return ts.forEachChild(ctx.sourceFile, function recur(node: ts.Node): void {
         if (isClassLikeDeclaration(node)) {
             for (const child of node.members) {
@@ -112,10 +125,10 @@ function walk(ctx: Lint.WalkContext<void>, noPublic: boolean, checkAccessor: boo
             ctx.addFailureAtNode(publicKeyword, Rule.FAILURE_STRING_NO_PUBLIC);
         }
         if (!noPublic && !isPublic) {
-            const nameNode = isConstructorDeclaration(node)
-                ? getChildOfKind(node, ts.SyntaxKind.ConstructorKeyword)!
+            const nameNode = node.kind === ts.SyntaxKind.Constructor
+                ? getChildOfKind(node, ts.SyntaxKind.ConstructorKeyword, ctx.sourceFile)!
                 : node.name !== undefined ? node.name : node;
-            const memberName = node.name !== undefined && isIdentifier(node.name) ? node.name.text : undefined;
+            const memberName = node.name !== undefined && node.name.kind === ts.SyntaxKind.Identifier ? node.name.text : undefined;
             ctx.addFailureAtNode(nameNode, Rule.FAILURE_STRING_FACTORY(memberType(node), memberName));
         }
     }

diff --git a/src/rules/memberOrderingRule.ts b/src/rules/memberOrderingRule.ts
@@ -17,6 +17,7 @@
 
 import * as ts from "typescript";
 
+import { showWarningOnce } from "../error";
 import * as Lint from "../index";
 import {flatMap, mapDefined} from "../utils";
 
@@ -194,7 +195,14 @@ export class Rule extends Lint.Rules.AbstractRule {
 
     /* tslint:enable:object-literal-sort-keys */
     public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] {
-        return this.applyWithWalker(new MemberOrderingWalker(sourceFile, this.ruleName, parseOptions(this.ruleArguments)));
+        let options: Options;
+        try {
+            options = parseOptions(this.ruleArguments);
+        } catch (e) {
+            showWarningOnce(`Warning: ${this.ruleName} - ${(e as Error).message}`);
+            return [];
+        }
+        return this.applyWithWalker(new MemberOrderingWalker(sourceFile, this.ruleName, options));
     }
 }
 

diff --git a/src/rules/quotemarkRule.ts b/src/rules/quotemarkRule.ts
@@ -18,6 +18,7 @@
 import { isNoSubstitutionTemplateLiteral, isSameLine, isStringLiteral } from "tsutils";
 import * as ts from "typescript";
 
+import { showWarningOnce } from "../error";
 import * as Lint from "../index";
 
 const OPTION_SINGLE = "single";
@@ -81,7 +82,8 @@ export class Rule extends Lint.Rules.AbstractRule {
         const args = this.ruleArguments;
         if (args.length > 0) {
             if (args[0] !== OPTION_SINGLE && args[0] !== OPTION_DOUBLE) {
-                throw new Error(`First argument to 'quotemark' rule should be "${OPTION_SINGLE}" or "${OPTION_DOUBLE}"`);
+                showWarningOnce(`Warning: First argument to 'quotemark' rule should be "${OPTION_SINGLE}" or "${OPTION_DOUBLE}"`);
+                return [];
             }
         }
         const quoteMark = args[0] === OPTION_SINGLE ? "'" : '"';