Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

194 changes: 194 additions & 0 deletions PLAN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
# Splinter Integration Plan

## Goal
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.

## Architecture Vision

### Crate Responsibilities

**`pgls_analyse`** - Generic framework for all analyzer types
- Generic traits: `RuleMeta`, `RuleGroup`, `GroupCategory`, `RegistryVisitor`
- Shared types: `RuleMetadata`, `RuleCategory`
- Configuration traits (execution-agnostic)
- Macros: `declare_rule!`, `declare_group!`, `declare_category!`

**`pgls_linter`** (renamed from `pgls_analyser`) - AST-based source code linting
- `LinterRule` trait (extends `RuleMeta`)
- `LinterRuleContext` (wraps AST nodes)
- `LinterDiagnostic` (span-based diagnostics)
- `LinterRuleRegistry` (type-erased executors)

**`pgls_splinter`** - Database-level linting
- `SplinterRule` trait (extends `RuleMeta`)
- `SplinterRuleRegistry` (metadata-based)
- `SplinterDiagnostic` (db-object-based diagnostics)
- Generated rule types from SQL files

**`pgls_configuration`**
- `analyser/linter/` - Generated from `pgls_linter`
- `analyser/splinter/` - Generated from `pgls_splinter`
- Per-rule configuration for both

## Implementation Phases

### Phase 1: Refactor pgls_analyse ⏳ IN PROGRESS
Extract AST-specific code into pgls_linter, keep only generic framework in pgls_analyse.

**Tasks:**
- [x] Analyze current `pgls_analyse` exports
- [x] Identify AST-specific vs generic code
- [x] Create new modules in `pgls_analyser`:
- [x] `linter_rule.rs` - LinterRule trait, LinterDiagnostic
- [x] `linter_context.rs` - LinterRuleContext, AnalysedFileContext
- [x] `linter_options.rs` - LinterOptions, LinterRules
- [x] `linter_registry.rs` - LinterRuleRegistry, LinterRegistryVisitor
- [x] Create `pgls_analyse/src/metadata.rs` - Generic traits only
- [x] Update `pgls_analyse/src/registry.rs` - Keep MetadataRegistry only
- [x] Update `pgls_analyse/src/lib.rs` - Export generic framework
- [x] Update `pgls_analyser/src/lib.rs` - Use new modules
- [x] Fix imports in filter.rs (RuleMeta instead of Rule)
- [x] Update generated files (options.rs, registry.rs)
- [x] Fix imports in all rule files
- [x] Add rustc-hash dependency
- [x] Verify compilation completes - **RESOLVED**
- [x] Separate visitor concerns from executor creation
- [x] Update codegen to generate factory function
- [x] Fix all import paths across workspace
- [x] Verify full workspace compiles
- [ ] Run tests

**Resolution:**
Separated two concerns:
1. **Visitor pattern** (generic): Collects rule keys that match the filter
- Implementation in `LinterRuleRegistryBuilder::record_rule`
- Only requires `R: RuleMeta` (satisfies trait)
2. **Factory mapping** (AST-specific): Maps rule keys to executor factories
- Function `get_linter_rule_factory` in `registry.rs`
- Will be generated by codegen with full type information
- Each factory can require `R: LinterRule<Options: Default>`

**Changes Made:**
- `LinterRuleRegistryBuilder` stores `Vec<RuleKey>` instead of factories
- `record_rule` just collects keys (no LinterRule bounds needed)
- `build()` calls `get_linter_rule_factory` to create executors
- Added stub `get_linter_rule_factory` in `registry.rs` (will be generated)

**Next Steps:**
- Update codegen to generate `get_linter_rule_factory` with match on all rules

**Design Decisions:**
- ✅ Keep `RuleDiagnostic` generic or make it linter-specific? → **Move to pgls_linter as LinterDiagnostic** (Option A)
- Rationale: Fundamentally different location models (spans vs db objects)
- LinterDiagnostic: span-based
- SplinterDiagnostic: db-object-based

**Code Classification:**

AST-specific (move to pgls_analyser):
- `Rule` trait
- `RuleContext`
- `RuleDiagnostic` → `LinterDiagnostic`
- `AnalysedFileContext`
- `RegistryRuleParams`
- `RuleRegistry`, `RuleRegistryBuilder` (AST execution)
- `AnalyserOptions`, `AnalyserRules` (rule options storage)

Generic (keep in pgls_analyse):
- `RuleMeta` trait
- `RuleMetadata` struct
- `RuleGroup` trait
- `GroupCategory` trait
- `RegistryVisitor` trait
- `RuleCategory` enum
- `RuleSource` enum
- `RuleFilter`, `AnalysisFilter`, `RuleKey`, `GroupKey`
- `MetadataRegistry`
- Macros: `declare_rule!`, `declare_lint_rule!`, `declare_lint_group!`, `declare_category!`

---

### Phase 2: Enhance pgls_splinter 📋 PLANNED
Add rule type generation and registry similar to linter.

**Tasks:**
- [ ] Create `pgls_splinter/src/rule.rs` with `SplinterRule` trait
- [ ] Create `pgls_splinter/src/rules/` directory structure
- [ ] Generate rule types from SQL files
- [ ] Generate registry with `visit_registry()` function
- [ ] Update diagnostics to use generated categories

**Structure:**
```
pgls_splinter/src/
rules/
performance/
unindexed_foreign_keys.rs
auth_rls_initplan.rs
security/
auth_users_exposed.rs
rule.rs # SplinterRule trait
registry.rs # Generated visit_registry()
```

---

### Phase 3: Update codegen for both linters 📋 PLANNED
Generalize codegen to handle both linter types.

**Tasks:**
- [ ] Rename `generate_analyser.rs` → `generate_linter.rs`
- [ ] Enhance `generate_splinter.rs` to generate rules + registry
- [ ] Update `generate_configuration.rs` for both linters
- [ ] Update justfile commands
- [ ] Test full generation cycle

**Codegen outputs:**
- Linter: registry.rs, options.rs, configuration
- Splinter: rules/, registry.rs, configuration

---

### Phase 4: Rename pgls_analyser → pgls_linter 📋 PLANNED
Final rename to clarify purpose.

**Tasks:**
- [ ] Rename crate in Cargo.toml
- [ ] Update all imports
- [ ] Update documentation
- [ ] Update CLAUDE.md / AGENTS.md
- [ ] Verify tests pass

---

## Progress Tracking

### Current Status
- [x] Requirements gathering & design
- [x] Architecture proposal (Option 1 - Dual-Track)
- [ ] Phase 1: Refactor pgls_analyse - **IN PROGRESS**
- [ ] Phase 2: Enhance pgls_splinter
- [ ] Phase 3: Update codegen
- [ ] Phase 4: Rename to pgls_linter

### Open Questions
None currently

### Decisions Made
1. Use `LinterRule` (not `ASTRule` or `SourceCodeRule`) for clarity
2. Use `SplinterRule` for database-level rules
3. Keep codegen in xtask (not build.rs) for consistency
4. Mirror file structure between linter and splinter

---

## Testing Strategy
- [ ] Existing linter tests continue to pass
- [ ] Splinter rules generate correctly from SQL
- [ ] Configuration schema validates
- [ ] Integration test: enable/disable rules via config
- [ ] Integration test: severity overrides work

---

Last updated: 2025-12-14
96 changes: 0 additions & 96 deletions crates/pgls_analyse/src/context.rs

This file was deleted.

8 changes: 4 additions & 4 deletions crates/pgls_analyse/src/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::fmt::{Debug, Display, Formatter};

use crate::{
categories::RuleCategories,
rule::{GroupCategory, Rule, RuleGroup},
metadata::{GroupCategory, RuleGroup, RuleMeta},
};

/// Allow filtering a single rule or group of rules by their names
Expand Down Expand Up @@ -52,7 +52,7 @@ impl<'analysis> AnalysisFilter<'analysis> {
}

/// Return `true` if the rule `R` matches this filter
pub fn match_rule<R: Rule>(&self) -> bool {
pub fn match_rule<R: RuleMeta>(&self) -> bool {
self.match_category::<<R::Group as RuleGroup>::Category>()
&& self.enabled_rules.is_none_or(|enabled_rules| {
enabled_rules.iter().any(|filter| filter.match_rule::<R>())
Expand Down Expand Up @@ -83,7 +83,7 @@ impl<'a> RuleFilter<'a> {
/// Return `true` if the rule `R` matches this filter
pub fn match_rule<R>(self) -> bool
where
R: Rule,
R: RuleMeta,
{
match self {
RuleFilter::Group(group) => group == <R::Group as RuleGroup>::NAME,
Expand Down Expand Up @@ -160,7 +160,7 @@ impl RuleKey {
Self { group, rule }
}

pub fn rule<R: Rule>() -> Self {
pub fn rule<R: RuleMeta>() -> Self {
Self::new(<R::Group as RuleGroup>::NAME, R::METADATA.name)
}

Expand Down
15 changes: 3 additions & 12 deletions crates/pgls_analyse/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,25 +1,16 @@
mod analysed_file_context;
mod categories;
pub mod context;
mod filter;
pub mod macros;
pub mod options;
mod metadata;
mod registry;
mod rule;

// Re-exported for use in the `declare_group` macro
pub use pgls_diagnostics::category_concat;

pub use crate::analysed_file_context::AnalysedFileContext;
pub use crate::categories::{
ActionCategory, RefactorKind, RuleCategories, RuleCategoriesBuilder, RuleCategory,
SUPPRESSION_ACTION_CATEGORY, SourceActionKind,
};
pub use crate::filter::{AnalysisFilter, GroupKey, RuleFilter, RuleKey};
pub use crate::options::{AnalyserOptions, AnalyserRules};
pub use crate::registry::{
MetadataRegistry, RegistryRuleParams, RegistryVisitor, RuleRegistry, RuleRegistryBuilder,
};
pub use crate::rule::{
GroupCategory, Rule, RuleDiagnostic, RuleGroup, RuleMeta, RuleMetadata, RuleSource,
};
pub use crate::metadata::{GroupCategory, RuleGroup, RuleMeta, RuleMetadata, RuleSource};
pub use crate::registry::{MetadataRegistry, RegistryVisitor};
Loading