diff --git a/.changeset/clean-flowers-carry.md b/.changeset/clean-flowers-carry.md new file mode 100644 index 000000000..283ae987e --- /dev/null +++ b/.changeset/clean-flowers-carry.md @@ -0,0 +1,5 @@ +--- +"@rnx-kit/eslint-plugin": patch +--- + +Added a rule for restricting asset imports diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 55cf0b418..decd96296 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -96,4 +96,5 @@ module.exports = [ | ✓ | | [`@rnx-kit/no-const-enum`](https://github.com/microsoft/rnx-kit/blob/main/packages/eslint-plugin/src/rules/no-const-enum.js) | disallow `const enum` ([why is it bad?](https://hackmd.io/bBcd6R-1TB6Zq95PSquooQ)) | | ✓ | 🔧 | [`@rnx-kit/no-export-all`](https://github.com/microsoft/rnx-kit/blob/main/packages/eslint-plugin/src/rules/no-export-all.js) | disallow `export *` ([why is it bad?](https://hackmd.io/Z021hgSGStKlYLwsqNMOcg)) | | ✓ | | [`@rnx-kit/no-foreach-with-captured-variables`](https://github.com/microsoft/rnx-kit/blob/main/packages/eslint-plugin/src/rules/no-foreach-with-captured-variables.js) | disallow `forEach` with outside variables; JavaScript is not efficient when it comes to using variables defined outside of its scope, and repeatedly calling that function can lead to performance issues. By using a `for...of` loop, you can avoid these performance pitfalls and also it is easier to debug. | +| | | [`@rnx-kit/restricted-asset-imports`](https://github.com/microsoft/rnx-kit/blob/main/packages/eslint-plugin/src/rules/restricted-asset-imports.js) | restrict asset imports (e.g., file names must be lowercased, exist on disk) | | | | [`@rnx-kit/type-definitions-only`](https://github.com/microsoft/rnx-kit/blob/main/packages/eslint-plugin/src/rules/type-definitions-only.js) | disallow anything but type definitions; useful for types only files or packages | diff --git a/packages/eslint-plugin/src/helpers/path.js b/packages/eslint-plugin/src/helpers/path.js new file mode 100644 index 000000000..aa1074721 --- /dev/null +++ b/packages/eslint-plugin/src/helpers/path.js @@ -0,0 +1,44 @@ +import * as nodefs from "node:fs"; +import * as path from "node:path"; + +/** + * Returns the name of a file as stored on disk. + * @param {string} p The path to the file. + * @param {string} relativeTo The file to resolve from. + * @returns {string | undefined} + */ +export function realname(p, relativeTo, /** @internal */ fs = nodefs) { + const base = path.dirname(relativeTo); + const fullPath = path.resolve(base, p); + if (!fs.existsSync(fullPath)) { + return undefined; + } + + // This is currently the only way to get the actual file name on disk, which + // is needed to check for case sensitivity. + // Note: We cannot use `fs.realpath.native()` because it resolves symbolic + // links while we want the actual file itself, symbolic link or not. + const needle = path.basename(p).toLowerCase(); + const matches = fs + .readdirSync(path.dirname(fullPath)) + .filter((file) => file.toLowerCase() === needle); + + const numMatches = matches.length; + if (numMatches === 0) { + // This can only happen if the file was deleted between `existsSync()` and + // the `readdirSync()` call, but we should still handle it gracefully. + return undefined; + } + + if (numMatches > 1) { + // Only case-sensitive file systems can return multiple matches, so we can + // be confident that the file name on disk is exact if it exists. + return p; + } + + // Ensure that the path string keeps the same prefix (e.g., "./") as the + // original value. + const filename = matches[0]; + const targetDir = path.dirname(p); + return targetDir === "." ? `./${filename}` : `${targetDir}/${filename}`; +} diff --git a/packages/eslint-plugin/src/rules.js b/packages/eslint-plugin/src/rules.js index 7be35f886..d31c7b226 100644 --- a/packages/eslint-plugin/src/rules.js +++ b/packages/eslint-plugin/src/rules.js @@ -6,6 +6,7 @@ module.exports = { "no-const-enum": require("./rules/no-const-enum"), "no-export-all": require("./rules/no-export-all"), "no-foreach-with-captured-variables": require("./rules/no-foreach-with-captured-variables"), + "restricted-asset-imports": require("./rules/restricted-asset-imports"), "type-definitions-only": require("./rules/type-definitions-only"), }, }; diff --git a/packages/eslint-plugin/src/rules/restricted-asset-imports.js b/packages/eslint-plugin/src/rules/restricted-asset-imports.js new file mode 100644 index 000000000..09b4f2bcd --- /dev/null +++ b/packages/eslint-plugin/src/rules/restricted-asset-imports.js @@ -0,0 +1,118 @@ +// @ts-check +"use strict"; + +/** @import { Rule } from "eslint"; */ +const { realname } = require("../helpers/path.js"); +const path = require("node:path"); + +const SOURCE_FILES = [ + ".cjs", + ".cts", + ".js", + ".jsx", + ".mjs", + ".mts", + ".ts", + ".tsx", +]; + +/** + * Returns whether the specified node is an import or require statement. + * @param {Rule.Node} node + * @returns {boolean} + */ +function isImportOrRequire(node) { + switch (node.type) { + case "CallExpression": { + // const m = require(...); + const callee = node.callee; + return callee.type === "Identifier" && callee.name === "require"; + } + case "ImportDeclaration": // import m from "..."; + case "ImportExpression": // const m = import(...); + return true; + default: + return false; + } +} + +/** + * @param {string} value + * @returns {boolean} + */ +function isSourceFile(value) { + const ext = path.extname(value); + return !ext || SOURCE_FILES.includes(ext); +} + +/** @type {Rule.RuleModule} */ +module.exports = { + meta: { + type: "problem", + docs: { + description: "asset imports must follow a set of rules", + category: "Possible Errors", + recommended: true, + url: require("../../package.json").homepage, + }, + messages: { + lowercase: "File name must be lowercase", + lowercaseDisk: "File name must be lowercase on disk", + mismatch: "File name does not match the file on disk", + noSuchFile: "No such file exists", + }, + schema: [ + { + type: "object", + properties: { + extensions: { type: "array", items: { type: "string" } }, + exists: { type: "boolean" }, + lowercase: { type: "boolean" }, + }, + additionalProperties: false, + }, + ], + }, + create: (context) => { + const { extensions, exists, lowercase } = context.options[0] || {}; + + /** @type {Rule.NodeListener} */ + return { + Literal: (node) => { + const { value, parent } = node; + if ( + typeof value !== "string" || + !value.startsWith("./") || + !isImportOrRequire(parent) + ) { + return; + } + + if (Array.isArray(extensions)) { + if (!extensions.includes(path.extname(value))) { + return; + } + } else if (isSourceFile(value)) { + return; + } + + if (lowercase !== false && value.toLowerCase() !== value) { + context.report({ node, messageId: "lowercase" }); + } + + if (exists !== false) { + const p = realname(value, context.filename); + if (!p) { + context.report({ node, messageId: "noSuchFile" }); + } else if (lowercase !== false) { + if (p.toLowerCase() !== p) { + context.report({ node, messageId: "lowercaseDisk" }); + } + } else if (p !== value) { + context.report({ node, messageId: "mismatch" }); + } + } + }, + }; + }, +}; diff --git a/packages/eslint-plugin/test/helpers/path.test.ts b/packages/eslint-plugin/test/helpers/path.test.ts new file mode 100644 index 000000000..c5632c8b7 --- /dev/null +++ b/packages/eslint-plugin/test/helpers/path.test.ts @@ -0,0 +1,33 @@ +import * as fs from "node:fs"; +import { realname } from "../../src/helpers/path.js"; + +describe("realname()", () => { + const thisFile = __filename; + + it("returns the file name on disk", () => { + const cases = [ + "../../README.md", + "../../package.json", + "../../../../README.md", + "../../../../package.json", + ]; + for (const c of cases) { + expect(realname(c, thisFile)).toEqual(c); + } + }); + + // The following tests only work on case-sensitive file systems + const isCaseSensitive = !fs.existsSync(thisFile.toUpperCase()); + const iit = isCaseSensitive ? it.skip : it; + + iit("returns the file name on disk (case-insensitive)", () => { + const cases = [ + ["../../rEaDmE.md", "../../README.md"], + ["../../PaCkAgE.json", "../../package.json"], + ["../../../../PaCkAgE.json", "../../../../package.json"], + ] as const; + for (const [input, expected] of cases) { + expect(realname(input, thisFile)).toEqual(expected); + } + }); +}); diff --git a/packages/eslint-plugin/test/restricted-asset-imports.test.ts b/packages/eslint-plugin/test/restricted-asset-imports.test.ts new file mode 100644 index 000000000..f82f24d64 --- /dev/null +++ b/packages/eslint-plugin/test/restricted-asset-imports.test.ts @@ -0,0 +1,136 @@ +import * as fs from "node:fs"; +import rule from "../src/rules/restricted-asset-imports.js"; +import { makeRuleTester } from "./RuleTester.ts"; + +describe("asset imports must follow a set of rules", () => { + const E_LOWERCASE = { messageId: "lowercase" }; + const E_LOWERCASEDISK = { messageId: "lowercaseDisk" }; + const E_MISMATCH = { messageId: "mismatch" }; + const E_NOSUCHFILE = { messageId: "noSuchFile" }; + + const ruleTester = makeRuleTester(); + + ruleTester.run("restricted-asset-imports", rule, { + valid: [ + '"./assets/Image.png";', + 'import t from "module.png";', + 'import t from "./types";', + 'import t from "./types.cjs";', + 'import t from "./types.cts";', + 'import t from "./types.js";', + 'import t from "./types.jsx";', + 'import t from "./types.mjs";', + 'import t from "./types.mts";', + 'import t from "./types.ts";', + 'import t from "./types.tsx";', + { + code: 'import i from "./assets/image.png";', + options: [{ exists: false }], + }, + { + code: 'import i from "./assets/Image.png";', + options: [{ extensions: [".jpg"], exists: false }], + }, + { + code: 'import i from "./assets/Image.png";', + options: [{ lowercase: false, exists: false }], + }, + { + code: 'const i = import("./assets/image.png");', + options: [{ exists: false }], + }, + { + code: 'const i = import("./assets/Image.png");', + options: [{ extensions: [".jpg"], exists: false }], + }, + { + code: 'const i = import("./assets/Image.png");', + options: [{ lowercase: false, exists: false }], + }, + { + code: 'const i = require("./assets/image.png");', + options: [{ exists: false }], + }, + { + code: 'const i = require("./assets/Image.png");', + options: [{ extensions: [".jpg"], exists: false }], + }, + { + code: 'const i = require("./assets/Image.png");', + options: [{ lowercase: false, exists: false }], + }, + ], + invalid: [ + { + code: 'import i from "./assets/Image.png";', + errors: [E_LOWERCASE, E_NOSUCHFILE], + }, + { + code: 'import i from "./assets/Image.png";', + options: [{ extensions: [".png"] }], + errors: [E_LOWERCASE, E_NOSUCHFILE], + }, + { + code: 'import i from "./assets/Image.png";', + options: [{ extensions: [".png"], exists: false }], + errors: [E_LOWERCASE], + }, + { + code: 'const i = import("./assets/Image.png");', + errors: [E_LOWERCASE, E_NOSUCHFILE], + }, + { + code: 'const i = import("./assets/Image.png");', + options: [{ extensions: [".png"] }], + errors: [E_LOWERCASE, E_NOSUCHFILE], + }, + { + code: 'const i = import("./assets/Image.png");', + options: [{ extensions: [".png"], exists: false }], + errors: [E_LOWERCASE], + }, + { + code: 'const i = require("./assets/Image.png");', + errors: [E_LOWERCASE, E_NOSUCHFILE], + }, + { + code: 'const i = require("./assets/Image.png");', + options: [{ extensions: [".png"] }], + errors: [E_LOWERCASE, E_NOSUCHFILE], + }, + { + code: 'const i = require("./assets/Image.png");', + options: [{ extensions: [".png"], exists: false }], + errors: [E_LOWERCASE], + }, + ], + }); + + // These tests only work on case-sensitive file systems + const thisFile = __filename; + if (fs.existsSync(thisFile.toUpperCase())) { + ruleTester.run("restricted-asset-imports (case-insensitive)", rule, { + valid: [ + { + code: 'import m from "./README.md";', + options: [{ lowercase: false }], + }, + ], + invalid: [ + { + code: 'import m from "./readme.md";', + errors: [E_LOWERCASEDISK], + }, + { + code: 'import m from "./ReadMe.md";', + errors: [E_LOWERCASE, E_LOWERCASEDISK], + }, + { + code: 'import m from "./ReadMe.md";', + options: [{ lowercase: false }], + errors: [E_MISMATCH], + }, + ], + }); + } +});