Skip to content

Commit e831f41

Browse files
josecelanojosecelano
committed
docs: add linter-auto-fix and linter-parallel-execution feature specs
1 parent 0d7e3ab commit e831f41

6 files changed

Lines changed: 1821 additions & 0 deletions

File tree

‎docs/features/linter-auto-fix/README.md‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
# Linter Auto-fix Feature
2+
3+
This folder contains the complete documentation for the linter auto-fix feature implementation.
4+
5+
## 📄 Documents
6+
7+
### [specification.md](./specification.md)
8+
9+
The main feature specification including:
10+
11+
- Overview and goals
12+
- Feature description with `--fix` flag usage
13+
- Decision rationale (why Option 3 was chosen)
14+
- Linter auto-fix support matrix
15+
- Expected behavior and output format using tracing
16+
- Safety considerations
17+
- Definition of done
18+
- Testing strategy
19+
20+
### [questions.md](./questions.md)
21+
22+
Clarifying questions that need to be answered before implementation:
23+
24+
- Linter scope and tools
25+
- Fix behavior (what gets fixed)
26+
- Output verbosity using tracing
27+
- Exit code behavior
28+
- Git integration
29+
- Error handling
30+
- Testing requirements
31+
- Documentation updates
32+
- Backward compatibility
33+
- Timeline and priority
34+
35+
## 📋 Status
36+
37+
**Current Phase**: Specification Complete
38+
39+
**Next Steps**:
40+
41+
1. ✅ Create feature specification
42+
2. ✅ Create questions document
43+
3. ✅ Answer clarifying questions in `questions.md`
44+
4. ✅ Update specification based on answers
45+
5. ⏳ Create detailed implementation plan
46+
6. ⏳ Review implementation plan
47+
7. ⏳ Commit documentation
48+
8. ⏳ Begin implementation
49+
50+
## 🎯 Quick Summary
51+
52+
Add a `--fix` flag to the linter binary that:
53+
54+
- Automatically fixes issues for linters that support auto-fix
55+
- Uses **yamlfmt** for YAML formatting (not prettier)
56+
- Uses tracing crate with **minimal verbosity** (show only "Fixed N files" summaries)
57+
- Shows only remaining errors after auto-fix
58+
- Allows developers to use git to see what changed
59+
- **Auto-installs missing tools** (matches current linter behavior)
60+
- Maintains backward compatibility (no `--fix` = current behavior)
61+
- **Incremental implementation**: One linter at a time for easier testing and commits
62+
63+
**Usage**:
64+
65+
```bash
66+
# Check only (current behavior)
67+
cargo run --bin linter all
68+
69+
# Fix and check
70+
cargo run --bin linter all --fix
71+
72+
# Individual linters
73+
cargo run --bin linter markdown --fix
74+
```
75+
76+
## � Key Decisions
77+
78+
Based on answers in [questions.md](./questions.md), the following key decisions were made:
79+
80+
1. **YAML Tool**: Use **yamlfmt** (not prettier) for YAML formatting
81+
82+
- Reason: More focused tool, simpler to integrate
83+
84+
2. **Fix Scope**: Fix the same files that current linter checks (Option B)
85+
86+
- No expansion to additional file types
87+
- Consistent with existing linter behavior
88+
89+
3. **Output Verbosity**: **Minimal** - show only "Fixed N files" summaries
90+
91+
- Keep output clean and focused
92+
- Users can see detailed changes via `git diff`
93+
- Only display errors that still need attention
94+
95+
4. **Error Handling**: **Auto-install missing tools** (Option D)
96+
97+
- Matches current linter behavior
98+
- Seamless developer experience
99+
- Logs installation for transparency
100+
101+
5. **Testing Strategy**: Unit + Integration + E2E + Manual
102+
103+
- Comprehensive but focused
104+
- No property-based testing needed for initial version
105+
106+
6. **Implementation**: Incremental - one linter at a time
107+
108+
- Easier to review, test, and commit
109+
- Reduces risk and complexity
110+
- Can deploy partial functionality
111+
112+
7. **Git Integration**: Don't auto-stage changes
113+
- Let git track changes naturally
114+
- Developers review with `git diff` before committing
115+
116+
## �🔗 Related Documentation
117+
118+
- [Error Handling Guide](../../contributing/error-handling.md)
119+
- [Development Principles](../../development-principles.md)
120+
- [Linting Guide](../../contributing/linting.md)
121+
- [Logging Guide](../../contributing/logging-guide.md)

‎docs/features/linter-auto-fix/questions.md‎

Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
# Clarifying Questions for Linter Auto-fix Feature
2+
3+
## 🤔 Questions for Implementation
4+
5+
Please answer these questions by editing this file directly. Add your answers after each question.
6+
7+
---
8+
9+
### 1. **Scope - Which Linters Get Auto-fix?**
10+
11+
Based on the research, I identified these linters with auto-fix:
12+
13+
- ✅ **markdown**: `npx markdownlint-cli --fix`
14+
- ✅ **yaml**: Should we use `prettier --write` or is there another tool?
15+
- ✅ **clippy**: `cargo clippy --fix --allow-dirty --allow-staged`
16+
- ✅ **rustfmt**: Already auto-formats (no change needed)
17+
- ✅ **taplo**: `taplo fmt` for TOML files
18+
- ❌ **shellcheck**: No auto-fix (skip)
19+
- ❌ **cspell**: No auto-fix (skip)
20+
21+
**Questions:**
22+
23+
- Is this the correct list of linters?
24+
- For YAML, should we use `prettier` or another tool? (I see prettier might not be installed)
25+
- Are there any other linters I'm missing?
26+
27+
**Your Answer:**
28+
29+
- Is this the correct list of linters?
30+
31+
Yes, it is.
32+
33+
- For YAML, should we use `prettier` or another tool? (I see prettier might not be installed)
34+
35+
We can use [yamlfmt](https://github.com/google/yamlfmt) if it's compatible with the YAML linter we are using.
36+
37+
- Are there any other linters I'm missing?
38+
39+
No, there are no other linters to add.
40+
41+
### 2. **Fix Behavior - What Gets Fixed?**
42+
43+
When running `cargo run --bin linter all --fix`, should we:
44+
45+
- **Option A**: Fix ALL files in the project (even those not staged/modified)
46+
- **Option B**: Only fix files that the linter would check anyway (current linter behavior)
47+
- **Option C**: Only fix staged files (git staged files only)
48+
49+
**My assumption:** Option B (same scope as current linting) - is that correct?
50+
51+
**Your Answer:**
52+
53+
Yes, you are right. Option B is the correct one.
54+
55+
### 3. **Output Verbosity and Logging**
56+
57+
You mentioned:
58+
59+
- "We only need to provide the remaining errors. Developers can check what was changed with git."
60+
- "The output should follow the current pattern. We are only using logging as output (tracing crate)."
61+
- "Logs can be nested using tracing instrumentation (spans) to group events related to one linter."
62+
63+
**Confirmed approach:**
64+
65+
- Use `tracing` crate for all output (consistent with current implementation)
66+
- Use tracing spans to group operations per linter (optional enhancement)
67+
- Log auto-fix operations at INFO level
68+
- Only show errors that remain after auto-fix
69+
- Maintain current logging targets (e.g., `target: "markdown"`)
70+
71+
**Example with tracing:**
72+
73+
```rust
74+
#[tracing::instrument(name = "markdown", skip_all)]
75+
pub fn run_markdown_linter_with_fix(fix: bool) -> Result<()> {
76+
if fix {
77+
info!("Applying auto-fix...");
78+
// Run fix command
79+
}
80+
info!("Scanning markdown files...");
81+
// Run check
82+
// ...
83+
}
84+
```
85+
86+
**Question:** Do you want tracing spans for grouping linter operations, or is the current flat logging with targets sufficient?
87+
88+
**Your Answer:**
89+
90+
The current flat logging with targets is sufficient. Regarding "Log auto-fix operations at INFO level", we should not be too verbose. We can show only a summary of the number of files fixed per linter.
91+
92+
### 4. **Exit Code Behavior**
93+
94+
What should the exit code be in these scenarios?
95+
96+
- All linters pass without fixes needed: **0** ✅
97+
- Some files auto-fixed, all linters pass after fix: **0** ✅
98+
- Some files auto-fixed, but errors remain: **non-zero** ✅
99+
- Auto-fix fails (command error): **non-zero** ✅
100+
101+
**Question:** Is this correct?
102+
103+
**Your Answer:**
104+
105+
Yes, it's correct.
106+
107+
### 5. **Git Integration**
108+
109+
Should we:
110+
111+
- **Option A**: Just run fix commands, let git track changes naturally (unstaged)
112+
- **Option B**: Automatically stage fixed files
113+
- **Option C**: Show git diff after fixes
114+
115+
**My assumption:** Option A (just fix, don't auto-stage) - correct?
116+
117+
**Your Answer:**
118+
119+
Yes, it's correct.
120+
121+
### 6. **Error Handling**
122+
123+
If auto-fix command fails (e.g., `npx` not found, network error):
124+
125+
- **Option A**: Fail immediately, don't continue with other linters
126+
- **Option B**: Log error, skip that linter's auto-fix, continue with checking
127+
- **Option C**: Log error, skip that linter entirely (no fix or check)
128+
129+
**My suggestion:** Option B (log error, still run check) - agree?
130+
131+
**Your Answer:**
132+
133+
I want a different option: **Option D - Auto-install missing tools**. Automatically install the missing tool (e.g., npm package) if it's not found, the same way we do for the linters. After installation, proceed with auto-fix and checking.
134+
135+
### 7. **Definition of Done - Testing**
136+
137+
What level of testing do you expect?
138+
139+
- Unit tests for CLI parsing? ✅
140+
- Integration tests for each linter? ✅
141+
- E2E test with actual broken files that get fixed? ✅
142+
- Manual testing checklist? ✅
143+
144+
**Question:** Is this sufficient?
145+
146+
**Your Answer:**
147+
148+
Yes, that should be enough.
149+
150+
### 8. **Documentation Updates**
151+
152+
Which documents need updating?
153+
154+
- `docs/contributing/commit-process.md` - Pre-commit checklist ✅
155+
- `docs/contributing/linting.md` - Linting documentation ✅
156+
- `README.md` - Main readme (if it mentions linting) ✅
157+
- Any others?
158+
159+
**Your Answer:**
160+
161+
No, that's enough.
162+
163+
### 9. **Backward Compatibility**
164+
165+
Should the behavior without `--fix` remain **exactly** the same as current?
166+
167+
- Same output format? ✅
168+
- Same exit codes? ✅
169+
- Same file paths checked? ✅
170+
171+
**Question:** Confirm this is correct?
172+
173+
**Your Answer:**
174+
175+
Yes, that's correct.
176+
177+
### 10. **Timeline & Priority**
178+
179+
Should we implement this as:
180+
181+
- **Option A**: Implement all linters at once
182+
- **Option B**: Start with markdown only, iterate with others
183+
- **Option C**: Core infrastructure first, then add linters incrementally
184+
185+
**My suggestion:** Option A (all at once, it's not that complex) - agree?
186+
187+
**Your Answer:**
188+
189+
Yes, we can do it all at once. However, during the implementation, I want to work on one linter at a time so we can test each one properly and commit the changes incrementally.
190+
191+
## 📋 Summary of Assumptions
192+
193+
Based on your feedback, here are my assumptions (please confirm or correct):
194+
195+
1. ✅ Add `--fix` flag to existing `cargo run --bin linter` binary
196+
2. ✅ Fix same files that linter checks (project-wide)
197+
3. ✅ **UPDATED**: Show only remaining errors after fix, not what was fixed
198+
4. ✅ Exit 0 only if all errors resolved (fixed or none)
199+
5. ✅ Don't auto-stage files, just fix them (developers use git to see changes)
200+
6. ✅ If fix command fails, log error and continue with check
201+
7. ✅ Implement all linters with auto-fix support at once
202+
8. ✅ Comprehensive testing (unit + integration + manual)
203+
9. ✅ Update documentation in commit-process and linting guides
204+
205+
**Your Confirmation:**
206+
207+
Regarding point 6: It's OK, but if the command fails because the tool is missing, I want it to be installed automatically (same as we do for linters).
208+
209+
Regarding point 7: Yes, but during the implementation, I want to work on one linter at a time so we can test each one properly and commit the changes incrementally.
210+
211+
### 11. **Parallel Execution (Moved to Separate Feature)**
212+
213+
**Note**: Parallel execution has been moved to a separate feature specification.
214+
215+
**See**: [Parallel Linter Execution Feature](../linter-parallel-execution/specification.md)
216+
217+
**Decision for auto-fix feature**: Use sequential execution (current approach)
218+
219+
**Rationale**:
220+
221+
- Current performance (~13s) is acceptable for pre-commit workflow
222+
- Parallel execution is a separate optimization that can be added later
223+
- Auto-fix works correctly with sequential execution
224+
- Focus on implementing auto-fix functionality first (higher priority)
225+
226+
**Compatibility**: Auto-fix will work correctly with parallel execution if that feature is implemented in the future.
227+
228+
---
229+
230+
## 🚀 Next Steps
231+
232+
Once you've answered these questions:
233+
234+
1. I'll update the specification based on your answers
235+
2. Create a detailed implementation plan
236+
3. You'll review the plan
237+
4. Commit the documentation
238+
5. Start implementing the feature

0 commit comments

Comments
 (0)