Skip to content

Commit 56484d9

Browse files
psteinroepsteinroe
committed
Merge main into chore/integrate-splinter-runtime
2 parents 22cfd84 + cf54370 commit 56484d9

File tree

2 files changed

+198
-0
lines changed

2 files changed

+198
-0
lines changed

‎PLAN.md‎

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
# Splinter Integration Plan
2+
3+
## Goal
4+
Integrate splinter into the codegen/rule setup used for the analyser, providing a consistent API (both internally and user-facing) for all types of analysers/linters.
5+
6+
## Architecture Vision
7+
8+
### Crate Responsibilities
9+
10+
**`pgls_analyse`** - Generic framework for all analyzer types
11+
- Generic traits: `RuleMeta`, `RuleGroup`, `GroupCategory`, `RegistryVisitor`
12+
- Shared types: `RuleMetadata`, `RuleCategory`
13+
- Configuration traits (execution-agnostic)
14+
- Macros: `declare_rule!`, `declare_group!`, `declare_category!`
15+
16+
**`pgls_linter`** (renamed from `pgls_analyser`) - AST-based source code linting
17+
- `LinterRule` trait (extends `RuleMeta`)
18+
- `LinterRuleContext` (wraps AST nodes)
19+
- `LinterDiagnostic` (span-based diagnostics)
20+
- `LinterRuleRegistry` (type-erased executors)
21+
22+
**`pgls_splinter`** - Database-level linting
23+
- `SplinterRule` trait (extends `RuleMeta`)
24+
- `SplinterRuleRegistry` (metadata-based)
25+
- `SplinterDiagnostic` (db-object-based diagnostics)
26+
- Generated rule types from SQL files
27+
28+
**`pgls_configuration`**
29+
- `analyser/linter/` - Generated from `pgls_linter`
30+
- `analyser/splinter/` - Generated from `pgls_splinter`
31+
- Per-rule configuration for both
32+
33+
## Implementation Phases
34+
35+
### Phase 1: Refactor pgls_analyse ⏳ IN PROGRESS
36+
Extract AST-specific code into pgls_linter, keep only generic framework in pgls_analyse.
37+
38+
**Tasks:**
39+
- [x] Analyze current `pgls_analyse` exports
40+
- [x] Identify AST-specific vs generic code
41+
- [x] Create new modules in `pgls_analyser`:
42+
- [x] `linter_rule.rs` - LinterRule trait, LinterDiagnostic
43+
- [x] `linter_context.rs` - LinterRuleContext, AnalysedFileContext
44+
- [x] `linter_options.rs` - LinterOptions, LinterRules
45+
- [x] `linter_registry.rs` - LinterRuleRegistry, LinterRegistryVisitor
46+
- [x] Create `pgls_analyse/src/metadata.rs` - Generic traits only
47+
- [x] Update `pgls_analyse/src/registry.rs` - Keep MetadataRegistry only
48+
- [x] Update `pgls_analyse/src/lib.rs` - Export generic framework
49+
- [x] Update `pgls_analyser/src/lib.rs` - Use new modules
50+
- [x] Fix imports in filter.rs (RuleMeta instead of Rule)
51+
- [x] Update generated files (options.rs, registry.rs)
52+
- [x] Fix imports in all rule files
53+
- [x] Add rustc-hash dependency
54+
- [x] Verify compilation completes - **RESOLVED**
55+
- [x] Separate visitor concerns from executor creation
56+
- [x] Update codegen to generate factory function
57+
- [x] Fix all import paths across workspace
58+
- [x] Verify full workspace compiles
59+
- [ ] Run tests
60+
61+
**Resolution:**
62+
Separated two concerns:
63+
1. **Visitor pattern** (generic): Collects rule keys that match the filter
64+
- Implementation in `LinterRuleRegistryBuilder::record_rule`
65+
- Only requires `R: RuleMeta` (satisfies trait)
66+
2. **Factory mapping** (AST-specific): Maps rule keys to executor factories
67+
- Function `get_linter_rule_factory` in `registry.rs`
68+
- Will be generated by codegen with full type information
69+
- Each factory can require `R: LinterRule<Options: Default>`
70+
71+
**Changes Made:**
72+
- `LinterRuleRegistryBuilder` stores `Vec<RuleKey>` instead of factories
73+
- `record_rule` just collects keys (no LinterRule bounds needed)
74+
- `build()` calls `get_linter_rule_factory` to create executors
75+
- Added stub `get_linter_rule_factory` in `registry.rs` (will be generated)
76+
77+
**Next Steps:**
78+
- Update codegen to generate `get_linter_rule_factory` with match on all rules
79+
80+
**Design Decisions:**
81+
- ✅ Keep `RuleDiagnostic` generic or make it linter-specific? → **Move to pgls_linter as LinterDiagnostic** (Option A)
82+
- Rationale: Fundamentally different location models (spans vs db objects)
83+
- LinterDiagnostic: span-based
84+
- SplinterDiagnostic: db-object-based
85+
86+
**Code Classification:**
87+
88+
AST-specific (move to pgls_analyser):
89+
- `Rule` trait
90+
- `RuleContext`
91+
- `RuleDiagnostic` → `LinterDiagnostic`
92+
- `AnalysedFileContext`
93+
- `RegistryRuleParams`
94+
- `RuleRegistry`, `RuleRegistryBuilder` (AST execution)
95+
- `AnalyserOptions`, `AnalyserRules` (rule options storage)
96+
97+
Generic (keep in pgls_analyse):
98+
- `RuleMeta` trait
99+
- `RuleMetadata` struct
100+
- `RuleGroup` trait
101+
- `GroupCategory` trait
102+
- `RegistryVisitor` trait
103+
- `RuleCategory` enum
104+
- `RuleSource` enum
105+
- `RuleFilter`, `AnalysisFilter`, `RuleKey`, `GroupKey`
106+
- `MetadataRegistry`
107+
- Macros: `declare_rule!`, `declare_lint_rule!`, `declare_lint_group!`, `declare_category!`
108+
109+
---
110+
111+
### Phase 2: Enhance pgls_splinter 📋 PLANNED
112+
Add rule type generation and registry similar to linter.
113+
114+
**Tasks:**
115+
- [ ] Create `pgls_splinter/src/rule.rs` with `SplinterRule` trait
116+
- [ ] Create `pgls_splinter/src/rules/` directory structure
117+
- [ ] Generate rule types from SQL files
118+
- [ ] Generate registry with `visit_registry()` function
119+
- [ ] Update diagnostics to use generated categories
120+
121+
**Structure:**
122+
```
123+
pgls_splinter/src/
124+
rules/
125+
performance/
126+
unindexed_foreign_keys.rs
127+
auth_rls_initplan.rs
128+
security/
129+
auth_users_exposed.rs
130+
rule.rs # SplinterRule trait
131+
registry.rs # Generated visit_registry()
132+
```
133+
134+
---
135+
136+
### Phase 3: Update codegen for both linters 📋 PLANNED
137+
Generalize codegen to handle both linter types.
138+
139+
**Tasks:**
140+
- [ ] Rename `generate_analyser.rs` → `generate_linter.rs`
141+
- [ ] Enhance `generate_splinter.rs` to generate rules + registry
142+
- [ ] Update `generate_configuration.rs` for both linters
143+
- [ ] Update justfile commands
144+
- [ ] Test full generation cycle
145+
146+
**Codegen outputs:**
147+
- Linter: registry.rs, options.rs, configuration
148+
- Splinter: rules/, registry.rs, configuration
149+
150+
---
151+
152+
### Phase 4: Rename pgls_analyser → pgls_linter 📋 PLANNED
153+
Final rename to clarify purpose.
154+
155+
**Tasks:**
156+
- [ ] Rename crate in Cargo.toml
157+
- [ ] Update all imports
158+
- [ ] Update documentation
159+
- [ ] Update CLAUDE.md / AGENTS.md
160+
- [ ] Verify tests pass
161+
162+
---
163+
164+
## Progress Tracking
165+
166+
### Current Status
167+
- [x] Requirements gathering & design
168+
- [x] Architecture proposal (Option 1 - Dual-Track)
169+
- [ ] Phase 1: Refactor pgls_analyse - **IN PROGRESS**
170+
- [ ] Phase 2: Enhance pgls_splinter
171+
- [ ] Phase 3: Update codegen
172+
- [ ] Phase 4: Rename to pgls_linter
173+
174+
### Open Questions
175+
None currently
176+
177+
### Decisions Made
178+
1. Use `LinterRule` (not `ASTRule` or `SourceCodeRule`) for clarity
179+
2. Use `SplinterRule` for database-level rules
180+
3. Keep codegen in xtask (not build.rs) for consistency
181+
4. Mirror file structure between linter and splinter
182+
183+
---
184+
185+
## Testing Strategy
186+
- [ ] Existing linter tests continue to pass
187+
- [ ] Splinter rules generate correctly from SQL
188+
- [ ] Configuration schema validates
189+
- [ ] Integration test: enable/disable rules via config
190+
- [ ] Integration test: severity overrides work
191+
192+
---
193+
194+
Last updated: 2025-12-14

‎crates/pgls_analyser/src/lib.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ pub use linter_registry::{
1919
};
2020
pub use linter_rule::{LinterDiagnostic, LinterRule};
2121

22+
// For convenience in macros and rule files - keep these shorter names
23+
pub use LinterDiagnostic as RuleDiagnostic;
24+
pub use LinterRule as Rule;
25+
pub use LinterRuleContext as RuleContext;
2226
pub static METADATA: LazyLock<MetadataRegistry> = LazyLock::new(|| {
2327
let mut metadata = MetadataRegistry::default();
2428
// Use a separate visitor for metadata that implements pgls_analyse::RegistryVisitor

0 commit comments

Comments
 (0)