From 654dc104ce0bb163bb45cd8c6e4315485a8e3ecf Mon Sep 17 00:00:00 2001 From: Xiaoji Chen Date: Sun, 4 Aug 2019 11:46:32 -0700 Subject: [PATCH] unbreak try catch use case --- src/rules/no-commonjs.js | 19 ++++++++++++++----- tests/src/rules/no-commonjs.js | 10 ++++------ 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/rules/no-commonjs.js b/src/rules/no-commonjs.js index 1c3f4cff8..892e855ea 100644 --- a/src/rules/no-commonjs.js +++ b/src/rules/no-commonjs.js @@ -30,6 +30,18 @@ function validateScope(scope) { return false } +// https://github.com/estree/estree/blob/master/es5.md +function isConditional(node) { + if ( + node.type === 'IfStatement' + || node.type === 'TryStatement' + || node.type === 'LogicalExpression' + || node.type === 'ConditionalExpression' + ) return true + if (node.parent) return isConditional(node.parent) + return false +} + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -93,11 +105,6 @@ module.exports = { }, 'CallExpression': function (call) { if (!validateScope(context.getScope())) return - if ( - call.parent.type !== 'ExpressionStatement' - && call.parent.type !== 'VariableDeclarator' - && call.parent.type !== 'AssignmentExpression' - ) return if (call.callee.type !== 'Identifier') return if (call.callee.name !== 'require') return @@ -110,6 +117,8 @@ module.exports = { if (allowRequire(call, options)) return + if (isConditional(call.parent)) return + // keeping it simple: all 1-string-arg `require` calls are reported context.report({ node: call.callee, diff --git a/tests/src/rules/no-commonjs.js b/tests/src/rules/no-commonjs.js index db7414395..b1ee137f4 100644 --- a/tests/src/rules/no-commonjs.js +++ b/tests/src/rules/no-commonjs.js @@ -58,7 +58,11 @@ ruleTester.run('no-commonjs', require('rules/no-commonjs'), { { code: 'module.exports = "foo"', options: [{ allowPrimitiveModules: true }] }, { code: 'if (typeof window !== "undefined") require("x")', options: [{ allowRequire: true }] }, + { code: 'if (typeof window !== "undefined") require("x")', options: [{ allowRequire: false }] }, { code: 'if (typeof window !== "undefined") { require("x") }', options: [{ allowRequire: true }] }, + { code: 'if (typeof window !== "undefined") { require("x") }', options: [{ allowRequire: false }] }, + + { code: 'try { require("x") } catch (error) {}' }, ], invalid: [ @@ -68,12 +72,6 @@ ruleTester.run('no-commonjs', require('rules/no-commonjs'), { { code: 'var x = require("x")', errors: [ { message: IMPORT_MESSAGE }] }, { code: 'x = require("x")', errors: [ { message: IMPORT_MESSAGE }] }, { code: 'require("x")', errors: [ { message: IMPORT_MESSAGE }] }, - { code: 'if (typeof window !== "undefined") require("x")', - errors: [ { message: IMPORT_MESSAGE }], - }, - { code: 'if (typeof window !== "undefined") { require("x") }', - errors: [ { message: IMPORT_MESSAGE }], - }, ]), // exports