Bug 1479515 - Add an ESLint rule to check for valid property accesses on Services. r=Gijs

This is manually run and requires a full build. MOZ_OBJDIR needs to be set to point to the object directory.

Differential Revision: https://phabricator.services.mozilla.com/D156426

Author: Standard8

devtools/client/shared/test-helpers/jest-fixtures/Services.js

@@ -552,6 +552,7 @@ const Services = {
 };
 
 function pref(name, value) {
+  // eslint-disable-next-line mozilla/valid-services-property
   const thePref = Services.prefs._findOrCreatePref(name, value, true, value);
   thePref._setDefault(value);
 }

docs/code-quality/lint/linters/eslint-plugin-mozilla.rst

@@ -74,6 +74,8 @@ The plugin implements the following rules:
    eslint-plugin-mozilla/use-services
    eslint-plugin-mozilla/valid-ci-uses
    eslint-plugin-mozilla/valid-lazy
+   eslint-plugin-mozilla/valid-services
+   eslint-plugin-mozilla/valid-services-property
    eslint-plugin-mozilla/var-only-at-top-level
 
 Tests

docs/code-quality/lint/linters/eslint-plugin-mozilla/valid-services-property.rst

@@ -0,0 +1,30 @@
+valid-services-property
+=======================
+
+Ensures that accesses of properties of items accessed via the ``Services``
+object are valid.
+
+This rule requires a full build to run, and is not turned on by default. To run
+this rule manually, use:
+
+.. code-block:: console
+
+    MOZ_OBJDIR=objdir-ff-opt ./mach eslint --rule="mozilla/valid-services-property: error" *
+
+Examples of incorrect code for this rule:
+-----------------------------------------
+
+Assuming ``foo`` is not defined within ``Ci.nsISearchService``.
+
+.. code-block:: js
+
+    Services.search.foo();
+
+Examples of correct code for this rule:
+---------------------------------------
+
+Assuming ``bar`` is defined within ``Ci.nsISearchService``.
+
+.. code-block:: js
+
+    Services.search.bar();

tools/lint/eslint/eslint-plugin-mozilla/lib/index.js

@@ -85,6 +85,7 @@ module.exports = {
     "valid-ci-uses": require("../lib/rules/valid-ci-uses"),
     "valid-lazy": require("../lib/rules/valid-lazy"),
     "valid-services": require("../lib/rules/valid-services"),
+    "valid-services-property": require("../lib/rules/valid-services-property"),
     "var-only-at-top-level": require("../lib/rules/var-only-at-top-level"),
   },
 };

tools/lint/eslint/eslint-plugin-mozilla/lib/rules/valid-services-property.js

@@ -0,0 +1,126 @@
+/**
+ * @fileoverview Ensures that property accesses on Services.<alias> are valid.
+ * Although this largely duplicates the valid-services rule, the checks here
+ * require an objdir and a manual run.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+"use strict";
+
+const helpers = require("../helpers");
+
+function findInterfaceNames(name) {
+  let interfaces = [];
+  for (let [key, value] of Object.entries(helpers.servicesData)) {
+    if (value == name) {
+      interfaces.push(key);
+    }
+  }
+  return interfaces;
+}
+
+function isInInterface(interfaceName, name) {
+  let interfaceDetails = helpers.xpidlData.get(interfaceName);
+
+  // TODO: Bug 1790261 - check only methods if the expression is callable.
+  if (interfaceDetails.methods.some(m => m.name == name)) {
+    return true;
+  }
+
+  if (interfaceDetails.consts.some(c => c.name == name)) {
+    return true;
+  }
+
+  if (interfaceDetails.parent) {
+    return isInInterface(interfaceDetails.parent, name);
+  }
+  return false;
+}
+
+module.exports = {
+  meta: {
+    docs: {
+      url:
+        "https://firefox-source-docs.mozilla.org/code-quality/lint/linters/eslint-plugin-mozilla/valid-services-property.html",
+    },
+    messages: {
+      unknownProperty:
+        "Unknown property access Services.{{ alias }}.{{ propertyName }}, Interfaces: {{ checkedInterfaces }}",
+    },
+    type: "problem",
+  },
+
+  create(context) {
+    let servicesInterfaceMap = helpers.servicesData;
+    let serviceAliases = new Set([
+      ...Object.values(servicesInterfaceMap),
+      // This is defined only for Android, so most builds won't pick it up.
+      "androidBridge",
+      // These are defined without interfaces and hence are not in the services map.
+      "cpmm",
+      "crashmanager",
+      "mm",
+      "ppmm",
+      // The new xulStore also does not have an interface.
+      "xulStore",
+    ]);
+    return {
+      MemberExpression(node) {
+        if (node.computed || node.object.type !== "Identifier") {
+          return;
+        }
+
+        let mainNode;
+        if (node.object.name == "Services") {
+          mainNode = node;
+        } else if (
+          node.property.name == "Services" &&
+          node.parent.type == "MemberExpression"
+        ) {
+          mainNode = node.parent;
+        } else {
+          return;
+        }
+
+        let alias = mainNode.property.name;
+        if (!serviceAliases.has(alias)) {
+          return;
+        }
+
+        if (
+          mainNode.parent.type == "MemberExpression" &&
+          !mainNode.parent.computed
+        ) {
+          let propertyName = mainNode.parent.property.name;
+          if (propertyName == "wrappedJSObject") {
+            return;
+          }
+          let interfaces = findInterfaceNames(alias);
+          if (!interfaces.length) {
+            return;
+          }
+
+          let checkedInterfaces = [];
+          for (let item of interfaces) {
+            if (isInInterface(item, propertyName)) {
+              return;
+            }
+            checkedInterfaces.push(item);
+          }
+          context.report({
+            node,
+            messageId: "unknownProperty",
+            data: {
+              alias,
+              propertyName,
+              checkedInterfaces,
+            },
+          });
+        }
+      },
+    };
+  },
+};

tools/lint/eslint/eslint-plugin-mozilla/lib/rules/valid-services.js

@@ -1,5 +1,5 @@
 /**
- * @fileoverview Require use of Services.* rather than getService.
+ * @fileoverview Ensures that Services uses have valid property names.
  *
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -13,7 +13,7 @@ module.exports = {
   meta: {
     docs: {
       url:
-        "https://firefox-source-docs.mozilla.org/code-quality/lint/linters/eslint-plugin-mozilla/use-services.html",
+        "https://firefox-source-docs.mozilla.org/code-quality/lint/linters/eslint-plugin-mozilla/valid-services.html",
     },
     type: "problem",
   },

tools/lint/eslint/eslint-plugin-mozilla/tests/valid-services-property.js

@@ -0,0 +1,42 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+var rule = require("../lib/rules/valid-services-property");
+var RuleTester = require("eslint").RuleTester;
+
+const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 13 } });
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+function invalidCode(code, messageId, data) {
+  return { code, errors: [{ messageId, data }] };
+}
+
+process.env.TEST_XPIDLDIR = `${__dirname}/xpidl`;
+
+ruleTester.run("valid-services-property", rule, {
+  valid: [
+    "Services.uriFixup.keywordToURI()",
+    "Services.uriFixup.FIXUP_FLAG_NONE",
+  ],
+  invalid: [
+    invalidCode("Services.uriFixup.UNKNOWN_CONSTANT", "unknownProperty", {
+      alias: "uriFixup",
+      propertyName: "UNKNOWN_CONSTANT",
+      checkedInterfaces: ["nsIURIFixup"],
+    }),
+    invalidCode("Services.uriFixup.foo()", "unknownProperty", {
+      alias: "uriFixup",
+      propertyName: "foo",
+      checkedInterfaces: ["nsIURIFixup"],
+    }),
+  ],
+});