Only lint call expressions and external module

... references in no-require-imports rule. Fixes #816.
palantir · Nov 23, 2015 · e31a1f6 · e31a1f6
1 parent 9be7b27
commit e31a1f6
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 32 deletions.
diff --git a/src/rules/noRequireImportsRule.ts b/src/rules/noRequireImportsRule.ts
@@ -26,27 +26,20 @@ export class Rule extends Lint.Rules.AbstractRule {
 }
 
 class NoRequireImportsWalker extends Lint.RuleWalker {
-    public visitVariableStatement(node: ts.VariableStatement) {
-        const declarations = node.declarationList.declarations;
-        for (let decl of declarations) {
-            this.handleDeclaration(decl);
+    public visitCallExpression(node: ts.CallExpression) {
+        if (node.arguments != null && node.expression != null) {
+            const callExpressionText = node.expression.getText(this.getSourceFile());
+            if (callExpressionText === "require") {
+                this.addFailure(this.createFailure(node.getStart(), node.getWidth(), Rule.FAILURE_STRING));
+            }
         }
-        super.visitVariableStatement(node);
     }
 
     public visitImportEqualsDeclaration(node: ts.ImportEqualsDeclaration) {
-        this.addFailure(this.createFailure(node.getStart(), node.getWidth(), Rule.FAILURE_STRING));
-        super.visitImportEqualsDeclaration(node);
-    }
-
-    private handleDeclaration(decl: ts.VariableDeclaration)  {
-        // make sure the RHS is a call expression.
-        const call = <ts.CallExpression> (decl.initializer);
-        if (call && call.arguments && call.expression) {
-            const callExpressionText = call.expression.getText(this.getSourceFile());
-            if (callExpressionText === "require") {
-                this.addFailure(this.createFailure(decl.getStart(), decl.getWidth(), Rule.FAILURE_STRING));
-            }
+        const {moduleReference} = node;
+        if (moduleReference.kind === ts.SyntaxKind.ExternalModuleReference) {
+            this.addFailure(this.createFailure(moduleReference.getStart(), moduleReference.getWidth(), Rule.FAILURE_STRING));
         }
+        super.visitImportEqualsDeclaration(node);
     }
 }
diff --git a/test/files/rules/norequireimports.test.ts b/test/files/rules/norequireimports.test.ts
@@ -1,17 +1,17 @@
-var lib = require('lib'); // error
+var lib = require('lib'); // failure
 
-let lib2 = require('lib2'); // error
+let lib2 = require('lib2'); // failure
 
 import {l} from 'lib';
 
 var lib3 = load('not_an_import');
 
 var lib4 = lib2.subImport;
 
-var lib5 = require('lib5'), // error
-    lib6 = require('lib6'), // error
+var lib5 = require('lib5'), // failure
+    lib6 = require('lib6'), // failure
     lib7 = 700;
 
-import lib8 = require('lib8'); // error
+import lib8 = require('lib8'); // failure
 
-import foo = bar; // error
+import lib9 = lib2.anotherSubImport;
diff --git a/test/rules/noRequireImportsRuleTests.ts b/test/rules/noRequireImportsRuleTests.ts
@@ -13,28 +13,27 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 import * as Lint from "../lint";
 
 describe("<no-require-imports>", () => {
     const Rule = Lint.Test.getRule("no-require-imports");
     const fileName = "rules/norequireimports.test.ts";
 
-    it("forbids CommmonJS style imports", () => {
+    it("forbids require() imports", () => {
         const actualFailures = Lint.Test.applyRuleOnFile(fileName, Rule);
 
-        assert(actualFailures.length === 6, "Expected 6 failures, got " + actualFailures.length);
-        const failure1 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING)([1, 5], [1, 25]);
-        const failure2 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING)([3, 5], [3, 27]);
-        const failure3 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING)([11, 5], [11, 27]);
-        const failure4 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING)([12, 5], [12, 27]);
-        const failure5 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING)([15, 1], [15, 31]);
-        const failure6 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING)([17, 1], [17, 18]);
+        assert(actualFailures.length === 5, "Expected 5 failures, got " + actualFailures.length);
+        const failure1 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING)([1, 11], [1, 25]);
+        const failure2 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING)([3, 12], [3, 27]);
+        const failure3 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING)([11, 12], [11, 27]);
+        const failure4 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING)([12, 12], [12, 27]);
+        const failure5 = Lint.Test.createFailuresOnFile(fileName, Rule.FAILURE_STRING)([15, 15], [15, 30]);
 
         Lint.Test.assertContainsFailure(actualFailures, failure1);
         Lint.Test.assertContainsFailure(actualFailures, failure2);
         Lint.Test.assertContainsFailure(actualFailures, failure3);
         Lint.Test.assertContainsFailure(actualFailures, failure4);
         Lint.Test.assertContainsFailure(actualFailures, failure5);
-        Lint.Test.assertContainsFailure(actualFailures, failure6);
     });
 });