diff --git a/CHANGELOG.md b/CHANGELOG.md index 61146ed..f85e4e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## v6 — Conservative Rule Expansion + +- Added 5 new conservative analysis rules: `no-console`, `no-empty-function`, `duplicate-imports`, `no-unreachable-after-return`, `no-throw-literal` +- All new rules registered in config defaults, all 4 presets, and rule registry +- Updated strict preset to elevate `no-console`, `no-unreachable-after-return`, and `no-throw-literal` to error severity +- Updated export workflow Known Limitations and ROADMAP for new rules +- Updated README with new rules table, configuration examples, and roadmap +- Added 20 new test cases covering all 5 new rules (194 total tests) + ## v5 — Summary Polish and Config Coverage - Improved human summary generator with complete sentence enforcement and deploy-readiness detection diff --git a/README.md b/README.md index 300eeaa..f2cfee9 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,11 @@ Manual code review is time-consuming and inconsistent. InspectoRepo provides det | `no-empty-catch` | warn | Flags empty catch blocks that silently hide errors — report only | | `no-useless-return` | info | Detects redundant `return;` at the end of functions — auto-fixable | | `ts-diagnostics` | error | Reports high-confidence TypeScript compiler diagnostics (unreachable code, duplicate identifiers, missing names, type mismatches) — report only | +| `no-console` | warn | Detects `console.log`, `console.warn`, `console.error` and similar calls left in production code | +| `no-empty-function` | info | Flags empty function bodies that may indicate incomplete implementation | +| `duplicate-imports` | info | Detects multiple import declarations from the same module and suggests combining them | +| `no-unreachable-after-return` | warn | Flags unreachable code after `return`, `throw`, `break`, or `continue` statements | +| `no-throw-literal` | warn | Detects throwing literal values instead of Error objects, which lose stack trace information | ## Tech Stack @@ -146,7 +151,7 @@ Apply safe code fixes interactively: inspectorepo fix ./my-project ``` -The fix command runs analysis, finds issues with safe auto-fix suggestions, shows a preview of each proposed change, and asks for confirmation before applying. Rules with auto-fix support: `optional-chaining`, `boolean-simplification`, `unused-imports`, `early-return`, `no-debugger`, and `no-useless-return`. Advisory rules like `complexity-hotspot`, `no-empty-catch`, and `ts-diagnostics` are never auto-applied. +The fix command runs analysis, finds issues with safe auto-fix suggestions, shows a preview of each proposed change, and asks for confirmation before applying. Rules with auto-fix support: `optional-chaining`, `boolean-simplification`, `unused-imports`, `early-return`, `no-debugger`, and `no-useless-return`. Advisory rules like `complexity-hotspot`, `no-empty-catch`, `ts-diagnostics`, `no-console`, `no-empty-function`, `duplicate-imports`, `no-unreachable-after-return`, and `no-throw-literal` are never auto-applied. ### Fix Preview Mode @@ -213,6 +218,11 @@ The CLI uses the same analysis engine as the web UI. Output is deterministic — | `no-empty-catch` | warn | ❌ | Flags empty catch blocks that silently hide errors | | `no-useless-return` | info | ✅ | Detects redundant `return;` at the end of functions | | `ts-diagnostics` | error | ❌ | Reports high-confidence TypeScript compiler diagnostics | +| `no-console` | warn | ❌ | Detects console calls left in production code | +| `no-empty-function` | info | ❌ | Flags empty function bodies that may indicate incomplete implementation | +| `duplicate-imports` | info | ❌ | Detects multiple imports from the same module | +| `no-unreachable-after-return` | warn | ❌ | Flags unreachable code after return/throw/break/continue | +| `no-throw-literal` | warn | ❌ | Detects throwing literals instead of Error objects | ## Configuration @@ -229,7 +239,12 @@ Create `.inspectorepo.json` in your project root to configure which rules run an "no-debugger": "warn", "no-empty-catch": "warn", "no-useless-return": "warn", - "ts-diagnostics": "off" + "ts-diagnostics": "off", + "no-console": "warn", + "no-empty-function": "warn", + "duplicate-imports": "warn", + "no-unreachable-after-return": "warn", + "no-throw-literal": "warn" } } ``` @@ -249,7 +264,7 @@ Available presets: | Preset | Description | |--------|-------------| | `recommended` | Balanced defaults — all rules at `warn` | -| `strict` | Stricter — `unused-imports` and `complexity-hotspot` at `error` | +| `strict` | Stricter — `unused-imports`, `complexity-hotspot`, `no-console`, `no-unreachable-after-return`, and `no-throw-literal` at `error` | | `cleanup` | Style-focused — disables `complexity-hotspot`, keeps simplification rules | | `react` | React/TS projects — `unused-imports` at `error`, all others `warn` | @@ -376,6 +391,7 @@ Download the `inspectorepo-report` artifact from the Actions tab to see the full - [x] VS Code extension for in-editor analysis - [x] Summary-only CLI mode (`--summary-only`) - [x] Improved PR comment summaries with top rules and package highlights +- [x] Conservative rules: `no-console`, `no-empty-function`, `duplicate-imports`, `no-unreachable-after-return`, `no-throw-literal` ### V4 — Planned diff --git a/ai/scripts/generate-repomix-exports.ts b/ai/scripts/generate-repomix-exports.ts index 68bd82f..697ba1f 100644 --- a/ai/scripts/generate-repomix-exports.ts +++ b/ai/scripts/generate-repomix-exports.ts @@ -566,6 +566,7 @@ const ROADMAP: RoadmapItem[] = [ { label: 'Web app onboarding About section and empty state', implemented: true }, { label: 'Richer complexity warnings with contributor breakdown', implemented: true }, { label: 'Conservative analysis rules (no-debugger, no-empty-catch, no-useless-return, ts-diagnostics)', implemented: true }, + { label: 'Additional conservative rules (no-console, no-empty-function, duplicate-imports, no-unreachable-after-return, no-throw-literal)', implemented: true }, { label: 'Deploy web app as a hosted service', implemented: false }, { label: 'Rule dependency graph and cascade analysis', implemented: false }, { label: 'Performance profiling for large codebases', implemented: false }, @@ -716,6 +717,7 @@ ${formatGroupedFiles(milestoneFiles)} ## Known Limitations - Auto-fix supports 6 rules (optional-chaining, boolean-simplification, unused-imports, early-return, no-debugger, no-useless-return) — more planned +- no-console, no-empty-function, duplicate-imports, no-unreachable-after-return, and no-throw-literal are advisory only — no auto-fix support - Complexity-hotspot, no-empty-catch, and ts-diagnostics are advisory only — no auto-fix support - Browser folder picker requires Chrome/Edge (File System Access API) diff --git a/packages/core/src/config.test.ts b/packages/core/src/config.test.ts index f916e50..81fe143 100644 --- a/packages/core/src/config.test.ts +++ b/packages/core/src/config.test.ts @@ -18,6 +18,11 @@ const ALL_RULE_IDS = [ 'no-empty-catch', 'no-useless-return', 'ts-diagnostics', + 'no-console', + 'no-empty-function', + 'duplicate-imports', + 'no-unreachable-after-return', + 'no-throw-literal', ]; describe('parseConfig', () => { diff --git a/packages/core/src/config.ts b/packages/core/src/config.ts index ddede46..b99cbcd 100644 --- a/packages/core/src/config.ts +++ b/packages/core/src/config.ts @@ -22,6 +22,11 @@ const DEFAULT_CONFIG: RuleConfig = { 'no-empty-catch': 'warn', 'no-useless-return': 'warn', 'ts-diagnostics': 'off', + 'no-console': 'warn', + 'no-empty-function': 'warn', + 'duplicate-imports': 'warn', + 'no-unreachable-after-return': 'warn', + 'no-throw-literal': 'warn', }; /** diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 32f1403..db92943 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -13,6 +13,11 @@ export { noEmptyCatchRule, noUselessReturnRule, tsDiagnosticsRule, + noConsoleRule, + noEmptyFunctionRule, + duplicateImportsRule, + noUnreachableAfterReturnRule, + noThrowLiteralRule, } from './rules/index.js'; export { isExcludedDir, diff --git a/packages/core/src/presets.ts b/packages/core/src/presets.ts index ece813e..557398f 100644 --- a/packages/core/src/presets.ts +++ b/packages/core/src/presets.ts @@ -13,6 +13,11 @@ const PRESETS: Record = { 'no-empty-catch': 'warn', 'no-useless-return': 'warn', 'ts-diagnostics': 'off', + 'no-console': 'warn', + 'no-empty-function': 'warn', + 'duplicate-imports': 'warn', + 'no-unreachable-after-return': 'warn', + 'no-throw-literal': 'warn', }, strict: { 'unused-imports': 'error', @@ -24,6 +29,11 @@ const PRESETS: Record = { 'no-empty-catch': 'error', 'no-useless-return': 'warn', 'ts-diagnostics': 'error', + 'no-console': 'error', + 'no-empty-function': 'warn', + 'duplicate-imports': 'warn', + 'no-unreachable-after-return': 'error', + 'no-throw-literal': 'error', }, cleanup: { 'unused-imports': 'warn', @@ -35,6 +45,11 @@ const PRESETS: Record = { 'no-empty-catch': 'warn', 'no-useless-return': 'warn', 'ts-diagnostics': 'off', + 'no-console': 'warn', + 'no-empty-function': 'warn', + 'duplicate-imports': 'warn', + 'no-unreachable-after-return': 'warn', + 'no-throw-literal': 'warn', }, react: { 'unused-imports': 'error', @@ -46,6 +61,11 @@ const PRESETS: Record = { 'no-empty-catch': 'warn', 'no-useless-return': 'warn', 'ts-diagnostics': 'off', + 'no-console': 'warn', + 'no-empty-function': 'warn', + 'duplicate-imports': 'warn', + 'no-unreachable-after-return': 'warn', + 'no-throw-literal': 'warn', }, }; diff --git a/packages/core/src/rules/conservative-rules.test.ts b/packages/core/src/rules/conservative-rules.test.ts new file mode 100644 index 0000000..4fa7a5c --- /dev/null +++ b/packages/core/src/rules/conservative-rules.test.ts @@ -0,0 +1,192 @@ +import { describe, it, expect } from 'vitest'; +import { analyzeCodebase } from '../analyzer.js'; +import { noConsoleRule } from './no-console.js'; +import { noEmptyFunctionRule } from './no-empty-function.js'; +import { duplicateImportsRule } from './duplicate-imports.js'; +import { noUnreachableAfterReturnRule } from './no-unreachable-after-return.js'; +import { noThrowLiteralRule } from './no-throw-literal.js'; + +function analyze(code: string, rules: NonNullable[0]['options']>['rules']) { + return analyzeCodebase({ + files: [{ path: 'src/test.ts', content: code }], + selectedDirectories: ['src'], + options: { rules }, + }); +} + +describe('no-console rule', () => { + it('detects console.log', () => { + const code = 'function foo() { console.log("hi"); }'; + const report = analyze(code, [noConsoleRule]); + expect(report.issues.length).toBe(1); + expect(report.issues[0].ruleId).toBe('no-console'); + expect(report.issues[0].message).toContain('console.log'); + }); + + it('detects console.warn and console.error', () => { + const code = [ + 'function a() { console.warn("w"); }', + 'function b() { console.error("e"); }', + ].join('\n'); + const report = analyze(code, [noConsoleRule]); + expect(report.issues.length).toBe(2); + }); + + it('does not flag non-console calls', () => { + const code = 'function foo() { logger.log("hi"); }'; + const report = analyze(code, [noConsoleRule]); + expect(report.issues.length).toBe(0); + }); + + it('does not flag console without method call', () => { + const code = 'const c = console;'; + const report = analyze(code, [noConsoleRule]); + expect(report.issues.length).toBe(0); + }); +}); + +describe('no-empty-function rule', () => { + it('detects empty function declaration', () => { + const code = 'function foo() {}'; + const report = analyze(code, [noEmptyFunctionRule]); + expect(report.issues.length).toBe(1); + expect(report.issues[0].ruleId).toBe('no-empty-function'); + }); + + it('detects empty arrow function', () => { + const code = 'const fn = () => {};'; + const report = analyze(code, [noEmptyFunctionRule]); + expect(report.issues.length).toBe(1); + }); + + it('does not flag function with body', () => { + const code = 'function foo() { return 1; }'; + const report = analyze(code, [noEmptyFunctionRule]); + expect(report.issues.length).toBe(0); + }); + + it('does not flag function with comment (intentional stub)', () => { + const code = [ + 'function foo() {', + ' // intentionally empty', + '}', + ].join('\n'); + const report = analyze(code, [noEmptyFunctionRule]); + expect(report.issues.length).toBe(0); + }); +}); + +describe('duplicate-imports rule', () => { + it('detects duplicate imports from same module', () => { + const code = [ + "import { a } from 'mod';", + "import { b } from 'mod';", + ].join('\n'); + const report = analyze(code, [duplicateImportsRule]); + expect(report.issues.length).toBe(1); + expect(report.issues[0].ruleId).toBe('duplicate-imports'); + expect(report.issues[0].message).toContain('mod'); + }); + + it('does not flag imports from different modules', () => { + const code = [ + "import { a } from 'mod-a';", + "import { b } from 'mod-b';", + ].join('\n'); + const report = analyze(code, [duplicateImportsRule]); + expect(report.issues.length).toBe(0); + }); + + it('does not flag single import', () => { + const code = "import { a } from 'mod';"; + const report = analyze(code, [duplicateImportsRule]); + expect(report.issues.length).toBe(0); + }); +}); + +describe('no-unreachable-after-return rule', () => { + it('detects unreachable code after return', () => { + const code = [ + 'function foo() {', + ' return 1;', + ' const x = 2;', + '}', + ].join('\n'); + const report = analyze(code, [noUnreachableAfterReturnRule]); + expect(report.issues.length).toBe(1); + expect(report.issues[0].ruleId).toBe('no-unreachable-after-return'); + expect(report.issues[0].message).toContain('Unreachable'); + }); + + it('detects unreachable code after throw', () => { + const code = [ + 'function foo() {', + ' throw new Error("fail");', + ' console.log("never");', + '}', + ].join('\n'); + const report = analyze(code, [noUnreachableAfterReturnRule]); + expect(report.issues.length).toBe(1); + }); + + it('does not flag reachable code', () => { + const code = [ + 'function foo(x: boolean) {', + ' if (x) return 1;', + ' return 2;', + '}', + ].join('\n'); + const report = analyze(code, [noUnreachableAfterReturnRule]); + expect(report.issues.length).toBe(0); + }); + + it('does not flag type declarations after return', () => { + const code = [ + 'function foo() {', + ' return 1;', + ' type MyType = string;', + '}', + ].join('\n'); + const report = analyze(code, [noUnreachableAfterReturnRule]); + expect(report.issues.length).toBe(0); + }); +}); + +describe('no-throw-literal rule', () => { + it('detects throw string literal', () => { + const code = 'function foo() { throw "error"; }'; + const report = analyze(code, [noThrowLiteralRule]); + expect(report.issues.length).toBe(1); + expect(report.issues[0].ruleId).toBe('no-throw-literal'); + expect(report.issues[0].message).toContain('literal'); + }); + + it('detects throw number literal', () => { + const code = 'function foo() { throw 42; }'; + const report = analyze(code, [noThrowLiteralRule]); + expect(report.issues.length).toBe(1); + }); + + it('does not flag throw new Error()', () => { + const code = 'function foo() { throw new Error("fail"); }'; + const report = analyze(code, [noThrowLiteralRule]); + expect(report.issues.length).toBe(0); + }); + + it('does not flag throw variable', () => { + const code = [ + 'function foo() {', + ' const err = new Error("fail");', + ' throw err;', + '}', + ].join('\n'); + const report = analyze(code, [noThrowLiteralRule]); + expect(report.issues.length).toBe(0); + }); + + it('does not flag throw function call result', () => { + const code = 'function foo() { throw createError(); }'; + const report = analyze(code, [noThrowLiteralRule]); + expect(report.issues.length).toBe(0); + }); +}); diff --git a/packages/core/src/rules/duplicate-imports.ts b/packages/core/src/rules/duplicate-imports.ts new file mode 100644 index 0000000..5845f5a --- /dev/null +++ b/packages/core/src/rules/duplicate-imports.ts @@ -0,0 +1,51 @@ +import type { Issue } from '@inspectorepo/shared'; +import type { Rule, RuleContext } from '../rule.js'; + +export const duplicateImportsRule: Rule = { + id: 'duplicate-imports', + title: 'Duplicate Imports', + severity: 'info', + + run(ctx: RuleContext): Issue[] { + const issues: Issue[] = []; + const sourceFile = ctx.sourceFile; + + // Group imports by module specifier + const importsByModule = new Map(); + + for (const importDecl of sourceFile.getImportDeclarations()) { + const module = importDecl.getModuleSpecifierValue(); + const entry = { + line: importDecl.getStartLineNumber(), + text: importDecl.getText(), + }; + const existing = importsByModule.get(module) ?? []; + existing.push(entry); + importsByModule.set(module, existing); + } + + for (const [module, imports] of importsByModule) { + if (imports.length < 2) continue; + + const lines = imports.map(i => i.line).join(', '); + + issues.push({ + id: `${ctx.filePath}:${imports[0].line}:duplicate-imports`, + ruleId: 'duplicate-imports', + severity: 'info', + message: `Multiple imports from '${module}' at lines ${lines} — consider combining them.`, + filePath: ctx.filePath, + range: { + start: { line: imports[0].line, column: 1 }, + end: { line: imports[imports.length - 1].line, column: 1 }, + }, + suggestion: { + summary: `Combine ${imports.length} imports from '${module}' into a single import statement.`, + details: `Found ${imports.length} separate import declarations from the same module.`, + }, + }); + } + + return issues; + }, +}; diff --git a/packages/core/src/rules/index.ts b/packages/core/src/rules/index.ts index ab8c9fb..ae3a078 100644 --- a/packages/core/src/rules/index.ts +++ b/packages/core/src/rules/index.ts @@ -8,6 +8,11 @@ import { noDebuggerRule } from './no-debugger.js'; import { noEmptyCatchRule } from './no-empty-catch.js'; import { noUselessReturnRule } from './no-useless-return.js'; import { tsDiagnosticsRule } from './ts-diagnostics.js'; +import { noConsoleRule } from './no-console.js'; +import { noEmptyFunctionRule } from './no-empty-function.js'; +import { duplicateImportsRule } from './duplicate-imports.js'; +import { noUnreachableAfterReturnRule } from './no-unreachable-after-return.js'; +import { noThrowLiteralRule } from './no-throw-literal.js'; export { unusedImportsRule } from './unused-imports.js'; export { complexityHotspotRule } from './complexity-hotspot.js'; @@ -18,6 +23,11 @@ export { noDebuggerRule } from './no-debugger.js'; export { noEmptyCatchRule } from './no-empty-catch.js'; export { noUselessReturnRule } from './no-useless-return.js'; export { tsDiagnosticsRule } from './ts-diagnostics.js'; +export { noConsoleRule } from './no-console.js'; +export { noEmptyFunctionRule } from './no-empty-function.js'; +export { duplicateImportsRule } from './duplicate-imports.js'; +export { noUnreachableAfterReturnRule } from './no-unreachable-after-return.js'; +export { noThrowLiteralRule } from './no-throw-literal.js'; export const allRules: Rule[] = [ unusedImportsRule, @@ -29,4 +39,9 @@ export const allRules: Rule[] = [ noEmptyCatchRule, noUselessReturnRule, tsDiagnosticsRule, + noConsoleRule, + noEmptyFunctionRule, + duplicateImportsRule, + noUnreachableAfterReturnRule, + noThrowLiteralRule, ]; diff --git a/packages/core/src/rules/no-console.ts b/packages/core/src/rules/no-console.ts new file mode 100644 index 0000000..18e798b --- /dev/null +++ b/packages/core/src/rules/no-console.ts @@ -0,0 +1,54 @@ +import type { Issue } from '@inspectorepo/shared'; +import type { Rule, RuleContext } from '../rule.js'; +import { SyntaxKind } from 'ts-morph'; + +const CONSOLE_METHODS = new Set(['log', 'warn', 'error', 'info', 'debug']); + +export const noConsoleRule: Rule = { + id: 'no-console', + title: 'No Console', + severity: 'warn', + + run(ctx: RuleContext): Issue[] { + const issues: Issue[] = []; + const sourceFile = ctx.sourceFile; + + sourceFile.forEachDescendant((node) => { + if (node.getKind() !== SyntaxKind.CallExpression) return; + + const expr = node.getChildren()[0]; + if (!expr || expr.getKind() !== SyntaxKind.PropertyAccessExpression) return; + + const children = expr.getChildren(); + if (children.length < 3) return; + + const obj = children[0]; + const method = children[2]; + if (obj.getText() !== 'console') return; + + const methodName = method.getText(); + if (!CONSOLE_METHODS.has(methodName)) return; + + const line = node.getStartLineNumber(); + const lineText = node.getText(); + + issues.push({ + id: `${ctx.filePath}:${line}:no-console`, + ruleId: 'no-console', + severity: 'warn', + message: `console.${methodName}() call found at line ${line}.`, + filePath: ctx.filePath, + range: { + start: { line, column: 1 }, + end: { line: node.getEndLineNumber(), column: 1 }, + }, + suggestion: { + summary: 'Remove or replace console usage before shipping.', + details: `Console statements should be removed from production code. Found: ${lineText}`, + }, + }); + }); + + return issues; + }, +}; diff --git a/packages/core/src/rules/no-empty-function.ts b/packages/core/src/rules/no-empty-function.ts new file mode 100644 index 0000000..d681eac --- /dev/null +++ b/packages/core/src/rules/no-empty-function.ts @@ -0,0 +1,66 @@ +import type { Issue } from '@inspectorepo/shared'; +import type { Rule, RuleContext } from '../rule.js'; +import { SyntaxKind } from 'ts-morph'; + +export const noEmptyFunctionRule: Rule = { + id: 'no-empty-function', + title: 'No Empty Function', + severity: 'info', + + run(ctx: RuleContext): Issue[] { + const issues: Issue[] = []; + const sourceFile = ctx.sourceFile; + + const functions = [ + ...sourceFile.getFunctions(), + ...sourceFile.getDescendantsOfKind(SyntaxKind.ArrowFunction), + ...sourceFile.getDescendantsOfKind(SyntaxKind.MethodDeclaration), + ]; + + for (const fn of functions) { + const body = fn.getBody?.(); + if (!body) continue; + if (body.getKind() !== SyntaxKind.Block) continue; + + const statements = body.getChildSyntaxList()?.getChildren() ?? []; + if (statements.length > 0) continue; + + // Skip functions with comments in the body (intentional stub) + const bodyText = body.getFullText(); + if (bodyText.includes('//') || bodyText.includes('/*')) continue; + + // Get function name + let name = '(anonymous)'; + if ('getName' in fn && typeof fn.getName === 'function') { + name = fn.getName() || '(anonymous)'; + } else { + const parent = fn.getParent(); + if (parent && parent.getKind() === SyntaxKind.VariableDeclaration) { + if ('getName' in parent && typeof parent.getName === 'function') { + name = parent.getName(); + } + } + } + + const line = fn.getStartLineNumber(); + + issues.push({ + id: `${ctx.filePath}:${line}:no-empty-function`, + ruleId: 'no-empty-function', + severity: 'info', + message: `Empty function "${name}" at line ${line}.`, + filePath: ctx.filePath, + range: { + start: { line, column: 1 }, + end: { line: fn.getEndLineNumber(), column: 1 }, + }, + suggestion: { + summary: 'Implement function logic, remove it, or add a comment documenting the intentional stub.', + details: 'Empty function bodies may indicate incomplete implementation.', + }, + }); + } + + return issues; + }, +}; diff --git a/packages/core/src/rules/no-throw-literal.ts b/packages/core/src/rules/no-throw-literal.ts new file mode 100644 index 0000000..770bc2e --- /dev/null +++ b/packages/core/src/rules/no-throw-literal.ts @@ -0,0 +1,53 @@ +import { SyntaxKind } from 'ts-morph'; +import type { Issue } from '@inspectorepo/shared'; +import type { Rule, RuleContext } from '../rule.js'; + +export const noThrowLiteralRule: Rule = { + id: 'no-throw-literal', + title: 'No Throw Literal', + severity: 'warn', + + run(ctx: RuleContext): Issue[] { + const issues: Issue[] = []; + const sourceFile = ctx.sourceFile; + + for (const throwStmt of sourceFile.getDescendantsOfKind(SyntaxKind.ThrowStatement)) { + const expression = throwStmt.getExpression(); + if (!expression) continue; + + const kind = expression.getKind(); + + // Allow: throw new Error(), throw new SomeError(), throw variable, throw functionCall() + if (kind === SyntaxKind.NewExpression) continue; + if (kind === SyntaxKind.Identifier) continue; + if (kind === SyntaxKind.CallExpression) continue; + if (kind === SyntaxKind.AwaitExpression) continue; + if (kind === SyntaxKind.PropertyAccessExpression) continue; + if (kind === SyntaxKind.ElementAccessExpression) continue; + if (kind === SyntaxKind.ConditionalExpression) continue; + + // Flag: throw 'string', throw 42, throw null, throw undefined, throw true, etc. + const line = throwStmt.getStartLineNumber(); + const thrownText = expression.getText().slice(0, 60); + + issues.push({ + id: `${ctx.filePath}:${line}:no-throw-literal`, + ruleId: 'no-throw-literal', + severity: 'warn', + message: `Throwing a literal value '${thrownText}' — throw an Error object instead for proper stack traces.`, + filePath: ctx.filePath, + range: { + start: { line, column: 1 }, + end: { line: throwStmt.getEndLineNumber(), column: 1 }, + }, + suggestion: { + summary: `Replace 'throw ${thrownText}' with 'throw new Error(${thrownText})'.`, + details: + 'Thrown literals lose stack trace information. Use Error objects or custom Error subclasses for reliable error handling.', + }, + }); + } + + return issues; + }, +}; diff --git a/packages/core/src/rules/no-unreachable-after-return.ts b/packages/core/src/rules/no-unreachable-after-return.ts new file mode 100644 index 0000000..d5a8467 --- /dev/null +++ b/packages/core/src/rules/no-unreachable-after-return.ts @@ -0,0 +1,72 @@ +import { SyntaxKind } from 'ts-morph'; +import type { Issue } from '@inspectorepo/shared'; +import type { Rule, RuleContext } from '../rule.js'; + +export const noUnreachableAfterReturnRule: Rule = { + id: 'no-unreachable-after-return', + title: 'No Unreachable Code After Return', + severity: 'warn', + + run(ctx: RuleContext): Issue[] { + const issues: Issue[] = []; + const sourceFile = ctx.sourceFile; + + // Terminal statement kinds: return, throw, continue, break + const terminalKinds = new Set([ + SyntaxKind.ReturnStatement, + SyntaxKind.ThrowStatement, + SyntaxKind.ContinueStatement, + SyntaxKind.BreakStatement, + ]); + + for (const block of sourceFile.getDescendantsOfKind(SyntaxKind.Block)) { + const statements = block.getStatements(); + if (statements.length < 2) continue; + + for (let i = 0; i < statements.length - 1; i++) { + const stmt = statements[i]; + if (!terminalKinds.has(stmt.getKind())) continue; + + // All statements after a terminal statement in the same block are unreachable + const unreachable = statements.slice(i + 1); + if (unreachable.length === 0) continue; + + // Skip if the unreachable code is only type/interface declarations (they don't execute) + const executableUnreachable = unreachable.filter( + s => + s.getKind() !== SyntaxKind.TypeAliasDeclaration && + s.getKind() !== SyntaxKind.InterfaceDeclaration, + ); + if (executableUnreachable.length === 0) continue; + + const terminalLine = stmt.getStartLineNumber(); + const terminalKind = stmt.getKindName().replace('Statement', '').toLowerCase(); + const firstUnreachable = executableUnreachable[0]; + + issues.push({ + id: `${ctx.filePath}:${firstUnreachable.getStartLineNumber()}:no-unreachable-after-return`, + ruleId: 'no-unreachable-after-return', + severity: 'warn', + message: `Unreachable code after '${terminalKind}' on line ${terminalLine} — ${executableUnreachable.length} statement(s) will never execute.`, + filePath: ctx.filePath, + range: { + start: { line: firstUnreachable.getStartLineNumber(), column: 1 }, + end: { + line: executableUnreachable[executableUnreachable.length - 1].getEndLineNumber(), + column: 1, + }, + }, + suggestion: { + summary: `Remove unreachable code after '${terminalKind}'.`, + details: `${executableUnreachable.length} statement(s) after '${terminalKind}' on line ${terminalLine} can never execute.`, + }, + }); + + // Only flag first terminal per block + break; + } + } + + return issues; + }, +};