Repository navigation
fix(lint): enforce explicit JSON utility extensions - #1432
RezoanurRahman wants to merge 2 commits into
Conversation
Adds an API-scoped ESLint rule for extensionless JSON utility requires. Preserves extensionless imports for JavaScript utilities. No current API offenders required changes. `npm run lint` and `npm run test:unit` pass. Closes cuttle-cards#1429
itsalaidbacklife
left a comment
There was a problem hiding this comment.
Thanks for picking this up — the selector works and does exactly what #1429 described.
I want to revisit the one limitation I flagged in the issue, though, because I think I was wrong about it. I wrote that the JSON basenames have to be hard-coded "because ESLint can't tell from the source whether utils/Foo resolves to .js or .json." That's true of the AST, but a rule isn't limited to the AST — it can resolve the specifier against the filesystem the same way Node does. That removes the hand-maintained list entirely.
I prototyped it and it holds up. Inline plugin in eslint.config.js, no new dependency:
const aliases = { _: __dirname, '@': path.join(__dirname, 'src') };
const resolveSpecifier = (fromDir, spec) => {
const [ head, ...rest ] = spec.split('/');
if (Object.hasOwn(aliases, head)) {
return path.join(aliases[head], ...rest);
}
return spec.startsWith('.') ? path.resolve(fromDir, spec) : null;
};
const isJsonCache = new Map();
const resolvesToJson = (target) => {
if (!isJsonCache.has(target)) {
isJsonCache.set(target, !fs.existsSync(`${target}.js`) && fs.existsSync(`${target}.json`));
}
return isJsonCache.get(target);
};…wrapped in a rule that visits require(), import, dynamic import(), and re-exports, skipping any specifier that already carries an extension and any bare package name. Full diff at the bottom.
What this buys over the current approach
| this PR | filesystem resolution | |
|---|---|---|
| New JSON util added | must edit the regex | works automatically |
JSON file outside utils/ |
not covered | covered |
| Scope | api/** only |
all 361 linted files |
Alias form _/utils/GameStatus |
not matched | matched |
| Autofix | no | yes |
That fourth row is the one that changed my mind. The frontend imports these enums through the _ alias in eight places — src/router.js:4, src/routes/home/HomeView.vue:165, src/stores/gameList.js:4, and others — and a utils\/(…)$ selector scoped to api/** can't see any of them. The convention is repo-wide but the rule as written guards one corner of it.
Verification
Probe file with the three cases from the issue plus the alias and re-export forms:
- Flagged:
require('../../utils/GameStatus'),import … from '_/utils/GameStatus','../utils/Transitions',export { default as GamePhase } from '_/utils/GamePhase' - Not flagged: anything already
.json, plusrulesVersion,game-utils, and the baresailspackage — i.e. the real.jsutils the issue worried a blanket rule would catch --fixrewrote all four correctlynpm run lint: 361 files, 0 errors, ~1.3s — no measurable cost from theexistsSynccalls
Two caveats, both smaller surfaces than the eight-name list:
- The alias map duplicates
vite.config.mjs:53. A CJS config can'trequirean.mjs, so the two entries are copied. If the aliases ever change this needs a matching edit — worth a comment pointing at the vite config. - A specifier like
./some.configwhose target issome.config.jsonis skipped, becausepath.extnamereads.configas an extension. No such file exists today.
Happy either way on how this lands — you take it into this PR, or merge yours as-is and I open a follow-up. Your call, it's your PR. If you do take it, the message string and (see #1397) reference are lifted straight from your version.
Full diff against eslint.config.js
diff --git a/eslint.config.js b/eslint.config.js
index 7958cf95..5a79ca07 100644
--- a/eslint.config.js
+++ b/eslint.config.js
@@ -1,3 +1,5 @@
+const fs = require('fs');
+const path = require('path');
const js = require('@eslint/js');
const globals = require('globals');
const vue = require('eslint-plugin-vue');
@@ -50,6 +52,67 @@ const browserFiles = [
'vitest.config.mjs',
];
+// ESLint can't tell from the source whether `utils/Foo` is a .js or a .json file, so instead of
+// hard-coding the enum names we resolve each specifier against the filesystem. Anything that is
+// genuinely JSON on disk must say so in the specifier, or the TS service can't resolve it (#1397).
+const aliases = { _: __dirname, '@': path.join(__dirname, 'src') };
+
+const resolveSpecifier = (fromDir, spec) => {
+ const [ head, ...rest ] = spec.split('/');
+ if (Object.hasOwn(aliases, head)) {
+ return path.join(aliases[head], ...rest);
+ }
+ return spec.startsWith('.') ? path.resolve(fromDir, spec) : null;
+};
+
+const isJsonCache = new Map();
+const resolvesToJson = (target) => {
+ if (!isJsonCache.has(target)) {
+ isJsonCache.set(target, !fs.existsSync(`${target}.js`) && fs.existsSync(`${target}.json`));
+ }
+ return isJsonCache.get(target);
+};
+
+const jsonExtensions = {
+ rules: {
+ 'explicit-json-extension': {
+ meta: {
+ type: 'problem',
+ fixable: 'code',
+ schema: [],
+ messages: {
+ missing: `'{{spec}}' resolves to a JSON file; write '{{spec}}.json' so the editor can resolve it (see #1397).`,
+ },
+ },
+ create(context) {
+ const fromDir = path.dirname(context.filename);
+ const check = (node) => {
+ if (node?.type !== 'Literal' || typeof node.value !== 'string' || path.extname(node.value) !== '') {
+ return;
+ }
+ const target = resolveSpecifier(fromDir, node.value);
+ if (!target || !resolvesToJson(target)) {
+ return;
+ }
+ context.report({
+ node,
+ messageId: 'missing',
+ data: { spec: node.value },
+ fix: (fixer) => fixer.replaceText(node, `'${node.value}.json'`),
+ });
+ };
+ return {
+ 'CallExpression[callee.name="require"]': (node) => check(node.arguments[0]),
+ ImportDeclaration: (node) => check(node.source),
+ ImportExpression: (node) => check(node.source),
+ ExportNamedDeclaration: (node) => check(node.source),
+ ExportAllDeclaration: (node) => check(node.source),
+ };
+ },
+ },
+ },
+};
+
module.exports = [
{
// The worktree dirs are git-ignored, but that doesn't stop eslint walking into them: each
@@ -66,6 +129,7 @@ module.exports = [
vuetify,
prettier,
'no-only-tests': noOnlyTests,
+ 'json-extensions': jsonExtensions,
},
languageOptions: {
ecmaVersion: 'latest',
@@ -73,6 +137,7 @@ module.exports = [
globals: nodeGlobals,
},
rules: {
+ 'json-extensions/explicit-json-extension': 'error',
'eol-last': [ 'error', 'always' ],
'max-len': [
'warn',|
Thanks for the prototype and for revisiting the limitation. I incorporated the filesystem-resolution approach in commit 1c6f727. The rule now resolves relative paths and the Vite |
Closes #1429
Summary
Adds an API-scoped ESLint rule for extensionless JSON utility requires.
Preserves extensionless imports for JavaScript utilities.
No current API offenders required changes.
npm run lintandnpm run test:unitpass.Changed files
eslint.config.jsVerification
npm ci --ignore-scripts --no-audit --no-fundpassednpm run test:unitpassednpm run lintpassedCode was written with Codex, reviewed and guided by me.