From b12cfc0a0f7743d59f67b747ce2da456b4ad23df Mon Sep 17 00:00:00 2001 From: psteinroe Date: Sun, 14 Dec 2025 18:30:18 +0100 Subject: [PATCH 1/5] refactor: analyse --- Cargo.lock | 1 + PLAN.md | 194 ++++++++++++++++++ crates/pgls_analyse/src/context.rs | 96 --------- crates/pgls_analyse/src/filter.rs | 8 +- crates/pgls_analyse/src/lib.rs | 15 +- crates/pgls_analyse/src/metadata.rs | 157 ++++++++++++++ crates/pgls_analyse/src/registry.rs | 118 +---------- crates/pgls_analyser/Cargo.toml | 1 + crates/pgls_analyser/src/lib.rs | 44 ++-- .../src/lint/safety/add_serial_column.rs | 3 +- .../lint/safety/adding_field_with_default.rs | 3 +- .../safety/adding_foreign_key_constraint.rs | 3 +- .../src/lint/safety/adding_not_null_field.rs | 3 +- .../safety/adding_primary_key_constraint.rs | 3 +- .../src/lint/safety/adding_required_field.rs | 3 +- .../src/lint/safety/ban_char_field.rs | 3 +- ...oncurrent_index_creation_in_transaction.rs | 3 +- .../src/lint/safety/ban_drop_column.rs | 3 +- .../src/lint/safety/ban_drop_database.rs | 3 +- .../src/lint/safety/ban_drop_not_null.rs | 3 +- .../src/lint/safety/ban_drop_table.rs | 3 +- .../src/lint/safety/ban_truncate_cascade.rs | 3 +- .../src/lint/safety/changing_column_type.rs | 3 +- .../safety/constraint_missing_not_valid.rs | 3 +- .../src/lint/safety/creating_enum.rs | 3 +- .../lint/safety/disallow_unique_constraint.rs | 3 +- .../src/lint/safety/lock_timeout_warning.rs | 3 +- .../src/lint/safety/multiple_alter_table.rs | 3 +- .../src/lint/safety/prefer_big_int.rs | 3 +- .../src/lint/safety/prefer_bigint_over_int.rs | 3 +- .../safety/prefer_bigint_over_smallint.rs | 3 +- .../src/lint/safety/prefer_identity.rs | 3 +- .../src/lint/safety/prefer_jsonb.rs | 3 +- .../src/lint/safety/prefer_robust_stmts.rs | 3 +- .../src/lint/safety/prefer_text_field.rs | 3 +- .../src/lint/safety/prefer_timestamptz.rs | 3 +- .../src/lint/safety/renaming_column.rs | 3 +- .../src/lint/safety/renaming_table.rs | 3 +- .../require_concurrent_index_creation.rs | 8 +- .../require_concurrent_index_deletion.rs | 3 +- ...tatement_while_holding_access_exclusive.rs | 3 +- .../src/lint/safety/transaction_nesting.rs | 5 +- .../src/linter_context.rs} | 94 +++++++++ .../src/linter_options.rs} | 19 +- crates/pgls_analyser/src/linter_registry.rs | 118 +++++++++++ .../src/linter_rule.rs} | 188 ++--------------- crates/pgls_analyser/src/options.rs | 67 +++--- crates/pgls_analyser/src/registry.rs | 167 ++++++++++++++- crates/pgls_configuration/src/linter/rules.rs | 3 +- .../src/rules/configuration.rs | 3 +- crates/pgls_splinter/src/convert.rs | 7 +- crates/pgls_splinter/src/diagnostics.rs | 10 +- crates/pgls_workspace/src/configuration.rs | 6 +- crates/pgls_workspace/src/workspace/server.rs | 7 +- .../src/workspace/server/analyser.rs | 6 +- docs/codegen/src/rules_docs.rs | 6 +- docs/codegen/src/utils.rs | 8 +- xtask/codegen/src/generate_analyser.rs | 61 +++++- xtask/codegen/src/generate_configuration.rs | 6 +- xtask/rules_check/src/lib.rs | 12 +- 60 files changed, 997 insertions(+), 531 deletions(-) create mode 100644 PLAN.md delete mode 100644 crates/pgls_analyse/src/context.rs create mode 100644 crates/pgls_analyse/src/metadata.rs rename crates/{pgls_analyse/src/analysed_file_context.rs => pgls_analyser/src/linter_context.rs} (66%) rename crates/{pgls_analyse/src/options.rs => pgls_analyser/src/linter_options.rs} (83%) create mode 100644 crates/pgls_analyser/src/linter_registry.rs rename crates/{pgls_analyse/src/rule.rs => pgls_analyser/src/linter_rule.rs} (50%) diff --git a/Cargo.lock b/Cargo.lock index b5f2f6f57..3425f1a83 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2667,6 +2667,7 @@ dependencies = [ "pgls_statement_splitter", "pgls_test_macros", "pgls_text_size", + "rustc-hash 2.1.0", "serde", "termcolor", ] diff --git a/PLAN.md b/PLAN.md new file mode 100644 index 000000000..86e0536db --- /dev/null +++ b/PLAN.md @@ -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` + +**Changes Made:** +- `LinterRuleRegistryBuilder` stores `Vec` 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 diff --git a/crates/pgls_analyse/src/context.rs b/crates/pgls_analyse/src/context.rs deleted file mode 100644 index bf3f48762..000000000 --- a/crates/pgls_analyse/src/context.rs +++ /dev/null @@ -1,96 +0,0 @@ -use pgls_schema_cache::SchemaCache; - -use crate::{ - AnalysedFileContext, - categories::RuleCategory, - rule::{GroupCategory, Rule, RuleGroup, RuleMetadata}, -}; - -pub struct RuleContext<'a, R: Rule> { - stmt: &'a pgls_query::NodeEnum, - options: &'a R::Options, - schema_cache: Option<&'a SchemaCache>, - file_context: &'a AnalysedFileContext<'a>, -} - -impl<'a, R> RuleContext<'a, R> -where - R: Rule + Sized + 'static, -{ - #[allow(clippy::too_many_arguments)] - pub fn new( - stmt: &'a pgls_query::NodeEnum, - options: &'a R::Options, - schema_cache: Option<&'a SchemaCache>, - file_context: &'a AnalysedFileContext, - ) -> Self { - Self { - stmt, - options, - schema_cache, - file_context, - } - } - - /// Returns the group that belongs to the current rule - pub fn group(&self) -> &'static str { - ::NAME - } - - /// Returns the category that belongs to the current rule - pub fn category(&self) -> RuleCategory { - <::Category as GroupCategory>::CATEGORY - } - - /// Returns the AST root - pub fn stmt(&self) -> &pgls_query::NodeEnum { - self.stmt - } - - pub fn file_context(&self) -> &AnalysedFileContext { - self.file_context - } - - pub fn schema_cache(&self) -> Option<&SchemaCache> { - self.schema_cache - } - - /// Returns the metadata of the rule - /// - /// The metadata contains information about the rule, such as the name, version, language, and whether it is recommended. - /// - /// ## Examples - /// ```rust,ignore - /// declare_lint_rule! { - /// /// Some doc - /// pub(crate) Foo { - /// version: "0.0.0", - /// name: "foo", - /// recommended: true, - /// } - /// } - /// - /// impl Rule for Foo { - /// const CATEGORY: RuleCategory = RuleCategory::Lint; - /// type State = (); - /// type Signals = (); - /// type Options = (); - /// - /// fn run(ctx: &RuleContext) -> Self::Signals { - /// assert_eq!(ctx.metadata().name, "foo"); - /// } - /// } - /// ``` - pub fn metadata(&self) -> &RuleMetadata { - &R::METADATA - } - - /// It retrieves the options that belong to a rule, if they exist. - /// - /// In order to retrieve a typed data structure, you have to create a deserializable - /// data structure and define it inside the generic type `type Options` of the [Rule] - /// - pub fn options(&self) -> &R::Options { - self.options - } -} diff --git a/crates/pgls_analyse/src/filter.rs b/crates/pgls_analyse/src/filter.rs index bb9cdc27c..72e33cf59 100644 --- a/crates/pgls_analyse/src/filter.rs +++ b/crates/pgls_analyse/src/filter.rs @@ -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(&self) -> bool { + pub fn match_rule(&self) -> bool { self.match_category::<::Category>() && self.enabled_rules.is_none_or(|enabled_rules| { enabled_rules.iter().any(|filter| filter.match_rule::()) @@ -83,7 +83,7 @@ impl<'a> RuleFilter<'a> { /// Return `true` if the rule `R` matches this filter pub fn match_rule(self) -> bool where - R: Rule, + R: RuleMeta, { match self { RuleFilter::Group(group) => group == ::NAME, @@ -160,7 +160,7 @@ impl RuleKey { Self { group, rule } } - pub fn rule() -> Self { + pub fn rule() -> Self { Self::new(::NAME, R::METADATA.name) } diff --git a/crates/pgls_analyse/src/lib.rs b/crates/pgls_analyse/src/lib.rs index 29b18f362..b327b843a 100644 --- a/crates/pgls_analyse/src/lib.rs +++ b/crates/pgls_analyse/src/lib.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}; diff --git a/crates/pgls_analyse/src/metadata.rs b/crates/pgls_analyse/src/metadata.rs new file mode 100644 index 000000000..a614e400e --- /dev/null +++ b/crates/pgls_analyse/src/metadata.rs @@ -0,0 +1,157 @@ +use pgls_diagnostics::Severity; +use std::cmp::Ordering; + +use crate::{categories::RuleCategory, registry::RegistryVisitor}; + +#[derive(Clone, Debug)] +#[cfg_attr( + feature = "serde", + derive(serde::Serialize), + serde(rename_all = "camelCase") +)] +/// Static metadata containing information about a rule +pub struct RuleMetadata { + /// It marks if a rule is deprecated, and if so a reason has to be provided. + pub deprecated: Option<&'static str>, + /// The version when the rule was implemented + pub version: &'static str, + /// The name of this rule, displayed in the diagnostics it emits + pub name: &'static str, + /// The content of the documentation comments for this rule + pub docs: &'static str, + /// Whether a rule is recommended or not + pub recommended: bool, + /// The source URL of the rule + pub sources: &'static [RuleSource], + /// The default severity of the rule + pub severity: Severity, +} + +impl RuleMetadata { + pub const fn new( + version: &'static str, + name: &'static str, + docs: &'static str, + severity: Severity, + ) -> Self { + Self { + deprecated: None, + version, + name, + docs, + sources: &[], + recommended: false, + severity, + } + } + + pub const fn recommended(mut self, recommended: bool) -> Self { + self.recommended = recommended; + self + } + + pub const fn deprecated(mut self, deprecated: &'static str) -> Self { + self.deprecated = Some(deprecated); + self + } + + pub const fn sources(mut self, sources: &'static [RuleSource]) -> Self { + self.sources = sources; + self + } +} + +pub trait RuleMeta { + type Group: RuleGroup; + const METADATA: RuleMetadata; +} + +/// A rule group is a collection of rules under a given name, serving as a +/// "namespace" for lint rules and allowing the entire set of rules to be +/// disabled at once +pub trait RuleGroup { + type Category: GroupCategory; + /// The name of this group, displayed in the diagnostics emitted by its rules + const NAME: &'static str; + /// Register all the rules belonging to this group into `registry` + fn record_rules(registry: &mut V); +} + +/// A group category is a collection of rule groups under a given category ID, +/// serving as a broad classification on the kind of diagnostic or code action +/// these rule emit, and allowing whole categories of rules to be disabled at +/// once depending on the kind of analysis being performed +pub trait GroupCategory { + /// The category ID used for all groups and rule belonging to this category + const CATEGORY: RuleCategory; + /// Register all the groups belonging to this category into `registry` + fn record_groups(registry: &mut V); +} + +#[derive(Debug, Clone, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize))] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +pub enum RuleSource { + /// Rules from [Squawk](https://squawkhq.com) + Squawk(&'static str), + /// Rules from [Eugene](https://github.com/kaaveland/eugene) + Eugene(&'static str), +} + +impl PartialEq for RuleSource { + fn eq(&self, other: &Self) -> bool { + std::mem::discriminant(self) == std::mem::discriminant(other) + } +} + +impl std::fmt::Display for RuleSource { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Squawk(_) => write!(f, "Squawk"), + Self::Eugene(_) => write!(f, "Eugene"), + } + } +} + +impl PartialOrd for RuleSource { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for RuleSource { + fn cmp(&self, other: &Self) -> Ordering { + let self_rule = self.as_rule_name(); + let other_rule = other.as_rule_name(); + self_rule.cmp(other_rule) + } +} + +impl RuleSource { + pub fn as_rule_name(&self) -> &'static str { + match self { + Self::Squawk(rule_name) => rule_name, + Self::Eugene(rule_name) => rule_name, + } + } + + pub fn to_namespaced_rule_name(&self) -> String { + match self { + Self::Squawk(rule_name) => format!("squawk/{rule_name}"), + Self::Eugene(rule_name) => format!("eugene/{rule_name}"), + } + } + + pub fn to_rule_url(&self) -> String { + match self { + Self::Squawk(rule_name) => format!("https://squawkhq.com/docs/{rule_name}"), + Self::Eugene(rule_name) => { + format!("https://kaveland.no/eugene/hints/{rule_name}/index.html") + } + } + } + + pub fn as_url_and_rule_name(&self) -> (String, &'static str) { + (self.to_rule_url(), self.as_rule_name()) + } +} diff --git a/crates/pgls_analyse/src/registry.rs b/crates/pgls_analyse/src/registry.rs index 3278c926d..413fa2e6c 100644 --- a/crates/pgls_analyse/src/registry.rs +++ b/crates/pgls_analyse/src/registry.rs @@ -1,11 +1,8 @@ use std::{borrow, collections::BTreeSet}; use crate::{ - AnalyserOptions, - analysed_file_context::AnalysedFileContext, - context::RuleContext, - filter::{AnalysisFilter, GroupKey, RuleKey}, - rule::{GroupCategory, Rule, RuleDiagnostic, RuleGroup}, + filter::{GroupKey, RuleKey}, + metadata::{GroupCategory, RuleGroup, RuleMeta}, }; pub trait RegistryVisitor { @@ -22,7 +19,7 @@ pub trait RegistryVisitor { /// Record the rule `R` to this visitor fn record_rule(&mut self) where - R: Rule + 'static; + R: RuleMeta + 'static; } /// Key struct for a rule in the metadata map, sorted alphabetically @@ -85,115 +82,8 @@ impl MetadataRegistry { impl RegistryVisitor for MetadataRegistry { fn record_rule(&mut self) where - R: Rule + 'static, + R: RuleMeta + 'static, { self.insert_rule(::NAME, R::METADATA.name); } } - -pub struct RuleRegistryBuilder<'a> { - filter: &'a AnalysisFilter<'a>, - // Rule Registry - registry: RuleRegistry, -} - -impl RegistryVisitor for RuleRegistryBuilder<'_> { - fn record_category(&mut self) { - if self.filter.match_category::() { - C::record_groups(self); - } - } - - fn record_group(&mut self) { - if self.filter.match_group::() { - G::record_rules(self); - } - } - - /// Add the rule `R` to the list of rules stored in this registry instance - fn record_rule(&mut self) - where - R: Rule + 'static, - { - if !self.filter.match_rule::() { - return; - } - - let rule = RegistryRule::new::(); - - self.registry.rules.push(rule); - } -} - -/// The rule registry holds type-erased instances of all active analysis rules -pub struct RuleRegistry { - pub rules: Vec, -} - -impl IntoIterator for RuleRegistry { - type Item = RegistryRule; - type IntoIter = std::vec::IntoIter; - - fn into_iter(self) -> Self::IntoIter { - self.rules.into_iter() - } -} - -/// Internal representation of a single rule in the registry -#[derive(Copy, Clone)] -pub struct RegistryRule { - pub run: RuleExecutor, -} - -impl RuleRegistry { - pub fn builder<'a>(filter: &'a AnalysisFilter<'a>) -> RuleRegistryBuilder<'a> { - RuleRegistryBuilder { - filter, - registry: RuleRegistry { - rules: Default::default(), - }, - } - } -} - -pub struct RegistryRuleParams<'a> { - pub root: &'a pgls_query::NodeEnum, - pub options: &'a AnalyserOptions, - pub analysed_file_context: &'a AnalysedFileContext<'a>, - pub schema_cache: Option<&'a pgls_schema_cache::SchemaCache>, -} - -/// Executor for rule as a generic function pointer -type RuleExecutor = fn(&RegistryRuleParams) -> Vec; - -impl RegistryRule { - fn new() -> Self - where - R: Rule + 'static, - { - /// Generic implementation of RuleExecutor for any rule type R - fn run(params: &RegistryRuleParams) -> Vec - where - R: Rule + 'static, - { - let options = params.options.rule_options::().unwrap_or_default(); - - let ctx = RuleContext::new( - params.root, - &options, - params.schema_cache, - params.analysed_file_context, - ); - - R::run(&ctx) - } - - Self { run: run:: } - } -} - -impl RuleRegistryBuilder<'_> { - pub fn build(self) -> RuleRegistry { - self.registry - } -} diff --git a/crates/pgls_analyser/Cargo.toml b/crates/pgls_analyser/Cargo.toml index ce9f0bf22..26c8ba3eb 100644 --- a/crates/pgls_analyser/Cargo.toml +++ b/crates/pgls_analyser/Cargo.toml @@ -19,6 +19,7 @@ pgls_query = { workspace = true } pgls_query_ext = { workspace = true } pgls_schema_cache = { workspace = true } pgls_text_size = { workspace = true } +rustc-hash = { workspace = true } serde = { workspace = true } [dev-dependencies] diff --git a/crates/pgls_analyser/src/lib.rs b/crates/pgls_analyser/src/lib.rs index fa1d407ba..038a53239 100644 --- a/crates/pgls_analyser/src/lib.rs +++ b/crates/pgls_analyser/src/lib.rs @@ -1,21 +1,41 @@ use std::{ops::Deref, sync::LazyLock}; -use pgls_analyse::{ - AnalysedFileContext, AnalyserOptions, AnalysisFilter, MetadataRegistry, RegistryRuleParams, - RuleDiagnostic, RuleRegistry, -}; +use pgls_analyse::{AnalysisFilter, MetadataRegistry}; pub use registry::visit_registry; mod lint; +mod linter_context; +mod linter_options; +mod linter_registry; +mod linter_rule; pub mod options; mod registry; +// Re-export linter-specific types +pub use linter_context::{AnalysedFileContext, LinterRuleContext}; +pub use linter_options::{LinterOptions, LinterRules, RuleOptions}; +pub use linter_registry::{ + LinterRegistryRuleParams, LinterRuleRegistry, LinterRuleRegistryBuilder, +}; +pub use linter_rule::{LinterDiagnostic, LinterRule}; + +// For convenience in macros and rule files - keep these shorter names +pub use LinterDiagnostic as RuleDiagnostic; +pub use LinterRule as Rule; +pub use LinterRuleContext as RuleContext; + pub static METADATA: LazyLock = LazyLock::new(|| { let mut metadata = MetadataRegistry::default(); - visit_registry(&mut metadata); + // Use a separate visitor for metadata that implements pgls_analyse::RegistryVisitor + visit_metadata_registry(&mut metadata); metadata }); +// Separate function for visiting metadata registry (uses pgls_analyse::RegistryVisitor) +fn visit_metadata_registry(registry: &mut V) { + registry.record_category::(); +} + /// Main entry point to the analyser. pub struct Analyser<'a> { /// Holds the metadata for all the rules statically known to the analyser @@ -24,10 +44,10 @@ pub struct Analyser<'a> { metadata: &'a MetadataRegistry, /// Holds all rule options - options: &'a AnalyserOptions, + options: &'a LinterOptions, /// Holds all rules - registry: RuleRegistry, + registry: LinterRuleRegistry, } #[derive(Debug)] @@ -42,13 +62,13 @@ pub struct AnalyserParams<'a> { } pub struct AnalyserConfig<'a> { - pub options: &'a AnalyserOptions, + pub options: &'a LinterOptions, pub filter: AnalysisFilter<'a>, } impl<'a> Analyser<'a> { pub fn new(conf: AnalyserConfig<'a>) -> Self { - let mut builder = RuleRegistry::builder(&conf.filter); + let mut builder = LinterRuleRegistry::builder(&conf.filter); visit_registry(&mut builder); let registry = builder.build(); @@ -59,7 +79,7 @@ impl<'a> Analyser<'a> { } } - pub fn run(&self, params: AnalyserParams) -> Vec { + pub fn run(&self, params: AnalyserParams) -> Vec { let mut diagnostics = vec![]; let roots: Vec = @@ -68,7 +88,7 @@ impl<'a> Analyser<'a> { for (i, stmt) in params.stmts.into_iter().enumerate() { let stmt_diagnostics: Vec<_> = { - let rule_params = RegistryRuleParams { + let rule_params = LinterRegistryRuleParams { root: &roots[i], options: self.options, analysed_file_context: &file_context, @@ -131,7 +151,7 @@ mod tests { let ast = pgls_query::parse(SQL).expect("failed to parse SQL"); let range = TextRange::new(0.into(), u32::try_from(SQL.len()).unwrap().into()); - let options = AnalyserOptions::default(); + let options = LinterOptions::default(); let analyser = Analyser::new(crate::AnalyserConfig { options: &options, diff --git a/crates/pgls_analyser/src/lint/safety/add_serial_column.rs b/crates/pgls_analyser/src/lint/safety/add_serial_column.rs index 0c53c2eb0..c249235fa 100644 --- a/crates/pgls_analyser/src/lint/safety/add_serial_column.rs +++ b/crates/pgls_analyser/src/lint/safety/add_serial_column.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/adding_field_with_default.rs b/crates/pgls_analyser/src/lint/safety/adding_field_with_default.rs index 604f60dab..e9b55cc8b 100644 --- a/crates/pgls_analyser/src/lint/safety/adding_field_with_default.rs +++ b/crates/pgls_analyser/src/lint/safety/adding_field_with_default.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/adding_foreign_key_constraint.rs b/crates/pgls_analyser/src/lint/safety/adding_foreign_key_constraint.rs index 58059b910..5c707339c 100644 --- a/crates/pgls_analyser/src/lint/safety/adding_foreign_key_constraint.rs +++ b/crates/pgls_analyser/src/lint/safety/adding_foreign_key_constraint.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/adding_not_null_field.rs b/crates/pgls_analyser/src/lint/safety/adding_not_null_field.rs index 8b14d5227..bdbbc43b0 100644 --- a/crates/pgls_analyser/src/lint/safety/adding_not_null_field.rs +++ b/crates/pgls_analyser/src/lint/safety/adding_not_null_field.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/adding_primary_key_constraint.rs b/crates/pgls_analyser/src/lint/safety/adding_primary_key_constraint.rs index 129812035..c38225271 100644 --- a/crates/pgls_analyser/src/lint/safety/adding_primary_key_constraint.rs +++ b/crates/pgls_analyser/src/lint/safety/adding_primary_key_constraint.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/adding_required_field.rs b/crates/pgls_analyser/src/lint/safety/adding_required_field.rs index c39597d53..22a978d4d 100644 --- a/crates/pgls_analyser/src/lint/safety/adding_required_field.rs +++ b/crates/pgls_analyser/src/lint/safety/adding_required_field.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/ban_char_field.rs b/crates/pgls_analyser/src/lint/safety/ban_char_field.rs index 39b1cdbe6..f9c948129 100644 --- a/crates/pgls_analyser/src/lint/safety/ban_char_field.rs +++ b/crates/pgls_analyser/src/lint/safety/ban_char_field.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/ban_concurrent_index_creation_in_transaction.rs b/crates/pgls_analyser/src/lint/safety/ban_concurrent_index_creation_in_transaction.rs index 2de0563d1..dcc4af04e 100644 --- a/crates/pgls_analyser/src/lint/safety/ban_concurrent_index_creation_in_transaction.rs +++ b/crates/pgls_analyser/src/lint/safety/ban_concurrent_index_creation_in_transaction.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/ban_drop_column.rs b/crates/pgls_analyser/src/lint/safety/ban_drop_column.rs index 868782a98..f499d7afb 100644 --- a/crates/pgls_analyser/src/lint/safety/ban_drop_column.rs +++ b/crates/pgls_analyser/src/lint/safety/ban_drop_column.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/ban_drop_database.rs b/crates/pgls_analyser/src/lint/safety/ban_drop_database.rs index 837cb8e77..8448eb26a 100644 --- a/crates/pgls_analyser/src/lint/safety/ban_drop_database.rs +++ b/crates/pgls_analyser/src/lint/safety/ban_drop_database.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/ban_drop_not_null.rs b/crates/pgls_analyser/src/lint/safety/ban_drop_not_null.rs index ba4eb09c2..a9ed90ea8 100644 --- a/crates/pgls_analyser/src/lint/safety/ban_drop_not_null.rs +++ b/crates/pgls_analyser/src/lint/safety/ban_drop_not_null.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/ban_drop_table.rs b/crates/pgls_analyser/src/lint/safety/ban_drop_table.rs index f98bc6d26..77265d24e 100644 --- a/crates/pgls_analyser/src/lint/safety/ban_drop_table.rs +++ b/crates/pgls_analyser/src/lint/safety/ban_drop_table.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/ban_truncate_cascade.rs b/crates/pgls_analyser/src/lint/safety/ban_truncate_cascade.rs index 892086a5f..e2bd9009b 100644 --- a/crates/pgls_analyser/src/lint/safety/ban_truncate_cascade.rs +++ b/crates/pgls_analyser/src/lint/safety/ban_truncate_cascade.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; use pgls_query::protobuf::DropBehavior; diff --git a/crates/pgls_analyser/src/lint/safety/changing_column_type.rs b/crates/pgls_analyser/src/lint/safety/changing_column_type.rs index 9e7cc9e75..f093b4242 100644 --- a/crates/pgls_analyser/src/lint/safety/changing_column_type.rs +++ b/crates/pgls_analyser/src/lint/safety/changing_column_type.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/constraint_missing_not_valid.rs b/crates/pgls_analyser/src/lint/safety/constraint_missing_not_valid.rs index 5599317ec..1e27fc06e 100644 --- a/crates/pgls_analyser/src/lint/safety/constraint_missing_not_valid.rs +++ b/crates/pgls_analyser/src/lint/safety/constraint_missing_not_valid.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/creating_enum.rs b/crates/pgls_analyser/src/lint/safety/creating_enum.rs index fa7eb61d7..e6b6ccf8a 100644 --- a/crates/pgls_analyser/src/lint/safety/creating_enum.rs +++ b/crates/pgls_analyser/src/lint/safety/creating_enum.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/disallow_unique_constraint.rs b/crates/pgls_analyser/src/lint/safety/disallow_unique_constraint.rs index 5fdf710c8..c7a081133 100644 --- a/crates/pgls_analyser/src/lint/safety/disallow_unique_constraint.rs +++ b/crates/pgls_analyser/src/lint/safety/disallow_unique_constraint.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/lock_timeout_warning.rs b/crates/pgls_analyser/src/lint/safety/lock_timeout_warning.rs index f3c5af62e..7660b9db4 100644 --- a/crates/pgls_analyser/src/lint/safety/lock_timeout_warning.rs +++ b/crates/pgls_analyser/src/lint/safety/lock_timeout_warning.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/multiple_alter_table.rs b/crates/pgls_analyser/src/lint/safety/multiple_alter_table.rs index 907a29b00..b7b7178b1 100644 --- a/crates/pgls_analyser/src/lint/safety/multiple_alter_table.rs +++ b/crates/pgls_analyser/src/lint/safety/multiple_alter_table.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/prefer_big_int.rs b/crates/pgls_analyser/src/lint/safety/prefer_big_int.rs index c89a08b5a..5aae8df47 100644 --- a/crates/pgls_analyser/src/lint/safety/prefer_big_int.rs +++ b/crates/pgls_analyser/src/lint/safety/prefer_big_int.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/prefer_bigint_over_int.rs b/crates/pgls_analyser/src/lint/safety/prefer_bigint_over_int.rs index 71db41684..58d0deff3 100644 --- a/crates/pgls_analyser/src/lint/safety/prefer_bigint_over_int.rs +++ b/crates/pgls_analyser/src/lint/safety/prefer_bigint_over_int.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/prefer_bigint_over_smallint.rs b/crates/pgls_analyser/src/lint/safety/prefer_bigint_over_smallint.rs index 06e92bb11..066923b02 100644 --- a/crates/pgls_analyser/src/lint/safety/prefer_bigint_over_smallint.rs +++ b/crates/pgls_analyser/src/lint/safety/prefer_bigint_over_smallint.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/prefer_identity.rs b/crates/pgls_analyser/src/lint/safety/prefer_identity.rs index ddc378b5c..bc82b1967 100644 --- a/crates/pgls_analyser/src/lint/safety/prefer_identity.rs +++ b/crates/pgls_analyser/src/lint/safety/prefer_identity.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/prefer_jsonb.rs b/crates/pgls_analyser/src/lint/safety/prefer_jsonb.rs index 637d32f01..16f51f9ff 100644 --- a/crates/pgls_analyser/src/lint/safety/prefer_jsonb.rs +++ b/crates/pgls_analyser/src/lint/safety/prefer_jsonb.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/prefer_robust_stmts.rs b/crates/pgls_analyser/src/lint/safety/prefer_robust_stmts.rs index 021c08473..0451db031 100644 --- a/crates/pgls_analyser/src/lint/safety/prefer_robust_stmts.rs +++ b/crates/pgls_analyser/src/lint/safety/prefer_robust_stmts.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/prefer_text_field.rs b/crates/pgls_analyser/src/lint/safety/prefer_text_field.rs index ce03c9958..847ad43d5 100644 --- a/crates/pgls_analyser/src/lint/safety/prefer_text_field.rs +++ b/crates/pgls_analyser/src/lint/safety/prefer_text_field.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/prefer_timestamptz.rs b/crates/pgls_analyser/src/lint/safety/prefer_timestamptz.rs index 7dc876783..8fd048e01 100644 --- a/crates/pgls_analyser/src/lint/safety/prefer_timestamptz.rs +++ b/crates/pgls_analyser/src/lint/safety/prefer_timestamptz.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/renaming_column.rs b/crates/pgls_analyser/src/lint/safety/renaming_column.rs index a7d151480..a54fbb51a 100644 --- a/crates/pgls_analyser/src/lint/safety/renaming_column.rs +++ b/crates/pgls_analyser/src/lint/safety/renaming_column.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/renaming_table.rs b/crates/pgls_analyser/src/lint/safety/renaming_table.rs index 6c3465da8..2cc1046fc 100644 --- a/crates/pgls_analyser/src/lint/safety/renaming_table.rs +++ b/crates/pgls_analyser/src/lint/safety/renaming_table.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/require_concurrent_index_creation.rs b/crates/pgls_analyser/src/lint/safety/require_concurrent_index_creation.rs index e2289ea8a..257536d3a 100644 --- a/crates/pgls_analyser/src/lint/safety/require_concurrent_index_creation.rs +++ b/crates/pgls_analyser/src/lint/safety/require_concurrent_index_creation.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; @@ -74,10 +75,7 @@ impl Rule for RequireConcurrentIndexCreation { } } -fn is_table_created_in_file( - file_context: &pgls_analyse::AnalysedFileContext, - table_name: &str, -) -> bool { +fn is_table_created_in_file(file_context: &crate::AnalysedFileContext, table_name: &str) -> bool { // Check all statements in the file to see if this table was created for stmt in file_context.stmts { if let pgls_query::NodeEnum::CreateStmt(create_stmt) = stmt { diff --git a/crates/pgls_analyser/src/lint/safety/require_concurrent_index_deletion.rs b/crates/pgls_analyser/src/lint/safety/require_concurrent_index_deletion.rs index cac317170..7117908c4 100644 --- a/crates/pgls_analyser/src/lint/safety/require_concurrent_index_deletion.rs +++ b/crates/pgls_analyser/src/lint/safety/require_concurrent_index_deletion.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/running_statement_while_holding_access_exclusive.rs b/crates/pgls_analyser/src/lint/safety/running_statement_while_holding_access_exclusive.rs index 7d18e7b16..f481f76a9 100644 --- a/crates/pgls_analyser/src/lint/safety/running_statement_while_holding_access_exclusive.rs +++ b/crates/pgls_analyser/src/lint/safety/running_statement_while_holding_access_exclusive.rs @@ -1,4 +1,5 @@ -use pgls_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule}; +use crate::{Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyser/src/lint/safety/transaction_nesting.rs b/crates/pgls_analyser/src/lint/safety/transaction_nesting.rs index 4f76c99db..d7205c945 100644 --- a/crates/pgls_analyser/src/lint/safety/transaction_nesting.rs +++ b/crates/pgls_analyser/src/lint/safety/transaction_nesting.rs @@ -1,6 +1,5 @@ -use pgls_analyse::{ - AnalysedFileContext, Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule, -}; +use crate::{AnalysedFileContext, Rule, RuleContext, RuleDiagnostic}; +use pgls_analyse::{RuleSource, declare_lint_rule}; use pgls_console::markup; use pgls_diagnostics::Severity; diff --git a/crates/pgls_analyse/src/analysed_file_context.rs b/crates/pgls_analyser/src/linter_context.rs similarity index 66% rename from crates/pgls_analyse/src/analysed_file_context.rs rename to crates/pgls_analyser/src/linter_context.rs index 5907d70e6..d445efb41 100644 --- a/crates/pgls_analyse/src/analysed_file_context.rs +++ b/crates/pgls_analyser/src/linter_context.rs @@ -1,3 +1,97 @@ +use pgls_analyse::{GroupCategory, RuleCategory, RuleGroup, RuleMetadata}; +use pgls_schema_cache::SchemaCache; + +use crate::linter_rule::LinterRule; + +pub struct LinterRuleContext<'a, R: LinterRule> { + stmt: &'a pgls_query::NodeEnum, + options: &'a R::Options, + schema_cache: Option<&'a SchemaCache>, + file_context: &'a AnalysedFileContext<'a>, +} + +impl<'a, R> LinterRuleContext<'a, R> +where + R: LinterRule + Sized + 'static, +{ + #[allow(clippy::too_many_arguments)] + pub fn new( + stmt: &'a pgls_query::NodeEnum, + options: &'a R::Options, + schema_cache: Option<&'a SchemaCache>, + file_context: &'a AnalysedFileContext, + ) -> Self { + Self { + stmt, + options, + schema_cache, + file_context, + } + } + + /// Returns the group that belongs to the current rule + pub fn group(&self) -> &'static str { + ::NAME + } + + /// Returns the category that belongs to the current rule + pub fn category(&self) -> RuleCategory { + <::Category as GroupCategory>::CATEGORY + } + + /// Returns the AST root + pub fn stmt(&self) -> &pgls_query::NodeEnum { + self.stmt + } + + pub fn file_context(&self) -> &AnalysedFileContext { + self.file_context + } + + pub fn schema_cache(&self) -> Option<&SchemaCache> { + self.schema_cache + } + + /// Returns the metadata of the rule + /// + /// The metadata contains information about the rule, such as the name, version, language, and whether it is recommended. + /// + /// ## Examples + /// ```rust,ignore + /// declare_lint_rule! { + /// /// Some doc + /// pub(crate) Foo { + /// version: "0.0.0", + /// name: "foo", + /// recommended: true, + /// } + /// } + /// + /// impl LinterRule for Foo { + /// const CATEGORY: RuleCategory = RuleCategory::Lint; + /// type State = (); + /// type Signals = (); + /// type Options = (); + /// + /// fn run(ctx: &LinterRuleContext) -> Self::Signals { + /// assert_eq!(ctx.metadata().name, "foo"); + /// } + /// } + /// ``` + pub fn metadata(&self) -> &RuleMetadata { + &R::METADATA + } + + /// It retrieves the options that belong to a rule, if they exist. + /// + /// In order to retrieve a typed data structure, you have to create a deserializable + /// data structure and define it inside the generic type `type Options` of the [LinterRule] + /// + pub fn options(&self) -> &R::Options { + self.options + } +} + pub struct AnalysedFileContext<'a> { pub stmts: &'a Vec, pos: usize, diff --git a/crates/pgls_analyse/src/options.rs b/crates/pgls_analyser/src/linter_options.rs similarity index 83% rename from crates/pgls_analyse/src/options.rs rename to crates/pgls_analyser/src/linter_options.rs index e46b7104f..65460236c 100644 --- a/crates/pgls_analyse/src/options.rs +++ b/crates/pgls_analyser/src/linter_options.rs @@ -1,9 +1,10 @@ +use pgls_analyse::RuleKey; use rustc_hash::FxHashMap; - -use crate::{Rule, RuleKey}; use std::any::{Any, TypeId}; use std::fmt::Debug; +use crate::linter_rule::LinterRule; + /// A convenient new type data structure to store the options that belong to a rule #[derive(Debug)] pub struct RuleOptions(TypeId, Box); @@ -28,9 +29,9 @@ impl RuleOptions { /// A convenient new type data structure to insert and get rules #[derive(Debug, Default)] -pub struct AnalyserRules(FxHashMap); +pub struct LinterRules(FxHashMap); -impl AnalyserRules { +impl LinterRules { /// It tracks the options of a specific rule pub fn push_rule(&mut self, rule_key: RuleKey, options: RuleOptions) { self.0.insert(rule_key, options); @@ -42,17 +43,17 @@ impl AnalyserRules { } } -/// A set of information useful to the analyser infrastructure +/// A set of information useful to the linter infrastructure #[derive(Debug, Default)] -pub struct AnalyserOptions { +pub struct LinterOptions { /// A data structured derived from the [`postgres-language-server.jsonc`] file - pub rules: AnalyserRules, + pub rules: LinterRules, } -impl AnalyserOptions { +impl LinterOptions { pub fn rule_options(&self) -> Option where - R: Rule + 'static, + R: LinterRule + 'static, { self.rules .get_rule_options::(&RuleKey::rule::()) diff --git a/crates/pgls_analyser/src/linter_registry.rs b/crates/pgls_analyser/src/linter_registry.rs new file mode 100644 index 000000000..5840d069f --- /dev/null +++ b/crates/pgls_analyser/src/linter_registry.rs @@ -0,0 +1,118 @@ +use pgls_analyse::{AnalysisFilter, GroupCategory, RuleGroup, RuleKey}; + +use crate::linter_context::{AnalysedFileContext, LinterRuleContext}; +use crate::linter_options::LinterOptions; +use crate::linter_rule::{LinterDiagnostic, LinterRule}; + +pub struct LinterRuleRegistryBuilder<'a> { + filter: &'a AnalysisFilter<'a>, + // Store rule keys discovered during traversal + rule_keys: Vec, +} + +// Implement RegistryVisitor - only needs RuleMeta! +impl pgls_analyse::RegistryVisitor for LinterRuleRegistryBuilder<'_> { + fn record_category(&mut self) { + if self.filter.match_category::() { + C::record_groups(self); + } + } + + fn record_group(&mut self) { + if self.filter.match_group::() { + G::record_rules(self); + } + } + + fn record_rule(&mut self) + where + R: pgls_analyse::RuleMeta + 'static, + { + // Visitor just collects which rules match the filter + // No executor creation happens here - that's AST-specific + if self.filter.match_rule::() { + self.rule_keys.push(RuleKey::rule::()); + } + } +} + +/// The rule registry holds type-erased instances of all active analysis rules +pub struct LinterRuleRegistry { + pub rules: Vec, +} + +impl IntoIterator for LinterRuleRegistry { + type Item = RegistryLinterRule; + type IntoIter = std::vec::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.rules.into_iter() + } +} + +/// Internal representation of a single rule in the registry +#[derive(Copy, Clone)] +pub struct RegistryLinterRule { + pub run: LinterRuleExecutor, +} + +impl LinterRuleRegistry { + pub fn builder<'a>(filter: &'a AnalysisFilter<'a>) -> LinterRuleRegistryBuilder<'a> { + LinterRuleRegistryBuilder { + filter, + rule_keys: Vec::new(), + } + } +} + +pub struct LinterRegistryRuleParams<'a> { + pub root: &'a pgls_query::NodeEnum, + pub options: &'a LinterOptions, + pub analysed_file_context: &'a AnalysedFileContext<'a>, + pub schema_cache: Option<&'a pgls_schema_cache::SchemaCache>, +} + +/// Executor for rule as a generic function pointer +type LinterRuleExecutor = fn(&LinterRegistryRuleParams) -> Vec; + +impl RegistryLinterRule { + pub fn new() -> Self + where + R: LinterRule + 'static, + { + /// Generic implementation of LinterRuleExecutor for any rule type R + fn run(params: &LinterRegistryRuleParams) -> Vec + where + R: LinterRule + 'static, + { + let options = params.options.rule_options::().unwrap_or_default(); + + let ctx = LinterRuleContext::new( + params.root, + &options, + params.schema_cache, + params.analysed_file_context, + ); + + R::run(&ctx) + } + + Self { run: run:: } + } +} + +impl LinterRuleRegistryBuilder<'_> { + pub fn build(self) -> LinterRuleRegistry { + // Look up factory for each collected rule key and create executors + let rules = self + .rule_keys + .into_iter() + .filter_map(|key| { + // This function will be generated by codegen + crate::registry::get_linter_rule_factory(&key).map(|factory| factory()) + }) + .collect(); + + LinterRuleRegistry { rules } + } +} diff --git a/crates/pgls_analyse/src/rule.rs b/crates/pgls_analyser/src/linter_rule.rs similarity index 50% rename from crates/pgls_analyse/src/rule.rs rename to crates/pgls_analyser/src/linter_rule.rs index 42a08cda3..d9fc8d006 100644 --- a/crates/pgls_analyse/src/rule.rs +++ b/crates/pgls_analyser/src/linter_rule.rs @@ -1,114 +1,28 @@ +use pgls_analyse::RuleMeta; use pgls_console::fmt::Display; use pgls_console::{MarkupBuf, markup}; use pgls_diagnostics::advice::CodeSuggestionAdvice; use pgls_diagnostics::{ Advices, Category, Diagnostic, DiagnosticTags, Location, LogCategory, MessageAndDescription, - Severity, Visit, + Visit, }; use pgls_text_size::TextRange; -use std::cmp::Ordering; use std::fmt::Debug; -use crate::{categories::RuleCategory, context::RuleContext, registry::RegistryVisitor}; +use crate::linter_context::LinterRuleContext; -#[derive(Clone, Debug)] -#[cfg_attr( - feature = "serde", - derive(serde::Serialize), - serde(rename_all = "camelCase") -)] -/// Static metadata containing information about a rule -pub struct RuleMetadata { - /// It marks if a rule is deprecated, and if so a reason has to be provided. - pub deprecated: Option<&'static str>, - /// The version when the rule was implemented - pub version: &'static str, - /// The name of this rule, displayed in the diagnostics it emits - pub name: &'static str, - /// The content of the documentation comments for this rule - pub docs: &'static str, - /// Whether a rule is recommended or not - pub recommended: bool, - /// The source URL of the rule - pub sources: &'static [RuleSource], - /// The default severity of the rule - pub severity: Severity, -} - -impl RuleMetadata { - pub const fn new( - version: &'static str, - name: &'static str, - docs: &'static str, - severity: Severity, - ) -> Self { - Self { - deprecated: None, - version, - name, - docs, - sources: &[], - recommended: false, - severity, - } - } - - pub const fn recommended(mut self, recommended: bool) -> Self { - self.recommended = recommended; - self - } - - pub const fn deprecated(mut self, deprecated: &'static str) -> Self { - self.deprecated = Some(deprecated); - self - } - - pub const fn sources(mut self, sources: &'static [RuleSource]) -> Self { - self.sources = sources; - self - } -} - -pub trait RuleMeta { - type Group: RuleGroup; - const METADATA: RuleMetadata; -} - -/// A rule group is a collection of rules under a given name, serving as a -/// "namespace" for lint rules and allowing the entire set of rules to be -/// disabled at once -pub trait RuleGroup { - type Category: GroupCategory; - /// The name of this group, displayed in the diagnostics emitted by its rules - const NAME: &'static str; - /// Register all the rules belonging to this group into `registry` - fn record_rules(registry: &mut V); -} - -/// A group category is a collection of rule groups under a given category ID, -/// serving as a broad classification on the kind of diagnostic or code action -/// these rule emit, and allowing whole categories of rules to be disabled at -/// once depending on the kind of analysis being performed -pub trait GroupCategory { - /// The category ID used for all groups and rule belonging to this category - const CATEGORY: RuleCategory; - /// Register all the groups belonging to this category into `registry` - fn record_groups(registry: &mut V); -} - -/// Trait implemented by all analysis rules: declares interest to a certain AstNode type, -/// and a callback function to be executed on all nodes matching the query to possibly -/// raise an analysis event -pub trait Rule: RuleMeta + Sized { +/// Trait implemented by all AST-based linter rules +pub trait LinterRule: RuleMeta + Sized { type Options: Default + Clone + Debug; + /// Execute the rule on the given AST context /// `schema_cache` will only be available if the user has a working database connection. - fn run(rule_context: &RuleContext) -> Vec; + fn run(rule_context: &LinterRuleContext) -> Vec; } -/// Diagnostic object returned by a single analysis rule +/// Diagnostic object returned by a single linter rule #[derive(Debug, Diagnostic, PartialEq)] -pub struct RuleDiagnostic { +pub struct LinterDiagnostic { #[category] pub(crate) category: &'static Category, #[location(span)] @@ -180,8 +94,8 @@ pub struct Detail { pub range: Option, } -impl RuleDiagnostic { - /// Creates a new [`RuleDiagnostic`] with a severity and title that will be +impl LinterDiagnostic { + /// Creates a new [`LinterDiagnostic`] with a severity and title that will be /// used in a builder-like way to modify labels. pub fn new(category: &'static Category, span: Option, title: impl Display) -> Self { let message = markup!({ title }).to_owned(); @@ -224,9 +138,9 @@ impl RuleDiagnostic { self } - /// Attaches a label to this [`RuleDiagnostic`]. + /// Attaches a label to this [`LinterDiagnostic`]. /// - /// The given span has to be in the file that was provided while creating this [`RuleDiagnostic`]. + /// The given span has to be in the file that was provided while creating this [`LinterDiagnostic`]. pub fn label(mut self, span: Option, msg: impl Display) -> Self { self.rule_advice.details.push(Detail { log_category: LogCategory::Info, @@ -236,12 +150,12 @@ impl RuleDiagnostic { self } - /// Attaches a detailed message to this [`RuleDiagnostic`]. + /// Attaches a detailed message to this [`LinterDiagnostic`]. pub fn detail(self, span: Option, msg: impl Display) -> Self { self.label(span, msg) } - /// Adds a footer to this [`RuleDiagnostic`], which will be displayed under the actual error. + /// Adds a footer to this [`LinterDiagnostic`], which will be displayed under the actual error. fn footer(mut self, log_category: LogCategory, msg: impl Display) -> Self { self.rule_advice .notes @@ -249,7 +163,7 @@ impl RuleDiagnostic { self } - /// Adds a footer to this [`RuleDiagnostic`], with the `Info` log category. + /// Adds a footer to this [`LinterDiagnostic`], with the `Info` log category. pub fn note(self, msg: impl Display) -> Self { self.footer(LogCategory::Info, msg) } @@ -270,7 +184,7 @@ impl RuleDiagnostic { self } - /// Adds a footer to this [`RuleDiagnostic`], with the `Warn` severity. + /// Adds a footer to this [`LinterDiagnostic`], with the `Warn` severity. pub fn warning(self, msg: impl Display) -> Self { self.footer(LogCategory::Warn, msg) } @@ -284,71 +198,3 @@ impl RuleDiagnostic { self.category.name() } } - -#[derive(Debug, Clone, Eq)] -#[cfg_attr(feature = "serde", derive(serde::Serialize))] -#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] -pub enum RuleSource { - /// Rules from [Squawk](https://squawkhq.com) - Squawk(&'static str), - /// Rules from [Eugene](https://github.com/kaaveland/eugene) - Eugene(&'static str), -} - -impl PartialEq for RuleSource { - fn eq(&self, other: &Self) -> bool { - std::mem::discriminant(self) == std::mem::discriminant(other) - } -} - -impl std::fmt::Display for RuleSource { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Squawk(_) => write!(f, "Squawk"), - Self::Eugene(_) => write!(f, "Eugene"), - } - } -} - -impl PartialOrd for RuleSource { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for RuleSource { - fn cmp(&self, other: &Self) -> Ordering { - let self_rule = self.as_rule_name(); - let other_rule = other.as_rule_name(); - self_rule.cmp(other_rule) - } -} - -impl RuleSource { - pub fn as_rule_name(&self) -> &'static str { - match self { - Self::Squawk(rule_name) => rule_name, - Self::Eugene(rule_name) => rule_name, - } - } - - pub fn to_namespaced_rule_name(&self) -> String { - match self { - Self::Squawk(rule_name) => format!("squawk/{rule_name}"), - Self::Eugene(rule_name) => format!("eugene/{rule_name}"), - } - } - - pub fn to_rule_url(&self) -> String { - match self { - Self::Squawk(rule_name) => format!("https://squawkhq.com/docs/{rule_name}"), - Self::Eugene(rule_name) => { - format!("https://kaveland.no/eugene/hints/{rule_name}/index.html") - } - } - } - - pub fn as_url_and_rule_name(&self) -> (String, &'static str) { - (self.to_rule_url(), self.as_rule_name()) - } -} diff --git a/crates/pgls_analyser/src/options.rs b/crates/pgls_analyser/src/options.rs index 0cd484bf2..b04539c45 100644 --- a/crates/pgls_analyser/src/options.rs +++ b/crates/pgls_analyser/src/options.rs @@ -2,53 +2,54 @@ use crate::lint; pub type AddSerialColumn = - ::Options; -pub type AddingFieldWithDefault = < lint :: safety :: adding_field_with_default :: AddingFieldWithDefault as pgls_analyse :: Rule > :: Options ; -pub type AddingForeignKeyConstraint = < lint :: safety :: adding_foreign_key_constraint :: AddingForeignKeyConstraint as pgls_analyse :: Rule > :: Options ; + ::Options; +pub type AddingFieldWithDefault = + ::Options; +pub type AddingForeignKeyConstraint = < lint :: safety :: adding_foreign_key_constraint :: AddingForeignKeyConstraint as crate::LinterRule > :: Options ; pub type AddingNotNullField = - ::Options; -pub type AddingPrimaryKeyConstraint = < lint :: safety :: adding_primary_key_constraint :: AddingPrimaryKeyConstraint as pgls_analyse :: Rule > :: Options ; + ::Options; +pub type AddingPrimaryKeyConstraint = < lint :: safety :: adding_primary_key_constraint :: AddingPrimaryKeyConstraint as crate::LinterRule > :: Options ; pub type AddingRequiredField = - ::Options; -pub type BanCharField = ::Options; -pub type BanConcurrentIndexCreationInTransaction = < lint :: safety :: ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction as pgls_analyse :: Rule > :: Options ; + ::Options; +pub type BanCharField = ::Options; +pub type BanConcurrentIndexCreationInTransaction = < lint :: safety :: ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction as crate::LinterRule > :: Options ; pub type BanDropColumn = - ::Options; + ::Options; pub type BanDropDatabase = - ::Options; + ::Options; pub type BanDropNotNull = - ::Options; -pub type BanDropTable = ::Options; + ::Options; +pub type BanDropTable = ::Options; pub type BanTruncateCascade = - ::Options; + ::Options; pub type ChangingColumnType = - ::Options; -pub type ConstraintMissingNotValid = < lint :: safety :: constraint_missing_not_valid :: ConstraintMissingNotValid as pgls_analyse :: Rule > :: Options ; -pub type CreatingEnum = ::Options; -pub type DisallowUniqueConstraint = < lint :: safety :: disallow_unique_constraint :: DisallowUniqueConstraint as pgls_analyse :: Rule > :: Options ; + ::Options; +pub type ConstraintMissingNotValid = < lint :: safety :: constraint_missing_not_valid :: ConstraintMissingNotValid as crate::LinterRule > :: Options ; +pub type CreatingEnum = ::Options; +pub type DisallowUniqueConstraint = < lint :: safety :: disallow_unique_constraint :: DisallowUniqueConstraint as crate::LinterRule > :: Options ; pub type LockTimeoutWarning = - ::Options; + ::Options; pub type MultipleAlterTable = - ::Options; -pub type PreferBigInt = ::Options; + ::Options; +pub type PreferBigInt = ::Options; pub type PreferBigintOverInt = - ::Options; -pub type PreferBigintOverSmallint = < lint :: safety :: prefer_bigint_over_smallint :: PreferBigintOverSmallint as pgls_analyse :: Rule > :: Options ; + ::Options; +pub type PreferBigintOverSmallint = < lint :: safety :: prefer_bigint_over_smallint :: PreferBigintOverSmallint as crate::LinterRule > :: Options ; pub type PreferIdentity = - ::Options; -pub type PreferJsonb = ::Options; + ::Options; +pub type PreferJsonb = ::Options; pub type PreferRobustStmts = - ::Options; + ::Options; pub type PreferTextField = - ::Options; + ::Options; pub type PreferTimestamptz = - ::Options; + ::Options; pub type RenamingColumn = - ::Options; + ::Options; pub type RenamingTable = - ::Options; -pub type RequireConcurrentIndexCreation = < lint :: safety :: require_concurrent_index_creation :: RequireConcurrentIndexCreation as pgls_analyse :: Rule > :: Options ; -pub type RequireConcurrentIndexDeletion = < lint :: safety :: require_concurrent_index_deletion :: RequireConcurrentIndexDeletion as pgls_analyse :: Rule > :: Options ; -pub type RunningStatementWhileHoldingAccessExclusive = < lint :: safety :: running_statement_while_holding_access_exclusive :: RunningStatementWhileHoldingAccessExclusive as pgls_analyse :: Rule > :: Options ; + ::Options; +pub type RequireConcurrentIndexCreation = < lint :: safety :: require_concurrent_index_creation :: RequireConcurrentIndexCreation as crate::LinterRule > :: Options ; +pub type RequireConcurrentIndexDeletion = < lint :: safety :: require_concurrent_index_deletion :: RequireConcurrentIndexDeletion as crate::LinterRule > :: Options ; +pub type RunningStatementWhileHoldingAccessExclusive = < lint :: safety :: running_statement_while_holding_access_exclusive :: RunningStatementWhileHoldingAccessExclusive as crate::LinterRule > :: Options ; pub type TransactionNesting = - ::Options; + ::Options; diff --git a/crates/pgls_analyser/src/registry.rs b/crates/pgls_analyser/src/registry.rs index 4978e40fd..29862d096 100644 --- a/crates/pgls_analyser/src/registry.rs +++ b/crates/pgls_analyser/src/registry.rs @@ -1,6 +1,171 @@ //! Generated file, do not edit by hand, see `xtask/codegen` -use pgls_analyse::RegistryVisitor; +use crate::linter_registry::RegistryLinterRule; +use pgls_analyse::{RegistryVisitor, RuleKey}; pub fn visit_registry(registry: &mut V) { registry.record_category::(); } +#[doc = r" Maps rule keys to factory functions that create rule executors"] +#[doc = r" This function is generated by codegen and includes all linter rules"] +pub fn get_linter_rule_factory(key: &RuleKey) -> Option RegistryLinterRule>> { + match key.rule_name() { + "addSerialColumn" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::add_serial_column::AddSerialColumn, + >() + })), + "addingFieldWithDefault" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::adding_field_with_default::AddingFieldWithDefault, + >() + })), + "addingForeignKeyConstraint" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::adding_foreign_key_constraint::AddingForeignKeyConstraint, + >() + })), + "addingNotNullField" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::adding_not_null_field::AddingNotNullField, + >() + })), + "addingPrimaryKeyConstraint" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::adding_primary_key_constraint::AddingPrimaryKeyConstraint, + >() + })), + "addingRequiredField" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::adding_required_field::AddingRequiredField, + >() + })), + "banCharField" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::ban_char_field::BanCharField, + >() + })), + "banConcurrentIndexCreationInTransaction" => Some(Box::new(|| { + crate :: linter_registry :: RegistryLinterRule :: new :: < crate::lint::safety::ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction > () + })), + "banDropColumn" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::ban_drop_column::BanDropColumn, + >() + })), + "banDropDatabase" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::ban_drop_database::BanDropDatabase, + >() + })), + "banDropNotNull" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::ban_drop_not_null::BanDropNotNull, + >() + })), + "banDropTable" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::ban_drop_table::BanDropTable, + >() + })), + "banTruncateCascade" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::ban_truncate_cascade::BanTruncateCascade, + >() + })), + "changingColumnType" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::changing_column_type::ChangingColumnType, + >() + })), + "constraintMissingNotValid" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::constraint_missing_not_valid::ConstraintMissingNotValid, + >() + })), + "creatingEnum" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::creating_enum::CreatingEnum, + >() + })), + "disallowUniqueConstraint" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::disallow_unique_constraint::DisallowUniqueConstraint, + >() + })), + "lockTimeoutWarning" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::lock_timeout_warning::LockTimeoutWarning, + >() + })), + "multipleAlterTable" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::multiple_alter_table::MultipleAlterTable, + >() + })), + "preferBigInt" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::prefer_big_int::PreferBigInt, + >() + })), + "preferBigintOverInt" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::prefer_bigint_over_int::PreferBigintOverInt, + >() + })), + "preferBigintOverSmallint" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::prefer_bigint_over_smallint::PreferBigintOverSmallint, + >() + })), + "preferIdentity" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::prefer_identity::PreferIdentity, + >() + })), + "preferJsonb" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::prefer_jsonb::PreferJsonb, + >() + })), + "preferRobustStmts" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::prefer_robust_stmts::PreferRobustStmts, + >() + })), + "preferTextField" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::prefer_text_field::PreferTextField, + >() + })), + "preferTimestamptz" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::prefer_timestamptz::PreferTimestamptz, + >() + })), + "renamingColumn" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::renaming_column::RenamingColumn, + >() + })), + "renamingTable" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::renaming_table::RenamingTable, + >() + })), + "requireConcurrentIndexCreation" => Some(Box::new(|| { + crate :: linter_registry :: RegistryLinterRule :: new :: < crate::lint::safety::require_concurrent_index_creation :: RequireConcurrentIndexCreation > () + })), + "requireConcurrentIndexDeletion" => Some(Box::new(|| { + crate :: linter_registry :: RegistryLinterRule :: new :: < crate::lint::safety::require_concurrent_index_deletion :: RequireConcurrentIndexDeletion > () + })), + "runningStatementWhileHoldingAccessExclusive" => Some(Box::new(|| { + crate :: linter_registry :: RegistryLinterRule :: new :: < crate::lint::safety::running_statement_while_holding_access_exclusive :: RunningStatementWhileHoldingAccessExclusive > () + })), + "transactionNesting" => Some(Box::new(|| { + crate::linter_registry::RegistryLinterRule::new::< + crate::lint::safety::transaction_nesting::TransactionNesting, + >() + })), + _ => None, + } +} diff --git a/crates/pgls_configuration/src/linter/rules.rs b/crates/pgls_configuration/src/linter/rules.rs index 5e27b92d1..32fc1a2fc 100644 --- a/crates/pgls_configuration/src/linter/rules.rs +++ b/crates/pgls_configuration/src/linter/rules.rs @@ -3,7 +3,8 @@ #![doc = r" Generated file, do not edit by hand, see `xtask/codegen`"] use crate::rules::{RuleConfiguration, RulePlainConfiguration}; use biome_deserialize_macros::Merge; -use pgls_analyse::{RuleFilter, options::RuleOptions}; +use pgls_analyse::RuleFilter; +use pgls_analyser::RuleOptions; use pgls_diagnostics::{Category, Severity}; use rustc_hash::FxHashSet; #[cfg(feature = "schema")] diff --git a/crates/pgls_configuration/src/rules/configuration.rs b/crates/pgls_configuration/src/rules/configuration.rs index a3e43f63c..ad93abeeb 100644 --- a/crates/pgls_configuration/src/rules/configuration.rs +++ b/crates/pgls_configuration/src/rules/configuration.rs @@ -1,6 +1,7 @@ use biome_deserialize::Merge; use biome_deserialize_macros::Deserializable; -use pgls_analyse::options::RuleOptions; +use pgls_analyse::RuleFilter; +use pgls_analyser::RuleOptions; use pgls_diagnostics::Severity; #[cfg(feature = "schema")] use schemars::JsonSchema; diff --git a/crates/pgls_splinter/src/convert.rs b/crates/pgls_splinter/src/convert.rs index 926de1d40..c5d0c7904 100644 --- a/crates/pgls_splinter/src/convert.rs +++ b/crates/pgls_splinter/src/convert.rs @@ -1,4 +1,4 @@ -use pgls_diagnostics::{Category, Severity, category}; +use pgls_diagnostics::{Category, DatabaseObjectOwned, Severity, category}; use serde_json::Value; use crate::{SplinterAdvices, SplinterDiagnostic, SplinterQueryResult}; @@ -22,6 +22,11 @@ impl From for SplinterDiagnostic { category: rule_name_to_category(&result.name, &group), message: result.detail.into(), severity, + db_object: object_name.as_ref().map(|name| DatabaseObjectOwned { + schema: schema.clone(), + name: name.clone(), + object_type: object_type.clone(), + }), advices: SplinterAdvices { description: result.description, schema, diff --git a/crates/pgls_splinter/src/diagnostics.rs b/crates/pgls_splinter/src/diagnostics.rs index 7ff6ca942..ac9e32d77 100644 --- a/crates/pgls_splinter/src/diagnostics.rs +++ b/crates/pgls_splinter/src/diagnostics.rs @@ -1,5 +1,6 @@ use pgls_diagnostics::{ - Advices, Category, Diagnostic, LogCategory, MessageAndDescription, Severity, Visit, + Advices, Category, DatabaseObjectOwned, Diagnostic, LogCategory, MessageAndDescription, + Severity, Visit, }; use serde_json::Value; use std::io; @@ -10,10 +11,9 @@ pub struct SplinterDiagnostic { #[category] pub category: &'static Category, - // TODO: add new location type for database objects - // This will map schema + object_name to source code location - // #[location(span)] - // pub span: Option, + #[location(database_object)] + pub db_object: Option, + #[message] #[description] pub message: MessageAndDescription, diff --git a/crates/pgls_workspace/src/configuration.rs b/crates/pgls_workspace/src/configuration.rs index 0804a1c84..ce5ce6e36 100644 --- a/crates/pgls_workspace/src/configuration.rs +++ b/crates/pgls_workspace/src/configuration.rs @@ -6,7 +6,7 @@ use std::{ }; use biome_deserialize::Merge; -use pgls_analyse::AnalyserRules; +use pgls_analyser::LinterRules; use pgls_configuration::{ ConfigurationDiagnostic, ConfigurationPathHint, ConfigurationPayload, PartialConfiguration, VERSION, diagnostics::CantLoadExtendFile, push_to_analyser_rules, @@ -216,8 +216,8 @@ pub fn create_config( } /// Returns the rules applied to a specific [Path], given the [Settings] -pub fn to_analyser_rules(settings: &Settings) -> AnalyserRules { - let mut analyser_rules = AnalyserRules::default(); +pub fn to_analyser_rules(settings: &Settings) -> LinterRules { + let mut analyser_rules = LinterRules::default(); if let Some(rules) = settings.linter.rules.as_ref() { push_to_analyser_rules(rules, pgls_analyser::METADATA.deref(), &mut analyser_rules); } diff --git a/crates/pgls_workspace/src/workspace/server.rs b/crates/pgls_workspace/src/workspace/server.rs index 2ff11ec5f..e5feed085 100644 --- a/crates/pgls_workspace/src/workspace/server.rs +++ b/crates/pgls_workspace/src/workspace/server.rs @@ -15,8 +15,9 @@ use document::{ }; use futures::{StreamExt, stream}; use pg_query::convert_to_positional_params; -use pgls_analyse::{AnalyserOptions, AnalysisFilter}; -use pgls_analyser::{Analyser, AnalyserConfig, AnalyserParams}; +use pgls_analyse::AnalysisFilter; +use pgls_analyser::{Analyser, AnalyserConfig, AnalyserParams, LinterOptions}; + use pgls_diagnostics::{ Diagnostic, DiagnosticExt, Error, Severity, serde::Diagnostic as SDiagnostic, }; @@ -601,7 +602,7 @@ impl Workspace for WorkspaceServer { .with_linter_rules(¶ms.only, ¶ms.skip) .finish(); - let options = AnalyserOptions { + let options = LinterOptions { rules: to_analyser_rules(settings), }; diff --git a/crates/pgls_workspace/src/workspace/server/analyser.rs b/crates/pgls_workspace/src/workspace/server/analyser.rs index b7382f2bb..d8de3bbb3 100644 --- a/crates/pgls_workspace/src/workspace/server/analyser.rs +++ b/crates/pgls_workspace/src/workspace/server/analyser.rs @@ -1,4 +1,4 @@ -use pgls_analyse::{GroupCategory, RegistryVisitor, Rule, RuleCategory, RuleFilter, RuleGroup}; +use pgls_analyse::{GroupCategory, RegistryVisitor, RuleCategory, RuleFilter, RuleGroup, RuleMeta}; use pgls_configuration::RuleSelector; use rustc_hash::FxHashSet; @@ -91,7 +91,7 @@ impl<'a, 'b> LintVisitor<'a, 'b> { fn push_rule(&mut self) where - R: Rule + 'static, + R: RuleMeta + 'static, { // Do not report unused suppression comment diagnostics if a single rule is run. for selector in self.only { @@ -132,7 +132,7 @@ impl RegistryVisitor for LintVisitor<'_, '_> { fn record_rule(&mut self) where - R: Rule + 'static, + R: RuleMeta + 'static, { self.push_rule::() } diff --git a/docs/codegen/src/rules_docs.rs b/docs/codegen/src/rules_docs.rs index eae0340e5..f68950315 100644 --- a/docs/codegen/src/rules_docs.rs +++ b/docs/codegen/src/rules_docs.rs @@ -1,7 +1,7 @@ use anyhow::{Result, bail}; use biome_string_case::Case; -use pgls_analyse::{AnalyserOptions, AnalysisFilter, RuleFilter, RuleMetadata}; -use pgls_analyser::{AnalysableStatement, Analyser, AnalyserConfig}; +use pgls_analyse::{AnalysisFilter, RuleFilter, RuleMetadata}; +use pgls_analyser::{AnalysableStatement, Analyser, AnalyserConfig, LinterOptions}; use pgls_console::StdDisplay; use pgls_diagnostics::{Diagnostic, DiagnosticExt, PrintDiagnostic}; use pgls_query_ext::diagnostics::SyntaxDiagnostic; @@ -431,7 +431,7 @@ fn print_diagnostics( ..AnalysisFilter::default() }; let settings = Settings::default(); - let options = AnalyserOptions::default(); + let options = LinterOptions::default(); let analyser = Analyser::new(AnalyserConfig { options: &options, filter, diff --git a/docs/codegen/src/utils.rs b/docs/codegen/src/utils.rs index 692ef9308..8d53cf909 100644 --- a/docs/codegen/src/utils.rs +++ b/docs/codegen/src/utils.rs @@ -1,4 +1,6 @@ -use pgls_analyse::{GroupCategory, RegistryVisitor, Rule, RuleCategory, RuleGroup, RuleMetadata}; +use pgls_analyse::{ + GroupCategory, RegistryVisitor, RuleCategory, RuleGroup, RuleMeta, RuleMetadata, +}; use regex::Regex; use std::collections::BTreeMap; @@ -32,7 +34,7 @@ pub(crate) struct LintRulesVisitor { impl LintRulesVisitor { fn push_rule(&mut self) where - R: Rule + 'static, + R: RuleMeta + 'static, { let group = self .groups @@ -52,7 +54,7 @@ impl RegistryVisitor for LintRulesVisitor { fn record_rule(&mut self) where - R: Rule + 'static, + R: RuleMeta + 'static, { self.push_rule::() } diff --git a/xtask/codegen/src/generate_analyser.rs b/xtask/codegen/src/generate_analyser.rs index 105ca7a82..fc1257d5c 100644 --- a/xtask/codegen/src/generate_analyser.rs +++ b/xtask/codegen/src/generate_analyser.rs @@ -15,11 +15,12 @@ pub fn generate_analyser() -> Result<()> { fn generate_linter() -> Result<()> { let base_path = project_root().join("crates/pgls_analyser/src"); let mut analysers = BTreeMap::new(); - generate_category("lint", &mut analysers, &base_path)?; + let mut all_rules = BTreeMap::new(); + generate_category("lint", &mut analysers, &mut all_rules, &base_path)?; generate_options(&base_path)?; - update_linter_registry_builder(analysers) + update_linter_registry_builder(analysers, all_rules) } fn generate_options(base_path: &Path) -> Result<()> { @@ -39,7 +40,7 @@ fn generate_options(base_path: &Path) -> Result<()> { let rule_module_name = format_ident!("{}", rule_filename); let rule_name = format_ident!("{}", rule_name); rules_options.insert(rule_filename.to_string(), quote! { - pub type #rule_name = <#category_name::#group_name::#rule_module_name::#rule_name as pgls_analyse::Rule>::Options; + pub type #rule_name = <#category_name::#group_name::#rule_module_name::#rule_name as crate::LinterRule>::Options; }); } } @@ -63,6 +64,7 @@ fn generate_options(base_path: &Path) -> Result<()> { fn generate_category( name: &'static str, entries: &mut BTreeMap<&'static str, TokenStream>, + all_rules: &mut BTreeMap, base_path: &Path, ) -> Result<()> { let path = base_path.join(name); @@ -81,7 +83,7 @@ fn generate_category( .to_str() .context("could not convert file name to string")?; - generate_group(name, file_name, base_path)?; + generate_group(name, file_name, all_rules, base_path)?; let module_name = format_ident!("{}", file_name); let group_name = format_ident!("{}", Case::Pascal.convert(file_name)); @@ -135,7 +137,12 @@ fn generate_category( Ok(()) } -fn generate_group(category: &'static str, group: &str, base_path: &Path) -> Result<()> { +fn generate_group( + category: &'static str, + group: &str, + all_rules: &mut BTreeMap, + base_path: &Path, +) -> Result<()> { let path = base_path.join(category).join(group); let mut rules = BTreeMap::new(); @@ -151,7 +158,21 @@ fn generate_group(category: &'static str, group: &str, base_path: &Path) -> Resu let key = rule_type.clone(); let module_name = format_ident!("{}", file_name); - let rule_type = format_ident!("{}", rule_type); + let rule_type_ident = format_ident!("{}", rule_type); + + // Collect for factory generation + // Key is the rule name (camelCase for config), value is (full_path_tokens, metadata_name) + let category_ident = format_ident!("{}", category); + let group_module_ident = format_ident!("{}", group); // Module name (lowercase) + all_rules.insert( + Case::Camel.convert(&key), + ( + quote! { + crate::#category_ident::#group_module_ident::#module_name::#rule_type_ident + }, + file_name.to_string(), + ), + ); rules.insert( key, @@ -160,7 +181,7 @@ fn generate_group(category: &'static str, group: &str, base_path: &Path) -> Resu pub mod #module_name; }, quote! { - self::#module_name::#rule_type + self::#module_name::#rule_type_ident }, ), ); @@ -199,17 +220,39 @@ fn generate_group(category: &'static str, group: &str, base_path: &Path) -> Resu Ok(()) } -fn update_linter_registry_builder(rules: BTreeMap<&'static str, TokenStream>) -> Result<()> { +fn update_linter_registry_builder( + rules: BTreeMap<&'static str, TokenStream>, + all_rules: BTreeMap, +) -> Result<()> { let path = project_root().join("crates/pgls_analyser/src/registry.rs"); let categories = rules.into_values(); + // Generate factory match arms for all rules + let factory_arms = all_rules.iter().map(|(rule_name, (rule_path, _))| { + quote! { + #rule_name => Some(Box::new(|| crate::linter_registry::RegistryLinterRule::new::<#rule_path>())) + } + }); + let tokens = xtask::reformat(quote! { - use pgls_analyse::RegistryVisitor; + use pgls_analyse::{RegistryVisitor, RuleKey}; + use crate::linter_registry::RegistryLinterRule; pub fn visit_registry(registry: &mut V) { #( #categories )* } + + /// Maps rule keys to factory functions that create rule executors + /// This function is generated by codegen and includes all linter rules + pub fn get_linter_rule_factory( + key: &RuleKey, + ) -> Option RegistryLinterRule>> { + match key.rule_name() { + #( #factory_arms, )* + _ => None, + } + } })?; fs2::write(path, tokens)?; diff --git a/xtask/codegen/src/generate_configuration.rs b/xtask/codegen/src/generate_configuration.rs index f4a922129..2a9913bf1 100644 --- a/xtask/codegen/src/generate_configuration.rs +++ b/xtask/codegen/src/generate_configuration.rs @@ -1,6 +1,8 @@ use crate::{to_capitalized, update}; use biome_string_case::Case; -use pgls_analyse::{GroupCategory, RegistryVisitor, Rule, RuleCategory, RuleGroup, RuleMetadata}; +use pgls_analyse::{ + GroupCategory, RegistryVisitor, RuleCategory, RuleGroup, RuleMeta, RuleMetadata, +}; use pgls_diagnostics::Severity; use proc_macro2::{Ident, Literal, Span, TokenStream}; use pulldown_cmark::{Event, Parser, Tag, TagEnd}; @@ -92,7 +94,7 @@ impl RegistryVisitor for CategoryRulesVisitor { fn record_rule(&mut self) where - R: Rule + 'static, + R: RuleMeta + 'static, { self.groups .entry(::NAME) diff --git a/xtask/rules_check/src/lib.rs b/xtask/rules_check/src/lib.rs index 3276e1a00..508d8c831 100644 --- a/xtask/rules_check/src/lib.rs +++ b/xtask/rules_check/src/lib.rs @@ -4,10 +4,10 @@ use std::{fmt::Write, slice}; use anyhow::bail; use pgls_analyse::{ - AnalyserOptions, AnalysisFilter, GroupCategory, RegistryVisitor, Rule, RuleCategory, - RuleFilter, RuleGroup, RuleMetadata, + AnalysisFilter, GroupCategory, RegistryVisitor, RuleCategory, RuleFilter, RuleGroup, RuleMeta, + RuleMetadata, }; -use pgls_analyser::{AnalysableStatement, Analyser, AnalyserConfig}; +use pgls_analyser::{AnalysableStatement, Analyser, AnalyserConfig, LinterOptions}; use pgls_console::{markup, Console}; use pgls_diagnostics::{Diagnostic, DiagnosticExt, PrintDiagnostic}; use pgls_query_ext::diagnostics::SyntaxDiagnostic; @@ -23,7 +23,7 @@ pub fn check_rules() -> anyhow::Result<()> { impl LintRulesVisitor { fn push_rule(&mut self) where - R: Rule + 'static, + R: RuleMeta + 'static, { self.groups .entry(::NAME) @@ -41,7 +41,7 @@ pub fn check_rules() -> anyhow::Result<()> { fn record_rule(&mut self) where - R: Rule + 'static, + R: RuleMeta + 'static, { self.push_rule::() } @@ -120,7 +120,7 @@ fn assert_lint( ..AnalysisFilter::default() }; let settings = Settings::default(); - let options = AnalyserOptions::default(); + let options = LinterOptions::default(); let analyser = Analyser::new(AnalyserConfig { options: &options, filter, From dc098c0a2c2307190b351567c0688541dfac14a4 Mon Sep 17 00:00:00 2001 From: psteinroe Date: Sun, 14 Dec 2025 18:40:52 +0100 Subject: [PATCH 2/5] fix: dont use dyn --- crates/pgls_analyser/src/linter_registry.rs | 4 +- crates/pgls_analyser/src/options.rs | 18 +-- crates/pgls_analyser/src/registry.rs | 165 +------------------- xtask/codegen/src/generate_analyser.rs | 14 +- 4 files changed, 21 insertions(+), 180 deletions(-) diff --git a/crates/pgls_analyser/src/linter_registry.rs b/crates/pgls_analyser/src/linter_registry.rs index 5840d069f..3d64db4b8 100644 --- a/crates/pgls_analyser/src/linter_registry.rs +++ b/crates/pgls_analyser/src/linter_registry.rs @@ -103,13 +103,13 @@ impl RegistryLinterRule { impl LinterRuleRegistryBuilder<'_> { pub fn build(self) -> LinterRuleRegistry { - // Look up factory for each collected rule key and create executors + // Look up executor for each collected rule key let rules = self .rule_keys .into_iter() .filter_map(|key| { // This function will be generated by codegen - crate::registry::get_linter_rule_factory(&key).map(|factory| factory()) + crate::registry::get_linter_rule_executor(&key) }) .collect(); diff --git a/crates/pgls_analyser/src/options.rs b/crates/pgls_analyser/src/options.rs index b04539c45..c7625a5a3 100644 --- a/crates/pgls_analyser/src/options.rs +++ b/crates/pgls_analyser/src/options.rs @@ -5,14 +5,14 @@ pub type AddSerialColumn = ::Options; pub type AddingFieldWithDefault = ::Options; -pub type AddingForeignKeyConstraint = < lint :: safety :: adding_foreign_key_constraint :: AddingForeignKeyConstraint as crate::LinterRule > :: Options ; +pub type AddingForeignKeyConstraint = < lint :: safety :: adding_foreign_key_constraint :: AddingForeignKeyConstraint as crate :: LinterRule > :: Options ; pub type AddingNotNullField = ::Options; -pub type AddingPrimaryKeyConstraint = < lint :: safety :: adding_primary_key_constraint :: AddingPrimaryKeyConstraint as crate::LinterRule > :: Options ; +pub type AddingPrimaryKeyConstraint = < lint :: safety :: adding_primary_key_constraint :: AddingPrimaryKeyConstraint as crate :: LinterRule > :: Options ; pub type AddingRequiredField = ::Options; pub type BanCharField = ::Options; -pub type BanConcurrentIndexCreationInTransaction = < lint :: safety :: ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction as crate::LinterRule > :: Options ; +pub type BanConcurrentIndexCreationInTransaction = < lint :: safety :: ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction as crate :: LinterRule > :: Options ; pub type BanDropColumn = ::Options; pub type BanDropDatabase = @@ -24,9 +24,9 @@ pub type BanTruncateCascade = ::Options; pub type ChangingColumnType = ::Options; -pub type ConstraintMissingNotValid = < lint :: safety :: constraint_missing_not_valid :: ConstraintMissingNotValid as crate::LinterRule > :: Options ; +pub type ConstraintMissingNotValid = < lint :: safety :: constraint_missing_not_valid :: ConstraintMissingNotValid as crate :: LinterRule > :: Options ; pub type CreatingEnum = ::Options; -pub type DisallowUniqueConstraint = < lint :: safety :: disallow_unique_constraint :: DisallowUniqueConstraint as crate::LinterRule > :: Options ; +pub type DisallowUniqueConstraint = < lint :: safety :: disallow_unique_constraint :: DisallowUniqueConstraint as crate :: LinterRule > :: Options ; pub type LockTimeoutWarning = ::Options; pub type MultipleAlterTable = @@ -34,7 +34,7 @@ pub type MultipleAlterTable = pub type PreferBigInt = ::Options; pub type PreferBigintOverInt = ::Options; -pub type PreferBigintOverSmallint = < lint :: safety :: prefer_bigint_over_smallint :: PreferBigintOverSmallint as crate::LinterRule > :: Options ; +pub type PreferBigintOverSmallint = < lint :: safety :: prefer_bigint_over_smallint :: PreferBigintOverSmallint as crate :: LinterRule > :: Options ; pub type PreferIdentity = ::Options; pub type PreferJsonb = ::Options; @@ -48,8 +48,8 @@ pub type RenamingColumn = ::Options; pub type RenamingTable = ::Options; -pub type RequireConcurrentIndexCreation = < lint :: safety :: require_concurrent_index_creation :: RequireConcurrentIndexCreation as crate::LinterRule > :: Options ; -pub type RequireConcurrentIndexDeletion = < lint :: safety :: require_concurrent_index_deletion :: RequireConcurrentIndexDeletion as crate::LinterRule > :: Options ; -pub type RunningStatementWhileHoldingAccessExclusive = < lint :: safety :: running_statement_while_holding_access_exclusive :: RunningStatementWhileHoldingAccessExclusive as crate::LinterRule > :: Options ; +pub type RequireConcurrentIndexCreation = < lint :: safety :: require_concurrent_index_creation :: RequireConcurrentIndexCreation as crate :: LinterRule > :: Options ; +pub type RequireConcurrentIndexDeletion = < lint :: safety :: require_concurrent_index_deletion :: RequireConcurrentIndexDeletion as crate :: LinterRule > :: Options ; +pub type RunningStatementWhileHoldingAccessExclusive = < lint :: safety :: running_statement_while_holding_access_exclusive :: RunningStatementWhileHoldingAccessExclusive as crate :: LinterRule > :: Options ; pub type TransactionNesting = ::Options; diff --git a/crates/pgls_analyser/src/registry.rs b/crates/pgls_analyser/src/registry.rs index 29862d096..35dc5e914 100644 --- a/crates/pgls_analyser/src/registry.rs +++ b/crates/pgls_analyser/src/registry.rs @@ -5,167 +5,8 @@ use pgls_analyse::{RegistryVisitor, RuleKey}; pub fn visit_registry(registry: &mut V) { registry.record_category::(); } -#[doc = r" Maps rule keys to factory functions that create rule executors"] +#[doc = r" Maps rule keys to rule executors"] #[doc = r" This function is generated by codegen and includes all linter rules"] -pub fn get_linter_rule_factory(key: &RuleKey) -> Option RegistryLinterRule>> { - match key.rule_name() { - "addSerialColumn" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::add_serial_column::AddSerialColumn, - >() - })), - "addingFieldWithDefault" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::adding_field_with_default::AddingFieldWithDefault, - >() - })), - "addingForeignKeyConstraint" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::adding_foreign_key_constraint::AddingForeignKeyConstraint, - >() - })), - "addingNotNullField" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::adding_not_null_field::AddingNotNullField, - >() - })), - "addingPrimaryKeyConstraint" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::adding_primary_key_constraint::AddingPrimaryKeyConstraint, - >() - })), - "addingRequiredField" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::adding_required_field::AddingRequiredField, - >() - })), - "banCharField" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::ban_char_field::BanCharField, - >() - })), - "banConcurrentIndexCreationInTransaction" => Some(Box::new(|| { - crate :: linter_registry :: RegistryLinterRule :: new :: < crate::lint::safety::ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction > () - })), - "banDropColumn" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::ban_drop_column::BanDropColumn, - >() - })), - "banDropDatabase" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::ban_drop_database::BanDropDatabase, - >() - })), - "banDropNotNull" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::ban_drop_not_null::BanDropNotNull, - >() - })), - "banDropTable" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::ban_drop_table::BanDropTable, - >() - })), - "banTruncateCascade" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::ban_truncate_cascade::BanTruncateCascade, - >() - })), - "changingColumnType" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::changing_column_type::ChangingColumnType, - >() - })), - "constraintMissingNotValid" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::constraint_missing_not_valid::ConstraintMissingNotValid, - >() - })), - "creatingEnum" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::creating_enum::CreatingEnum, - >() - })), - "disallowUniqueConstraint" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::disallow_unique_constraint::DisallowUniqueConstraint, - >() - })), - "lockTimeoutWarning" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::lock_timeout_warning::LockTimeoutWarning, - >() - })), - "multipleAlterTable" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::multiple_alter_table::MultipleAlterTable, - >() - })), - "preferBigInt" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::prefer_big_int::PreferBigInt, - >() - })), - "preferBigintOverInt" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::prefer_bigint_over_int::PreferBigintOverInt, - >() - })), - "preferBigintOverSmallint" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::prefer_bigint_over_smallint::PreferBigintOverSmallint, - >() - })), - "preferIdentity" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::prefer_identity::PreferIdentity, - >() - })), - "preferJsonb" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::prefer_jsonb::PreferJsonb, - >() - })), - "preferRobustStmts" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::prefer_robust_stmts::PreferRobustStmts, - >() - })), - "preferTextField" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::prefer_text_field::PreferTextField, - >() - })), - "preferTimestamptz" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::prefer_timestamptz::PreferTimestamptz, - >() - })), - "renamingColumn" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::renaming_column::RenamingColumn, - >() - })), - "renamingTable" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::renaming_table::RenamingTable, - >() - })), - "requireConcurrentIndexCreation" => Some(Box::new(|| { - crate :: linter_registry :: RegistryLinterRule :: new :: < crate::lint::safety::require_concurrent_index_creation :: RequireConcurrentIndexCreation > () - })), - "requireConcurrentIndexDeletion" => Some(Box::new(|| { - crate :: linter_registry :: RegistryLinterRule :: new :: < crate::lint::safety::require_concurrent_index_deletion :: RequireConcurrentIndexDeletion > () - })), - "runningStatementWhileHoldingAccessExclusive" => Some(Box::new(|| { - crate :: linter_registry :: RegistryLinterRule :: new :: < crate::lint::safety::running_statement_while_holding_access_exclusive :: RunningStatementWhileHoldingAccessExclusive > () - })), - "transactionNesting" => Some(Box::new(|| { - crate::linter_registry::RegistryLinterRule::new::< - crate::lint::safety::transaction_nesting::TransactionNesting, - >() - })), - _ => None, - } +pub fn get_linter_rule_executor(key: &RuleKey) -> Option { + match key . rule_name () { "addSerialColumn" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: add_serial_column :: AddSerialColumn > ()) , "addingFieldWithDefault" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_field_with_default :: AddingFieldWithDefault > ()) , "addingForeignKeyConstraint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_foreign_key_constraint :: AddingForeignKeyConstraint > ()) , "addingNotNullField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_not_null_field :: AddingNotNullField > ()) , "addingPrimaryKeyConstraint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_primary_key_constraint :: AddingPrimaryKeyConstraint > ()) , "addingRequiredField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_required_field :: AddingRequiredField > ()) , "banCharField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_char_field :: BanCharField > ()) , "banConcurrentIndexCreationInTransaction" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction > ()) , "banDropColumn" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_column :: BanDropColumn > ()) , "banDropDatabase" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_database :: BanDropDatabase > ()) , "banDropNotNull" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_not_null :: BanDropNotNull > ()) , "banDropTable" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_table :: BanDropTable > ()) , "banTruncateCascade" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_truncate_cascade :: BanTruncateCascade > ()) , "changingColumnType" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: changing_column_type :: ChangingColumnType > ()) , "constraintMissingNotValid" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: constraint_missing_not_valid :: ConstraintMissingNotValid > ()) , "creatingEnum" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: creating_enum :: CreatingEnum > ()) , "disallowUniqueConstraint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: disallow_unique_constraint :: DisallowUniqueConstraint > ()) , "lockTimeoutWarning" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: lock_timeout_warning :: LockTimeoutWarning > ()) , "multipleAlterTable" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: multiple_alter_table :: MultipleAlterTable > ()) , "preferBigInt" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_big_int :: PreferBigInt > ()) , "preferBigintOverInt" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_bigint_over_int :: PreferBigintOverInt > ()) , "preferBigintOverSmallint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_bigint_over_smallint :: PreferBigintOverSmallint > ()) , "preferIdentity" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_identity :: PreferIdentity > ()) , "preferJsonb" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_jsonb :: PreferJsonb > ()) , "preferRobustStmts" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_robust_stmts :: PreferRobustStmts > ()) , "preferTextField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_text_field :: PreferTextField > ()) , "preferTimestamptz" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_timestamptz :: PreferTimestamptz > ()) , "renamingColumn" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: renaming_column :: RenamingColumn > ()) , "renamingTable" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: renaming_table :: RenamingTable > ()) , "requireConcurrentIndexCreation" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: require_concurrent_index_creation :: RequireConcurrentIndexCreation > ()) , "requireConcurrentIndexDeletion" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: require_concurrent_index_deletion :: RequireConcurrentIndexDeletion > ()) , "runningStatementWhileHoldingAccessExclusive" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: running_statement_while_holding_access_exclusive :: RunningStatementWhileHoldingAccessExclusive > ()) , "transactionNesting" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: transaction_nesting :: TransactionNesting > ()) , _ => None , } } diff --git a/xtask/codegen/src/generate_analyser.rs b/xtask/codegen/src/generate_analyser.rs index fc1257d5c..8698795fb 100644 --- a/xtask/codegen/src/generate_analyser.rs +++ b/xtask/codegen/src/generate_analyser.rs @@ -228,10 +228,10 @@ fn update_linter_registry_builder( let categories = rules.into_values(); - // Generate factory match arms for all rules - let factory_arms = all_rules.iter().map(|(rule_name, (rule_path, _))| { + // Generate match arms that directly create executors (no closure/Box overhead) + let executor_arms = all_rules.iter().map(|(rule_name, (rule_path, _))| { quote! { - #rule_name => Some(Box::new(|| crate::linter_registry::RegistryLinterRule::new::<#rule_path>())) + #rule_name => Some(crate::linter_registry::RegistryLinterRule::new::<#rule_path>()) } }); @@ -243,13 +243,13 @@ fn update_linter_registry_builder( #( #categories )* } - /// Maps rule keys to factory functions that create rule executors + /// Maps rule keys to rule executors (zero-cost abstraction) /// This function is generated by codegen and includes all linter rules - pub fn get_linter_rule_factory( + pub fn get_linter_rule_executor( key: &RuleKey, - ) -> Option RegistryLinterRule>> { + ) -> Option { match key.rule_name() { - #( #factory_arms, )* + #( #executor_arms, )* _ => None, } } From 6fae04119cd8d705cc4e945ef3cc1fb8d0e4323a Mon Sep 17 00:00:00 2001 From: psteinroe Date: Mon, 15 Dec 2025 12:06:49 +0100 Subject: [PATCH 3/5] fix: correct AnalyserRules reference in codegen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changed pgls_analyse::AnalyserRules to pgls_analyser::LinterRules and fixed import statements to use the correct crate for RuleOptions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- crates/pgls_analyser/src/registry.rs | 2 +- crates/pgls_configuration/src/linter/rules.rs | 2 +- crates/pgls_configuration/src/rules/configuration.rs | 1 - xtask/codegen/src/generate_configuration.rs | 8 +++++--- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/pgls_analyser/src/registry.rs b/crates/pgls_analyser/src/registry.rs index 35dc5e914..7e3872905 100644 --- a/crates/pgls_analyser/src/registry.rs +++ b/crates/pgls_analyser/src/registry.rs @@ -5,7 +5,7 @@ use pgls_analyse::{RegistryVisitor, RuleKey}; pub fn visit_registry(registry: &mut V) { registry.record_category::(); } -#[doc = r" Maps rule keys to rule executors"] +#[doc = r" Maps rule keys to rule executors (zero-cost abstraction)"] #[doc = r" This function is generated by codegen and includes all linter rules"] pub fn get_linter_rule_executor(key: &RuleKey) -> Option { match key . rule_name () { "addSerialColumn" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: add_serial_column :: AddSerialColumn > ()) , "addingFieldWithDefault" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_field_with_default :: AddingFieldWithDefault > ()) , "addingForeignKeyConstraint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_foreign_key_constraint :: AddingForeignKeyConstraint > ()) , "addingNotNullField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_not_null_field :: AddingNotNullField > ()) , "addingPrimaryKeyConstraint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_primary_key_constraint :: AddingPrimaryKeyConstraint > ()) , "addingRequiredField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_required_field :: AddingRequiredField > ()) , "banCharField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_char_field :: BanCharField > ()) , "banConcurrentIndexCreationInTransaction" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction > ()) , "banDropColumn" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_column :: BanDropColumn > ()) , "banDropDatabase" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_database :: BanDropDatabase > ()) , "banDropNotNull" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_not_null :: BanDropNotNull > ()) , "banDropTable" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_table :: BanDropTable > ()) , "banTruncateCascade" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_truncate_cascade :: BanTruncateCascade > ()) , "changingColumnType" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: changing_column_type :: ChangingColumnType > ()) , "constraintMissingNotValid" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: constraint_missing_not_valid :: ConstraintMissingNotValid > ()) , "creatingEnum" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: creating_enum :: CreatingEnum > ()) , "disallowUniqueConstraint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: disallow_unique_constraint :: DisallowUniqueConstraint > ()) , "lockTimeoutWarning" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: lock_timeout_warning :: LockTimeoutWarning > ()) , "multipleAlterTable" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: multiple_alter_table :: MultipleAlterTable > ()) , "preferBigInt" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_big_int :: PreferBigInt > ()) , "preferBigintOverInt" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_bigint_over_int :: PreferBigintOverInt > ()) , "preferBigintOverSmallint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_bigint_over_smallint :: PreferBigintOverSmallint > ()) , "preferIdentity" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_identity :: PreferIdentity > ()) , "preferJsonb" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_jsonb :: PreferJsonb > ()) , "preferRobustStmts" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_robust_stmts :: PreferRobustStmts > ()) , "preferTextField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_text_field :: PreferTextField > ()) , "preferTimestamptz" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_timestamptz :: PreferTimestamptz > ()) , "renamingColumn" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: renaming_column :: RenamingColumn > ()) , "renamingTable" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: renaming_table :: RenamingTable > ()) , "requireConcurrentIndexCreation" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: require_concurrent_index_creation :: RequireConcurrentIndexCreation > ()) , "requireConcurrentIndexDeletion" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: require_concurrent_index_deletion :: RequireConcurrentIndexDeletion > ()) , "runningStatementWhileHoldingAccessExclusive" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: running_statement_while_holding_access_exclusive :: RunningStatementWhileHoldingAccessExclusive > ()) , "transactionNesting" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: transaction_nesting :: TransactionNesting > ()) , _ => None , } diff --git a/crates/pgls_configuration/src/linter/rules.rs b/crates/pgls_configuration/src/linter/rules.rs index 32fc1a2fc..4bc3d4c98 100644 --- a/crates/pgls_configuration/src/linter/rules.rs +++ b/crates/pgls_configuration/src/linter/rules.rs @@ -908,7 +908,7 @@ impl Safety { pub fn push_to_analyser_rules( rules: &Rules, metadata: &pgls_analyse::MetadataRegistry, - analyser_rules: &mut pgls_analyse::AnalyserRules, + analyser_rules: &mut pgls_analyser::LinterRules, ) { if let Some(rules) = rules.safety.as_ref() { for rule_name in Safety::GROUP_RULES { diff --git a/crates/pgls_configuration/src/rules/configuration.rs b/crates/pgls_configuration/src/rules/configuration.rs index ad93abeeb..6b86fef69 100644 --- a/crates/pgls_configuration/src/rules/configuration.rs +++ b/crates/pgls_configuration/src/rules/configuration.rs @@ -1,6 +1,5 @@ use biome_deserialize::Merge; use biome_deserialize_macros::Deserializable; -use pgls_analyse::RuleFilter; use pgls_analyser::RuleOptions; use pgls_diagnostics::Severity; #[cfg(feature = "schema")] diff --git a/xtask/codegen/src/generate_configuration.rs b/xtask/codegen/src/generate_configuration.rs index 2a9913bf1..43224d48e 100644 --- a/xtask/codegen/src/generate_configuration.rs +++ b/xtask/codegen/src/generate_configuration.rs @@ -287,7 +287,8 @@ fn generate_lint_rules_file( use crate::rules::{RuleConfiguration, RulePlainConfiguration}; use biome_deserialize_macros::Merge; - use pgls_analyse::{RuleFilter, options::RuleOptions}; + use pgls_analyse::RuleFilter; + use pgls_analyser::RuleOptions; use pgls_diagnostics::{Category, Severity}; use rustc_hash::FxHashSet; #[cfg(feature = "schema")] @@ -426,7 +427,7 @@ fn generate_lint_rules_file( pub fn push_to_analyser_rules( rules: &Rules, metadata: &pgls_analyse::MetadataRegistry, - analyser_rules: &mut pgls_analyse::AnalyserRules, + analyser_rules: &mut pgls_analyser::LinterRules, ) { #( if let Some(rules) = rules.#group_idents.as_ref() { @@ -789,7 +790,8 @@ fn generate_action_actions_file( use crate::rules::{RuleAssistConfiguration, RuleAssistPlainConfiguration}; use biome_deserialize_macros::{Deserializable, Merge}; - use pgls_analyse::{RuleFilter, options::RuleOptions}; + use pgls_analyse::RuleFilter; + use pgls_analyser::RuleOptions; use pgls_diagnostics::{Category, Severity}; use rustc_hash::FxHashSet; #[cfg(feature = "schema")] From 4e9757f31a97d149dd0d3db0597ef69fc26ab417 Mon Sep 17 00:00:00 2001 From: psteinroe Date: Mon, 15 Dec 2025 12:32:53 +0100 Subject: [PATCH 4/5] fix: update test imports to use LinterOptions and LinterDiagnostic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updated test files to use the new type names after the refactor: - Changed AnalyserOptions to LinterOptions - Changed RuleDiagnostic to LinterDiagnostic - Fixed import paths to import from pgls_analyser instead of pgls_analyse 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- crates/pgls_analyser/src/lib.rs | 3 ++- crates/pgls_analyser/tests/rules_tests.rs | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/pgls_analyser/src/lib.rs b/crates/pgls_analyser/src/lib.rs index 038a53239..577a2a886 100644 --- a/crates/pgls_analyser/src/lib.rs +++ b/crates/pgls_analyser/src/lib.rs @@ -116,7 +116,8 @@ impl<'a> Analyser<'a> { mod tests { use core::slice; - use pgls_analyse::{AnalyserOptions, AnalysisFilter, RuleFilter}; + use pgls_analyse::{AnalysisFilter, RuleFilter}; + use crate::LinterOptions; use pgls_console::{ Markup, fmt::{Formatter, Termcolor}, diff --git a/crates/pgls_analyser/tests/rules_tests.rs b/crates/pgls_analyser/tests/rules_tests.rs index 2b023d472..4749b3401 100644 --- a/crates/pgls_analyser/tests/rules_tests.rs +++ b/crates/pgls_analyser/tests/rules_tests.rs @@ -1,8 +1,8 @@ use core::slice; use std::{collections::HashMap, fmt::Write, fs::read_to_string, path::Path}; -use pgls_analyse::{AnalyserOptions, AnalysisFilter, RuleDiagnostic, RuleFilter}; -use pgls_analyser::{AnalysableStatement, Analyser, AnalyserConfig, AnalyserParams}; +use pgls_analyse::{AnalysisFilter, RuleFilter}; +use pgls_analyser::{AnalysableStatement, Analyser, AnalyserConfig, AnalyserParams, LinterDiagnostic, LinterOptions}; use pgls_console::StdDisplay; use pgls_diagnostics::PrintDiagnostic; @@ -25,7 +25,7 @@ fn rule_test(full_path: &'static str, _: &str, _: &str) { let query = read_to_string(full_path).unwrap_or_else(|_| panic!("Failed to read file: {full_path} ")); - let options = AnalyserOptions::default(); + let options = LinterOptions::default(); let analyser = Analyser::new(AnalyserConfig { options: &options, filter, @@ -79,7 +79,7 @@ fn parse_test_path(path: &Path) -> (String, String, String) { (group.into(), rule.into(), fname.into()) } -fn write_snapshot(snapshot: &mut String, query: &str, diagnostics: &[RuleDiagnostic]) { +fn write_snapshot(snapshot: &mut String, query: &str, diagnostics: &[LinterDiagnostic]) { writeln!(snapshot, "# Input").unwrap(); writeln!(snapshot, "```").unwrap(); writeln!(snapshot, "{query}").unwrap(); @@ -140,7 +140,7 @@ impl Expectation { ); } - fn assert(&self, diagnostics: &[RuleDiagnostic]) { + fn assert(&self, diagnostics: &[LinterDiagnostic]) { match self { Self::NoDiagnostics => { if !diagnostics.is_empty() { From 4f16e91572205ea50f75f14efcae6a9bda203bc1 Mon Sep 17 00:00:00 2001 From: psteinroe Date: Mon, 15 Dec 2025 12:50:37 +0100 Subject: [PATCH 5/5] style: apply rustfmt to test files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed import ordering and line length formatting issues. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- crates/pgls_analyser/src/lib.rs | 2 +- crates/pgls_analyser/tests/rules_tests.rs | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/pgls_analyser/src/lib.rs b/crates/pgls_analyser/src/lib.rs index 577a2a886..5cfd01897 100644 --- a/crates/pgls_analyser/src/lib.rs +++ b/crates/pgls_analyser/src/lib.rs @@ -116,8 +116,8 @@ impl<'a> Analyser<'a> { mod tests { use core::slice; - use pgls_analyse::{AnalysisFilter, RuleFilter}; use crate::LinterOptions; + use pgls_analyse::{AnalysisFilter, RuleFilter}; use pgls_console::{ Markup, fmt::{Formatter, Termcolor}, diff --git a/crates/pgls_analyser/tests/rules_tests.rs b/crates/pgls_analyser/tests/rules_tests.rs index 4749b3401..39b74e62c 100644 --- a/crates/pgls_analyser/tests/rules_tests.rs +++ b/crates/pgls_analyser/tests/rules_tests.rs @@ -2,7 +2,9 @@ use core::slice; use std::{collections::HashMap, fmt::Write, fs::read_to_string, path::Path}; use pgls_analyse::{AnalysisFilter, RuleFilter}; -use pgls_analyser::{AnalysableStatement, Analyser, AnalyserConfig, AnalyserParams, LinterDiagnostic, LinterOptions}; +use pgls_analyser::{ + AnalysableStatement, Analyser, AnalyserConfig, AnalyserParams, LinterDiagnostic, LinterOptions, +}; use pgls_console::StdDisplay; use pgls_diagnostics::PrintDiagnostic;