[IMP] web: check for undesirable module dependencies

Previously, we have accidentally made some core modules depend on some
modules outside of the core folder (eg views), this is undesirable as it
can make assets bundle hard to write using globs, and can make it easy
to introduce circular dependencies, it also doesn't make sense from a
code-organisation perspective.

This commit introduces a QUnit test that checks the modules in some
folders only depend on other whitelisted folders.

closes odoo/odoo#129982

Signed-off-by: Aaron Bohy (aab) <aab@odoo.com>

addons/web/__manifest__.py

@@ -620,8 +620,10 @@ This module provides the core of the Odoo Web Client.
         ],
         'web.qunit_suite_tests': [
             'web/static/tests/env_tests.js',
+            'web/static/tests/dependencies_tests.js',
             'web/static/tests/core/**/*.js',
             'web/static/tests/search/**/*.js',
+            'web/static/tests/model/**/*.js',
             ('remove', 'web/static/tests/search/helpers.js'),
             'web/static/tests/views/**/*.js',
             ('remove', 'web/static/tests/views/helpers.js'),

addons/web/static/src/boot.js

@@ -49,18 +49,17 @@
     odoo.__DEBUG__ = {
         didLogInfo: didLogInfoPromise,
         getDependencies: function (name, transitive) {
-            var deps = name instanceof Array ? name : [name];
-            var changed;
+            const deps = new Set();
+            let queue = [name];
             do {
-                changed = false;
-                jobDeps.forEach(function (dep) {
-                    if (deps.indexOf(dep.to) >= 0 && deps.indexOf(dep.from) < 0) {
-                        deps.push(dep.from);
-                        changed = true;
-                    }
-                });
-            } while (changed && transitive);
-            return deps;
+                queue = queue.flatMap((job) =>
+                    jobDeps.filter((dep) => dep.to === job).map((dep) => dep.from)
+                );
+                for (const dep of queue) {
+                    deps.add(dep);
+                }
+            } while (queue.length && transitive);
+            return [...deps];
         },
         getDependents: function (name) {
             return jobDeps

addons/web/static/tests/dependencies_tests.js

@@ -0,0 +1,65 @@
+/** @odoo-module **/
+
+/**
+ * @param {string} folder folder that can only import from `allowedFolders`
+ * @param {string[]} allowedFolders folders from which `folder` can import
+ * @returns {{[key: string]: string[]}} an object where the keys are modules and
+ *  the values are an array of imports that the module is not allowed to import
+ */
+function invalidImportsFrom(folder, allowedFolders) {
+    // modules within a folder can always depend on one another
+    allowedFolders.push(folder);
+    const modulesToCheck = Object.keys(odoo.__DEBUG__.services).filter((module) =>
+        module.startsWith(`@web/${folder}/`)
+    );
+    const invalidDeps = {};
+    for (const module of modulesToCheck) {
+        const invalid = odoo.__DEBUG__.getDependencies(module).filter((dep) => {
+            // owl and @web/session are allowed everywhere
+            if (dep === "@odoo/owl" || dep === "@web/session") {
+                return false;
+            }
+            return !allowedFolders.some((allowed) => dep.startsWith(`@web/${allowed}/`));
+        });
+        if (invalid.length) {
+            invalidDeps[module] = invalid;
+        }
+    }
+    return invalidDeps;
+}
+
+QUnit.module("module dependencies", {}, () => {
+    QUnit.test("modules only import from allowed folders", (assert) => {
+        // Using deepEqual with {} in this test to get a good diff view when failing
+        assert.deepEqual(
+            invalidImportsFrom("core", []),
+            // FIXME: this dependency should not exist. Temporarily whitelist it so we don't add more, and remove ASAP
+            { "@web/core/utils/hooks": ["@web/env"] },
+            "Core modules don't depend on any other folder"
+        );
+        assert.deepEqual(
+            invalidImportsFrom("search", ["core"]),
+            // FIXME: this dependency should not exist. Temporarily whitelist it so we don't add more, and remove ASAP
+            { "@web/search/with_search/with_search": ["@web/webclient/actions/action_hook"] },
+            "Search modules only depend on core"
+        );
+        assert.deepEqual(
+            invalidImportsFrom("model", ["core", "search"]),
+            // FIXME: this dependency should not exist. Temporarily whitelist it so we don't add more, and remove ASAP
+            { "@web/model/model": ["@web/views/view_hook"] },
+            "Model modules only depend on core and search"
+        );
+        assert.deepEqual(
+            invalidImportsFrom("views", ["core", "search", "model"]),
+            // FIXME: these dependencies should not exist. Temporarily whitelist them so we don't add more, and remove ASAP
+            {
+                "@web/views/fields/many2one/many2one_field": [
+                    "@web/webclient/barcode/barcode_scanner",
+                ],
+                "@web/views/pivot/pivot_renderer": ["@web/legacy/js/fields/field_utils"],
+                "@web/views/view_hook": ["@web/webclient/actions/action_hook"],
+            },
+            "View modules only depend on core, search and model"
+        );
+    });
+});