eslint-plugin-mozilla - npm Package Compare versions

Comparing version 0.2.3 to 0.2.5

lib/globals.js

@@ -73,2 +73,3 @@ /**
   this.path = path;
+  this.root = helpers.getRootDir(path);
 }
@@ -96,4 +97,4 @@
     let isGlobal = helpers.getIsGlobalScope(parents);
-    let name = helpers.convertExpressionToGlobal(node, isGlobal);
-    return name ? [{ name, writable: true}] : [];
+    let names = helpers.convertExpressionToGlobals(node, isGlobal, this.root);
+    return names.map(name => { return { name, writable: true }});
   },
@@ -100,0 +101,0 @@ };

lib/helpers.js

@@ -14,3 +14,7 @@ /**
 var fs = require("fs");
+var ini = require("ini-parser");
 
+var modules = null;
+var directoryManifests = new Map();
+
 var definitions = [
@@ -34,3 +38,3 @@ /^loader\.lazyGetter\(this, "(\w+)"/,
 var imports = [
-  /^(?:Cu|Components\.utils)\.import\(".*\/(.*?)\.jsm?"(?:, this)?\)/,
+  /^(?:Cu|Components\.utils)\.import\(".*\/((.*?)\.jsm?)"(?:, this)?\)/,
 ];
@@ -157,4 +161,4 @@
   /**
-   * Attempts to convert an ExpressionStatement to a likely global variable
-   * definition.
+   * Attempts to convert an ExpressionStatement to likely global variable
+   * definitions.
    *
@@ -164,8 +168,14 @@ * @param  {Object} node
    * @param  {boolean} isGlobal
-   *         True if the current node is in the global scope
+   *         True if the current node is in the global scope.
+   * @param  {String} repository
+   *         The root of the repository.
    *
-   * @return {String or null}
-   *         The variable name defined.
+   * @return {Array}
+   *         An array of variable names defined.
    */
-  convertExpressionToGlobal: function(node, isGlobal) {
+  convertExpressionToGlobals: function(node, isGlobal, repository) {
+    if (!modules) {
+      modules = require(path.join(repository, "tools", "lint", "eslint", "modules.json"));
+    }
+
     try {
@@ -175,3 +185,3 @@ var source = this.getASTSource(node);
     catch (e) {
-      return null;
+      return [];
     }
@@ -184,6 +194,6 @@
         if (!isGlobal) {
-          return null;
+          return [];
         }
 
-        return match[1];
+        return [match[1]];
       }
@@ -197,10 +207,14 @@ }
         if (node.expression.arguments.length > 1 && !isGlobal) {
-          return null;
+          return [];
         }
 
-        return match[1];
+        if (match[1] in modules) {
+          return modules[match[1]];
+        }
+
+        return [match[2]];
       }
     }
 
-    return null;
+    return [];
   },
@@ -315,31 +329,85 @@
   /**
-   * Check whether we might be in an xpcshell test.
+   * Gets the head files for a potential test file
    *
    * @param  {RuleContext} scope
    *         You should pass this from within a rule
-   *         e.g. helpers.getIsXpcshellTest(this)
+   *         e.g. helpers.getIsHeadFile(this)
    *
-   * @return {Boolean}
-   *         True or false
+   * @return {String[]}
+   *         Paths to head files to load for the test
    */
-  getIsXpcshellTest: function(scope) {
-    var pathAndFilename = this.cleanUpPath(scope.getFilename());
+  getTestHeadFiles: function(scope) {
+    if (!this.getIsTest(scope)) {
+      return [];
+    }
 
-    return /.*[\\/]test_.+\.js$/.test(pathAndFilename);
+    let filepath = this.cleanUpPath(scope.getFilename());
+    let dir = path.dirname(filepath);
+
+    let names = fs.readdirSync(dir)
+                  .filter(name => name.startsWith("head") && name.endsWith(".js"))
+                  .map(name => path.join(dir, name));
+    return names;
   },
 
   /**
-   * Check whether we are in a browser mochitest.
+   * Gets all the test manifest data for a directory
    *
+   * @param  {String} dir
+   *         The directory
+   *
+   * @return {Array}
+   *         An array of objects with file and manifest properties
+   */
+  getManifestsForDirectory: function(dir) {
+    if (directoryManifests.has(dir)) {
+      return directoryManifests.get(dir);
+    }
+
+    let manifests = [];
+
+    let names = fs.readdirSync(dir);
+    for (let name of names) {
+      if (!name.endsWith(".ini")) {
+        continue;
+      }
+
+      try {
+        let manifest = ini.parse(fs.readFileSync(path.join(dir, name), 'utf8'));
+
+        manifests.push({
+          file: path.join(dir, name),
+          manifest
+        })
+      } catch (e) {
+      }
+    }
+
+    directoryManifests.set(dir, manifests);
+    return manifests;
+  },
+
+  /**
+   * Gets the manifest file a test is listed in
+   *
    * @param  {RuleContext} scope
    *         You should pass this from within a rule
-   *         e.g. helpers.getIsBrowserMochitest(this)
+   *         e.g. helpers.getIsHeadFile(this)
    *
-   * @return {Boolean}
-   *         True or false
+   * @return {String}
+   *         The path to the test manifest file
    */
-  getIsBrowserMochitest: function(scope) {
-    var pathAndFilename = this.cleanUpPath(scope.getFilename());
+  getTestManifest: function(scope) {
+    let filepath = this.cleanUpPath(scope.getFilename());
 
-    return /.*[\\/]browser_.+\.js$/.test(pathAndFilename);
+    let dir = path.dirname(filepath);
+    let filename = path.basename(filepath);
+
+    for (let manifest of this.getManifestsForDirectory(dir)) {
+      if (filename in manifest.manifest) {
+        return manifest.file;
+      }
+    }
+
+    return null;
   },
@@ -358,19 +426,55 @@
   getIsTest: function(scope) {
-    if (this.getIsXpcshellTest(scope)) {
+    // Regardless of the manifest name being in a manifest means we're a test.
+    let manifest = this.getTestManifest(scope);
+    if (manifest) {
       return true;
     }
 
-    return this.getIsBrowserMochitest(scope);
+    return !!this.getTestType(scope);
   },
 
   /**
+   * Gets the type of test or null if this isn't a test.
+   *
+   * @param  {RuleContext} scope
+   *         You should pass this from within a rule
+   *         e.g. helpers.getIsHeadFile(this)
+   *
+   * @return {String or null}
+   *         Test type: xpcshell, browser, chrome, mochitest
+   */
+  getTestType: function(scope) {
+    let manifest = this.getTestManifest(scope);
+    if (manifest) {
+      let name = path.basename(manifest);
+      for (let testType of ["browser", "xpcshell", "chrome", "mochitest"]) {
+        if (name.startsWith(testType)) {
+          return testType;
+        }
+      }
+    }
+
+    let filepath = this.cleanUpPath(scope.getFilename());
+    let filename = path.basename(filepath);
+
+    if (filename.startsWith("browser_")) {
+      return "browser";
+    }
+
+    if (filename.startsWith("test_")) {
+      return "xpcshell";
+    }
+
+    return null;
+  },
+
+  /**
    * Gets the root directory of the repository by walking up directories until
    * a .eslintignore file is found.
-   * @param {ASTContext} context
-   *        The current context.
+   * @param {String} fileName
+   *        The absolute path of a file in the repository
    *
    * @return {String} The absolute path of the repository directory
    */
-  getRootDir: function(context) {
-    var fileName = this.getAbsoluteFilePath(context);
+  getRootDir: function(fileName) {
     var dirName = path.dirname(fileName);
@@ -377,0 +481,0 @@

lib/rules/.eslintrc.js

@@ -18,23 +18,23 @@ "use strict";
   "rules": {
-    "brace-style": [2, "1tbs"],
-    "camelcase": 2,
-    "comma-dangle": [2, "never"],
-    "comma-spacing": 2,
-    "comma-style": [2, "last"],
-    "curly": [2, "multi-line"],
-    "handle-callback-err": [2, "er"],
-    "indent": [2, 2, {"SwitchCase": 1}],
-    "max-len": [2, 80, 2],
-    "no-multiple-empty-lines": [2, {"max": 1}],
-    "no-undef": 2,
-    "no-undef-init": 2,
-    "no-unexpected-multiline": 2,
-    "object-curly-spacing": 0,
-    "one-var": [2, "never"],
-    "operator-linebreak": [2, "after"],
-    "semi": [2, "always"],
-    "space-before-blocks": 2,
-    "space-before-function-paren": [2, "never"],
-    "keyword-spacing": 2,
-    "strict": [2, "global"],
+    "brace-style": ["error", "1tbs"],
+    "camelcase": "error",
+    "comma-dangle": ["error", "never"],
+    "comma-spacing": "error",
+    "comma-style": ["error", "last"],
+    "curly": ["error", "multi-line"],
+    "handle-callback-err": ["error", "er"],
+    "indent": ["error", 2, {"SwitchCase": 1}],
+    "max-len": ["error", 80, "error"],
+    "no-multiple-empty-lines": ["error", {"max": 1}],
+    "no-undef": "error",
+    "no-undef-init": "error",
+    "no-unexpected-multiline": "error",
+    "object-curly-spacing": "off",
+    "one-var": ["error", "never"],
+    "operator-linebreak": ["error", "after"],
+    "semi": ["error", "always"],
+    "space-before-blocks": "error",
+    "space-before-function-paren": ["error", "never"],
+    "keyword-spacing": "error",
+    "strict": ["error", "global"],
   },
@@ -41,0 +41,0 @@

lib/rules/import-browserjs-globals.js

@@ -58,3 +58,3 @@ /**
     Program: function(node) {
-      if (!helpers.getIsBrowserMochitest(this) &&
+      if (helpers.getTestType(this) != "browser" &&
           !helpers.getIsHeadFile(this)) {
@@ -64,3 +64,4 @@ return;
 
-      let root = helpers.getRootDir(context);
+      let filepath = helpers.getAbsoluteFilePath(context);
+      let root = helpers.getRootDir(filepath);
       for (let script of SCRIPTS) {
@@ -67,0 +68,0 @@ let fileName = path.join(root, script);

lib/rules/import-headjs-globals.js

@@ -43,22 +43,8 @@ /**
     Program: function(node) {
-      if (!helpers.getIsTest(this)) {
-        return;
+      let heads = helpers.getTestHeadFiles(this);
+      for (let head of heads) {
+        importHead(head, node);
       }
-
-      var currentFilePath = helpers.getAbsoluteFilePath(context);
-      var dirName = path.dirname(currentFilePath);
-      importHead(path.resolve(dirName, "head.js"), node);
-
-      if (!helpers.getIsXpcshellTest(this)) {
-        return;
-      }
-
-      let names = fs.readdirSync(dirName);
-      for (let name of names) {
-        if (name.startsWith("head_") && name.endsWith(".js")) {
-          importHead(path.resolve(dirName, name), node);
-        }
-      }
     }
   };
 };

lib/rules/mark-test-function-used.js

 /**
- * @fileoverview Simply marks test (the test method) as used. This avoids ESLint
- * telling us that the function is never called..
+ * @fileoverview Simply marks `test` (the test method) or `run_test` as used when
+ * in mochitests or xpcshell tests respectively. This avoids ESLint telling us
+ * that the function is never called.
  *
@@ -25,9 +26,13 @@ * This Source Code Form is subject to the terms of the Mozilla Public
     Program: function() {
-      if (!helpers.getIsBrowserMochitest(this)) {
+      if (helpers.getTestType(this) == "browser") {
+        context.markVariableAsUsed("test");
         return;
       }
 
-      context.markVariableAsUsed("test");
+      if (helpers.getTestType(this) == "xpcshell") {
+        context.markVariableAsUsed("run_test");
+        return;
+      }
     }
   };
 };

lib/rules/no-cpows-in-tests.js

@@ -71,3 +71,3 @@ /**
     MemberExpression: function(node) {
-      if (!helpers.getIsBrowserMochitest(this)) {
+      if (helpers.getTestType(this) != "browser") {
         return;
@@ -96,3 +96,3 @@ }
     Identifier: function(node) {
-      if (!helpers.getIsBrowserMochitest(this)) {
+      if (helpers.getTestType(this) != "browser") {
         return;
@@ -99,0 +99,0 @@ }

package.json

 {
   "name": "eslint-plugin-mozilla",
-  "version": "0.2.3",
+  "version": "0.2.5",
   "description": "A collection of rules that help enforce JavaScript coding standard in the Mozilla project.",
@@ -22,2 +22,3 @@ "keywords": [
     "estraverse": "^4.2.0",
+    "ini-parser": "^0.0.2",
     "sax": "^1.1.4"
@@ -24,0 +25,0 @@ },

New alerts

Dynamic require

Supply chain risk

Dynamic require can indicate the package is performing dangerous or unsafe dynamic code execution.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

71187

increased by3.87%

Lines of code

1545

increased by5.68%

Worsened metrics

Dependency count

5

increased by25%

Number of low supply chain risk alerts

6

increased by20%

Dependency changes

• + Addedini-parser@^0.0.2

• + Addedini-parser@0.0.2(transitive)