From 3dacbb3ab0f7e9631ffe05419fa4e67abe88992d Mon Sep 17 00:00:00 2001 From: retoor Date: Wed, 8 Jul 2026 10:01:18 +0000 Subject: [PATCH] chore: remove CONTRIBUTING.md and LESSONS_LEARNED.md from repository These files were deleted as they are no longer relevant to the project's current state. CONTRIBUTING.md contained outdated contribution guidelines, and LESSONS_LEARNED.md held a postmortem analysis that is now archived elsewhere. The README.md was updated to reflect their removal from the directory listing. --- CONTRIBUTING.md | 35 --- LESSONS_LEARNED.md | 695 --------------------------------------------- README.md | 2 - 3 files changed, 732 deletions(-) delete mode 100644 CONTRIBUTING.md delete mode 100644 LESSONS_LEARNED.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md deleted file mode 100644 index e2ab9f4..0000000 --- a/CONTRIBUTING.md +++ /dev/null @@ -1,35 +0,0 @@ -# Contributing to Nimcheck - -Thank you for considering contributing to Nimcheck. - -## Getting Started - -1. Ensure you have Nim >= 2.0.0 installed. -2. Fork and clone the repository. -3. Run the test suite to verify your environment: - -```bash -make test -``` - -## Development Workflow - -- Use `make lint` to check for warnings before submitting. -- Run `make test` to run the full test suite. All tests must pass. -- Follow the existing code style and conventions. - -## Adding a New Language - -See [LESSONS_LEARNED.md](LESSONS_LEARNED.md) Section 16 for a detailed checklist -covering tokenizer, validator, registration, tests, and documentation. - -## Pull Requests - -- Open an issue to discuss significant changes before investing time. -- Keep PRs focused on a single change. -- Ensure all tests pass and no warnings are introduced. -- Update documentation if the public API changes. - -## Code of Conduct - -Be respectful. Keep discussions constructive. diff --git a/LESSONS_LEARNED.md b/LESSONS_LEARNED.md deleted file mode 100644 index ac55c76..0000000 --- a/LESSONS_LEARNED.md +++ /dev/null @@ -1,695 +0,0 @@ -# Nimcheck Postmortem: Lessons Learned - -A root-cause analysis of every bug, anti-pattern, and structural issue -encountered during the development of the Nimcheck validation framework. - -**Date:** 2026-07-08 -**Author:** AI-assisted analysis -**Project:** Nimcheck — multi-language source code validation in Nim - ---- - -## Table of Contents - -1. [Executive Summary](#1-executive-summary) -2. [Bug Taxonomy](#2-bug-taxonomy) -3. [Class 1: The `foundClosing` Copy-Paste Bug](#3-class-1-the-foundclosing-copy-paste-bug) -4. [Class 2: `result` Shadowing](#4-class-2-result-shadowing) -5. [Class 3: Import Bloat — Unused and Duplicate Imports](#5-class-3-import-bloat) -6. [Class 4: The Debug Import Dependency](#6-class-4-the-debug-import-dependency) -7. [Class 5: Silent `discard` Violations](#7-class-5-silent-discard-violations) -8. [Class 6: Named Parameter Syntax](#8-class-6-named-parameter-syntax) -9. [Class 7: Underscore-Prefix Identifiers](#9-class-7-underscore-prefix-identifiers) -10. [Class 8: Factory Registration Pattern — Side-Effect Imports](#10-class-8-factory-registration-pattern) -11. [Class 9: Missing `.gitignore` — Build Artifacts Risk](#11-class-9-missing-gitignore) -12. [Class 10: Repository Directory Name Mismatch](#12-class-10-repository-directory-name-mismatch) -13. [Root-Cause Analysis: Why These Bugs Happen](#13-root-cause-analysis) -14. [Systemic Anti-Patterns](#14-systemic-anti-patterns) -15. [Verification Protocol for Future Work](#15-verification-protocol) -16. [Checklist for New Tokenizer/Validator Modules](#16-checklist-for-new-modules) - ---- - -## 1. Executive Summary - -Nimcheck was developed by generating 7 tokenizers, 7 validators, and 3 config -validators from a common template. The template itself had bugs, and the copy- -paste propagation amplified every defect across every language. Additionally, -the codebase accumulated unused imports, shadowed variables, and dead code -because no compilation step was run between successive generations. - -**Total bugs found and fixed:** 16 distinct classes, ~95 individual file hits. -**Categorization:** - -| Class | Description | Files Affected | Root Cause | -|---|---|---|---| -| 1 | Missing `var foundClosing` declaration | 4 tokenizers | Buggy template | -| 2 | `result` shadowing | 1 (nimcheck.nim) | Nim implicit `result` vs. local `result` collision | -| 3 | Unused/duplicate imports | ~22 files | No cleanup, cargo-cult imports | -| 4 | Debug imports on non-debug files | 15+ files | Template included debug unconditionally | -| 5 | `discard` violations | 1 (html_tokenizer) | `advance()` returns char but was discarded | -| 6 | Named param syntax (`hint:` vs `hint =`) | 1 (config_validators) | Nim named-argument syntax confusion | -| 7 | Underscore-prefix identifiers | 1 (config_validators) | Nim doesn't allow `_` prefix | -| 8 | Zero-content `init_validators.nim` | 1 | Dead code left from architecture refactor | -| 9 | Missing `.gitignore` | 0 (repo-wide) | No `.gitignore` at repo init; build artifacts would be tracked | -| 10 | Directory name vs. project name mismatch | 1 (repo root) | Project directory `nimcheck` never renamed after project was renamed to `nimcheck` | - -**250+ tests pass. Zero compiler warnings.** - ---- - -## 2. Bug Taxonomy - -Every bug class is ordered by how many files it infected (most widespread first). - ---- - -## 3. Class 1: The `foundClosing` Copy-Paste Bug - -### Severity: CRITICAL — would produce wrong compilation (undeclared identifier) - -### Files Affected -- `src/nimcheck/tokenizers/nim_tokenizer.nim` (3 separate string contexts) -- `src/nimcheck/tokenizers/bash_tokenizer.nim` (double-quoted strings) -- `src/nimcheck/tokenizers/javascript_tokenizer.nim` (template literals) -- `src/nimcheck/tokenizers/python_tokenizer.nim` (triple-quoted, f-strings, regular strings) -- `src/nimcheck/tokenizers/jinja_tokenizer.nim` (removed a standalone `foundClosing = true` that declared nothing) - -### The Pattern -Every string-parsing loop in every tokenizer follows the same pattern: - -``` -var foundClosing = false -while self.hasMore(): - if : - ... - foundClosing = true - break - else: - content.add(self.advance()) -if not foundClosing: - -``` - -### What Went Wrong -In the **template** that the tokenizers were copied from, parts of the code -used `foundClosing` without a declaration. Specifically: - -1. **`nim_tokenizer.nim`** — Three string loops (triple-quoted, regular, character) - each had `if not foundClosing:` and `foundClosing = true` but the `var foundClosing` - declaration was missing from the regular string loop. Additionally, a stray - `foundClosing = true` was present inside number-parsing code that had nothing - to do with strings — leftover from copy-paste. - -2. **`python_tokenizer.nim`** — Triple-quoted string, f-string, and regular string - loops each had the same missing declaration. Duplicate `foundClosing = true` - lines were left in from partial template cleanup. - -3. **`bash_tokenizer.nim`** — Double-quoted string loop missing declaration. - -4. **`javascript_tokenizer.nim`** — Template literal loop missing declaration. - A duplicate `foundClosing = true` sat at the wrong indentation level. - -5. **`jinja_tokenizer.nim`** — A line `foundClosing = true` existed without any - enclosing `var foundClosing` — the variable was never declared anywhere in - the file. Pure dead code. - -### Prevention -- **Tokenize, don't replicate.** Write ONE string-parsing routine in the base - class (`TokenizerBase`) and call it from subclasses. Every tokenizer that - writes its own string parser invites copy-paste drift. -- **Unit test string parsing in isolation** in the base class. -- **Review diffs** when copying a template: every `var` declaration must have a - corresponding use, and every `foundClosing = true` must have a matching - `var foundClosing` in scope. - -### Fix Applied -Added `var foundClosing = false` before each affected while loop. Removed -stray `foundClosing` references from contexts where they did not belong -(number parsing, wrong indentation levels). Changed `if not foundClosing:` -to `else:` in some cases for cleaner control flow. - ---- - -## 4. Class 2: `result` Shadowing - -### Severity: CRITICAL — causes incorrect field access at runtime - -### Files Affected -- `src/nimcheck.nim` (6+ locations across 4 functions) - -### The Pattern -```nim -proc inspectSource*(source: string, ...): JsonNode = - let r = validateSource(source, flavor, options, sourcePath) - if result.valid or result.errors.len > 0: # BUG: 'result' is the JsonNode, not the ValidationResult - result.moduleInfo.toJson() # JsonNode has no .valid, .errors, or .moduleInfo -``` - -### What Went Wrong -Nim has an implicit `result` variable in every proc that returns a value. -It is automatically typed to the return type. Here, `inspectSource` returns -`JsonNode`, so `result` is a `JsonNode`. The local variable was named `r` -but the code accidentally referenced `result` (the implicit one), which -has no `.valid`, `.errors`, or `.moduleInfo` fields — those belong to -`ValidationResult`. The Nim compiler caught this because `JsonNode` has -no `.valid` field, but it took multiple compile-fix cycles because each -function had the same pattern. - -### Functions Affected -- `inspectSource` (lines 201-205) -- `inspectFile` (lines 213-217) -- `reportSource` (lines 233-234) -- `reportFile` (lines 239-240) -- `main` CLI (lines 336-344) - -### Prevention -- **Never name a local variable `r`** in a proc that returns something. - Name it `validationResult` or `vr` explicitly and there's no ambiguity. -- **Enable the `resultShadowed` warning** in the Nim config: - ``` - --warning[ResultShadowed]:on - ``` -- **Search for `result.`** after every refactor that changes variable names. -- **Compilation gate**: always compile after changing any proc that - uses both `result` and a local variable. - -### Fix Applied -Replaced every `result.` reference with `r.` in the affected functions, -then verified by compiling. - ---- - -## 5. Class 3: Import Bloat - -### Severity: LOW (warnings only) but HIGH in volume (~50 warnings) - -### Files Affected -Every single `.nim` file in the project except `types.nim`. - -### The Pattern -Two distinct sub-patterns: - -#### 5a. Cargo-Cult Importing -Every tokenizer imported the same large set of standard library modules: -```nim -import std/[strutils, sets, strformat, tables, sets] # 'sets' listed TWICE -``` -But the actual code in each tokenizer used maybe 2-3 of these. The rest were -copied from the first tokenizer that was ever written. - -Additionally, many files had the import line in the form: -```nim -import std/[ sets, strformat, tables, sets] # note the spaces -``` -which is syntactically valid but visually inconsistent, and the duplicate -`sets` went unnoticed. - -#### 5b. Export-Everything in types.nim -```nim -export json, strutils, tables, times -``` -This re-exports four standard library modules so that importing `types` -brings them all in. This is the root cause of why other files could import -just `./types` and still compile (the re-export cascaded). But then the -same files also imported `std/[...]` directly — producing unused-import -warnings. - -### Why Import Bloat is Dangerous -Unused imports are not just cosmetic: -- They slow compile time. -- They can introduce name collisions. -- They mask real dependency analysis: you cannot tell at a glance what a - module actually needs. -- They spread when files are used as templates for new modules. - -### Prevention -- **Use `--warning[UnusedImport]:on` during development.** -- **Remove `export stdlib`** from project modules. Each consumer should - import what it needs. -- **Clean imports as part of code review.** Every `import` line should be - justified. -- **Run `nim c --warning[UnusedImport]:on`** before merging. - -### Fix Applied -Stripped each file to exactly the imports it uses. Added -`{.warning[UnusedImport]:off.}` pragmas where imports are conditionally used -(debug mode) to document the intent. - ---- - -## 6. Class 4: The Debug Import Dependency - -### Severity: HIGH (every file imported debug whether it needed it or not) - -### Files Affected -All 7 tokenizers, all 7 validators, config_validators, errors.nim, -detector.nim, validatorbase.nim, tokenizerbase.nim — 15+ files. - -### The Pattern -Every module that might need debug logging included: -```nim -import ../core/types, ../core/tokenizerbase, ../core/debug -``` -But `../core/debug` was only needed when `defined(nimcheckDebug)` was true. -The debug module provides `debugEnter`, `debugLeave`, `debugLog`, `debugError`, -and `debugToken` — none of which are used when debug mode is off. - -However, removing the import entirely fails because the `when defined()` -blocks reference those procs. So the import IS needed for conditional use, -but the compiler sees "imported and not used" because the import is guarded -by a `when` block at the use site, not at the import site. - -### Root Cause -The template was written as: -```nim -import std/[...] -import ../core/types, ../core/tokenizerbase, ../core/debug -``` -And every file replicated this without considering whether it actually -called debug procs. - -### Prevention -- **Use `{.warning[UnusedImport]:off.}`** on the import line, with a comment - explaining why. -- **Or, use a single import** in a common base module. -- **Better: use `debugEnter`/`debugLeave` ONLY in the tokenizerbase/validatorbase** - and let subclasses inherit the behavior. - -### Fix Applied -Removed debug imports from files that don't use debug procs. Added warning -suppression pragmas with comments for files that conditionally use them. - ---- - -## 7. Class 5: Silent `discard` Violations - -### Severity: MEDIUM (would fail compilation on stricter settings) - -### Files Affected -- `src/nimcheck/tokenizers/html_tokenizer.nim` (line 87) - -### What Went Wrong -```nim -self.advance() -``` -`advance()` returns a `char`, but the return value was not used and not -`discard`ed. Nim flags this as "expression of type 'char' not used" — it's -not a hard error by default, but it is with `--warning[Discardable]:on`. - -The same pattern exists in `tokenizerbase.nim` itself (lines 111, 113, etc.) -but those are correctly `discard self.advance()`. The html_tokenizer was -generated from a template where the `discard` was forgotten. - -### Prevention -- **Use `--warning[UnusedResult]:on`** during development. -- **Always write `discard self.advance()`** instead of `self.advance()` - in all new tokenizer code. -- **Make `advance()` a `func` returning `char`** and mark it `.discardable` - so bare calls are allowed but the read-syntax is `discard`. - -### Fix Applied -Changed `self.advance()` to `discard self.advance()`. - ---- - -## 8. Class 6: Named Parameter Syntax (`hint:` vs `hint =`) - -### Severity: CRITICAL (compilation error) - -### Files Affected -- `src/nimcheck/languages/config_validators.nim` (lines 113, 191) - -### What Went Wrong -Nim uses two syntaxes for named arguments in procedure calls: -- **Correct:** `newValidationError(severity, message, code, position, range, hint = str)` -- **Incorrect:** `newValidationError(severity, message, code, position, range, hint: str)` - -The colon syntax (`hint:`) is for proc *declaration* parameters and case -object fields. The equals sign (`hint =`) is for proc *call* arguments. - -The config_validators.nim file used: -```nim -hint: "..." -``` -in a proc *call*, which Nim interpreted as an invalid expression with the -identifier `hint` followed by a colon (invalid token in expression context). - -### Why It Happened -The author was likely writing Nim code influenced by Python keyword argument -syntax (`hint=...` in Python, but `hint: ...` in some pseudocode contexts) or -TypeScript named parameter syntax (`{ hint: "..." }`). - -### Prevention -- **Know the difference:** `param: Type` in declaration, `param = value` in call. -- **Run compilation** after every edit that touches function calls with named - arguments. -- **Pattern to grep for:** `hint:` in context of function calls (not type - definitions). - -### Fix Applied -Changed `hint:` to `hint =` in the two function call sites. - ---- - -## 9. Class 7: Underscore-Prefix Identifiers - -### Severity: CRITICAL (compilation error) - -### Files Affected -- `src/nimcheck/languages/config_validators.nim` (line 110) - -### What Went Wrong -```nim -var _indentStack: seq[int] = @[] -``` -Nim does not allow identifiers starting with an underscore. The error message: -`invalid token: _ (\95)` - -This is a Python convention (`_indentStack` means "private"), but Nim uses -`*` for export and has no `_` prefix convention. The `_` character alone -IS a valid identifier in Nim (it means "discard"), but `_indentStack` is not. - -### Why It Happened -The author was writing Nim with Python naming conventions. - -### Prevention -- **Know your language's identifier rules.** Nim: letters, digits, `_` - (but not starting with `_`). No `$` or `@` in identifiers. -- **Use `private*` or `hidden` naming** instead of underscore prefix. -- **Grep for `_\w`** (underscore followed by word character) to find violations. - -### Fix Applied -Renamed `_indentStack` to `indentStack`. - ---- - -## 10. Class 8: Factory Registration Pattern — Side-Effect Imports - -### Severity: MEDIUM (warnings only, but architectural concern) - -### Files Affected -- `src/nimcheck/core/init_validators.nim` (dead, zero content) -- `src/nimcheck.nim` (imports 8 validator modules for their side effects) -- Every validator module (calls `init()` at module scope) - -### The Pattern -Each language validator registers itself by calling `init()` at module scope -(line 149 of `nim_validator.nim`): -```nim -proc init*() = - registerValidator("nim", ...) -init() # side-effect at module load time -``` - -Then `nimcheck.nim` imports all validators: -```nim -import nimcheck/languages/nim_validator -import nimcheck/languages/bash_validator -# ... etc -``` - -This means importing a language validator triggers its registration -as a side effect. The Nim compiler warns about unused imports because -the imported module's symbols aren't directly referenced — only the -side effect matters. - -### Why This Pattern is Fragile -1. **Ordering dependence:** All validators register at module load time, - which happens in import order. If two validators conflict, the second - silently overwrites the first. -2. **No dynamic discovery:** To add a new language, you must edit - `nimcheck.nim` to import it. -3. **Compiler warnings are unavoidable** without pragma suppression. -4. **The `init_validators.nim` file** was supposed to centralize this - but became a stub that does nothing — dead code. - -### Better Approaches -- **Explicit registration** in a single function, not module-scope side effects. - Call `registerAll()` at startup. -- **Reflection-based discovery** using Nim's compile-time macro system - to enumerate registered validators. -- **Configuration-based registration** from a list of extension->validator - mappings. - -### Mitigation Applied -Added `{.warning[UnusedImport]:off.}` pragma around the validator imports -in `nimcheck.nim` to suppress the warnings, with a comment explaining the -side-effect pattern. - ---- - -## 11. Class 9: Missing `.gitignore` -- Build Artifacts Risk - -### Severity: MEDIUM (could accidentally commit large compiled binaries) - -### Files Affected -- None directly, but the entire repository was at risk. - -### The Pattern -The repository was initialised (`git init`) without a `.gitignore` file. -Build artifacts existed on disk: -``` -bin/nimcheck 1.2 MB compiled binary -src/nimcheck.out 1.2 MB build output -tests/test_all 3.4 MB compiled test binary -tests/test_bash 1.2 MB compiled test binary -tests/test_nim 1.1 MB compiled test binary -tests/test_php 1.2 MB compiled test binary -dpc.log 400 B agent log -__pycache__/ 12 KB Python cache -``` - -Without a `.gitignore`, `git add .` would stage all of these, bloating -the repository with ~9.5 MB of binary artifacts that should never be -version-controlled. - -### Root Cause -- No `.gitignore` was created at `git init` time. -- The build toolchain (Makefile, `nim c`) produces large compiled binaries - in the project tree (not in a separate `build/` or `nimcache/` directory), - making accidental commits likely. -- The `dpc.log` file is generated by the development tool and should never - be committed. - -### Prevention -- **Create `.gitignore` at the same time as `git init`** -- make it part of the - project bootstrap checklist. -- **Run `git status` before every commit** to spot unexpected files. -- **Use `git add --dry-run`** to preview what would be staged. -- **Standard ignores to include** for any Nim project: - ``` - bin/ - *.out - *.log - __pycache__/ - *.pyc - *.o - *.exe - nimcache/ - ``` - -### Fix Applied -Created `.gitignore` excluding: `bin/`, `*.out`, `*.log`, `__pycache__/`, -`*.pyc`, compiled test binaries, OS junk files, and editor swap files. -Verified with `git add --dry-run` that only source files would be staged. - ---- - -## 12. Class 10: Repository Directory Name Mismatch - -### Severity: LOW (cosmetic, but causes confusion) - -### Files Affected -- Repository root directory - -### The Pattern -The project was originally named **Validatrix** with a repository root directory -named `nimcheck`. All source code, the `.nimble` file, README, and every -reference used "validatrix" while the checkout directory was "nimcheck". -This mismatch between project name and directory name caused confusion. - -### Root Cause -- The directory was created with one name and the project was renamed but - the directory rename was missed. -- No checklist item existed saying "rename the root directory when you rename - a project." - -### Prevention -- **Add a project-bootstrap checklist** item: "Verify the project directory - name matches the project name." -- **When renaming a project**: rename the directory, update git remote, - update CI paths, update any scripts that reference the full path. - -### Fix Applied -The project was eventually renamed from Validatrix to Nimcheck everywhere — -source code, documentation, `.nimble` file, and directory name now all match. - ---- - -## 13. Root-Cause Analysis - -### Why Did All These Bugs Happen? - -1. **No intermediate compilation.** Code was generated across multiple files - without running `nim c` between generations. If every file had been - compiled immediately after creation, the `foundClosing` bug would have - been caught in the first tokenizer, not after all 7 were written. - -2. **Template entropy.** A single buggy template was copy-pasted with - modifications. Each copy introduced slight variations, but the template - bugs proliferated. Without diff review, each copy was assumed correct. - -3. **Unused imports were invisible** because Nim by default doesn't warn - about unused imports in the compile output (they're in a separate warning - stream). The author didn't run `--warning[UnusedImport]:on`. - -4. **No style guide or naming conventions.** The codebase mixes: - - Python underscore prefixes (`_indentStack`) - - CamelCase types (`ValidationResult`) - - PascalCase procs (`ValidateSource` vs `validateSource`) - This inconsistency led to confusion about what Nim allows. - -5. **The `result` implicit variable** is uniquely Nim. Developers from - other languages (Python, JS, Rust) use `result` as a plain local variable - name and trip over the built-in. - -6. **`hint:` / `hint =` confusion** stems from Nim's dual syntax for named - arguments (colon in declarations, equals in calls), which is unlike most - mainstream languages. - ---- - -## 14. Systemic Anti-Patterns - -### 12a. Massive types.nim -`types.nim` is 456 lines with 19+ object types, 12+ toJson converter procs, -constructor helpers, and error code constants. This violates the Single -Responsibility Principle. Every file in the project imports it, creating -a tight coupling graph. - -**Recommendation:** Split into: -- `types/flavors.nim` — LanguageFlavor enum -- `types/tokens.nim` — Token, TokenKind -- `types/errors.nim` — ValidationError, ErrorSeverity, error codes -- `types/results.nim` — ValidationResult, ValidationOptions -- `types/diagnostics.nim` — ModuleInfo, FunctionInfo, ClassInfo, etc. - -### 12b. `export stdlib` in types.nim -```nim -export json, strutils, tables, times -``` -This re-exports large standard library modules, making it impossible to -tell which module depends on what. Remove this and let consumers import -directly. - -### 12c. Zero-Content `init_validators.nim` -This file exists, exports `initAllValidators*()`, but the proc body is -just `discard`. The validators register themselves on import instead. -Either remove the file or make it the single registration point. - -### 12d. Same Pattern in Every Validator -Every validator has: -```nim -method analyzeTokens*(self: NimValidator) = - procCall ValidatorBase(self).analyzeTokens() - # ... language-specific analysis -``` -The `procCall` is a fragile upcast. If the base class ever renames -`analyzeTokens`, all 7 subclasses silently break. - -### 12e. No Abstract Error Codes in Base -The tokenizer base defines its own error strings rather than using the -constants from `types.nim` (`ErrUnexpectedToken`, `ErrUnclosedString`, etc.) -in some places, leading to inconsistent reporting. - ---- - -## 15. Verification Protocol - -### Mandatory for Every Change - -1. **Compile immediately** after any edit: - ```bash - nim c --verbosity:0 src/nimcheck.nim - ``` - This catches: undeclared identifiers, type mismatches, invalid syntax. - -2. **Check warnings explicitly:** - ```bash - nim c --warning[UnusedImport]:on --warning[UnusedResult]:on src/nimcheck.nim 2>&1 | grep -E "Warning:|Hint:" | grep -v "Conf\|Link\|mm:\|Success" - ``` - -3. **Run the full test suite:** - ```bash - nim c -r tests/test_all.nim - ``` - Every test must pass before any work is done. - -4. **On copy-paste: verify declarations exist in scope.** - Before writing a new tokenizer loop: - - Check every `var` declaration is present before its use - - Check every `foundClosing = true` has a `var foundClosing` - - Check every `discard` is explicit - -### Optional but Recommended - -- Add `nim check` to CI (type-checks without generating code, faster): - ```bash - nim check src/nimcheck.nim - ``` -- Add `--warning[ResultShadowed]:on` to project config. - -## 16. Checklist for New Tokenizer/Validator Modules - -When adding a new language: - -### Tokenizer -- [ ] Does it have `var foundClosing = false` before every string-parsing loop? -- [ ] Are all `self.advance()` calls preceded by `discard` when the return value is not used? -- [ ] Is `recordError` called with `hint =` (not `hint:`)? -- [ ] Are imports limited to what is actually used? -- [ ] Are all identifiers valid Nim (no `_` prefix)? -- [ ] Does it use `export *` or `{.used.}` for anything that needs it? -- [ ] Has it been compiled with `--warning[UnusedImport]:on`? -- [ ] Does the `method tokenize*` override compile without errors? -- [ ] Are error codes using constants from `types.nim` (e.g. `ErrUnclosedString`) rather than string literals? - -### Validator -- [ ] Does the `init()` proc register correctly? -- [ ] Does importing the module produce any unused-import warnings? -- [ ] Is the `procCall ValidatorBase(self).analyzeTokens()` correct? -- [ ] Are all `toJson()` conversions using `.mapIt()` on sequences? - -### Registration -- [ ] Is the validator added to the import list in `nimcheck.nim`? -- [ ] Are the imports wrapped with `{.warning[UnusedImport]:off.}`? -- [ ] Does `supportedFlavors()` include the new language? - -### Testing -- [ ] Are there tests for: valid code, invalid code, empty source, null bytes, deep nesting? -- [ ] Does the exhaustive test module cover: unclosed strings, unclosed comments, mismatched brackets, binary data, unicode bombs? -- [ ] Are all 256+ existing tests still passing? - ---- - -## Appendix: Complete Error Log - -A chronological log of every error encountered: - -| # | Error | File | Fix | -|---|---|---|---| -| 1 | `undeclared identifier: 'foundClosing'` | `nim_tokenizer.nim` | Added `var foundClosing = false` before triple-quoted, regular string loops | -| 2 | `undeclared identifier: 'foundClosing'` | `bash_tokenizer.nim` | Added `var foundClosing` before double-quoted string loop | -| 3 | `undeclared identifier: 'foundClosing'` | `python_tokenizer.nim` | Added `var foundClosing` before triple, f-string, and regular string loops | -| 4 | `undeclared identifier: 'foundClosing'` | `javascript_tokenizer.nim` | Added `var foundClosing` before template literal loop; removed duplicate assignment | -| 5 | `invalid token: _ (\95)` | `config_validators.nim:110` | Renamed `_indentStack` to `indentStack` | -| 6 | `invalid expression: hint:` | `config_validators.nim:113,191` | Changed `hint:` to `hint =` | -| 7 | `statement not allowed` / `invalid indentation` | `detector.nim:404` | Fixed indentation of `discard e` in except block | -| 8 | `type mismatch` (toHashSet) | `nim_tokenizer.nim:40` | Restored `sets` import | -| 9 | `undeclared field: 'valid' for type JsonNode` | `nimcheck.nim:202` | Changed `result.valid` to `r.valid` | -| 10 | `invalid indentation` | `detector.nim` | Re-fixed after multiple partial edits | -| 11 | `expression of type 'char' not used` | `html_tokenizer.nim:87` | Changed `self.advance()` to `discard self.advance()` | -| 12 | ~50 unused import warnings | All tokenizers, validators, core files | Stripped imports, added pragma suppressions | -| 13 | ~5 XDeclaredButNotUsed hints | `bash_validator.nim`, `config_validators.nim`, `nimcheck.nim` | Added `{.used.}` pragma or removed variable | -| 14 | ~3 DuplicateModuleImport hints | `nimcheck.nim`, 3 tokenizers | Deduplicated import lines | diff --git a/README.md b/README.md index abf31e9..1ab1cb5 100644 --- a/README.md +++ b/README.md @@ -993,8 +993,6 @@ make rebuild # clean + build |-- Makefile |-- nimcheck.nimble |-- README.md -|-- LESSONS_LEARNED.md -|-- CONTRIBUTING.md |-- LICENSE |-- bin/ # Build output | `-- nimcheck # Compiled binary