From 03173dbe5cef130362efb36faf5febf3d8b2e068 Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 10 Feb 2025 10:05:24 +0100 Subject: [PATCH 1/6] test(analyser): add debug_test --- Cargo.lock | 3 + crates/pg_analyse/src/rule.rs | 8 +-- crates/pg_analyser/CONTRIBUTING.md | 29 +++++++--- crates/pg_analyser/Cargo.toml | 5 ++ crates/pg_analyser/src/lib.rs | 58 ++++++++++++++++++++ crates/pg_diagnostics/src/advice.rs | 2 +- crates/pg_diagnostics/src/display/message.rs | 2 +- 7 files changed, 94 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8ad8fef0..48303ff9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2213,8 +2213,11 @@ version = "0.0.0" dependencies = [ "pg_analyse", "pg_console", + "pg_diagnostics", + "pg_query", "pg_query_ext", "serde", + "termcolor", ] [[package]] diff --git a/crates/pg_analyse/src/rule.rs b/crates/pg_analyse/src/rule.rs index f159861d..3cf397cc 100644 --- a/crates/pg_analyse/src/rule.rs +++ b/crates/pg_analyse/src/rule.rs @@ -94,7 +94,7 @@ pub trait Rule: RuleMeta + Sized { } /// Diagnostic object returned by a single analysis rule -#[derive(Debug, Diagnostic)] +#[derive(Debug, Diagnostic, PartialEq)] pub struct RuleDiagnostic { #[category] pub(crate) category: &'static Category, @@ -109,7 +109,7 @@ pub struct RuleDiagnostic { pub(crate) rule_advice: RuleAdvice, } -#[derive(Debug, Default)] +#[derive(Debug, Default, PartialEq)] /// It contains possible advices to show when printing a diagnostic that belong to the rule pub struct RuleAdvice { pub(crate) details: Vec, @@ -118,7 +118,7 @@ pub struct RuleAdvice { pub(crate) code_suggestion_list: Vec>, } -#[derive(Debug, Default)] +#[derive(Debug, Default, PartialEq)] pub struct SuggestionList { pub(crate) message: MarkupBuf, pub(crate) list: Vec, @@ -160,7 +160,7 @@ impl Advices for RuleAdvice { } } -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub struct Detail { pub log_category: LogCategory, pub message: MarkupBuf, diff --git a/crates/pg_analyser/CONTRIBUTING.md b/crates/pg_analyser/CONTRIBUTING.md index 200be87c..3f968042 100644 --- a/crates/pg_analyser/CONTRIBUTING.md +++ b/crates/pg_analyser/CONTRIBUTING.md @@ -35,6 +35,7 @@ We follow a naming convention according to what the rule does: A rule should be informative to the user, and give as much explanation as possible. When writing a rule, you must adhere to the following **pillars**: + 1. Explain to the user the error. Generally, this is the message of the diagnostic. 1. Explain to the user **why** the error is triggered. Generally, this is implemented with an additional node. 1. Tell the user what they should do. Generally, this is implemented using a code action. If a code action is not applicable a note should tell the user what they should do to fix the error. @@ -53,6 +54,7 @@ Let's say we want to create a new **lint** rule called `useMyRuleName`, follow t ```shell just new-lintrule safety useMyRuleName ``` + The script will generate a bunch of files inside the `pg_analyser` crate. Among the other files, you'll find a file called `use_my_new_rule_name.rs` inside the `pg_analyser/lib/src/lint/safety` folder. You'll implement your rule in this file. @@ -127,6 +129,7 @@ The compiler should warn you that `MyRuleOptions` does not implement some requir We currently require implementing _serde_'s traits `Deserialize`/`Serialize`. Also, we use other `serde` macros to adjust the JSON configuration: + - `rename_all = "snake_case"`: it renames all fields in camel-case, so they are in line with the naming style of the `pglsp.toml`. - `deny_unknown_fields`: it raises an error if the configuration contains extraneous fields. - `default`: it uses the `Default` value when the field is missing from `pglsp.toml`. This macro makes the field optional. @@ -159,7 +162,6 @@ pub enum Behavior { Below, there are many tips and guidelines on how to create a lint rule using our infrastructure. - #### `declare_lint_rule` This macro is used to declare an analyzer rule type, and implement the [RuleMeta] trait for it. @@ -235,6 +237,7 @@ impl Rule for BanDropColumn { ### Document the rule The documentation needs to adhere to the following rules: + - The **first** paragraph of the documentation is used as brief description of the rule, and it **must** be written in one single line. Breaking the paragraph in multiple lines will break the table content of the rules page. - The next paragraphs can be used to further document the rule with as many details as you see fit. - The documentation must have a `## Examples` header, followed by two headers: `### Invalid` and `### Valid`. `### Invalid` must go first because we need to show when the rule is triggered. @@ -246,7 +249,7 @@ The documentation needs to adhere to the following rules: Here's an example of how the documentation could look like: -```rust +````rust declare_lint_rule! { /// Dropping a column may break existing clients. /// @@ -269,12 +272,26 @@ declare_lint_rule! { sources: &[RuleSource::Squawk("ban-drop-column")], } } -``` +```` This will cause the documentation generator to ensure the rule does emit exactly one diagnostic for this code, and to include a snapshot for the diagnostic in the resulting documentation page. +### Testing the Rule + +#### Quick Test + +To quickly test your rule, head to the `pg_analyser/src/lib.rs` file and modify the `debug_test` function. + +You should: + +- remove the `#[ignore]` macro if present +- change the content of the `SQL` static `&str` to whatever you need +- pass your group and rule to the `RuleFilter::Rule(..)` + +If you run the test, you'll see any diagnostics your rule created in your console. + ### Code generation For simplicity, use `just` to run all the commands with: @@ -294,7 +311,6 @@ Stage and commit your changes: > git commit -m 'feat(pg_analyser): myRuleName' ``` - ### Deprecate a rule There are occasions when a rule must be deprecated, to avoid breaking changes. The reason @@ -302,7 +318,7 @@ of deprecation can be multiple. In order to do, the macro allows adding additional field to add the reason for deprecation -```rust +````rust use pg_analyse::declare_lint_rule; declare_lint_rule! { @@ -328,5 +344,4 @@ declare_lint_rule! { sources: &[RuleSource::Squawk("ban-drop-column")], } } -``` - +```` diff --git a/crates/pg_analyser/Cargo.toml b/crates/pg_analyser/Cargo.toml index 6e3ce4c5..70b7c750 100644 --- a/crates/pg_analyser/Cargo.toml +++ b/crates/pg_analyser/Cargo.toml @@ -16,3 +16,8 @@ pg_analyse = { workspace = true } pg_console = { workspace = true } pg_query_ext = { workspace = true } serde = { workspace = true } + +[dev-dependencies] +termcolor = { workspace = true } +pg_query = { workspace = true} +pg_diagnostics = { workspace = true} \ No newline at end of file diff --git a/crates/pg_analyser/src/lib.rs b/crates/pg_analyser/src/lib.rs index d6c14d62..9d74d483 100644 --- a/crates/pg_analyser/src/lib.rs +++ b/crates/pg_analyser/src/lib.rs @@ -65,3 +65,61 @@ impl<'a> Analyser<'a> { .collect::>() } } + +#[cfg(test)] +mod tests { + use core::slice; + + use pg_analyse::{AnalyserOptions, AnalysisFilter, RuleFilter}; + use pg_console::{ + fmt::{Formatter, Termcolor}, + markup, Markup, + }; + use pg_diagnostics::PrintDiagnostic; + use termcolor::NoColor; + + use crate::Analyser; + + #[ignore] + #[test] + fn debug_test() { + fn markup_to_string(markup: Markup) -> String { + let mut buffer = Vec::new(); + let mut write = Termcolor(NoColor::new(&mut buffer)); + let mut fmt = Formatter::new(&mut write); + fmt.write_markup(markup).unwrap(); + + String::from_utf8(buffer).unwrap() + } + + const SQL: &str = r#"alter table test drop column id;"#; + let rule_filter = RuleFilter::Rule("safety", "banDropColumn"); + + let filter = AnalysisFilter { + enabled_rules: Some(slice::from_ref(&rule_filter)), + ..Default::default() + }; + + let ast = pg_query_ext::parse(SQL).expect("failed to parse SQL"); + + let options = AnalyserOptions::default(); + + let analyser = Analyser::new(crate::AnalyserConfig { + options: &options, + filter, + }); + + let results = analyser.run(crate::AnalyserContext { root: &ast }); + + println!("*******************"); + for result in &results { + let text = markup_to_string(markup! { + {PrintDiagnostic::simple(result)} + }); + eprintln!("{}", text); + } + println!("*******************"); + + // assert_eq!(results, vec![]); + } +} diff --git a/crates/pg_diagnostics/src/advice.rs b/crates/pg_diagnostics/src/advice.rs index 1488ed81..c8d6e485 100644 --- a/crates/pg_diagnostics/src/advice.rs +++ b/crates/pg_diagnostics/src/advice.rs @@ -195,7 +195,7 @@ where } } -#[derive(Debug)] +#[derive(Debug, PartialEq)] /// Utility type implementing [Advices] that emits a /// code suggestion with the provided text pub struct CodeSuggestionAdvice { diff --git a/crates/pg_diagnostics/src/display/message.rs b/crates/pg_diagnostics/src/display/message.rs index 460f7038..5141e111 100644 --- a/crates/pg_diagnostics/src/display/message.rs +++ b/crates/pg_diagnostics/src/display/message.rs @@ -18,7 +18,7 @@ use termcolor::NoColor; /// message: MessageAndDescription /// } /// ``` -#[derive(Clone, Deserialize, Serialize)] +#[derive(Clone, Deserialize, Serialize, PartialEq)] pub struct MessageAndDescription { /// Shown when medium supports custom markup message: MarkupBuf, From b33c3f5a1b9d9d2d0a05db2024d1b655cf13b210 Mon Sep 17 00:00:00 2001 From: Julian Date: Mon, 10 Feb 2025 10:08:12 +0100 Subject: [PATCH 2/6] format --- Cargo.lock | 1 - crates/pg_analyser/Cargo.toml | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 48303ff9..fd7df859 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2214,7 +2214,6 @@ dependencies = [ "pg_analyse", "pg_console", "pg_diagnostics", - "pg_query", "pg_query_ext", "serde", "termcolor", diff --git a/crates/pg_analyser/Cargo.toml b/crates/pg_analyser/Cargo.toml index 70b7c750..43c56cdb 100644 --- a/crates/pg_analyser/Cargo.toml +++ b/crates/pg_analyser/Cargo.toml @@ -18,6 +18,5 @@ pg_query_ext = { workspace = true } serde = { workspace = true } [dev-dependencies] -termcolor = { workspace = true } -pg_query = { workspace = true} -pg_diagnostics = { workspace = true} \ No newline at end of file +pg_diagnostics = { workspace = true } +termcolor = { workspace = true } From bba83777449e485ac12d16afc15ab3d3c24f183f Mon Sep 17 00:00:00 2001 From: Julian Date: Wed, 12 Feb 2025 09:07:55 +0100 Subject: [PATCH 3/6] ok? --- Cargo.lock | 265 ----------------------------------------------------- 1 file changed, 265 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ef5274a2..7f0be321 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2191,271 +2191,6 @@ dependencies = [ ] [[package]] -<<<<<<< HEAD -name = "pg_analyse" -version = "0.0.0" -dependencies = [ - "biome_deserialize", - "biome_deserialize_macros", - "enumflags2", - "pg_console", - "pg_diagnostics", - "pg_query_ext", - "pg_schema_cache", - "rustc-hash 2.1.0", - "schemars", - "serde", - "text-size", -] - -[[package]] -name = "pg_analyser" -version = "0.0.0" -dependencies = [ - "pg_analyse", - "pg_console", - "pg_diagnostics", - "pg_query_ext", - "serde", - "termcolor", -] - -[[package]] -name = "pg_base_db" -version = "0.0.0" -dependencies = [ - "line_index", - "pg_fs", - "pg_statement_splitter", - "text-size", -] - -[[package]] -name = "pg_cli" -version = "0.0.0" -dependencies = [ - "anyhow", - "biome_deserialize", - "biome_deserialize_macros", - "bpaf", - "crossbeam", - "dashmap 5.5.3", - "hdrhistogram", - "libc", - "mimalloc", - "path-absolutize", - "pg_analyse", - "pg_configuration", - "pg_console", - "pg_diagnostics", - "pg_flags", - "pg_fs", - "pg_lsp", - "pg_text_edit", - "pg_workspace", - "quick-junit", - "rayon", - "rustc-hash 2.1.0", - "serde", - "serde_json", - "tikv-jemallocator", - "tokio", - "tracing", - "tracing-appender", - "tracing-subscriber", - "tracing-tree", -] - -[[package]] -name = "pg_commands" -version = "0.0.0" -dependencies = [ - "anyhow", - "async-std", - "sqlx", - "text-size", -] - -[[package]] -name = "pg_completions" -version = "0.0.0" -dependencies = [ - "async-std", - "pg_schema_cache", - "pg_test_utils", - "pg_treesitter_queries", - "serde", - "serde_json", - "sqlx", - "text-size", - "tokio", - "tree-sitter", - "tree_sitter_sql", -] - -[[package]] -name = "pg_configuration" -version = "0.0.0" -dependencies = [ - "biome_deserialize", - "biome_deserialize_macros", - "bpaf", - "pg_analyse", - "pg_analyser", - "pg_console", - "pg_diagnostics", - "rustc-hash 2.1.0", - "schemars", - "serde", - "serde_json", - "text-size", - "toml", -] - -[[package]] -name = "pg_console" -version = "0.0.0" -dependencies = [ - "pg_markup", - "schemars", - "serde", - "termcolor", - "text-size", - "trybuild", - "unicode-segmentation", - "unicode-width", -] - -[[package]] -name = "pg_diagnostics" -version = "0.0.0" -dependencies = [ - "backtrace", - "bpaf", - "enumflags2", - "pg_console", - "pg_diagnostics_categories", - "pg_diagnostics_macros", - "pg_text_edit", - "schemars", - "serde", - "serde_json", - "termcolor", - "text-size", - "unicode-width", -] - -[[package]] -name = "pg_diagnostics_categories" -version = "0.0.0" -dependencies = [ - "quote", - "schemars", - "serde", -] - -[[package]] -name = "pg_diagnostics_macros" -version = "0.0.0" -dependencies = [ - "proc-macro-error", - "proc-macro2", - "quote", - "syn 1.0.109", -] - -[[package]] -name = "pg_flags" -version = "0.0.0" -dependencies = [ - "pg_console", -] - -[[package]] -name = "pg_fs" -version = "0.0.0" -dependencies = [ - "crossbeam", - "directories", - "enumflags2", - "parking_lot", - "pg_diagnostics", - "rayon", - "rustc-hash 2.1.0", - "schemars", - "serde", - "smallvec", - "tracing", -] - -[[package]] -name = "pg_lexer" -version = "0.0.0" -dependencies = [ - "cstree", - "insta", - "pg_lexer_codegen", - "pg_query", - "regex", - "text-size", -] - -[[package]] -name = "pg_lexer_codegen" -version = "0.0.0" -dependencies = [ - "pg_query_proto_parser", - "proc-macro2", - "quote", -] - -[[package]] -name = "pg_lsp" -version = "0.0.0" -dependencies = [ - "anyhow", - "biome_deserialize", - "futures", - "pg_analyse", - "pg_completions", - "pg_configuration", - "pg_console", - "pg_diagnostics", - "pg_fs", - "pg_lsp_converters", - "pg_text_edit", - "pg_workspace", - "rustc-hash 2.1.0", - "serde", - "serde_json", - "text-size", - "tokio", - "tower-lsp", - "tracing", -] - -[[package]] -name = "pg_lsp_converters" -version = "0.0.0" -dependencies = [ - "anyhow", - "rustc-hash 2.1.0", - "text-size", - "tower-lsp", -] - -[[package]] -name = "pg_markup" -version = "0.0.0" -dependencies = [ - "proc-macro-error", - "proc-macro2", - "quote", -] - -[[package]] -======= ->>>>>>> main name = "pg_query" version = "6.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" From 253e1e0c3d008ddd9014c223327be7b3b54ac208 Mon Sep 17 00:00:00 2001 From: Julian Date: Wed, 12 Feb 2025 09:10:23 +0100 Subject: [PATCH 4/6] resolved? --- Cargo.lock | 2 ++ crates/pglt_analyser/Cargo.toml | 4 ++++ crates/pglt_analyser/src/lib.rs | 8 ++++---- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7f0be321..b133b902 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2231,8 +2231,10 @@ version = "0.0.0" dependencies = [ "pglt_analyse", "pglt_console", + "pglt_diagnostics", "pglt_query_ext", "serde", + "termcolor", ] [[package]] diff --git a/crates/pglt_analyser/Cargo.toml b/crates/pglt_analyser/Cargo.toml index f235bcc3..c8f29619 100644 --- a/crates/pglt_analyser/Cargo.toml +++ b/crates/pglt_analyser/Cargo.toml @@ -16,3 +16,7 @@ pglt_analyse = { workspace = true } pglt_console = { workspace = true } pglt_query_ext = { workspace = true } serde = { workspace = true } + +[dev-dependencies] +pglt_diagnostics = { workspace = true } +termcolor ={ workspace = true } \ No newline at end of file diff --git a/crates/pglt_analyser/src/lib.rs b/crates/pglt_analyser/src/lib.rs index 12214a6b..9311ca54 100644 --- a/crates/pglt_analyser/src/lib.rs +++ b/crates/pglt_analyser/src/lib.rs @@ -70,12 +70,12 @@ impl<'a> Analyser<'a> { mod tests { use core::slice; - use pg_analyse::{AnalyserOptions, AnalysisFilter, RuleFilter}; - use pg_console::{ + use pglt_analyse::{AnalyserOptions, AnalysisFilter, RuleFilter}; + use pglt_console::{ fmt::{Formatter, Termcolor}, markup, Markup, }; - use pg_diagnostics::PrintDiagnostic; + use pglt_diagnostics::PrintDiagnostic; use termcolor::NoColor; use crate::Analyser; @@ -100,7 +100,7 @@ mod tests { ..Default::default() }; - let ast = pg_query_ext::parse(SQL).expect("failed to parse SQL"); + let ast = pglt_query_ext::parse(SQL).expect("failed to parse SQL"); let options = AnalyserOptions::default(); From 478bf656de448054d402db07f2c061f07c16cdc2 Mon Sep 17 00:00:00 2001 From: Julian Date: Wed, 12 Feb 2025 09:11:15 +0100 Subject: [PATCH 5/6] docs --- crates/pglt_analyser/CONTRIBUTING.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/crates/pglt_analyser/CONTRIBUTING.md b/crates/pglt_analyser/CONTRIBUTING.md index 2a746221..88f6f287 100644 --- a/crates/pglt_analyser/CONTRIBUTING.md +++ b/crates/pglt_analyser/CONTRIBUTING.md @@ -284,7 +284,7 @@ diagnostic in the resulting documentation page. #### Quick Test -To quickly test your rule, head to the `pg_analyser/src/lib.rs` file and modify the `debug_test` function. +To quickly test your rule, head to the `pglt_analyser/src/lib.rs` file and modify the `debug_test` function. You should: @@ -321,11 +321,7 @@ of deprecation can be multiple. In order to do, the macro allows adding additional field to add the reason for deprecation ````rust -<<<<<<< HEAD:crates/pg_analyser/CONTRIBUTING.md -use pg_analyse::declare_lint_rule; -======= use pglt_analyse::declare_lint_rule; ->>>>>>> main:crates/pglt_analyser/CONTRIBUTING.md declare_lint_rule! { /// Dropping a column may break existing clients. From 12dae21f3cc02e07e70e65ec045e09e4656dec09 Mon Sep 17 00:00:00 2001 From: Julian Date: Wed, 12 Feb 2025 09:17:28 +0100 Subject: [PATCH 6/6] format --- crates/pglt_analyser/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/pglt_analyser/Cargo.toml b/crates/pglt_analyser/Cargo.toml index c8f29619..e9de97f9 100644 --- a/crates/pglt_analyser/Cargo.toml +++ b/crates/pglt_analyser/Cargo.toml @@ -19,4 +19,4 @@ serde = { workspace = true } [dev-dependencies] pglt_diagnostics = { workspace = true } -termcolor ={ workspace = true } \ No newline at end of file +termcolor = { workspace = true }