feat(sfcc): forbid legacy Rhino import globals

Author: jenssimon

README.md

@@ -84,14 +84,15 @@ That rule stays enabled in the recommended config by default, because it is stil
 
 The new `sfcc` plugin contains the general Rhino/SFCC runtime rules:
 
-| Rule                        | Description                                                                                                                                                                                                  | Default |
-| --------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------- |
-| `sfcc/no-e4x-syntax`        | Disallows JSX/E4X-like tag syntax (e.g. `<a/>`) in SFCC JavaScript to avoid parser ambiguity and unsupported runtime patterns.                                                                               | `error` |
-| `sfcc/no-type-annotations`  | Disallows type annotation syntax in JavaScript files (e.g. `const x: string = ...`, `function y(): number {}`). Rhino/E4X may accept it, but it is invalid in standard JavaScript; use JSDoc typing instead. | `error` |
-| `sfcc/prefer-const`         | Requires `const` for `let` declarations that are never reassigned, excluding Rhino-sensitive nested/loop contexts.                                                                                           | `error` |
-| `sfcc/rhino-const-compat`   | Enforces `let` instead of `const` in Rhino loop-critical contexts (loop headers and declarations inside loop bodies) and supports auto-fix.                                                                  | `error` |
-| `sfcc/rhino-const-conflict` | Detects same-name `const` declarations in nested blocks within the same function (Rhino treats them as function-scoped) and supports auto-fix to `let`.                                                      | `error` |
-| `sfcc/valid-require-path`   | Validates SFCC-compatible `require()` paths (`dw/*`, `cartridgeName/*`, `./*`, `../*`, `*/*`, `~/*`) and supports optional filesystem existence checks.                                                      | `error` |
+| Rule                           | Description                                                                                                                                                                                                  | Default |
+| ------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------- |
+| `sfcc/no-e4x-syntax`           | Disallows JSX/E4X-like tag syntax (e.g. `<a/>`) in SFCC JavaScript to avoid parser ambiguity and unsupported runtime patterns.                                                                               | `error` |
+| `sfcc/no-type-annotations`     | Disallows type annotation syntax in JavaScript files (e.g. `const x: string = ...`, `function y(): number {}`). Rhino/E4X may accept it, but it is invalid in standard JavaScript; use JSDoc typing instead. | `error` |
+| `sfcc/no-rhino-import-globals` | Disallows legacy Rhino globals `importScript(...)`, `importPackage(...)`, and `importClass(...)`. Use CommonJS `require()` instead.                                                                          | `error` |
+| `sfcc/prefer-const`            | Requires `const` for `let` declarations that are never reassigned, excluding Rhino-sensitive nested/loop contexts.                                                                                           | `error` |
+| `sfcc/rhino-const-compat`      | Enforces `let` instead of `const` in Rhino loop-critical contexts (loop headers and declarations inside loop bodies) and supports auto-fix.                                                                  | `error` |
+| `sfcc/rhino-const-conflict`    | Detects same-name `const` declarations in nested blocks within the same function (Rhino treats them as function-scoped) and supports auto-fix to `let`.                                                      | `error` |
+| `sfcc/valid-require-path`      | Validates SFCC-compatible `require()` paths (`dw/*`, `cartridgeName/*`, `./*`, `../*`, `*/*`, `~/*`) and supports optional filesystem existence checks.                                                      | `error` |
 
 The recommended config intentionally combines these `sfcc/*` rules so `--fix` does not bounce between conflicting suggestions: Rhino-unsafe `const` becomes `let`, while genuinely safe top-level function bindings still become `const`.
 
@@ -210,6 +211,10 @@ Q: Are type annotations allowed in `.js` files?
 
 A: No. `sfcc/no-type-annotations` reports annotation syntax in JavaScript files (for example `const x: string = "foo"` or `function y(): number {}`). Rhino/E4X may accept this syntax, but `.js` here follows standard JavaScript where it is invalid. Use JSDoc types instead.
 
+Q: Are legacy Rhino import globals allowed?
+
+A: No. `sfcc/no-rhino-import-globals` reports `importScript(...)`, `importPackage(...)`, and `importClass(...)` and points you to CommonJS `require()` instead.
+
 Q: What suggestion is shown for multiline static markup?
 
 A: For static multiline JSX/E4X-like markup, `sfcc/no-e4x-syntax` suggests converting to `XML(\`...\`)`. For dynamic markup (for example with `{value}`), no conversion suggestion is offered.

src/plugins/sfcc/index.ts

@@ -1,4 +1,5 @@
 import noE4xSyntax from "./no-e4x-syntax.js"
+import noRhinoImportGlobals from "./no-rhino-import-globals.js"
 import noTypeAnnotations from "./no-type-annotations.js"
 import preferConst from "./prefer-const.js"
 import rhinoConstCompat from "./rhino-const-compat.js"
@@ -9,6 +10,7 @@ const sfcc = {
   rules: {
     "no-e4x-syntax": noE4xSyntax,
     "no-type-annotations": noTypeAnnotations,
+    "no-rhino-import-globals": noRhinoImportGlobals,
     "prefer-const": preferConst,
     "rhino-const-compat": rhinoConstCompat,
     "rhino-const-conflict": rhinoConstConflict,

src/plugins/sfcc/no-rhino-import-globals.ts

@@ -0,0 +1,54 @@
+import type { Rule } from "eslint"
+
+import { withSfccSettings } from "../_utils/sfcc-settings.js"
+
+const LEGACY_RHINO_IMPORTS = new Set(["importScript", "importPackage", "importClass"])
+
+function isJavaScriptTarget(filename: string): boolean {
+  if (filename === "<input>") {
+    return true
+  }
+
+  return /\.(?:[cm]?js|ds)$/iu.test(filename)
+}
+
+const noRhinoImportGlobals: Rule.RuleModule = {
+  meta: {
+    type: "problem",
+    docs: {
+      description:
+        "Disallow legacy Rhino globals importScript, importPackage, and importClass in JavaScript files. Use CommonJS require() instead.",
+      recommended: true,
+    },
+    schema: [],
+    messages: {
+      forbiddenLegacyImport: "{{name}}() is a legacy Rhino global. Use CommonJS require() instead.",
+    },
+  },
+  create: withSfccSettings((context) => {
+    if (!isJavaScriptTarget(context.filename)) {
+      return {}
+    }
+
+    return {
+      CallExpression(node: Rule.Node) {
+        const callExpression = node as Rule.Node & {
+          callee: Rule.Node & { type?: string; name?: string }
+        }
+        const callee = callExpression.callee
+
+        if (callee.type !== "Identifier" || !LEGACY_RHINO_IMPORTS.has(callee.name ?? "")) {
+          return
+        }
+
+        context.report({
+          node: callee,
+          messageId: "forbiddenLegacyImport",
+          data: { name: callee.name },
+        })
+      },
+    }
+  }),
+}
+
+export default noRhinoImportGlobals

src/rules/sfcc.ts

@@ -3,6 +3,7 @@ import type { Linter } from "eslint"
 const sfcc: Linter.RulesRecord = {
   "sfcc/no-e4x-syntax": "error",
   "sfcc/no-type-annotations": "error",
+  "sfcc/no-rhino-import-globals": "error",
   "sfcc/prefer-const": "error",
   "sfcc/rhino-const-compat": "error",
   "sfcc/rhino-const-conflict": "error",

tests/sfcc-no-rhino-import-globals.test.ts

@@ -0,0 +1,53 @@
+import tseslint from "@typescript-eslint/eslint-plugin"
+import { ESLint, type Linter } from "eslint"
+import { describe, expect, test } from "vite-plus/test"
+
+import { createRecommendedConfig } from "../src/index.js"
+
+const tsRecommended = tseslint.configs["flat/recommended"] as unknown as
+  | Linter.Config
+  | Linter.Config[]
+
+const config: Linter.Config[] = [
+  ...(Array.isArray(tsRecommended) ? tsRecommended : [tsRecommended]),
+  ...createRecommendedConfig({
+    files: ["**/*.js"],
+    ignores: [],
+  }),
+]
+
+async function lint(code: string, filename = "cartridges/app_sfra/cartridge/scripts/fixture.js") {
+  const eslint = new ESLint({
+    overrideConfigFile: true,
+    overrideConfig: config,
+  })
+
+  const results = await eslint.lintText(code, { filePath: filename })
+  return results[0]?.messages || []
+}
+
+describe("sfcc/no-rhino-import-globals", () => {
+  test("reports importScript usage", async () => {
+    const messages = await lint('importScript("scripts/util");')
+
+    expect(messages.some((m) => m.ruleId === "sfcc/no-rhino-import-globals")).toBe(true)
+  })
+
+  test("reports importPackage usage", async () => {
+    const messages = await lint('importPackage("dw.catalog")')
+
+    expect(messages.some((m) => m.ruleId === "sfcc/no-rhino-import-globals")).toBe(true)
+  })
+
+  test("reports importClass usage", async () => {
+    const messages = await lint("importClass(Packages.java.lang.String)")
+
+    expect(messages.some((m) => m.ruleId === "sfcc/no-rhino-import-globals")).toBe(true)
+  })
+
+  test("allows CommonJS require", async () => {
+    const messages = await lint('const Logger = require("dw/system/Logger")')
+
+    expect(messages.some((m) => m.ruleId === "sfcc/no-rhino-import-globals")).toBe(false)
+  })
+})