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
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-cpows-in-tests.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-cpows-in-tests.js
@@ -9,22 +9,24 @@
"use strict";
// -----------------------------------------------------------------------------
// Rule Definition
// -----------------------------------------------------------------------------
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;
module.exports = function(context) {
// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------
@@ -81,32 +83,60 @@ module.exports = function(context) {
showError(node, expression);
return true;
}
return false;
});
if (!someCpowFound && helpers.getIsGlobalScope(context.getAncestors())) {
if (/^content\./.test(expression)) {
showError(node, expression);
-
}
}
},
Identifier(node) {
if (helpers.getTestType(context) != "browser") {
return;
}
+ 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);
- 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);
-
- }
+ showError(node, expression);
}
};
};
--- a/tools/lint/eslint/eslint-plugin-mozilla/tests/no-cpows-in-tests.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/tests/no-cpows-in-tests.js
@@ -15,28 +15,34 @@ const ruleTester = new RuleTester({ pars
// ------------------------------------------------------------------------------
// Tests
// ------------------------------------------------------------------------------
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")
]
});