Resolve custom paths when loading rules (palantir#3640)

PomanoB · Jan 10, 2018 · a4dc874 · a4dc874
1 parent 2b8a737
commit a4dc874
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 7 deletions.
diff --git a/src/ruleLoader.ts b/src/ruleLoader.ts
@@ -81,6 +81,7 @@ export function loadRules(ruleOptionsList: IOptions[],
     return rules;
 }
 
+/** @internal private API */
 export function findRule(name: string, rulesDirectories?: string | string[]): RuleConstructor | undefined {
     const camelizedName = transformName(name);
     // first check for core rules
@@ -123,13 +124,13 @@ function loadCachedRule(directory: string, ruleName: string, isCustomPath?: bool
         return cachedRule === "not-found" ? undefined : cachedRule;
     }
 
-    // get absolute path
+    // treat directory as a relative path (which needs to be resolved) if it's a custom rule directory
     let absolutePath: string = directory;
     if (isCustomPath) {
-        if (!fs.existsSync(directory)) {
-            throw new FatalError(`Could not find custom rule directory: ${directory}`);
-        }
         absolutePath = path.resolve(directory);
+        if (!fs.existsSync(absolutePath)) {
+            throw new FatalError(`Could not find custom rule directory: ${absolutePath}`);
+        }
     }
 
     const Rule = loadRule(absolutePath, ruleName);

diff --git a/test/ruleLoaderTests.ts b/test/ruleLoaderTests.ts
@@ -21,7 +21,7 @@ import * as path from "path";
 import { rules as allRules, RULES_EXCLUDED_FROM_ALL_CONFIG } from "../src/configs/all";
 import { camelize } from "../src/utils";
 import { IOptions } from "./../src/language/rule/rule";
-import { loadRules } from "./lint";
+import { findRule, loadRules, RuleConstructor } from "./lint";
 
 const builtRulesDir = "build/src/rules";
 const srcRulesDir = "src/rules";
@@ -85,7 +85,7 @@ describe("Rule Loader", () => {
         assert.equal(rules.length, 5);
     });
 
-    it("loads js rules", () => {
+    it("loads rules for JS files, excluding typescript-only ones", () => {
         const validConfiguration: IOptions[] = [
             { ruleName: "class-name", ruleArguments: [], ruleSeverity: "error", disabledIntervals: [] },
             { ruleName: "await-promise", ruleArguments: [], ruleSeverity: "error", disabledIntervals: [] },
@@ -95,7 +95,7 @@ describe("Rule Loader", () => {
         assert.equal(rules.length, 1);
     });
 
-    it("tests every rule", () => {
+    it("tests exist for every rule", () => {
         const tests = fs.readdirSync(testRulesDir)
             .filter((file) => !file.startsWith("_") && fs.statSync(path.join(testRulesDir, file)).isDirectory())
             .map(camelize)
@@ -110,6 +110,27 @@ describe("Rule Loader", () => {
 
         assert.deepEqual(expectedAllRules, tslintAllRules, "rule is missing in tslint:all");
     });
+
+    it("resolves custom rule directories as relative paths", () => {
+        let rule: RuleConstructor | undefined;
+        assert.doesNotThrow(() => {
+            rule = findRule("always-fail", "test/files/custom-rules");
+        });
+        assert.isDefined(rule);
+    });
+
+    it("supports rulesDirectory set to empty string", () => {
+        // see https://github.com/palantir/tslint/issues/3638
+        assert.doesNotThrow(() => {
+            findRule("always-fail", "");
+        });
+    });
+
+    it("throws an error for invalid rulesDirectories", () => {
+        assert.throws(() => {
+            findRule("always-fail", "some/invalid/dir");
+        });
+    });
 });
 
 function everyRule(): string[] {