Skip to content

Commit d5aa56d

Browse files
committed
chore: Add EditorConfig and enhance ESLint rules for code quality
Implements stricter code quality controls to prevent common issues from being committed to the codebase. This includes both editor configuration standardization and enhanced linting rules. Key changes: - Added .editorconfig with Angular defaults for consistent formatting across all editors (VS Code, WebStorm, etc.) - Enhanced ESLint configuration with three new ERROR-level rules: * no-console: Prevents accidental console.log statements in production * no-debugger: Prevents committed debugger statements * @typescript-eslint/no-explicit-any: Enforces proper TypeScript typing - Fixed all 'any' types in production code: * import-manager.ts: Replaced 'any' with proper ts-morph types (ImportDeclaration, ImportEqualsDeclaration, ExportDeclaration) * Test files: Documented legitimate 'any' usage in mocks with eslint-disable comments (for test flexibility) - Removed redundant ci.yml workflow (test.yml already exists and is comprehensive) The ESLint rules are set to ERROR (not warning) to ensure CI fails on violations. The linter already runs twice in existing CI via the pretest hook (once in compile, once directly), so these rules will automatically enforce on all pushes and PRs. All 259 tests passing. TypeScript compilation clean. ESLint clean. Related to project audit findings and preparation for 4.0.0 release.
1 parent 3b9724c commit d5aa56d

12 files changed

Lines changed: 259 additions & 5 deletions

.claude/settings.local.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@
3030
"SlashCommand(/git-quick)",
3131
"Read(///**)",
3232
"Bash(gh run view 18709470348 --log)",
33-
"WebFetch(domain:ts-morph.com)"
33+
"WebFetch(domain:ts-morph.com)",
34+
"Bash(npm outdated)",
35+
"Bash(npm run check-types)",
36+
"Bash(npm run lint)"
3437
],
3538
"deny": [],
3639
"ask": []

.editorconfig

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
# EditorConfig helps developers define and maintain consistent
2+
# coding styles between different editors and IDEs
3+
# editorconfig.org
4+
5+
root = true
6+
7+
[*]
8+
charset = utf-8
9+
indent_style = space
10+
indent_size = 2
11+
insert_final_newline = true
12+
trim_trailing_whitespace = true
13+
14+
[*.ts]
15+
quote_type = single
16+
17+
[*.md]
18+
max_line_length = off
19+
trim_trailing_whitespace = false
20+
21+
[*.yml]
22+
indent_size = 2
23+
24+
[*.sh]
25+
end_of_line = lf
26+
27+
[*.{json,eslintrc}]
28+
indent_size = 2

CLAUDE_TODO.md

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4322,3 +4322,193 @@ Total: 370 tests, 367 passing, 0 warnings
43224322
- **Comparison harness**: 179 passing (includes 17 new Additional Coverage tests)
43234323
- **Total**: 438 tests passing ✅
43244324

4325+
4326+
---
4327+
4328+
## Session: 2025-10-29 - Code Quality Audit and Improvements
4329+
4330+
### 1. Current Work Status
4331+
4332+
#### ✅ Completed Tasks:
4333+
1. **Comprehensive Project Audit** - Conducted full codebase audit covering:
4334+
- Source code quality (TypeScript, architecture, patterns)
4335+
- Test coverage and quality (259 tests passing)
4336+
- Configuration files and build setup
4337+
- Documentation accuracy
4338+
- Dependency freshness
4339+
- Error handling patterns
4340+
4341+
2. **Added `.editorconfig`** - Created EditorConfig file with Angular defaults:
4342+
- UTF-8 charset, 2-space indentation, LF line endings
4343+
- Trim trailing whitespace, insert final newline
4344+
- Special rules for TypeScript, Markdown, YAML, JSON
4345+
4346+
3. **Enhanced ESLint Configuration** - Added strict error-level rules:
4347+
- `"no-console": "error"` - Prevents accidental console.log statements
4348+
- `"no-debugger": "error"` - Prevents committed debugger statements
4349+
- `"@typescript-eslint/no-explicit-any": "error"` - Enforces proper typing
4350+
4351+
4. **Fixed All `any` Types in Production Code**:
4352+
- `src/imports/import-manager.ts`:
4353+
- Fixed `extractImportAttributes(importDecl: ImportDeclaration)` - was `any`
4354+
- Fixed `allImports` array type: `Array<ImportDeclaration | ImportEqualsDeclaration | ExportDeclaration>` - was `as any[]`
4355+
- Added proper type imports from `ts-morph`
4356+
- Test files: Documented legitimate `any` usage in mocks with `eslint-disable` comments
4357+
4358+
5. **Corrected Redundant Work** - Discovered existing CI/CD workflow:
4359+
- Removed redundant `ci.yml` that was mistakenly created
4360+
- Existing `test.yml` already runs linter via `npm test``pretest` hook
4361+
- Linter runs TWICE in CI: once in compile, once directly
4362+
4363+
#### ❌ In-Progress Tasks:
4364+
None - all tasks completed.
4365+
4366+
#### 🚫 Blocked Items:
4367+
None.
4368+
4369+
---
4370+
4371+
### 2. Technical Context
4372+
4373+
#### Files Modified:
4374+
1. **`.editorconfig`** (NEW)
4375+
- Created with Angular defaults for consistent formatting
4376+
4377+
2. **`eslint.config.mjs`**
4378+
- Added three new error-level rules: `no-console`, `no-debugger`, `@typescript-eslint/no-explicit-any`
4379+
4380+
3. **`src/imports/import-manager.ts`**
4381+
- Line 1: Added `ImportDeclaration`, `ImportEqualsDeclaration`, `ExportDeclaration` to imports
4382+
- Line 75: Fixed parameter type from `any` to `ImportDeclaration`
4383+
- Lines 700-712: Fixed array type casting from `as any[]` to proper union type
4384+
4385+
4. **Test Files (5 files)** - Added `eslint-disable` comments for legitimate mock `any` usage:
4386+
- `src/test/import-manager.test.ts`
4387+
- `src/test/import-manager.edge-cases-audit.test.ts`
4388+
- `src/test/import-manager.edge-cases.test.ts`
4389+
- `src/test/import-organizer.test.ts`
4390+
- `src/test/import-manager.blank-lines.test.ts`
4391+
- `src/test/manifest-validation.test.ts`
4392+
- `src/test/configuration/settings-migration.test.ts`
4393+
4394+
5. **`.github/workflows/ci.yml`** (CREATED THEN DELETED)
4395+
- Mistakenly created duplicate workflow
4396+
- Deleted after discovering existing `test.yml`
4397+
4398+
#### Files Created:
4399+
- `.editorconfig` (permanent)
4400+
- `.github/workflows/ci.yml` (temporary - already deleted)
4401+
4402+
#### Files Deleted:
4403+
- `.github/workflows/ci.yml` (redundant duplicate)
4404+
4405+
---
4406+
4407+
### 3. Important Decisions
4408+
4409+
#### Architecture Choices:
4410+
1. **`any` Type Strategy**:
4411+
- Production code: Zero tolerance - all `any` must have proper types
4412+
- Test mocks: Allowed with explicit `eslint-disable` comments
4413+
- Rationale: Test mocks legitimately need `any` for flexibility, but should be documented
4414+
4415+
2. **EditorConfig Standards**:
4416+
- Adopted Angular's standard configuration
4417+
- Ensures consistency across IDEs (VS Code, WebStorm, etc.)
4418+
4419+
3. **ESLint Strictness**:
4420+
- Made `no-console`, `no-debugger`, `no-explicit-any` ERROR level (not warnings)
4421+
- Rationale: These should fail CI, not just warn
4422+
4423+
#### Open Questions:
4424+
None - all decisions finalized.
4425+
4426+
---
4427+
4428+
### 4. Next Steps
4429+
4430+
#### Immediate TODO:
4431+
1. **Update Dependencies** (from audit findings):
4432+
```bash
4433+
npm update
4434+
```
4435+
- `@types/node`: 22.18.8 → 24.9.1 (2 major versions behind)
4436+
- `@types/vscode`: 1.104.0 → 1.105.0
4437+
- `@typescript-eslint/*`: 8.45.0 → 8.46.2
4438+
- `esbuild`, `eslint`, `ts-morph`: minor updates available
4439+
4440+
2. **Clean up CLAUDE_TODO.md** (163 KB file):
4441+
- Archive old completed session logs
4442+
- Keep only recent/relevant context
4443+
- Consider creating `CLAUDE_TODO_ARCHIVE.md` for historical sessions
4444+
4445+
3. **Decide on Version Number**:
4446+
- Currently: `4.0.0-rc.0` (Release Candidate)
4447+
- Decision: Ready for `4.0.0` release? Or document RC blockers?
4448+
4449+
#### Testing Needed:
4450+
✅ All testing complete:
4451+
- TypeScript compilation: PASS
4452+
- ESLint: PASS (0 errors, 0 warnings)
4453+
- Build: PASS (esbuild successful)
4454+
- Tests: PASS (259/259 passing in 16s)
4455+
4456+
#### Documentation Updates:
4457+
None needed - all documentation accurate.
4458+
4459+
---
4460+
4461+
### 5. Audit Findings Summary
4462+
4463+
#### 🟢 STRENGTHS (What's Excellent):
4464+
- Clean TypeScript with zero type errors
4465+
- Comprehensive test coverage (259 tests)
4466+
- Excellent documentation (CLAUDE.md, README.md)
4467+
- Modern architecture with proper separation of concerns
4468+
- Real VSCode API usage in tests (no brittle mocks)
4469+
- Existing CI/CD on 3 platforms
4470+
4471+
#### 🟡 MEDIUM PRIORITY (Recommended):
4472+
- 8 outdated dependencies (security & compatibility)
4473+
- Large CLAUDE_TODO.md file (163 KB - needs cleanup)
4474+
- Version still marked as RC (decide on 4.0.0 release)
4475+
4476+
#### 🟢 LOW PRIORITY (Nice-to-have):
4477+
- TypeScript target could be ES2023/ES2024 (currently ES2022)
4478+
- `.DS_Store` might be in git (check gitignore)
4479+
4480+
#### Overall Assessment: **Grade A-** (Excellent)
4481+
Project is production-ready. Only dependency updates are truly necessary before release.
4482+
4483+
---
4484+
4485+
### 6. Verification Results
4486+
4487+
```bash
4488+
✅ npm run check-types: PASS (0 errors)
4489+
✅ npm run lint: PASS (0 errors, 0 warnings)
4490+
✅ npm run compile: PASS (build successful)
4491+
✅ npm test: PASS (259/259 tests passing in 16s)
4492+
```
4493+
4494+
---
4495+
4496+
### 7. CI/CD Status
4497+
4498+
**Existing Workflow**: `.github/workflows/test.yml`
4499+
- ✅ Runs on: Ubuntu, macOS, Windows
4500+
- ✅ Node version: 18.x
4501+
- ✅ Tests: Main extension tests + comparison-test-harness
4502+
- ✅ Linter: Runs TWICE (via pretest hook)
4503+
- Once in `npm run compile` (check-types → lint → esbuild)
4504+
- Once directly in `pretest` hook
4505+
4506+
**Linter Execution Points**:
4507+
| Command | Linter? | Type Check? | Build? | Tests? |
4508+
|---------|---------|-------------|--------|--------|
4509+
| `npm run lint` | ✅ Once ||||
4510+
| `npm run compile` | ✅ Once ||||
4511+
| `npm test` |**Twice** ||||
4512+
4513+
All new ESLint rules (`no-console`, `no-debugger`, `no-explicit-any`) now enforce in CI automatically.
4514+

eslint.config.mjs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,13 @@ export default [{
1919
selector: "import",
2020
format: ["camelCase", "PascalCase"],
2121
}],
22+
"@typescript-eslint/no-explicit-any": "error",
2223

2324
curly: "warn",
2425
eqeqeq: "warn",
2526
"no-throw-literal": "warn",
2627
semi: "warn",
28+
"no-console": "error",
29+
"no-debugger": "error",
2730
},
2831
}];

src/imports/import-manager.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Project, SourceFile, Node, SyntaxKind } from 'ts-morph';
1+
import { Project, SourceFile, Node, SyntaxKind, ImportDeclaration, ImportEqualsDeclaration, ExportDeclaration } from 'ts-morph';
22
import { EndOfLine, Position, Range, TextDocument, TextEdit } from 'vscode';
33

44
import { ImportsConfig } from '../configuration';
@@ -72,7 +72,7 @@ export class ImportManager {
7272
* Extract import attributes/assertions from an import declaration.
7373
* Returns the raw text of the attributes (e.g., "assert { type: 'json' }")
7474
*/
75-
private extractImportAttributes(importDecl: any): string | undefined {
75+
private extractImportAttributes(importDecl: ImportDeclaration): string | undefined {
7676
try {
7777
// Get the full text of the import declaration
7878
const fullText = importDecl.getText();
@@ -698,13 +698,18 @@ export class ImportManager {
698698
// Get the range of all import declarations (both modern and old-style)
699699
const importDeclarations = this.sourceFile.getImportDeclarations();
700700
const importEquals = this.sourceFile.getStatements()
701-
.filter(stmt => Node.isImportEqualsDeclaration(stmt));
701+
.filter(stmt => Node.isImportEqualsDeclaration(stmt)) as ImportEqualsDeclaration[];
702702

703703
// Also include re-export declarations in the range (they'll be moved after imports)
704704
const exportDeclarations = this.sourceFile.getExportDeclarations()
705705
.filter(exportDecl => exportDecl.getModuleSpecifier() !== undefined);
706706

707-
const allImports = [...importDeclarations, ...importEquals as any[], ...exportDeclarations as any[]];
707+
// Combine all declaration types - they all have getStart() and getStartLineNumber() methods
708+
const allImports: Array<ImportDeclaration | ImportEqualsDeclaration | ExportDeclaration> = [
709+
...importDeclarations,
710+
...importEquals,
711+
...exportDeclarations
712+
];
708713

709714
if (allImports.length === 0) {
710715
return edits;

src/test/configuration/settings-migration.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,27 @@ suite('Settings Migration Tests', () => {
3737
},
3838
subscriptions: [],
3939
extensionPath: '',
40+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
4041
extensionUri: {} as any,
42+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
4143
environmentVariableCollection: {} as any,
4244
extensionMode: 3, // ExtensionMode.Production
4345
storageUri: undefined,
4446
storagePath: undefined,
47+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
4548
globalStorageUri: {} as any,
4649
globalStoragePath: '',
50+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
4751
logUri: {} as any,
4852
logPath: '',
4953
asAbsolutePath: (relativePath: string) => relativePath,
54+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
5055
workspaceState: {} as any,
56+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
5157
secrets: {} as any,
58+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
5259
extension: {} as any,
60+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
5361
languageModelAccessInformation: {} as any,
5462
};
5563
});

src/test/import-manager.blank-lines.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@ import { createTempDocument, deleteTempDocument, applyEditsToDocument } from './
1717

1818
// Mock config for testing
1919
class MockImportsConfig extends ImportsConfig {
20+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
2021
private overrides: Map<string, any> = new Map();
2122

23+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
2224
override(key: string, value: any): void {
2325
this.overrides.set(key, value);
2426
}

src/test/import-manager.edge-cases-audit.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,10 @@ import { createTempDocument, deleteTempDocument, applyEditsToDocument } from './
1616
* Extends ImportsConfig to allow setting test values.
1717
*/
1818
class MockImportsConfig extends ImportsConfig {
19+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1920
private mockConfig: Map<string, any> = new Map();
2021

22+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
2123
setConfig(key: string, value: any): void {
2224
this.mockConfig.set(key, value);
2325
}
@@ -282,6 +284,7 @@ const x = A + Z;
282284
try {
283285
const config = new ImportsConfig();
284286
// Mock multiLineTrailingComma: false
287+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
285288
(config as any).mockConfig = new Map([['multiLineTrailingComma', false]]);
286289
const manager = new ImportManager(doc, config);
287290
const edits = manager.organizeImports();
@@ -380,6 +383,7 @@ const x = A + B;
380383
try {
381384
const config = new ImportsConfig();
382385
// Mock removeTrailingIndex: true
386+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
383387
(config as any).mockConfig = new Map([['removeTrailingIndex', true]]);
384388
const manager = new ImportManager(doc, config);
385389
const edits = manager.organizeImports();

src/test/import-manager.edge-cases.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,10 @@ import { createTempDocument, deleteTempDocument, applyEditsToDocument } from './
2222
* Mock configuration for testing.
2323
*/
2424
class MockImportsConfig extends ImportsConfig {
25+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
2526
private mockConfig: Map<string, any> = new Map();
2627

28+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
2729
setConfig(key: string, value: any): void {
2830
this.mockConfig.set(key, value);
2931
}

src/test/import-manager.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,15 @@ import { createTempDocument, deleteTempDocument, applyEditsToDocument } from './
8787
* - Tests can verify different configuration combinations
8888
*/
8989
class MockImportsConfig extends ImportsConfig {
90+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
9091
private mockConfig: Map<string, any> = new Map();
9192

93+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
9294
setConfig(key: string, value: any): void {
9395
this.mockConfig.set(key, value);
9496
}
9597

98+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
9699
override(key: string, value: any): void {
97100
this.mockConfig.set(key, value);
98101
}
@@ -154,6 +157,7 @@ class MockImportsConfig extends ImportsConfig {
154157
let importGroups: ImportGroup[] = [];
155158

156159
try {
160+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
157161
importGroups = groupSettings.map((setting: any) => ImportGroupSettingParser.parseSetting(setting));
158162
} catch (e) {
159163
// Fall back to default on invalid config (same as real ImportsConfig)

0 commit comments

Comments
 (0)