From 2837477e3d8a6c85fcafeccd60c7414d1355bba5 Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Sat, 10 May 2025 16:49:25 -0400 Subject: [PATCH 1/2] linter: improve prefer-robust-stmts error messages also update internal vscode extension readme and fix typo in playground linter source name `tea` instead of `squawk` --- .vscode/extensions/squawk-dev/README.md | 28 +++++++++++ .../src/rules/prefer_robust_stmts.rs | 46 +++++++++++++++++-- ...tmts__test__alter_column_set_not_null.snap | 12 +++++ ...ts__test__alter_table_drop_column_err.snap | 2 +- ...test__alter_table_drop_constraint_err.snap | 2 +- ..._test__disable_row_level_security_err.snap | 2 +- ...tmts__test__double_add_after_drop_err.snap | 2 +- ...__test__enable_row_level_security_err.snap | 2 +- ...vel_security_without_exists_check_err.snap | 2 +- playground/src/App.tsx | 4 +- 10 files changed, 89 insertions(+), 13 deletions(-) create mode 100644 crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_column_set_not_null.snap diff --git a/.vscode/extensions/squawk-dev/README.md b/.vscode/extensions/squawk-dev/README.md index e1e9be27..abd71788 100644 --- a/.vscode/extensions/squawk-dev/README.md +++ b/.vscode/extensions/squawk-dev/README.md @@ -2,6 +2,34 @@ Extension to make developing squawk a little nicer. +## Install + +```shell + +``` + +1. install deps and build + +```sh +cd .vscode/extensions/squawk-dev +pnpm install +pnpm run compile +``` + +2. enable extension in vscode + +In the command palette run: + +``` +Extensions: Show Recommended Extensions +``` + +You should then see the `recipeyak-dev` extension in the results. + +Press the `Install Workspace Extension` button. + +Done! + ## Features ### Jump from snapshot to source file diff --git a/crates/squawk_linter/src/rules/prefer_robust_stmts.rs b/crates/squawk_linter/src/rules/prefer_robust_stmts.rs index dda83490..04e645c9 100644 --- a/crates/squawk_linter/src/rules/prefer_robust_stmts.rs +++ b/crates/squawk_linter/src/rules/prefer_robust_stmts.rs @@ -30,6 +30,12 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { return; } + enum ActionErrorMessage { + IfExists, + IfNotExists, + None, + } + for item in file.items() { match item { ast::Item::Begin(_) => { @@ -40,7 +46,7 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { } ast::Item::AlterTable(alter_table) => { for action in alter_table.actions() { - match &action { + let message_type = match &action { ast::AlterTableAction::DropConstraint(drop_constraint) => { if let Some(constraint_name) = drop_constraint.name_ref() { constraint_names.insert( @@ -51,11 +57,13 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { if drop_constraint.if_exists().is_some() { continue; } + ActionErrorMessage::IfExists } ast::AlterTableAction::AddColumn(add_column) => { if add_column.if_not_exists().is_some() { continue; } + ActionErrorMessage::IfNotExists } ast::AlterTableAction::ValidateConstraint(validate_constraint) => { if let Some(constraint_name) = validate_constraint.name_ref() { @@ -65,6 +73,7 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { continue; } } + ActionErrorMessage::None } ast::AlterTableAction::AddConstraint(add_constraint) => { let constraint = add_constraint.constraint(); @@ -78,22 +87,36 @@ pub(crate) fn prefer_robust_stmts(ctx: &mut Linter, parse: &Parse) { } } } + ActionErrorMessage::None } ast::AlterTableAction::DropColumn(drop_column) => { if drop_column.if_exists().is_some() { continue; } + ActionErrorMessage::IfExists } - _ => (), - } + _ => ActionErrorMessage::None, + }; if inside_transaction { continue; } + let message = match message_type { + ActionErrorMessage::IfExists => { + "Missing `IF EXISTS`, the migration can't be rerun if it fails part way through.".to_string() + }, + ActionErrorMessage::IfNotExists => { + "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.".to_string() + }, + ActionErrorMessage::None => { + "Missing transaction, the migration can't be rerun if it fails part way through.".to_string() + }, + }; + ctx.report(Violation::new( Rule::PreferRobustStmts, - "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.".into(), + message, action.syntax().text_range(), None, )); @@ -547,7 +570,20 @@ ALTER TABLE IF EXISTS test DISABLE ROW LEVEL SECURITY; ALTER TABLE "app_email" DROP CONSTRAINT IF EXISTS "email_uniq"; ALTER TABLE "app_email" ADD CONSTRAINT "email_uniq" UNIQUE USING INDEX "email_idx"; -- this second add constraint should error because it's not robust -ALTER TABLE "app_email" ADD CONSTRAINT "email_uniq" UNIQUE USING INDEX "email_idx"; +ALTER TABLE "app_email" ADD CONSTRAIN "email_uniq" UNIQUE USING INDEX "email_idx"; + "#; + let file = squawk_syntax::SourceFile::parse(sql); + let mut linter = Linter::from([Rule::PreferRobustStmts]); + let errors = linter.lint(file, sql); + assert_ne!(errors.len(), 0); + assert_debug_snapshot!(errors); + } + + #[test] + fn alter_column_set_not_null() { + let sql = r#" +select 1; -- so we don't skip checking +alter table t alter column c set not null; "#; let file = squawk_syntax::SourceFile::parse(sql); let mut linter = Linter::from([Rule::PreferRobustStmts]); diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_column_set_not_null.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_column_set_not_null.snap new file mode 100644 index 00000000..4380370a --- /dev/null +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_column_set_not_null.snap @@ -0,0 +1,12 @@ +--- +source: crates/squawk_linter/src/rules/prefer_robust_stmts.rs +expression: errors +--- +[ + Violation { + code: PreferRobustStmts, + message: "Missing transaction, the migration can't be rerun if it fails part way through.", + text_range: 54..81, + help: None, + }, +] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_column_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_column_err.snap index e18b8c61..acc9f26a 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_column_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_column_err.snap @@ -5,7 +5,7 @@ expression: errors [ Violation { code: PreferRobustStmts, - message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", + message: "Missing `IF EXISTS`, the migration can't be rerun if it fails part way through.", text_range: 54..75, help: None, }, diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_constraint_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_constraint_err.snap index 57b0af67..35e6946a 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_constraint_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__alter_table_drop_constraint_err.snap @@ -5,7 +5,7 @@ expression: errors [ Violation { code: PreferRobustStmts, - message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", + message: "Missing `IF EXISTS`, the migration can't be rerun if it fails part way through.", text_range: 63..93, help: None, }, diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__disable_row_level_security_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__disable_row_level_security_err.snap index 1bb33be7..d0947f39 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__disable_row_level_security_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__disable_row_level_security_err.snap @@ -5,7 +5,7 @@ expression: errors [ Violation { code: PreferRobustStmts, - message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", + message: "Missing transaction, the migration can't be rerun if it fails part way through.", text_range: 63..89, help: None, }, diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__double_add_after_drop_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__double_add_after_drop_err.snap index 0b837fb7..c3188e6e 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__double_add_after_drop_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__double_add_after_drop_err.snap @@ -6,7 +6,7 @@ expression: errors Violation { code: PreferRobustStmts, message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", - text_range: 240..298, + text_range: 240..297, help: None, }, ] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_err.snap index e4608142..767b4305 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_err.snap @@ -5,7 +5,7 @@ expression: errors [ Violation { code: PreferRobustStmts, - message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", + message: "Missing transaction, the migration can't be rerun if it fails part way through.", text_range: 63..88, help: None, }, diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_without_exists_check_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_without_exists_check_err.snap index 8d1a1e8c..a2272c37 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_without_exists_check_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__prefer_robust_stmts__test__enable_row_level_security_without_exists_check_err.snap @@ -5,7 +5,7 @@ expression: errors [ Violation { code: PreferRobustStmts, - message: "Missing `IF NOT EXISTS`, the migration can't be rerun if it fails part way through.", + message: "Missing transaction, the migration can't be rerun if it fails part way through.", text_range: 53..78, help: None, }, diff --git a/playground/src/App.tsx b/playground/src/App.tsx index 7cc03f80..86e2bf7a 100644 --- a/playground/src/App.tsx +++ b/playground/src/App.tsx @@ -246,7 +246,7 @@ function Editor({ } const model = editorRef.current?.getModel() if (model != null) { - monaco.editor.setModelMarkers(model, "tea", markers) + monaco.editor.setModelMarkers(model, "squawk", markers) } }, [markers]) @@ -375,7 +375,7 @@ function useMarkers(text: string): Array { // https://github.com/microsoft/monaco-editor/issues/1264 // https://stackoverflow.com/questions/62362741/using-markdown-in-imarkerdata message: x.message, - source: "tea", + source: "squawk", } }) } From d98947816fa1a632a52cd3a3913a595ea2a752fe Mon Sep 17 00:00:00 2001 From: Steve Dignam Date: Sat, 10 May 2025 16:50:03 -0400 Subject: [PATCH 2/2] fix --- .vscode/extensions/squawk-dev/README.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.vscode/extensions/squawk-dev/README.md b/.vscode/extensions/squawk-dev/README.md index abd71788..9d9815b7 100644 --- a/.vscode/extensions/squawk-dev/README.md +++ b/.vscode/extensions/squawk-dev/README.md @@ -4,10 +4,6 @@ Extension to make developing squawk a little nicer. ## Install -```shell - -``` - 1. install deps and build ```sh