From e31a1f6279c8f4126d2ba604246e0d1220cd7651 Mon Sep 17 00:00:00 2001 From: Adi Dahiya Date: Mon, 23 Nov 2015 17:24:31 -0500 Subject: [PATCH] Only lint call expressions and external module ... references in no-require-imports rule. Fixes #816. --- src/rules/noRequireImportsRule.ts | 27 +++++++++-------------- test/files/rules/norequireimports.test.ts | 12 +++++----- test/rules/noRequireImportsRuleTests.ts | 17 +++++++------- 3 files changed, 24 insertions(+), 32 deletions(-) diff --git a/src/rules/noRequireImportsRule.ts b/src/rules/noRequireImportsRule.ts index 524766ecd43..477ef6a36c7 100644 --- 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 = (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 index d4288f7767a..6d4ba626888 100644 --- a/test/files/rules/norequireimports.test.ts +++ b/test/files/rules/norequireimports.test.ts @@ -1,6 +1,6 @@ -var lib = require('lib'); // error +var lib = require('lib'); // failure -let lib2 = require('lib2'); // error +let lib2 = require('lib2'); // failure import {l} from 'lib'; @@ -8,10 +8,10 @@ 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 index 7cf074b4c3a..ddfbd468e09 100644 --- 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("", () => { 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); }); });