Skip to content

chore: expose schema_cache & file_context in lint rules #449

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Jul 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Cargo.lock

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

9 changes: 5 additions & 4 deletions crates/pgt_analyse/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ version = "0.0.0"


[dependencies]
pgt_console.workspace = true
pgt_diagnostics.workspace = true
pgt_query_ext.workspace = true
rustc-hash = { workspace = true }
pgt_console.workspace = true
pgt_diagnostics.workspace = true
pgt_query_ext.workspace = true
pgt_schema_cache.workspace = true
rustc-hash = { workspace = true }

biome_deserialize = { workspace = true, optional = true }
biome_deserialize_macros = { workspace = true, optional = true }
Expand Down
7 changes: 7 additions & 0 deletions crates/pgt_analyse/src/analysed_file_context.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#[derive(Default)]
pub struct AnalysedFileContext {}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's just a scaffold for the file context.
We can add properties in future linting PRs.
eugene works the same way: link


impl AnalysedFileContext {
#[allow(unused)]
pub fn update_from(&mut self, stmt_root: &pgt_query_ext::NodeEnum) {}
}
27 changes: 25 additions & 2 deletions crates/pgt_analyse/src/context.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,35 @@
use pgt_schema_cache::SchemaCache;

use crate::{
AnalysedFileContext,
categories::RuleCategory,
rule::{GroupCategory, Rule, RuleGroup, RuleMetadata},
};

pub struct RuleContext<'a, R: Rule> {
stmt: &'a pgt_query_ext::NodeEnum,
options: &'a R::Options,
schema_cache: Option<&'a SchemaCache>,
file_context: &'a AnalysedFileContext,
}

impl<'a, R> RuleContext<'a, R>
where
R: Rule + Sized + 'static,
{
#[allow(clippy::too_many_arguments)]
pub fn new(stmt: &'a pgt_query_ext::NodeEnum, options: &'a R::Options) -> Self {
Self { stmt, options }
pub fn new(
stmt: &'a pgt_query_ext::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
Expand All @@ -32,6 +47,14 @@ where
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.
Expand Down
2 changes: 2 additions & 0 deletions crates/pgt_analyse/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
mod analysed_file_context;
mod categories;
pub mod context;
mod filter;
Expand All @@ -9,6 +10,7 @@ mod rule;
// Re-exported for use in the `declare_group` macro
pub use pgt_diagnostics::category_concat;

pub use crate::analysed_file_context::AnalysedFileContext;
pub use crate::categories::{
ActionCategory, RefactorKind, RuleCategories, RuleCategoriesBuilder, RuleCategory,
SUPPRESSION_ACTION_CATEGORY, SourceActionKind,
Expand Down
12 changes: 11 additions & 1 deletion crates/pgt_analyse/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::{borrow, collections::BTreeSet};

use crate::{
AnalyserOptions,
analysed_file_context::AnalysedFileContext,
context::RuleContext,
filter::{AnalysisFilter, GroupKey, RuleKey},
rule::{GroupCategory, Rule, RuleDiagnostic, RuleGroup},
Expand Down Expand Up @@ -158,6 +159,8 @@ impl RuleRegistry {
pub struct RegistryRuleParams<'a> {
pub root: &'a pgt_query_ext::NodeEnum,
pub options: &'a AnalyserOptions,
pub analysed_file_context: &'a AnalysedFileContext,
pub schema_cache: Option<&'a pgt_schema_cache::SchemaCache>,
}

/// Executor for rule as a generic function pointer
Expand All @@ -174,7 +177,14 @@ impl RegistryRule {
R: Rule<Options: Default> + 'static,
{
let options = params.options.rule_options::<R>().unwrap_or_default();
let ctx = RuleContext::new(params.root, &options);

let ctx = RuleContext::new(
params.root,
&options,
params.schema_cache,
params.analysed_file_context,
);

R::run(&ctx)
}

Expand Down
9 changes: 8 additions & 1 deletion crates/pgt_analyse/src/rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ pub trait GroupCategory {
pub trait Rule: RuleMeta + Sized {
type Options: Default + Clone + Debug;

fn run(ctx: &RuleContext<Self>) -> Vec<RuleDiagnostic>;
/// `schema_cache` will only be available if the user has a working database connection.
fn run(rule_context: &RuleContext<Self>) -> Vec<RuleDiagnostic>;
}

/// Diagnostic object returned by a single analysis rule
Expand Down Expand Up @@ -208,6 +209,12 @@ impl RuleDiagnostic {
self
}

/// Sets the span of this diagnostic.
pub fn span(mut self, span: TextRange) -> Self {
self.span = Some(span);
self
}

/// Marks this diagnostic as unnecessary code, which will
/// be displayed in the language server.
///
Expand Down
12 changes: 7 additions & 5 deletions crates/pgt_analyser/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ repository.workspace = true
version = "0.0.0"

[dependencies]
pgt_analyse = { workspace = true }
pgt_console = { workspace = true }
pgt_diagnostics = { workspace = true }
pgt_query_ext = { workspace = true }
serde = { workspace = true }
pgt_analyse = { workspace = true }
pgt_console = { workspace = true }
pgt_diagnostics = { workspace = true }
pgt_query_ext = { workspace = true }
pgt_schema_cache = { workspace = true }
pgt_text_size = { workspace = true }
serde = { workspace = true }

[dev-dependencies]
insta = { version = "1.42.1" }
Expand Down
58 changes: 42 additions & 16 deletions crates/pgt_analyser/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use std::{ops::Deref, sync::LazyLock};

use pgt_analyse::{
AnalyserOptions, AnalysisFilter, MetadataRegistry, RegistryRuleParams, RuleDiagnostic,
RuleRegistry,
AnalysedFileContext, AnalyserOptions, AnalysisFilter, MetadataRegistry, RegistryRuleParams,
RuleDiagnostic, RuleRegistry,
};
pub use registry::visit_registry;

Expand Down Expand Up @@ -30,8 +30,15 @@ pub struct Analyser<'a> {
registry: RuleRegistry,
}

pub struct AnalyserContext<'a> {
pub root: &'a pgt_query_ext::NodeEnum,
#[derive(Debug)]
pub struct AnalysableStatement {
pub root: pgt_query_ext::NodeEnum,
pub range: pgt_text_size::TextRange,
}

pub struct AnalyserParams<'a> {
pub stmts: Vec<AnalysableStatement>,
pub schema_cache: Option<&'a pgt_schema_cache::SchemaCache>,
}

pub struct AnalyserConfig<'a> {
Expand All @@ -52,17 +59,31 @@ impl<'a> Analyser<'a> {
}
}

pub fn run(&self, ctx: AnalyserContext) -> Vec<RuleDiagnostic> {
let params = RegistryRuleParams {
root: ctx.root,
options: self.options,
};
pub fn run(&self, params: AnalyserParams) -> Vec<RuleDiagnostic> {
let mut diagnostics = vec![];

let mut file_context = AnalysedFileContext::default();

for stmt in params.stmts {
let rule_params = RegistryRuleParams {
root: &stmt.root,
options: self.options,
analysed_file_context: &file_context,
schema_cache: params.schema_cache,
};

self.registry
.rules
.iter()
.flat_map(|rule| (rule.run)(&params))
.collect::<Vec<_>>()
diagnostics.extend(
self.registry
.rules
.iter()
.flat_map(|rule| (rule.run)(&rule_params))
.map(|r| r.span(stmt.range)),
);

file_context.update_from(&stmt.root);
}

diagnostics
}
}

Expand All @@ -77,9 +98,10 @@ mod tests {
markup,
};
use pgt_diagnostics::PrintDiagnostic;
use pgt_text_size::TextRange;
use termcolor::NoColor;

use crate::Analyser;
use crate::{AnalysableStatement, Analyser};

#[ignore]
#[test]
Expand All @@ -102,6 +124,7 @@ mod tests {
};

let ast = pgt_query_ext::parse(SQL).expect("failed to parse SQL");
let range = TextRange::new(0.into(), u32::try_from(SQL.len()).unwrap().into());

let options = AnalyserOptions::default();

Expand All @@ -110,7 +133,10 @@ mod tests {
filter,
});

let results = analyser.run(crate::AnalyserContext { root: &ast });
let results = analyser.run(crate::AnalyserParams {
stmts: vec![AnalysableStatement { root: ast, range }],
schema_cache: None,
});

println!("*******************");
for result in &results {
Expand Down
12 changes: 10 additions & 2 deletions crates/pgt_analyser/tests/rules_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use core::slice;
use std::{fmt::Write, fs::read_to_string, path::Path};

use pgt_analyse::{AnalyserOptions, AnalysisFilter, RuleDiagnostic, RuleFilter};
use pgt_analyser::{Analyser, AnalyserConfig, AnalyserContext};
use pgt_analyser::{AnalysableStatement, Analyser, AnalyserConfig, AnalyserParams};
use pgt_console::StdDisplay;
use pgt_diagnostics::PrintDiagnostic;

Expand Down Expand Up @@ -32,7 +32,15 @@ fn rule_test(full_path: &'static str, _: &str, _: &str) {
filter,
});

let results = analyser.run(AnalyserContext { root: &ast });
let stmt = AnalysableStatement {
root: ast,
range: pgt_text_size::TextRange::new(0.into(), u32::try_from(query.len()).unwrap().into()),
};

let results = analyser.run(AnalyserParams {
stmts: vec![stmt],
schema_cache: None,
});

let mut snapshot = String::new();
write_snapshot(&mut snapshot, query.as_str(), results.as_slice());
Expand Down
18 changes: 16 additions & 2 deletions crates/pgt_lsp/tests/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1734,13 +1734,24 @@ $$;

server.open_document(initial_content).await?;

let notification = tokio::time::timeout(Duration::from_secs(5), async {
let got_notification = tokio::time::timeout(Duration::from_secs(5), async {
loop {
match receiver.next().await {
Some(ServerNotification::PublishDiagnostics(msg)) => {
if msg.diagnostics.iter().any(|d| {
d.message
.contains("Invalid statement: syntax error at or near \"declre\"")
&& d.range
== Range {
start: Position {
line: 5,
character: 9,
},
end: Position {
line: 11,
character: 0,
},
}
}) {
return true;
}
Expand All @@ -1752,7 +1763,10 @@ $$;
.await
.is_ok();

assert!(notification, "expected diagnostics for unknown column");
assert!(
got_notification,
"expected diagnostics for invalid declare statement"
);

server.shutdown().await?;
reader.abort();
Expand Down
7 changes: 6 additions & 1 deletion crates/pgt_query_ext/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use pgt_text_size::TextRange;
pub struct SyntaxDiagnostic {
/// The location where the error is occurred
#[location(span)]
span: Option<TextRange>,
pub span: Option<TextRange>,
#[message]
#[description]
pub message: MessageAndDescription,
Expand All @@ -23,6 +23,11 @@ impl SyntaxDiagnostic {
message: MessageAndDescription::from(message.into()),
}
}

pub fn span(mut self, span: TextRange) -> Self {
self.span = Some(span);
self
}
}

impl From<pg_query::Error> for SyntaxDiagnostic {
Expand Down
4 changes: 3 additions & 1 deletion crates/pgt_workspace/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ schema = [
]

[dev-dependencies]
tempfile = "3.15.0"
pgt_test_utils = { workspace = true }
sqlx = { workspace = true }
tempfile = "3.15.0"

[lib]
doctest = false
Loading