diff --git a/package.json b/package.json index 9ffebad198..a9069d4535 100644 --- a/package.json +++ b/package.json @@ -35,7 +35,7 @@ "browserify": "13.1.0", "closure-util": "1.15.0", "derequire": "2.0.3", - "eslint-plugin-openlayers-internal": "file:test/rules", + "eslint-plugin-openlayers-internal": "file:rules", "fs-extra": "0.30.0", "glob": "7.0.5", "handlebars": "4.0.5", @@ -96,7 +96,10 @@ } ], "no-constant-condition": 0, - "openlayers-internal/no-unused-requires": 2 + "openlayers-internal/no-duplicate-requires": 2, + "openlayers-internal/no-unused-requires": 2, + "openlayers-internal/requires-first": 2, + "openlayers-internal/valid-requires": 2 } }, "ext": [ diff --git a/test/rules/.eslintrc b/rules/.eslintrc similarity index 100% rename from test/rules/.eslintrc rename to rules/.eslintrc diff --git a/rules/index.js b/rules/index.js new file mode 100644 index 0000000000..2658bc9c98 --- /dev/null +++ b/rules/index.js @@ -0,0 +1,8 @@ +module.exports = { + rules: { + 'no-duplicate-requires': require('./no-duplicate-requires').rule, + 'no-unused-requires': require('./no-unused-requires').rule, + 'requires-first': require('./requires-first').rule, + 'valid-requires': require('./valid-requires').rule + } +}; diff --git a/rules/no-duplicate-requires.js b/rules/no-duplicate-requires.js new file mode 100644 index 0000000000..7574572a29 --- /dev/null +++ b/rules/no-duplicate-requires.js @@ -0,0 +1,46 @@ +const util = require('./util'); + +exports.rule = { + meta: { + docs: { + description: 'disallow duplicate goog.require() calls' + }, + fixable: 'code' + }, + + create: function(context) { + const alreadyRequired = {}; + + return { + ExpressionStatement: function(statement) { + if (util.isRequireStatement(statement)) { + const expression = statement.expression; + + if (!expression.arguments[0]) { + return; + } + const name = expression.arguments[0].value; + + if (alreadyRequired[name]) { + const source = context.getSourceCode(); + + return context.report({ + node: statement, + message: `Duplicate goog.require('${name}')`, + fix: function(fixer) { + const afterToken = source.getTokenAfter(statement); + const range = [ + statement.range[0], + afterToken ? afterToken.range[0] : statement.range[1] + ]; + return fixer.removeRange(range); + } + }); + } + alreadyRequired[name] = true; + } + } + }; + } + +}; diff --git a/test/rules/no-unused-requires.js b/rules/no-unused-requires.js similarity index 56% rename from test/rules/no-unused-requires.js rename to rules/no-unused-requires.js index 1606d68ad4..a5ba7d272c 100644 --- a/test/rules/no-unused-requires.js +++ b/rules/no-unused-requires.js @@ -1,9 +1,4 @@ -function isGoogRequire(node) { - const callee = node.callee; - return callee.type === 'MemberExpression' && - callee.object.type === 'Identifier' && callee.object.name === 'goog' && - callee.property.type === 'Identifier' && !callee.property.computed && callee.property.name === 'require'; -} +const util = require('./util'); function getName(node) { if (node.type !== 'MemberExpression') { @@ -29,50 +24,42 @@ exports.rule = { docs: { description: 'disallow unused goog.require() calls' }, - fixable: 'code', - schema: [] + fixable: 'code' }, + create: function(context) { // a lookup of goog.require() nodes by argument - const requireNodes = {}; + const requireStatements = {}; // used names from member expressions that match the goog.require() arg const usedNames = {}; return { - CallExpression: function(node) { + ExpressionStatement: function(statement) { + if (util.isRequireStatement(statement)) { + const expression = statement.expression; + const arg = expression.arguments[0]; + if (!arg || !arg.value) { + return; + } - if (isGoogRequire(node)) { - if (node.arguments.length !== 1) { - return context.report(node, 'Expected one argument for goog.require()'); - } - const arg = node.arguments[0]; - if (arg.type !== 'Literal' || !arg.value || typeof arg.value !== 'string') { - return context.report(node, 'Expected goog.require() to be called with a string'); - } const name = arg.value; - if (name in requireNodes) { - return context.report(node, `Duplicate goog.require('${name}')`); - } const ancestors = context.getAncestors(); const parent = ancestors[ancestors.length - 1]; - if (!parent || parent.type !== 'ExpressionStatement') { - return context.report(node, 'Expected goog.require() to be in an expression statement'); + if (!parent) { + return; } - requireNodes[name] = { - expression: node, - statement: parent - }; - } + requireStatements[name] = statement; + } }, MemberExpression: function(node) { const name = getName(node); - if (name in requireNodes) { - const requiredAncestor = context.getAncestors().some(ancestorNode => !!requireNodes[getName(ancestorNode)]); + if (name in requireStatements) { + const requiredAncestor = context.getAncestors().some(ancestorNode => !!requireStatements[getName(ancestorNode)]); if (!requiredAncestor) { usedNames[name] = true; } @@ -81,10 +68,10 @@ exports.rule = { Identifier: function(node) { const name = node.name; - if (name in requireNodes) { + if (name in requireStatements) { const ancestors = context.getAncestors(); if (ancestors.length && ancestors[0].type === 'MemberExpression') { - const requiredAncestor = context.getAncestors().some(ancestorNode => !!requireNodes[getName(ancestorNode)]); + const requiredAncestor = context.getAncestors().some(ancestorNode => !!requireStatements[getName(ancestorNode)]); if (!requiredAncestor) { usedNames[name] = true; } @@ -96,21 +83,25 @@ exports.rule = { 'Program:exit': function(node) { const source = context.getSourceCode(); - for (let name in requireNodes) { + + for (let name in requireStatements) { + if (!usedNames[name]) { - const unusedRequire = requireNodes[name]; + const unusedRequire = requireStatements[name]; + context.report({ - node: unusedRequire.expression, - message: `Unused ${source.getText(unusedRequire.expression)}`, + node: unusedRequire, + message: `Unused ${source.getText(unusedRequire)}`, fix: function(fixer) { - const afterToken = source.getTokenAfter(unusedRequire.statement); + const afterToken = source.getTokenAfter(unusedRequire); const range = [ - unusedRequire.statement.range[0], - afterToken ? afterToken.range[0] : unusedRequire.statement.range[1] + unusedRequire.range[0], + afterToken ? afterToken.range[0] : unusedRequire.range[1] ]; return fixer.removeRange(range); } }); + } } } diff --git a/test/rules/package.json b/rules/package.json similarity index 100% rename from test/rules/package.json rename to rules/package.json diff --git a/rules/requires-first.js b/rules/requires-first.js new file mode 100644 index 0000000000..0641019c52 --- /dev/null +++ b/rules/requires-first.js @@ -0,0 +1,28 @@ +const util = require('./util'); + +exports.rule = { + meta: { + docs: { + description: 'require that all goog.require() precede other statements (except goog.provide())' + } + }, + + create: function(context) { + return { + Program: function(program) { + let otherSeen = false; + + program.body.forEach(statement => { + if (util.isRequireStatement(statement)) { + if (otherSeen) { + return context.report(statement, 'Expected goog.require() to precede other statements'); + } + } else if (!util.isProvideStatement(statement)) { + otherSeen = true; + } + }); + + } + }; + } +}; diff --git a/rules/util.js b/rules/util.js new file mode 100644 index 0000000000..00b3fefbb5 --- /dev/null +++ b/rules/util.js @@ -0,0 +1,28 @@ +function isGoogCallExpression(node, name) { + const callee = node.callee; + return callee && callee.type === 'MemberExpression' && + callee.object.type === 'Identifier' && callee.object.name === 'goog' && + callee.property.type === 'Identifier' && !callee.property.computed && + callee.property.name === name; +} + +function isGoogStatement(node, name) { + return node.expression && node.expression.type === 'CallExpression' && + isGoogCallExpression(node.expression, name); +} + +exports.isProvideExpression = function(node) { + return isGoogCallExpression(node, 'provide'); +}; + +exports.isProvideStatement = function(node) { + return isGoogStatement(node, 'provide'); +}; + +exports.isRequireExpression = function(node) { + return isGoogCallExpression(node, 'require'); +}; + +exports.isRequireStatement = function(node) { + return isGoogStatement(node, 'require'); +}; diff --git a/rules/valid-requires.js b/rules/valid-requires.js new file mode 100644 index 0000000000..b7f7479df0 --- /dev/null +++ b/rules/valid-requires.js @@ -0,0 +1,36 @@ +const util = require('./util'); + +exports.rule = { + meta: { + docs: { + description: 'require that all goog.require() have a valid arg and appear at the top level' + } + }, + + create: function(context) { + return { + CallExpression: function(expression) { + if (util.isRequireExpression(expression)) { + const ancestors = context.getAncestors(); + const parent = ancestors[ancestors.length - 1]; + if (parent.type !== 'ExpressionStatement') { + return context.report(expression, 'Expected goog.require() to in an expression statement'); + } + + if (ancestors.length !== 2) { + return context.report(expression, 'Expected goog.require() to be at the top level'); + } + + if (expression.arguments.length !== 1) { + return context.report(expression, 'Expected one argument for goog.require()'); + } + + const arg = expression.arguments[0]; + if (arg.type !== 'Literal' || !arg.value || typeof arg.value !== 'string') { + return context.report(expression, 'Expected goog.require() to be called with a string'); + } + } + } + }; + } +}; diff --git a/test/rules/index.js b/test/rules/index.js deleted file mode 100644 index 857d652fda..0000000000 --- a/test/rules/index.js +++ /dev/null @@ -1,5 +0,0 @@ -module.exports = { - rules: { - 'no-unused-requires': require('./no-unused-requires').rule - } -};