Refactored rules

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": [

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
+  }
+};

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;
+        }
+      }
+    };
+  }
+
+};

rules/no-unused-requires.js

@@ -1,9 +1,4 @@
-function isGoogRequire(node) {
+const util = require('./util');
-  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';
-}
 
 function getName(node) {
   if (node.type !== 'MemberExpression') {
@@ -29,50 +24,42 @@ exports.rule = {
     docs: {
       description: 'disallow unused goog.require() calls'
     },
-    fixable: 'code',
+    fixable: 'code'
-    schema: []
   },
+
   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') {
+          if (!parent) {
-            return context.report(node, 'Expected goog.require() to be in an expression statement');
+            return;
           }
-          requireNodes[name] = {
-            expression: node,
-            statement: parent
-          };
-        }
 
+          requireStatements[name] = statement;
+        }
       },
 
       MemberExpression: function(node) {
         const name = getName(node);
-        if (name in requireNodes) {
+        if (name in requireStatements) {
-          const requiredAncestor = context.getAncestors().some(ancestorNode => !!requireNodes[getName(ancestorNode)]);
+          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,
+              node: unusedRequire,
-              message: `Unused ${source.getText(unusedRequire.expression)}`,
+              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],
+                  unusedRequire.range[0],
-                  afterToken ? afterToken.range[0] : unusedRequire.statement.range[1]
+                  afterToken ? afterToken.range[0] : unusedRequire.range[1]
                 ];
                 return fixer.removeRange(range);
               }
             });
+
           }
         }
       }

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;
+          }
+        });
+
+      }
+    };
+  }
+};

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');
+};

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');
+          }
+        }
+      }
+    };
+  }
+};

test/rules/index.js

@@ -1,5 +0,0 @@
-module.exports = {
-  rules: {
-    'no-unused-requires': require('./no-unused-requires').rule
-  }
-};