Skip to content

Commit bfae9b5

Browse files
tido64tido64
committed
feat(eslint-plugin): add rule for restricting asset imports
1 parent 58b763d commit bfae9b5

File tree

5 files changed

+325
-0
lines changed

5 files changed

+325
-0
lines changed

‎.changeset/clean-flowers-carry.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@rnx-kit/eslint-plugin": patch
3+
---
4+
5+
Added a rule for restricting asset imports

‎packages/eslint-plugin/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,4 +96,5 @@ module.exports = [
9696
| ✓ | | [`@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)) |
9797
| ✓ | 🔧 | [`@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)) |
9898
| ✓ | | [`@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. |
99+
| | | [`@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) |
99100
| | | [`@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 |

‎packages/eslint-plugin/src/rules.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ module.exports = {
66
"no-const-enum": require("./rules/no-const-enum"),
77
"no-export-all": require("./rules/no-export-all"),
88
"no-foreach-with-captured-variables": require("./rules/no-foreach-with-captured-variables"),
9+
"restricted-asset-imports": require("./rules/restricted-asset-imports"),
910
"type-definitions-only": require("./rules/type-definitions-only"),
1011
},
1112
};

‎packages/eslint-plugin/src/rules/restricted-asset-imports.js‎

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
// @ts-check
2+
"use strict";
3+
4+
/** @import { Rule } from "eslint"; */
5+
const nodefs = require("node:fs");
6+
const path = require("node:path");
7+
8+
/**
9+
* @typedef {{
10+
* __internalForTestingPurposesOnly: {
11+
* realpath: typeof realpath
12+
* }
13+
* }} Testable;
14+
*/
15+
16+
const SOURCE_FILES = [
17+
".cjs",
18+
".cts",
19+
".js",
20+
".jsx",
21+
".mjs",
22+
".mts",
23+
".ts",
24+
".tsx",
25+
];
26+
27+
/**
28+
* Returns whether the specified node is an import or require statement.
29+
* @param {Rule.Node} node
30+
* @returns {boolean}
31+
*/
32+
function isImportOrRequire(node) {
33+
switch (node.type) {
34+
case "CallExpression": {
35+
// const m = require(...);
36+
const callee = node.callee;
37+
return callee.type === "Identifier" && callee.name === "require";
38+
}
39+
case "ImportDeclaration": // import m from "...";
40+
case "ImportExpression": // const m = import(...);
41+
return true;
42+
default:
43+
return false;
44+
}
45+
}
46+
47+
/**
48+
* @param {string} value
49+
* @returns {boolean}
50+
*/
51+
function isSourceFile(value) {
52+
const ext = path.extname(value);
53+
return !ext || SOURCE_FILES.includes(ext);
54+
}
55+
56+
/**
57+
* Returns the real path of a file.
58+
* @param {string} p
59+
* @param {string} relativeTo
60+
* @returns {string | undefined}
61+
*/
62+
function realpath(p, relativeTo, fs = nodefs) {
63+
const base = path.dirname(relativeTo);
64+
const fullPath = path.resolve(base, p);
65+
if (!fs.existsSync(fullPath)) {
66+
return undefined;
67+
}
68+
69+
// This is currently the only way to get the actual file name on disk, which
70+
// is needed to check for case sensitivity.
71+
const needle = path.basename(p).toLowerCase();
72+
const matches = fs
73+
.readdirSync(path.dirname(fullPath))
74+
.filter((file) => file.toLowerCase() === needle);
75+
76+
const numMatches = matches.length;
77+
if (numMatches === 0) {
78+
// This can only happen if it was deleted between `existsSync()` and the
79+
// `readdirSync()` call, but we should still handle it gracefully.
80+
return undefined;
81+
}
82+
83+
if (numMatches > 1) {
84+
// Only case-sensitive file systems can return multiple matches, so we can
85+
// be confident that the file name on disk is exact if it exists.
86+
return p;
87+
}
88+
89+
// Ensure that the path string keeps the same prefix (e.g., "./") as the
90+
// original value.
91+
const filename = matches[0];
92+
const targetDir = path.dirname(p);
93+
return targetDir === "." ? `./${filename}` : `${targetDir}/${filename}`;
94+
}
95+
96+
/** @type {Rule.RuleModule & Testable} */
97+
module.exports = {
98+
meta: {
99+
type: "problem",
100+
docs: {
101+
description: "asset imports must follow a set of rules",
102+
category: "Possible Errors",
103+
recommended: true,
104+
url: require("../../package.json").homepage,
105+
},
106+
messages: {
107+
lowercase: "File name must be lowercase",
108+
lowercaseDisk: "File name must be lowercase on disk",
109+
mismatch: "File name does not match the file on disk",
110+
noSuchFile: "No such file exists",
111+
},
112+
schema: [
113+
{
114+
type: "object",
115+
properties: {
116+
extensions: { type: "array", items: { type: "string" } },
117+
exists: { type: "boolean" },
118+
lowercase: { type: "boolean" },
119+
},
120+
additionalProperties: false,
121+
},
122+
],
123+
},
124+
create: (context) => {
125+
const { extensions, exists, lowercase } = context.options[0] || {};
126+
127+
/** @type {Rule.NodeListener} */
128+
return {
129+
Literal: (node) => {
130+
const { value, parent } = node;
131+
if (
132+
typeof value !== "string" ||
133+
!value.startsWith("./") ||
134+
!isImportOrRequire(parent)
135+
) {
136+
return;
137+
}
138+
139+
if (Array.isArray(extensions)) {
140+
if (!extensions.includes(path.extname(value))) {
141+
return;
142+
}
143+
} else if (isSourceFile(value)) {
144+
return;
145+
}
146+
147+
if (lowercase !== false && value.toLowerCase() !== value) {
148+
context.report({ node, messageId: "lowercase" });
149+
}
150+
151+
if (exists !== false) {
152+
const p = realpath(value, context.filename);
153+
if (!p) {
154+
context.report({ node, messageId: "noSuchFile" });
155+
} else if (lowercase !== false) {
156+
if (p.toLowerCase() !== p) {
157+
context.report({ node, messageId: "lowercaseDisk" });
158+
}
159+
} else if (p !== value) {
160+
context.report({ node, messageId: "mismatch" });
161+
}
162+
}
163+
},
164+
};
165+
},
166+
__internalForTestingPurposesOnly: { realpath },
167+
};

‎packages/eslint-plugin/test/restricted-asset-imports.test.ts‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
import * as fs from "node:fs";
2+
import rule from "../src/rules/restricted-asset-imports.js";
3+
import { makeRuleTester } from "./RuleTester.ts";
4+
5+
describe("asset imports must follow a set of rules", () => {
6+
const E_LOWERCASE = { messageId: "lowercase" };
7+
const E_LOWERCASEDISK = { messageId: "lowercaseDisk" };
8+
const E_MISMATCH = { messageId: "mismatch" };
9+
const E_NOSUCHFILE = { messageId: "noSuchFile" };
10+
11+
const ruleTester = makeRuleTester();
12+
13+
ruleTester.run("restricted-asset-imports", rule, {
14+
valid: [
15+
'"./assets/Image.png";',
16+
'import t from "module.png";',
17+
'import t from "./types";',
18+
'import t from "./types.cjs";',
19+
'import t from "./types.cts";',
20+
'import t from "./types.js";',
21+
'import t from "./types.jsx";',
22+
'import t from "./types.mjs";',
23+
'import t from "./types.mts";',
24+
'import t from "./types.ts";',
25+
'import t from "./types.tsx";',
26+
{
27+
code: 'import i from "./assets/image.png";',
28+
options: [{ exists: false }],
29+
},
30+
{
31+
code: 'import i from "./assets/Image.png";',
32+
options: [{ extensions: [".jpg"], exists: false }],
33+
},
34+
{
35+
code: 'import i from "./assets/Image.png";',
36+
options: [{ lowercase: false, exists: false }],
37+
},
38+
{
39+
code: 'const i = import("./assets/image.png");',
40+
options: [{ exists: false }],
41+
},
42+
{
43+
code: 'const i = import("./assets/Image.png");',
44+
options: [{ extensions: [".jpg"], exists: false }],
45+
},
46+
{
47+
code: 'const i = import("./assets/Image.png");',
48+
options: [{ lowercase: false, exists: false }],
49+
},
50+
{
51+
code: 'const i = require("./assets/image.png");',
52+
options: [{ exists: false }],
53+
},
54+
{
55+
code: 'const i = require("./assets/Image.png");',
56+
options: [{ extensions: [".jpg"], exists: false }],
57+
},
58+
{
59+
code: 'const i = require("./assets/Image.png");',
60+
options: [{ lowercase: false, exists: false }],
61+
},
62+
],
63+
invalid: [
64+
{
65+
code: 'import i from "./assets/Image.png";',
66+
errors: [E_LOWERCASE, E_NOSUCHFILE],
67+
},
68+
{
69+
code: 'import i from "./assets/Image.png";',
70+
options: [{ extensions: [".png"] }],
71+
errors: [E_LOWERCASE, E_NOSUCHFILE],
72+
},
73+
{
74+
code: 'import i from "./assets/Image.png";',
75+
options: [{ extensions: [".png"], exists: false }],
76+
errors: [E_LOWERCASE],
77+
},
78+
{
79+
code: 'const i = import("./assets/Image.png");',
80+
errors: [E_LOWERCASE, E_NOSUCHFILE],
81+
},
82+
{
83+
code: 'const i = import("./assets/Image.png");',
84+
options: [{ extensions: [".png"] }],
85+
errors: [E_LOWERCASE, E_NOSUCHFILE],
86+
},
87+
{
88+
code: 'const i = import("./assets/Image.png");',
89+
options: [{ extensions: [".png"], exists: false }],
90+
errors: [E_LOWERCASE],
91+
},
92+
{
93+
code: 'const i = require("./assets/Image.png");',
94+
errors: [E_LOWERCASE, E_NOSUCHFILE],
95+
},
96+
{
97+
code: 'const i = require("./assets/Image.png");',
98+
options: [{ extensions: [".png"] }],
99+
errors: [E_LOWERCASE, E_NOSUCHFILE],
100+
},
101+
{
102+
code: 'const i = require("./assets/Image.png");',
103+
options: [{ extensions: [".png"], exists: false }],
104+
errors: [E_LOWERCASE],
105+
},
106+
],
107+
});
108+
109+
const {
110+
__internalForTestingPurposesOnly: { realpath },
111+
} = rule;
112+
113+
// These tests only work on case-sensitive file systems
114+
const thisFile = __filename;
115+
if (fs.existsSync(thisFile.toUpperCase())) {
116+
test("realpath returns the file path on disk", () => {
117+
expect(realpath("../PaCkAgE.json", thisFile)).toEqual("../package.json");
118+
expect(realpath("../package.json", thisFile)).toEqual("../package.json");
119+
expect(realpath("../../../PaCkAgE.json", thisFile)).toEqual(
120+
"../../../package.json"
121+
);
122+
expect(realpath("../../../package.json", thisFile)).toEqual(
123+
"../../../package.json"
124+
);
125+
});
126+
127+
ruleTester.run("restricted-asset-imports (case-insensitive)", rule, {
128+
valid: [
129+
{
130+
code: 'import m from "./README.md";',
131+
options: [{ lowercase: false }],
132+
},
133+
],
134+
invalid: [
135+
{
136+
code: 'import m from "./readme.md";',
137+
errors: [E_LOWERCASEDISK],
138+
},
139+
{
140+
code: 'import m from "./ReadMe.md";',
141+
errors: [E_LOWERCASE, E_LOWERCASEDISK],
142+
},
143+
{
144+
code: 'import m from "./ReadMe.md";',
145+
options: [{ lowercase: false }],
146+
errors: [E_MISMATCH],
147+
},
148+
],
149+
});
150+
}
151+
});

0 commit comments

Comments
 (0)