Merge pull request #953 from palantir/error-if-no-rule

Throw error if rule specified in config cannot be found.
palantir · Feb 5, 2016 · 8c58ef2 · 8c58ef2
2 parents 40c37dc + 5bcee13
commit 8c58ef2
Show file tree

Hide file tree

Showing 24 changed files with 98 additions and 74 deletions.
diff --git a/src/configuration.ts b/src/configuration.ts
@@ -39,17 +39,17 @@ export const DEFAULT_CONFIG = {
             "index-signature": "nospace",
             "parameter": "nospace",
             "property-declaration": "nospace",
-            "variable-declaration": "nospace"
-        }],
+            "variable-declaration": "nospace",
+        }, ],
         "variable-name": [true, "ban-keywords"],
         "whitespace": [true,
             "check-branch",
             "check-decl",
             "check-operator",
             "check-separator",
-            "check-type"
+            "check-type",
         ],
-    }
+    },
 };
 
 export function findConfiguration(configFile: string, inputFileLocation: string): any {
@@ -110,7 +110,7 @@ function getHomeDir() {
         environment.USERPROFILE,
         environment.HOME,
         environment.HOMEPATH,
-        environment.HOMEDRIVE + environment.HOMEPATH
+        environment.HOMEDRIVE + environment.HOMEPATH,
     ];
 
     for (const homePath of paths) {

diff --git a/src/enableDisableRules.ts b/src/enableDisableRules.ts
@@ -64,7 +64,7 @@ export class EnableDisableRulesWalker extends SkippableTokenAwareRuleWalker {
                     }
                     this.enableDisableRuleMap[ruleToAdd].push({
                         isEnabled: isEnabled,
-                        position: startingPosition
+                        position: startingPosition,
                     });
                 }
             }

diff --git a/src/language/languageServiceHost.ts b/src/language/languageServiceHost.ts
@@ -26,7 +26,7 @@ export function createLanguageServiceHost(fileName: string, source: string): ts.
         getScriptFileNames: () => [fileName],
         getScriptSnapshot: (name: string) => ts.ScriptSnapshot.fromString(name === fileName ? source : ""),
         getScriptVersion: () => "1",
-        log: () => { /* */ }
+        log: () => { /* */ },
     };
 }
 

diff --git a/src/language/rule/abstractRule.ts b/src/language/rule/abstractRule.ts
@@ -35,7 +35,7 @@ export abstract class AbstractRule implements IRule {
         this.options = {
             disabledIntervals: disabledIntervals,
             ruleArguments: ruleArguments,
-            ruleName: ruleName
+            ruleName: ruleName,
         };
     }
 

diff --git a/src/language/rule/rule.ts b/src/language/rule/rule.ts
@@ -57,7 +57,7 @@ export class RuleFailurePosition {
         return {
             character: this.lineAndCharacter.character,
             line: this.lineAndCharacter.line,
-            position: this.position
+            position: this.position,
         };
     }
 
@@ -119,7 +119,7 @@ export class RuleFailure {
             failure: this.failure,
             name: this.fileName,
             ruleName: this.ruleName,
-            startPosition: this.startPosition.toJson()
+            startPosition: this.startPosition.toJson(),
         };
     }
 

diff --git a/src/language/utils.ts b/src/language/utils.ts
@@ -36,7 +36,7 @@ export function getSourceFile(fileName: string, source: string): ts.SourceFile {
         },
         readFile: () => null,
         useCaseSensitiveFileNames: () => true,
-        writeFile: () => null
+        writeFile: () => null,
     };
 
     const program = ts.createProgram([normalizedName], compilerOptions, compilerHost);
@@ -47,7 +47,7 @@ export function getSourceFile(fileName: string, source: string): ts.SourceFile {
 export function createCompilerOptions(): ts.CompilerOptions {
     return {
         noResolve: true,
-        target: ts.ScriptTarget.ES5
+        target: ts.ScriptTarget.ES5,
     };
 }
 

diff --git a/src/ruleLoader.ts b/src/ruleLoader.ts
@@ -33,11 +33,15 @@ export function loadRules(ruleConfiguration: {[name: string]: any},
                           enableDisableRuleMap: {[rulename: string]: IEnableDisablePosition[]},
                           rulesDirectories?: string | string[]): IRule[] {
     const rules: IRule[] = [];
+    const notFoundRules: string[] = [];
+
     for (const ruleName in ruleConfiguration) {
         if (ruleConfiguration.hasOwnProperty(ruleName)) {
             const ruleValue = ruleConfiguration[ruleName];
             const Rule = findRule(ruleName, rulesDirectories);
-            if (Rule !== undefined) {
+            if (Rule == null) {
+                notFoundRules.push(ruleName);
+            } else {
                 const all = "all"; // make the linter happy until we can turn it on and off
                 const allList = (all in enableDisableRuleMap ? enableDisableRuleMap[all] : []);
                 const ruleSpecificList = (ruleName in enableDisableRuleMap ? enableDisableRuleMap[ruleName] : []);
@@ -47,7 +51,12 @@ export function loadRules(ruleConfiguration: {[name: string]: any},
         }
     }
 
-    return rules;
+    if (notFoundRules.length > 0) {
+        const errorMessage = `Could not find the following rules specified in the configuration:\n${notFoundRules.join("\n")}`;
+        throw new Error(errorMessage);
+    } else {
+        return rules;
+    }
 }
 
 export function findRule(name: string, rulesDirectories?: string | string[]) {
@@ -134,7 +143,7 @@ function buildDisabledIntervalsFromSwitches(ruleSpecificList: IEnableDisablePosi
                 // we're currently disabled and about to enable -- end the interval
                 disabledIntervalList.push({
                     endPosition: newPositionToCheck.position,
-                    startPosition: disabledStartPosition
+                    startPosition: disabledStartPosition,
                 });
                 isCurrentlyDisabled = false;
             }
@@ -145,7 +154,7 @@ function buildDisabledIntervalsFromSwitches(ruleSpecificList: IEnableDisablePosi
         // we started an interval but didn't finish one -- so finish it with an Infinity
         disabledIntervalList.push({
             endPosition: Infinity,
-            startPosition: disabledStartPosition
+            startPosition: disabledStartPosition,
         });
     }
 

diff --git a/src/rules/memberOrderingRule.ts b/src/rules/memberOrderingRule.ts
@@ -37,7 +37,7 @@ function getModifiers(isMethod: boolean, modifiers?: ts.ModifiersArray): IModifi
     return {
         isInstance: !Lint.hasModifier(modifiers, ts.SyntaxKind.StaticKeyword),
         isMethod: isMethod,
-        isPrivate: Lint.hasModifier(modifiers, ts.SyntaxKind.PrivateKeyword)
+        isPrivate: Lint.hasModifier(modifiers, ts.SyntaxKind.PrivateKeyword),
     };
 }
 
@@ -46,7 +46,7 @@ function toString(modifiers: IModifiers): string {
         modifiers.isPrivate ? "private" : "public",
         modifiers.isInstance ? "instance" : "static",
         "member",
-        modifiers.isMethod ? "function" : "variable"
+        modifiers.isMethod ? "function" : "variable",
     ].join(" ");
 }
 
@@ -95,7 +95,7 @@ export class MemberOrderingWalker extends Lint.RuleWalker {
         this.previousMember = {
             isInstance: false,
             isMethod: false,
-            isPrivate: false
+            isPrivate: false,
         };
     }
 

diff --git a/src/rules/noConstructRule.ts b/src/rules/noConstructRule.ts
@@ -30,7 +30,7 @@ class NoConstructWalker extends Lint.RuleWalker {
     private static FORBIDDEN_CONSTRUCTORS = [
         "Boolean",
         "Number",
-        "String"
+        "String",
     ];
 
     public visitNewExpression(node: ts.NewExpression) {

diff --git a/src/test.ts b/src/test.ts
@@ -64,12 +64,12 @@ export function runTest(testDirectory: string): TestResult {
             return {
                 endPos: {
                     col: endLineAndCharacter.character,
-                    line: endLineAndCharacter.line
+                    line: endLineAndCharacter.line,
                 },
                 message: failure.getFailure(),
                 startPos: {
                     col: startLineAndCharacter.character,
-                    line: startLineAndCharacter.line
+                    line: startLineAndCharacter.line,
                 },
             };
         });
@@ -105,7 +105,7 @@ export function consoleTestResultHandler(testResult: TestResult): boolean {
             didAllTestsPass = false;
 
             for (const diffResult of diffResults) {
-                let color = colors.gray;
+                let color = colors.grey;
                 if (diffResult.added) {
                     color = colors.green;
                 } else if (diffResult.removed) {

diff --git a/src/test/parse.ts b/src/test/parse.ts
@@ -15,8 +15,16 @@
  */
 
 import {LintError, errorComparator, lintSyntaxError} from "./lintError";
-import {Line, ErrorLine, CodeLine, MultilineErrorLine, EndErrorLine, MessageSubstitutionLine,
-        parseLine, printLine} from "./lines";
+import {
+    Line,
+    ErrorLine,
+    CodeLine,
+    MultilineErrorLine,
+    EndErrorLine,
+    MessageSubstitutionLine,
+    parseLine,
+    printLine,
+} from "./lines";
 
 /**
  * Takes the full text of a .lint file and returns the contents of the file
@@ -65,7 +73,7 @@ export function parseErrorsFromMarkup(text: string): LintError[] {
                 lintErrors.push({
                     startPos: errorStartPos,
                     endPos: { line: lineNo, col: errorLine.endCol },
-                    message: messageSubstitutions[errorLine.message] || errorLine.message
+                    message: messageSubstitutions[errorLine.message] || errorLine.message,
                 });
 
             // if the error is the start of a multiline error
@@ -85,7 +93,7 @@ export function parseErrorsFromMarkup(text: string): LintError[] {
                             lintErrors.push({
                                 startPos: errorStartPos,
                                 endPos: { line: nextLineNo, col: nextErrorLine.endCol },
-                                message: messageSubstitutions[nextErrorLine.message] || nextErrorLine.message
+                                message: messageSubstitutions[nextErrorLine.message] || nextErrorLine.message,
                             });
                             break;
                         }

diff --git a/src/tslint-cli.ts b/src/tslint-cli.ts
@@ -25,7 +25,7 @@ import {
     DEFAULT_CONFIG,
     findConfigurationPath,
     getRulesDirectories,
-    loadConfigurationFromPath
+    loadConfigurationFromPath,
 } from "./configuration";
 import {consoleTestResultHandler, runTest} from "./test";
 
@@ -44,48 +44,48 @@ let processed = optimist
     .options({
         "c": {
             alias: "config",
-            describe: "configuration file"
+            describe: "configuration file",
         },
         "h": {
             alias: "help",
-            describe: "display detailed help"
+            describe: "display detailed help",
         },
         "i": {
             alias: "init",
-            describe: "generate a tslint.json config file in the current working directory"
+            describe: "generate a tslint.json config file in the current working directory",
         },
         "o": {
             alias: "out",
-            describe: "output file"
+            describe: "output file",
         },
         "r": {
             alias: "rules-dir",
-            describe: "rules directory"
+            describe: "rules directory",
         },
         "s": {
             alias: "formatters-dir",
-            describe: "formatters directory"
+            describe: "formatters directory",
         },
         "t": {
             alias: "format",
             default: "prose",
-            describe: "output format (prose, json, verbose)"
+            describe: "output format (prose, json, verbose)",
         },
         "test": {
             describe: "test that tslint produces the correct output for the specified directory"
         },
         "v": {
             alias: "version",
-            describe: "current version"
-        }
+            describe: "current version",
+        },
     });
 const argv = processed.argv;
 
 let outputStream: any;
 if (argv.o != null) {
     outputStream = fs.createWriteStream(argv.o, {
         flags: "w+",
-        mode: 420
+        mode: 420,
     });
 } else {
     outputStream = process.stdout;
@@ -206,7 +206,7 @@ const processFile = (file: string) => {
         configuration: configuration,
         formatter: argv.t,
         formattersDirectory: argv.s,
-        rulesDirectory: rulesDirectories
+        rulesDirectory: rulesDirectories,
     });
 
     const lintResult = linter.lint();

diff --git a/src/tslint.ts b/src/tslint.ts
@@ -45,7 +45,7 @@ class Linter {
         // walk the code first to find all the intervals where rules are disabled
         const rulesWalker = new EnableDisableRulesWalker(sourceFile, {
             disabledIntervals: [],
-            ruleName: ""
+            ruleName: "",
         });
         rulesWalker.walk(sourceFile);
         const enableDisableRuleMap = rulesWalker.enableDisableRuleMap;
@@ -78,7 +78,7 @@ class Linter {
             failureCount: failures.length,
             failures: failures,
             format: this.options.formatter,
-            output: output
+            output: output,
         };
     }
 

diff --git a/test/files/custom-rules-2/noFail.js → test/files/custom-rules-2/noFailRule.js b/test/files/custom-rules-2/noFail.js → test/files/custom-rules-2/noFailRule.js
diff --git a/test/formatters/externalFormatterTest.ts b/test/formatters/externalFormatterTest.ts
@@ -34,7 +34,7 @@ describe("External Formatter", () => {
         const failures = [
             new RuleFailure(sourceFile, 0, 1, "first failure", "first-name"),
             new RuleFailure(sourceFile, 32, 36, "mid failure", "mid-name"),
-            new RuleFailure(sourceFile, maxPosition - 1, maxPosition, "last failure", "last-name")
+            new RuleFailure(sourceFile, maxPosition - 1, maxPosition, "last failure", "last-name"),
         ];
 
         const expectedResult =