Bug 1877484 - Allow conditional lazy getter definitions in if-then-else. r=Standard8,devtools-reviewers,sync-reviewers,ochameau

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

devtools/server/actors/watcher/SessionDataHelpers.jsm

@@ -31,15 +31,11 @@ if (typeof module == "object") {
     true
   );
 } else {
-  // Ignore the "duplicate" definitions here as this are also defined
-  // in the if block above.
-  // eslint-disable-next-line mozilla/valid-lazy
   ChromeUtils.defineLazyGetter(lazy, "validateBreakpointLocation", () => {
     return ChromeUtils.import(
       "resource://devtools/shared/validate-breakpoint.jsm"
     ).validateBreakpointLocation;
   });
-  // eslint-disable-next-line mozilla/valid-lazy
   ChromeUtils.defineLazyGetter(lazy, "validateEventBreakpoint", () => {
     const { loader } = ChromeUtils.importESModule(
       "resource://devtools/shared/loader/Loader.sys.mjs"

services/sync/modules/main.sys.mjs

@@ -8,7 +8,6 @@ const lazy = {};
 
 // We want these to be lazily loaded, which helps performance and also tests
 // to not have these loaded before they are ready.
-// eslint-disable-next-line mozilla/valid-lazy
 ChromeUtils.defineESModuleGetters(lazy, {
   Service: "resource://services-sync/service.sys.mjs",
   Status: "resource://services-sync/status.sys.mjs",
@@ -16,7 +15,6 @@ ChromeUtils.defineESModuleGetters(lazy, {
   Utils: "resource://services-sync/util.sys.mjs",
 });
 
-// eslint-disable-next-line mozilla/valid-lazy
 ChromeUtils.defineLazyGetter(lazy, "Crypto", () => {
   let { WeaveCrypto } = ChromeUtils.importESModule(
     "resource://services-crypto/WeaveCrypto.sys.mjs"

toolkit/modules/subprocess/Subprocess.sys.mjs

@@ -20,9 +20,6 @@ if (AppConstants.platform == "win") {
     SubprocessImpl: "resource://gre/modules/subprocess/subprocess_win.sys.mjs",
   });
 } else {
-  // Ignore the "duplicate" definitions here as this are also defined
-  // in the "win" block above.
-  // eslint-disable-next-line mozilla/valid-lazy
   ChromeUtils.defineESModuleGetters(lazy, {
     SubprocessImpl: "resource://gre/modules/subprocess/subprocess_unix.sys.mjs",
   });

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

@@ -67,8 +67,46 @@ module.exports = {
     let unknownProperties = [];
     let isLazyExported = false;
 
+    function getAncestorNodes(node) {
+      const ancestors = [];
+      node = node.parent;
+      while (node) {
+        ancestors.unshift(node);
+        node = node.parent;
+      }
+      return ancestors;
+    }
+
+    // Returns true if lazy getter definitions in prevNode and currNode are
+    // duplicate.
+    // This returns false if prevNode and currNode have the same IfStatement as
+    // ancestor and they're in different branches.
+    function isDuplicate(prevNode, currNode) {
+      const prevAncestors = getAncestorNodes(prevNode);
+      const currAncestors = getAncestorNodes(currNode);
+
+      for (
+        let i = 0;
+        i < prevAncestors.length && i < currAncestors.length;
+        i++
+      ) {
+        const prev = prevAncestors[i];
+        const curr = currAncestors[i];
+        if (prev === curr && prev.type === "IfStatement") {
+          if (prevAncestors[i + 1] !== currAncestors[i + 1]) {
+            return false;
+          }
+        }
+      }
+
+      return true;
+    }
+
     function addProp(node, name) {
-      if (lazyProperties.has(name)) {
+      if (
+        lazyProperties.has(name) &&
+        isDuplicate(lazyProperties.get(name).node, node)
+      ) {
         context.report({
           node,
           messageId: "duplicateSymbol",
@@ -135,7 +173,10 @@ module.exports = {
         for (let reg of callExpressionDefinitions) {
           let match = source.match(reg);
           if (match) {
-            if (lazyProperties.has(match[1])) {
+            if (
+              lazyProperties.has(match[1]) &&
+              isDuplicate(lazyProperties.get(match[1]).node, node)
+            ) {
               context.report({
                 node,
                 messageId: "duplicateSymbol",

tools/lint/eslint/eslint-plugin-mozilla/tests/valid-lazy.js

@@ -102,6 +102,15 @@ ruleTester.run("valid-lazy", rule, {
        ChromeUtils.defineLazyGetter(lazy, "foo", () => {});
        export { lazy as Foo };
      `,
+    `
+       const lazy = {};
+       if (cond) {
+         ChromeUtils.defineLazyGetter(lazy, "foo", () => { return 1; });
+       } else {
+         ChromeUtils.defineLazyGetter(lazy, "foo", () => { return 2; });
+       }
+       if (x) { lazy.foo; }
+    `,
   ],
   invalid: [
     invalidCode("if (x) { lazy.bar; }", "bar", "unknownProperty"),