Bug 1246896 - Improve detection for mozilla/no-cpows-in-tests.js 'content' - only notify when content is used as an identifier. r=florian

MozReview-Commit-ID: 1X38Wq4hUmZ

--HG--
extra : rebase_source : 6d7b0e8c9048045b47af02590455e060411953e3

tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-cpows-in-tests.js

@@ -14,12 +14,14 @@
 
 var helpers = require("../helpers");
 
+// Note: we match to the end of the string as well as the beginning, to avoid
+// multiple reports from MemberExpression statements.
 var cpows = [
-  /^gBrowser\.contentWindow/,
-  /^gBrowser\.contentDocument/,
-  /^gBrowser\.selectedBrowser.contentWindow/,
-  /^browser\.contentDocument/,
-  /^window\.content/
+  /^gBrowser\.contentWindow$/,
+  /^gBrowser\.contentDocument$/,
+  /^gBrowser\.selectedBrowser\.contentWindow$/,
+  /^browser\.contentDocument$/,
+  /^window\.content$/
 ];
 
 var isInContentTask = false;
@@ -86,7 +88,6 @@ module.exports = function(context) {
       if (!someCpowFound && helpers.getIsGlobalScope(context.getAncestors())) {
         if (/^content\./.test(expression)) {
           showError(node, expression);
-
         }
       }
     },
@@ -96,17 +97,46 @@ module.exports = function(context) {
         return;
       }
 
-      var expression = context.getSource(node);
-      if (expression == "content" || /^content\./.test(expression)) {
-        if (node.parent.type === "MemberExpression" &&
-            node.parent.object &&
-            node.parent.object.type === "Identifier" &&
-            node.parent.object.name != "content") {
-          return;
-        }
-        showError(node, expression);
-
+      if (node.name !== "content" ||
+          // Don't complain if this is part of a member expression - the
+          // MemberExpression() function will handle those.
+          node.parent.type === "MemberExpression" ||
+          // If this is a declared variable in a function, then don't complain.
+          node.parent.type === "FunctionDeclaration") {
+        return;
       }
+
+      // Don't error in the case of `let content = foo`.
+      if (node.parent.type === "VariableDeclarator" &&
+          node.parent.id && node.parent.id.name === "content") {
+        return;
+      }
+
+      // Walk up the parents, see if we can find if this is a local variable.
+      let parent = node;
+      do {
+        parent = parent.parent;
+
+        // Don't error if 'content' is one of the function parameters.
+        if (parent.type === "FunctionDeclaration" &&
+            context.getDeclaredVariables(parent).some(variable => variable.name === "content")) {
+          return;
+        } else if (parent.type === "BlockStatement" || parent.type === "Program") {
+          // Don't error if the block or program includes their own definition of content.
+          for (let item of parent.body) {
+            if (item.type === "VariableDeclaration" && item.declarations.length) {
+              for (let declaration of item.declarations) {
+                if (declaration.id && declaration.id.name === "content") {
+                  return;
+                }
+              }
+            }
+          }
+        }
+      } while (parent.parent);
+
+      var expression = context.getSource(node);
+      showError(node, expression);
     }
   };
 };

tools/lint/eslint/eslint-plugin-mozilla/tests/no-cpows-in-tests.js

@@ -20,23 +20,29 @@ function wrapCode(code, filename = "browser_fake.js") {
   return {code, filename};
 }
 
-function invalidCode(code, item) {
+function invalidCode(code, item, type = "MemberExpression") {
   let message = `${item} is a possible Cross Process Object Wrapper (CPOW).`;
   let obj = wrapCode(code);
-  obj.errors = [{message, type: "MemberExpression"}];
+  obj.errors = [{message, type}];
   return obj;
 }
 
 ruleTester.run("no-cpows-in-tests", rule, {
   valid: [
     "window.document",
-    wrapCode("ContentTask.spawn(browser, null, () => { content.document; });")
+    wrapCode("ContentTask.spawn(browser, null, () => { content.document; });"),
+    wrapCode("let x = cssDocs.tooltip.content.contentDocument;"),
+    wrapCode("function test(content) { let x = content; }"),
+    wrapCode('let content = "content"; foo(content);')
   ],
   invalid: [
     invalidCode("let x = gBrowser.contentWindow;", "gBrowser.contentWindow"),
     invalidCode("let x = gBrowser.contentDocument;", "gBrowser.contentDocument"),
     invalidCode("let x = gBrowser.selectedBrowser.contentWindow;", "gBrowser.selectedBrowser.contentWindow"),
     invalidCode("let x = browser.contentDocument;", "browser.contentDocument"),
-    invalidCode("let x = window.content;", "window.content")
+    invalidCode("let x = window.content;", "window.content"),
+    invalidCode("content.document;", "content.document"),
+    invalidCode("let x = content;", "content", "Identifier"),
+    invalidCode("let x = gBrowser.contentWindow.wrappedJSObject", "gBrowser.contentWindow")
   ]
 });