cli: add rendering of diffs for fix suggestions (#662)

Also update a change to CI that I don't think is necessary.

Author: sbdchd

.github/workflows/rust.yml

@@ -20,16 +20,6 @@ jobs:
         with:
           paths_ignore: '["docs/**", "*.md"]'
 
-  lint_pre_job:
-    runs-on: ubuntu-22.04
-    outputs:
-      should_skip_lint: ${{ steps.skip_check_lint.outputs.should_skip }}
-    steps:
-      - id: skip_check_lint
-        uses: fkirc/skip-duplicate-actions@c449d86cf33a2a6c7a4193264cc2578e2c3266d4 # pin@v4
-        with:
-          paths: '["**/*.rs", "**/Cargo.toml", "Cargo.lock", "s/lint"]'
-
   build:
     needs: pre_job
     if: needs.pre_job.outputs.should_skip != 'true' || startsWith(github.ref, 'refs/tags/')
@@ -283,8 +273,8 @@ jobs:
           NETLIFY_SITE_ID: ${{ secrets.PLAYGROUND_NETLIFY_SITE_ID }}
 
   lint:
-    needs: lint_pre_job
-    if: needs.lint_pre_job.outputs.should_skip_lint != 'true'
+    needs: pre_job
+    if: needs.pre_job.outputs.should_skip != 'true'
 
     runs-on: ubuntu-22.04
 

Cargo.lock

@@ -51,6 +51,7 @@ ungrammar = "1.1.4"
 quote = "1.0.40"
 xshell = "0.2.7"
 proc-macro2 = "1.0.95"
+snapbox = { version = "0.6.0", features = ["diff", "term-svg", "cmd"] }
 
 # local
 # we have to make the versions explicit otherwise `cargo publish` won't work

Cargo.toml

@@ -50,28 +50,42 @@ warning[prefer-bigint-over-int]: Using 32-bit integer fields can result in hitti
 6 │     "id" serial NOT NULL PRIMARY KEY,
   │          ━━━━━━
   │
-  ╰ help: Use 64-bit integer values instead to prevent hitting this limit.
+  ├ help: Use 64-bit integer values instead to prevent hitting this limit.
+  ╭╴
+6 │     "id" bigserial NOT NULL PRIMARY KEY,
+  ╰╴         +++
 warning[prefer-identity]: Serial types make schema, dependency, and permission management difficult.
   ╭▸ example.sql:6:10
   │
 6 │     "id" serial NOT NULL PRIMARY KEY,
   │          ━━━━━━
   │
-  ╰ help: Use an `IDENTITY` column instead.
+  ├ help: Use an `IDENTITY` column instead.
+  ╭╴
+6 -     "id" serial NOT NULL PRIMARY KEY,
+6 +     "id" integer generated by default as identity NOT NULL PRIMARY KEY,
+  ╰╴
 warning[prefer-text-field]: Changing the size of a `varchar` field requires an `ACCESS EXCLUSIVE` lock, that will prevent all reads and writes to the table.
   ╭▸ example.sql:7:13
   │
 7 │     "alpha" varchar(100) NOT NULL
   │             ━━━━━━━━━━━━
   │
-  ╰ help: Use a `TEXT` field with a `CHECK` constraint.
+  ├ help: Use a `TEXT` field with a `CHECK` constraint.
+  ╭╴
+7 -     "alpha" varchar(100) NOT NULL
+7 +     "alpha" text NOT NULL
+  ╰╴
 warning[require-concurrent-index-creation]: During normal index creation, table updates are blocked, but reads are still allowed.
    ╭▸ example.sql:10:1
    │
 10 │ CREATE INDEX "field_name_idx" ON "table_name" ("field_name");
    │ ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
    │
-   ╰ help: Use `concurrently` to avoid blocking writes.
+   ├ help: Use `concurrently` to avoid blocking writes.
+   ╭╴
+10 │ CREATE INDEX concurrently "field_name_idx" ON "table_name" ("field_name");
+   ╰╴             ++++++++++++
 warning[constraint-missing-not-valid]: By default new constraints require a table scan and block writes to the table while that scan occurs.
    ╭▸ example.sql:12:24
    │

README.md

@@ -40,6 +40,7 @@ lsp-types.workspace = true
 
 [dev-dependencies]
 insta.workspace = true
+snapbox.workspace = true
 
 [lints]
 workspace = true

crates/squawk/Cargo.toml

@@ -394,6 +394,7 @@ mod test_github_comment {
                 help: Some("Make the field nullable.".to_string()),
                 column_end: 9,
                 line_end: 1,
+                fix: None,
             }],
         }];
 
@@ -486,6 +487,7 @@ ALTER TABLE "core_recipe" ADD COLUMN "foo" integer DEFAULT 10;
                 help: Some("Use bigint instead.".to_string()),
                 column_end: 0,
                 line_end: 1,
+                fix: None,
             }],
         }];
 
@@ -520,6 +522,7 @@ ALTER TABLE "core_recipe" ADD COLUMN "foo" integer DEFAULT 10;
                 help: Some("Use bigint instead.".to_string()),
                 column_end: 0,
                 line_end: 1,
+                fix: None,
             }],
         }];
 

crates/squawk/src/github.rs

@@ -1,13 +1,11 @@
-use annotate_snippets::{AnnotationKind, Level, Renderer, Snippet, renderer::DecorStyle};
+use annotate_snippets::{AnnotationKind, Level, Patch, Renderer, Snippet, renderer::DecorStyle};
 use anyhow::Result;
 use console::style;
 use line_index::LineIndex;
 use line_index::TextRange;
 use log::info;
 use serde::Serialize;
-use squawk_linter::Linter;
-use squawk_linter::Rule;
-use squawk_linter::Version;
+use squawk_linter::{Fix, Linter, Rule, Version};
 use squawk_syntax::SourceFile;
 use std::hash::DefaultHasher;
 use std::hash::Hash;
@@ -56,6 +54,7 @@ fn check_sql(
             range: e.range(),
             message: e.message().to_string(),
             rule_name: "syntax-error".to_string(),
+            fix: None,
         })
     }
     for e in errors {
@@ -74,6 +73,7 @@ fn check_sql(
             level: ViolationLevel::Warning,
             message: e.message,
             rule_name: e.code.to_string(),
+            fix: e.fix,
         })
     }
 
@@ -100,17 +100,30 @@ fn render_lint_error<W: std::io::Write>(
         ViolationLevel::Error => Level::ERROR,
     };
 
-    let mut group = level.primary_title(title).id(error_name).element(
-        Snippet::source(sql)
-            .path(filename)
-            .fold(true)
-            .annotation(AnnotationKind::Primary.span(err.range.into())),
-    );
+    let snippet = Snippet::source(sql)
+        .path(filename)
+        .fold(true)
+        .annotation(AnnotationKind::Primary.span(err.range.into()));
+
+    let mut group = level.primary_title(title).id(error_name).element(snippet);
 
     if let Some(help) = &err.help {
         group = group.element(Level::HELP.message(help));
     }
 
+    if let Some(fix) = &err.fix {
+        let mut patch_snippet = Snippet::source(sql).path(filename).fold(true);
+
+        for edit in &fix.edits {
+            let start: usize = edit.text_range.start().into();
+            let end: usize = edit.text_range.end().into();
+            let replacement = edit.text.as_deref().unwrap_or("");
+            patch_snippet = patch_snippet.patch(Patch::new(start..end, replacement));
+        }
+
+        group = group.element(patch_snippet);
+    }
+
     writeln!(f, "{}", renderer.render(&[group]))?;
     Ok(())
 }
@@ -286,6 +299,8 @@ pub struct ReportViolation {
     pub rule_name: String,
     pub column_end: usize,
     pub line_end: usize,
+    #[serde(skip_serializing)]
+    pub fix: Option<Fix>,
 }
 
 fn fmt_gcc<W: io::Write>(f: &mut W, reports: &[CheckReport]) -> Result<()> {