From 4f2833f5e2e2af8484ff744d270e7a850f49f3d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 17 Jun 2026 00:08:56 +0000 Subject: [PATCH 1/2] =?UTF-8?q?A=20new=20enforced=20smell=20for=20excessiv?= =?UTF-8?q?ely=20nested=20blocks.=20Adding=20it=20touches=20only=20the=20c?= =?UTF-8?q?atalogue=20(source=20eslint,=20sourceRuleId=20max-depth)=20?= =?UTF-8?q?=E2=80=94=20the=20eslint=20translation=20and=20preset=20produce?= =?UTF-8?q?s=20auto-derive=20via=20#24,=20so=20the=20runner,=20sensors,=20?= =?UTF-8?q?checks,=20and=20registry=20need=20zero=20edits.=20Ships=20a=20R?= =?UTF-8?q?OSE-pattern=20prompt=20and=20an=20end-to-end=20test=20that=20fi?= =?UTF-8?q?res=20it=20through=20the=20real=20runner.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Nq6xHU6JtSNpYWhMYjXB15 --- docs/smell-vocabulary.md | 6 +++- src/config/catalogue-unused.ts | 40 ++++++++++++++++++++++++++ src/config/catalogue.ts | 52 +++++++--------------------------- src/deep-nesting.test.ts | 46 ++++++++++++++++++++++++++++++ src/prompts/deep-nesting.md | 13 +++++++++ 5 files changed, 115 insertions(+), 42 deletions(-) create mode 100644 src/config/catalogue-unused.ts create mode 100644 src/deep-nesting.test.ts create mode 100644 src/prompts/deep-nesting.md diff --git a/docs/smell-vocabulary.md b/docs/smell-vocabulary.md index c414be5..d230054 100644 --- a/docs/smell-vocabulary.md +++ b/docs/smell-vocabulary.md @@ -26,6 +26,7 @@ exits 0. The mapper config can override it per project. | `oversized-function` | Oversized function | enforced | | `too-many-parameters` | Too many parameters | enforced | | `high-complexity` | High cyclomatic complexity | enforced | +| `deep-nesting` | Deep nesting | enforced | | `oversized-file` | Oversized file | enforced | | `unused-variable` | Unused variable | enforced | | `loose-equality` | Loose equality | enforced | @@ -59,6 +60,7 @@ to. | `eslint:max-lines-per-function` | `oversized-function` | | `eslint:max-params` | `too-many-parameters` | | `eslint:complexity` | `high-complexity` | +| `eslint:max-depth` | `deep-nesting` | | `eslint:max-lines` | `oversized-file` | | `eslint:no-unused-vars` | `unused-variable` | | `eslint:eqeqeq` | `loose-equality` | @@ -97,7 +99,9 @@ TS-only smells (`explicit-any`, `var-declaration`, …) simply do not appear in the Python preset. `oversized-file` has no clean ruff rule, so the Python preset emits it from a language-agnostic line-count sensor whose threshold (`max-module-lines`, default 200) is read from the consumer's config text (see -`DECISIONS.md`). +`DECISIONS.md`). `deep-nesting` ships for TypeScript only (ESLint `max-depth`); +the Python equivalent (ruff `PLR1702`) is preview/unstable, so it is deferred +rather than opting the default preset into ruff `--preview`. ## Combinations diff --git a/src/config/catalogue-unused.ts b/src/config/catalogue-unused.ts new file mode 100644 index 0000000..5ded796 --- /dev/null +++ b/src/config/catalogue-unused.ts @@ -0,0 +1,40 @@ +import type { Rule } from '../types.js'; + +// The "unused" family: knip (TS), deptry (unused-dependency), ruff (unused-import). +export const unusedRules: Rule[] = [ + { + id: 'unused-class-member', + source: 'knip', + severity: 'enforced', + title: 'Unused class member', + description: 'Class methods or properties not referenced anywhere are dead weight.', + }, + { + id: 'unused-file', + source: 'knip', + severity: 'enforced', + title: 'Unused file', + description: 'Files not referenced anywhere are dead weight; remove them.', + }, + { + id: 'unused-export', + source: 'knip', + severity: 'enforced', + title: 'Unused export', + description: 'Exports not imported anywhere are dead weight; drop the export or the symbol.', + }, + { + id: 'unused-dependency', + source: 'knip', + severity: 'enforced', + title: 'Unused dependency', + description: 'Dependencies declared but never used should be removed from the manifest.', + }, + { + id: 'unused-import', + source: 'eslint', + severity: 'enforced', + title: 'Unused import', + description: 'Imports that are never used should be removed.', + }, +]; diff --git a/src/config/catalogue.ts b/src/config/catalogue.ts index 11642d3..bc5a634 100644 --- a/src/config/catalogue.ts +++ b/src/config/catalogue.ts @@ -1,7 +1,7 @@ import type { Rule } from '../types.js'; +import { unusedRules } from './catalogue-unused.js'; -// The canonical, tool-independent smell catalogue (docs/smell-vocabulary.md): a -// rule here makes the smell coached and activates its producing sensor. +// The canonical, tool-independent smell catalogue (docs/smell-vocabulary.md). const tier1Rules: Rule[] = [ { @@ -30,6 +30,15 @@ const tier1Rules: Rule[] = [ title: 'High cyclomatic complexity', description: 'Complex functions are harder to understand, test, and maintain.', }, + { + id: 'deep-nesting', + source: 'eslint', + sourceRuleId: 'max-depth', + severity: 'enforced', + changedFilesOnly: true, + title: 'Deep nesting', + description: 'Deeply nested blocks want guard clauses, early returns, or an extracted helper.', + }, { id: 'oversized-file', source: 'eslint', @@ -151,45 +160,6 @@ const jscpdRules: Rule[] = [ }, ]; -// The "unused" family: knip (TS), deptry (unused-dependency), ruff (unused-import). -const unusedRules: Rule[] = [ - { - id: 'unused-class-member', - source: 'knip', - severity: 'enforced', - title: 'Unused class member', - description: 'Class methods or properties not referenced anywhere are dead weight.', - }, - { - id: 'unused-file', - source: 'knip', - severity: 'enforced', - title: 'Unused file', - description: 'Files not referenced anywhere are dead weight; remove them.', - }, - { - id: 'unused-export', - source: 'knip', - severity: 'enforced', - title: 'Unused export', - description: 'Exports not imported anywhere are dead weight; drop the export or the symbol.', - }, - { - id: 'unused-dependency', - source: 'knip', - severity: 'enforced', - title: 'Unused dependency', - description: 'Dependencies declared but never used should be removed from the manifest.', - }, - { - id: 'unused-import', - source: 'eslint', - severity: 'enforced', - title: 'Unused import', - description: 'Imports that are never used should be removed.', - }, -]; - export const defaultRules: Rule[] = [ ...tier1Rules, ...tier2Rules, diff --git a/src/deep-nesting.test.ts b/src/deep-nesting.test.ts new file mode 100644 index 0000000..e1453c7 --- /dev/null +++ b/src/deep-nesting.test.ts @@ -0,0 +1,46 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { run } from './runner.js'; + +const NESTED = `export function deep(items) { + for (const a of items) { + if (a > 0) { + while (a < 100) { + if (a % 2 === 0) { + return a; + } + } + } + } + return 0; +} +`; + +describe('deep-nesting smell (eslint max-depth)', () => { + let dir: string; + + afterEach(() => { + if (dir) rmSync(dir, { recursive: true, force: true }); + }); + + function setup(maxDepth: number): void { + dir = mkdtempSync(join(tmpdir(), 'hh-deep-nesting-')); + writeFileSync( + join(dir, 'eslint.config.js'), + `export default [{ languageOptions: { sourceType: 'module', ecmaVersion: 2022 }, rules: { 'max-depth': ['error', { max: ${String(maxDepth)} }] } }];\n`, + ); + writeFileSync(join(dir, 'nested.js'), NESTED); + } + + it('fires deep-nesting end-to-end when nesting exceeds the configured threshold', async () => { + setup(2); + expect((await run(dir)).violations.some((v) => v.ruleId === 'deep-nesting')).toBe(true); + }); + + it('does not fire when nesting is within the configured threshold', async () => { + setup(6); + expect((await run(dir)).violations.some((v) => v.ruleId === 'deep-nesting')).toBe(false); + }); +}); diff --git a/src/prompts/deep-nesting.md b/src/prompts/deep-nesting.md new file mode 100644 index 0000000..9ffcd3c --- /dev/null +++ b/src/prompts/deep-nesting.md @@ -0,0 +1,13 @@ +Deeply nested blocks — `if` inside `for` inside `try` inside `if` — force the reader to hold every enclosing condition in their head at once. The smell is not the indentation; it is that the function is doing too much branching in one place. + +Read the nesting from the inside out and ask what the innermost block actually needs. Usually most of the enclosing conditions are *guards* — preconditions that should be checked and bailed on early, not wrapped around the real work. + +Prefer, in order: + +1. **Guard clauses / early returns.** Invert a condition and `return` (or `continue`/`throw`) early so the happy path stays at the top indentation level. Each guard you lift removes one level of nesting from everything below it. +2. **Extract a helper.** When an inner block is a coherent sub-step, pull it into a named function. The name documents the intent and the nesting moves into a flat, separately-readable unit. +3. **Replace the structure.** A deep `if/else` ladder is often a lookup table or a polymorphic dispatch in disguise; a nested loop is often a `filter`/`map`/`flatMap` pipeline. + +Avoid the mechanical fix of merging conditions with `&&` just to drop a level — that trades vertical nesting for an unreadable horizontal condition. The goal is a function whose shape you can take in at a glance, not one that merely passes the depth threshold. + +If the nesting is genuinely irreducible (a real algorithm with interacting conditions), extracting the inner loops into well-named helpers is still the move: keep each function shallow even when the algorithm as a whole is deep. From 35328cb4dbc282480fb8e0652128ea5557f7c837 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 17 Jun 2026 00:08:56 +0000 Subject: [PATCH 2/2] A first-time consumer's `habit-hooks init` now writes a tunable `max-depth: 4` rule alongside the other thresholds, so deep-nesting is configurable from the eslint config like the other TS smells. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Nq6xHU6JtSNpYWhMYjXB15 --- docs/DECISIONS.md | 12 +++++++++++- src/cli/init/scaffold-eslint-config.test.ts | 2 ++ src/cli/init/templates/eslint-config.ts | 1 + 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index a6ba4a8..f129dcd 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -332,7 +332,6 @@ phases (PR #13). Each call below is an _agent decision_. gated out, so a config that disables an input can't crash the run. The catalogue entry lives in `customRules` (source `custom`) to stay under the 200-line cap. - - **Smell knowledge lives only in config; the sensor layer consumes it.** _(agent decision, #24)_ All tool/smell mappings moved to `src/config/tool-smells.ts`: the eslint raw→smell map, eslint/knip/jscpd/comment produces, and the ruff + @@ -346,3 +345,14 @@ phases (PR #13). Each call below is an _agent decision_. (catalogue) + `src/cli/init/` (the scaffolded eslint config) — proved live by the `deep-nesting` smell (#26). Realizes the locked "spec from config" via config-derived constants rather than per-call DI, keeping the refactor low-risk. + +- **`deep-nesting` (TS, ESLint `max-depth`) is #24's live demonstrator.** _(agent + decision, #26)_ Added the smell touching only `src/config/catalogue.ts` (the + rule, `source: eslint`, `sourceRuleId: max-depth`, severity **enforced** to + mirror `high-complexity`) and `src/cli/init/templates/eslint-config.ts` (the + scaffolded `max-depth: 4`). The eslint raw→smell translation and the preset's + produces **auto-derived** from the catalogue with zero edits to the runner, + sensors, checks, or rules registry — the proof that #24 worked. Python + `deep-nesting` (ruff `PLR1702`) is **deferred**: it is preview/unstable, and we + do not opt the default preset into ruff `--preview`. Reversible — Python can be + enabled once PLR1702 stabilises or via an AST detector. diff --git a/src/cli/init/scaffold-eslint-config.test.ts b/src/cli/init/scaffold-eslint-config.test.ts index 7e3a16b..e7e7e86 100644 --- a/src/cli/init/scaffold-eslint-config.test.ts +++ b/src/cli/init/scaffold-eslint-config.test.ts @@ -34,6 +34,8 @@ describe('scaffoldEslintConfig', () => { expect(contents).toContain('max: 3'); expect(contents).toContain("'complexity'"); expect(contents).toContain('max: 10'); + expect(contents).toContain("'max-depth'"); + expect(contents).toContain('max: 4'); expect(contents).toContain("'max-lines'"); expect(contents).toContain('max: 200'); }); diff --git a/src/cli/init/templates/eslint-config.ts b/src/cli/init/templates/eslint-config.ts index 94647f7..a67557d 100644 --- a/src/cli/init/templates/eslint-config.ts +++ b/src/cli/init/templates/eslint-config.ts @@ -18,6 +18,7 @@ export default tseslint.config( 'max-lines-per-function': ['error', { max: 12, skipBlankLines: false, skipComments: false, IIFEs: true }], 'max-params': ['error', { max: 3 }], 'complexity': ['error', { max: 10 }], + 'max-depth': ['error', { max: 4 }], 'max-lines': ['error', { max: 200, skipBlankLines: false, skipComments: false }], 'no-unused-vars': ['error', { argsIgnorePattern: '^_', varsIgnorePattern: '^_' }], '@typescript-eslint/no-unused-vars': 'off',