refactor(analyser): separate concerns (#617)

Refactors `pgls_analyse` and `pgls_analyser` so that "source code
linter"-concerns only live in `analyser`. `analyse` only contains shared
concerns such as `RuleMeta` and a generic `RuleVisitor`.

This is in preparation for the integration of other type of linters
(e.g. `splinter`), which will use the same infrastructure as much as
possible. Every execution concern was moved to `pgls_analyser`, since
other linters will not run on the AST and have a different execution
model.

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: psteinroe

Cargo.lock

@@ -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

PLAN.md

@@ -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
@@ -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>())
@@ -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,
@@ -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)
     }
 

crates/pgls_analyse/src/context.rs

@@ -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};