diff --git a/CLAUDE.md b/CLAUDE.md index 290d275..784d6de 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -120,6 +120,13 @@ This package is currently on v3, which uses ESLint 9's flat config format. The p - v1 → v2: See `docs/MIGRATION-V2.md` - v2 → v3: See `docs/MIGRATION-V3.md` (ESLint 9 flat config migration requiring Node.js 24+) +## Versioning Convention + +When adding rules to `configs.recommended` in the `hs-web-team` plugin (or any other bundled config): + +- **`'warn'` severity** → minor bump (`feat:` commit). Warn-only rules don't fail standard CI, so consumers aren't broken without opt-in (`--max-warnings 0`). +- **`'error'` severity** → breaking change (`feat!:` commit, major bump). Error rules can fail CI for any consumer with a violation, regardless of their settings. + ## Important Notes - This package supports multiple use cases: diff --git a/package.json b/package.json index 61bf4ac..8d1ebb8 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,8 @@ "./cypress": { "types": "./cypress/index.d.ts", "default": "./cypress/index.js" - } + }, + "./plugin": "./plugins/hs-web-team/index.js" }, "typesVersions": { "*": { diff --git a/plugins/hs-web-team/index.js b/plugins/hs-web-team/index.js index 8e1593f..5bed0e8 100644 --- a/plugins/hs-web-team/index.js +++ b/plugins/hs-web-team/index.js @@ -1,7 +1,9 @@ +import { noAbbreviations } from './rules/no-abbreviations.js'; import { noReduceAccumulatorCopy } from './rules/no-reduce-accumulator-copy.js'; export const hsWebTeamPlugin = { rules: { + 'no-abbreviations': noAbbreviations, 'no-reduce-accumulator-copy': noReduceAccumulatorCopy, }, }; @@ -12,6 +14,7 @@ hsWebTeamPlugin.configs = { recommended: { plugins: { 'hs-web-team': hsWebTeamPlugin }, rules: { + 'hs-web-team/no-abbreviations': 'warn', 'hs-web-team/no-reduce-accumulator-copy': 'error', }, }, diff --git a/plugins/hs-web-team/rules/no-abbreviations.js b/plugins/hs-web-team/rules/no-abbreviations.js new file mode 100644 index 0000000..cdea019 --- /dev/null +++ b/plugins/hs-web-team/rules/no-abbreviations.js @@ -0,0 +1,135 @@ +const DEFAULT_EXCEPTIONS = new Set(['e', '_']); +const DEFAULT_MIN_LENGTH = 2; + +const SORT_METHODS = new Set(['sort', 'toSorted']); +const ARRAY_ITERATION_METHODS = new Set([ + 'map', + 'flatMap', + 'forEach', + 'filter', + 'find', + 'findIndex', + 'findLast', + 'findLastIndex', + 'some', + 'every', + // reduce/reduceRight are intentionally excluded: their callback signature is + // (accumulator, currentValue, currentIndex, …), so position 1 is the current + // *value*, not the index. The i-at-position-1 heuristic does not apply. +]); + +/** + * Returns true if the VariableDeclarator is in the init of a for/for...of/for...in statement. + * Walks the parent chain directly — no scope API needed. + * + * @param {import('eslint').Rule.Node} declaratorNode - VariableDeclarator node + */ +function isDeclaredInForLoopInit(declaratorNode) { + const declaration = declaratorNode.parent; // VariableDeclaration + const enclosing = declaration?.parent; + if (!enclosing) return false; + return ( + (enclosing.type === 'ForStatement' && enclosing.init === declaration) || + enclosing.type === 'ForInStatement' || + enclosing.type === 'ForOfStatement' + ); +} + +/** + * Returns true if funcNode is a direct inline callback to a method whose name is in methodNames. + * Only matches non-computed method calls (arr.sort(...), not arr['sort'](...)). + * + * @param {import('eslint').Rule.Node} funcNode - ArrowFunctionExpression or FunctionExpression + * @param {Set} methodNames + */ +function isCallbackToMethod(funcNode, methodNames) { + const callExpr = funcNode.parent; + if (callExpr?.type !== 'CallExpression') return false; + if (!callExpr.arguments.includes(funcNode)) return false; + const { callee } = callExpr; + return ( + callee?.type === 'MemberExpression' && + !callee.computed && + methodNames.has(callee.property.name) + ); +} + +/** @type {import('eslint').Rule.RuleModule} */ +export const noAbbreviations = { + meta: { + type: 'suggestion', + docs: { + description: 'Disallow abbreviated identifier names (e.g. `w` instead of `warnings`)', + url: 'https://docs.hubwt.com/docs/developers/coding-on-the-web-team/coding-standards/javascript-standards/#avoid-abbreviation', + recommended: true, + }, + messages: { + tooShort: + 'Identifier "{{name}}" is too short ({{length}} < {{min}}). Use a descriptive name.', + }, + schema: [ + { + type: 'object', + properties: { + exceptions: { type: 'array', items: { type: 'string' }, uniqueItems: true }, + minLength: { type: 'integer', minimum: 1 }, + }, + additionalProperties: false, + }, + ], + }, + + create(context) { + const options = context.options[0] ?? {}; + const exceptions = new Set([...DEFAULT_EXCEPTIONS, ...(options.exceptions ?? [])]); + const minLength = options.minLength ?? DEFAULT_MIN_LENGTH; + + return { + Identifier(node) { + const { name, parent } = node; + + if (name.length >= minLength) return; + if (exceptions.has(name)) return; + + // Only flag declaration sites — not usage sites. + // + // Object destructuring shorthand (const { w } = response) is intentionally + // excluded: `w` may be an external API property name the developer doesn't + // control. The idiomatic fix — const { w: warning } = response — is correct + // but non-obvious enough to leave as opt-in. Array destructuring IS included + // because the element binding name is always chosen by the developer. + const isDeclaration = + (parent.type === 'VariableDeclarator' && parent.id === node) || + (parent.type === 'ArrayPattern' && parent.elements.includes(node)) || + (parent.type === 'FunctionDeclaration' && parent.id === node) || + (parent.type === 'FunctionDeclaration' && parent.params.includes(node)) || + (parent.type === 'ArrowFunctionExpression' && parent.params.includes(node)) || + (parent.type === 'FunctionExpression' && parent.params.includes(node)); + + if (!isDeclaration) return; + + // For-loop variables (i, j, k…) are exempt regardless of length. + if (parent.type === 'VariableDeclarator' && isDeclaredInForLoopInit(parent)) return; + + // a and b are conventional sort-comparator names — exempt only inside .sort()/.toSorted(). + if ((name === 'a' || name === 'b') && isCallbackToMethod(parent, SORT_METHODS)) return; + + // i is a conventional index name — exempt only when it is the index param (position 1) + // of an array iteration callback: items.map((item, i) => ...). + // Position 1 is the index slot for every array iteration method except reduce/reduceRight + // (where it's position 2), but naming the reduce index `i` is uncommon enough to skip. + if ( + name === 'i' && + parent.params?.indexOf(node) === 1 && + isCallbackToMethod(parent, ARRAY_ITERATION_METHODS) + ) return; + + context.report({ + node, + messageId: 'tooShort', + data: { name, length: name.length, min: minLength }, + }); + }, + }; + }, +}; diff --git a/tests/plugins/hs-web-team/no-abbreviations.test.js b/tests/plugins/hs-web-team/no-abbreviations.test.js new file mode 100644 index 0000000..aa63fae --- /dev/null +++ b/tests/plugins/hs-web-team/no-abbreviations.test.js @@ -0,0 +1,237 @@ +import { describe, it } from 'node:test'; +import { RuleTester } from 'eslint'; +import { noAbbreviations as rule } from '../../../plugins/hs-web-team/rules/no-abbreviations.js'; + +const ruleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: 'module', + }, +}); + +describe('no-abbreviations', () => { + describe('valid', () => { + it('does not flag full-length names', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [ + { code: 'const warnings = [];' }, + { code: 'const event = new Event("click");' }, + { code: 'function handleClick(event) {}' }, + ], + invalid: [], + }); + }); + + it('does not flag for-loop init variables', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [ + { code: 'for (let i = 0; i < 10; i++) {}' }, + { code: 'for (let j = 0; j < arr.length; j++) {}' }, + { code: 'for (const x of arr) {}' }, + { code: 'for (const k in obj) {}' }, + ], + invalid: [], + }); + }); + + it('does not flag default exceptions (e, _)', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [ + { code: 'btn.addEventListener("click", e => e.preventDefault());' }, + { code: 'const _ = unused;' }, + { code: 'arr.forEach((item, _) => doThing(item));' }, + ], + invalid: [], + }); + }); + + it('does not flag a and b as sort-comparator params in .sort()/.toSorted()', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [ + { code: 'arr.sort((a, b) => a - b);' }, + { code: 'arr.sort((a, b) => a.name.localeCompare(b.name));' }, + { code: 'arr.toSorted((a, b) => a - b);' }, + { code: 'arr.sort(function(a, b) { return a - b; });' }, + ], + invalid: [], + }); + }); + + it('does not flag i as the index param (position 1) of array iteration callbacks', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [ + { code: 'items.map((item, i) => ({ ...item, index: i }));' }, + { code: 'items.forEach((item, i) => doThing(item, i));' }, + { code: 'items.filter((item, i) => i % 2 === 0);' }, + { code: 'items.find((item, i) => i > 3);' }, + { code: 'items.some((item, i) => i === 0);' }, + { code: 'items.every((item, i) => i < 10);' }, + { code: 'items.flatMap((item, i) => [item, i]);' }, + ], + invalid: [], + }); + }); + + it('does not flag property access or usage sites (not declarations)', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [ + { code: 'obj.w = 1;' }, + { code: 'const foo = obj.w;' }, + { code: 'doThing(w);' }, + ], + invalid: [], + }); + }); + + it('does not flag object destructuring shorthand (external property name may not be developer-owned)', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [ + { code: 'const { w } = response;' }, + { code: 'const { w: warning } = response;' }, + { code: 'function process({ w }) {}' }, + ], + invalid: [], + }); + }); + + it('does not flag names covered by the custom exceptions option', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [ + { code: 'const cb = () => {};', options: [{ exceptions: ['cb'] }] }, + ], + invalid: [], + }); + }); + }); + + describe('invalid', () => { + it('flags a single-char array destructuring binding', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [], + invalid: [ + { + code: 'const [w] = items;', + errors: [{ messageId: 'tooShort' }], + }, + { + code: 'const [item, w] = items;', + errors: [{ messageId: 'tooShort' }], + }, + ], + }); + }); + + it('flags a single-char function declaration name', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [], + invalid: [ + { + code: 'function w() {}', + errors: [{ messageId: 'tooShort' }], + }, + ], + }); + }); + + it('flags a single-char variable declaration', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [], + invalid: [ + { + code: 'const w = [];', + errors: [{ messageId: 'tooShort', data: { name: 'w', length: 1, min: 2 } }], + }, + ], + }); + }); + + it('flags a single-char arrow function parameter', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [], + invalid: [ + { + code: 'items.map(w => w.id);', + errors: [{ messageId: 'tooShort' }], + }, + ], + }); + }); + + it('flags a single-char named function parameter', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [], + invalid: [ + { + code: 'function process(w) {}', + errors: [{ messageId: 'tooShort' }], + }, + ], + }); + }); + + it('flags a single-char function expression parameter', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [], + invalid: [ + { + code: 'const fn = function(w) {};', + errors: [{ messageId: 'tooShort' }], + }, + ], + }); + }); + + it('flags a and b outside of sort callbacks', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [], + invalid: [ + { + code: 'const a = 1;', + errors: [{ messageId: 'tooShort' }], + }, + { + // Not a sort method — a and b should still be flagged + code: 'items.map((a, b) => a + b);', + errors: [{ messageId: 'tooShort' }, { messageId: 'tooShort' }], + }, + ], + }); + }); + + it('flags i when it is not the index param (position 1) of an array method', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [], + invalid: [ + { + // i as the first (item) param — not the index position + code: 'items.map(i => i.id);', + errors: [{ messageId: 'tooShort' }], + }, + { + // i outside an array method callback entirely + code: 'const i = getIndex();', + errors: [{ messageId: 'tooShort' }], + }, + ], + }); + }); + + it('flags two-char names when minLength is raised to 3', () => { + ruleTester.run('no-abbreviations', rule, { + valid: [], + invalid: [ + { + code: 'const cb = () => {};', + options: [{ minLength: 3 }], + errors: [{ messageId: 'tooShort', data: { name: 'cb', length: 2, min: 3 } }], + }, + { + code: 'items.map(fn => fn());', + options: [{ minLength: 3 }], + errors: [{ messageId: 'tooShort' }], + }, + ], + }); + }); + }); +});