refactor: make github native a fomatter

crates/cli/Cargo.toml

@@ -32,6 +32,10 @@ harness = false
 name = "ui_with_python"
 harness = false
 
+[[test]]
+name = "ui_github"
+harness = false
+
 [features]
 python = ["sqruff-lib/python"]
 codegen-docs = ["clap-markdown", "minijinja", "serde", "python"]

crates/cli/src/commands_lint.rs

@@ -10,29 +10,8 @@ pub(crate) fn run_lint(
 ) -> i32 {
     let LintArgs { paths, format } = args;
     let mut linter = linter(config, format);
-    let result = linter.lint_paths(paths, false, &ignorer);
-    let count: usize = result.paths.iter().map(|path| path.files.len()).sum();
+    linter.lint_paths(paths, false, &ignorer);
 
-    // TODO this should be cleaned up better
-    if matches!(format, Format::GithubAnnotationNative) {
-        for path in result.paths {
-            for file in path.files {
-                for violation in file.violations {
-                    let line = format!(
-                        "::error title=sqruff,file={},line={},col={}::{}: {}",
-                        file.path,
-                        violation.line_no,
-                        violation.line_pos,
-                        violation.rule.as_ref().unwrap().code,
-                        violation.description
-                    );
-                    eprintln!("{line}");
-                }
-            }
-        }
-    }
-
-    eprintln!("The linter processed {count} file(s).");
     linter.formatter().unwrap().completion_message();
     if linter.formatter().unwrap().has_fail() {
         1
@@ -45,13 +24,13 @@ pub(crate) fn run_lint_stdin(config: FluffConfig, format: Format) -> i32 {
     let read_in = crate::stdin::read_std_in().unwrap();
 
     let linter = linter(config, format);
-    let result = linter.lint_string(&read_in, None, false);
+    linter.lint_string(&read_in, None, false);
 
     linter.formatter().unwrap().completion_message();
 
-    if result.get_violations(None).is_empty() {
-        0
-    } else {
+    if linter.formatter().unwrap().has_fail() {
         1
+    } else {
+        0
     }
 }

crates/cli/src/main.rs

@@ -1,6 +1,10 @@
 use clap::Parser as _;
 use commands::Format;
-use sqruff_lib::cli::formatters::OutputStreamFormatter;
+use sqruff_lib::cli::formatters::Formatter;
+use sqruff_lib::cli::{
+    formatters::OutputStreamFormatter,
+    github_annotation_native_formatter::GithubAnnotationNativeFormatter,
+};
 use sqruff_lib::core::config::FluffConfig;
 use sqruff_lib::core::linter::core::Linter;
 use std::path::Path;
@@ -85,17 +89,23 @@ fn main() {
 }
 
 pub(crate) fn linter(config: FluffConfig, format: Format) -> Linter {
-    let output_stream = match format {
-        Format::Human => std::io::stderr().into(),
-        Format::GithubAnnotationNative => None,
+    let formatter: Arc<dyn Formatter> = match format {
+        Format::Human => {
+            let output_stream = std::io::stderr().into();
+            let formatter = OutputStreamFormatter::new(
+                output_stream,
+                config.get("nocolor", "core").as_bool().unwrap_or_default(),
+            );
+            Arc::new(formatter)
+        }
+        Format::GithubAnnotationNative => {
+            let output_stream = std::io::stderr();
+            let formatter = GithubAnnotationNativeFormatter::new(output_stream);
+            Arc::new(formatter)
+        }
     };
 
-    let formatter = OutputStreamFormatter::new(
-        output_stream,
-        config.get("nocolor", "core").as_bool().unwrap_or_default(),
-    );
-
-    Linter::new(config, Some(Arc::new(formatter)), None)
+    Linter::new(config, Some(formatter), None)
 }
 
 fn check_user_input() -> Option<bool> {

crates/cli/tests/github/LT01_noqa.exitcode

@@ -0,0 +1 @@
+0

crates/cli/tests/github/LT01_noqa.sql

@@ -0,0 +1 @@
+SELECT   1; --noqa: LT01

crates/cli/tests/github/hql_file.exitcode

@@ -0,0 +1 @@
+1

crates/cli/tests/github/hql_file.hql

@@ -0,0 +1 @@
+SELECT   1;

crates/cli/tests/github/hql_file.stderr

@@ -0,0 +1,2 @@
+::error title=sqruff,file=tests/lint/hql_file.hql,line=1,col=7::LT01: Expected only single space before "1". Found "   ".
+::error title=sqruff,file=tests/lint/hql_file.hql,line=1,col=11::LT12: Files must end with a single trailing newline.

crates/cli/tests/github/test_fail_whitespace_before_comma.exitcode

@@ -0,0 +1 @@
+1

crates/cli/tests/github/test_fail_whitespace_before_comma.sql

@@ -0,0 +1 @@
+SELECT 1 ,4

crates/cli/tests/github/test_fail_whitespace_before_comma.stderr

@@ -0,0 +1,5 @@
+::error title=sqruff,file=tests/lint/test_fail_whitespace_before_comma.sql,line=1,col=8::AL03: Column expression without alias. Use explicit `AS` clause.
+::error title=sqruff,file=tests/lint/test_fail_whitespace_before_comma.sql,line=1,col=9::LT01: Unexpected whitespace before comma.
+::error title=sqruff,file=tests/lint/test_fail_whitespace_before_comma.sql,line=1,col=11::AL03: Column expression without alias. Use explicit `AS` clause.
+::error title=sqruff,file=tests/lint/test_fail_whitespace_before_comma.sql,line=1,col=11::LT01: Expected single whitespace between "," and "4".
+::error title=sqruff,file=tests/lint/test_fail_whitespace_before_comma.sql,line=1,col=12::LT12: Files must end with a single trailing newline.

crates/cli/tests/stdin/stdin.stderr

@@ -1,4 +1,5 @@
 == [<string>] FAIL
 L:   1 | P:  20 | LT12 | Files must end with a single trailing newline.
                        | [layout.end_of_file]
+The linter processed 1 file(s).
 All Finished

crates/cli/tests/ui_github.rs

@@ -0,0 +1,66 @@
+use std::fs;
+use std::path::PathBuf;
+
+use assert_cmd::Command;
+use expect_test::expect_file;
+
+fn main() {
+    let profile = if cfg!(debug_assertions) {
+        "debug"
+    } else {
+        "release"
+    };
+    let mut lint_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+    lint_dir.push("tests/github");
+
+    // Iterate over each test file in the directory
+    for entry in fs::read_dir(&lint_dir).unwrap() {
+        let entry = entry.unwrap();
+        let path = entry.path();
+
+        // Check if the file has a .sql or .hql extension
+        if path
+            .extension()
+            .and_then(|e| e.to_str())
+            .map_or(false, |ext| ext == "sql" || ext == "hql")
+        {
+            // Construct the path to the sqruff binary
+            let mut sqruff_path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+            sqruff_path.push(format!("../../target/{}/sqruff", profile));
+
+            // Set up the command with arguments
+            let mut cmd = Command::new(sqruff_path);
+
+            cmd.arg("lint");
+            cmd.arg(path.to_str().unwrap());
+            // Set the HOME environment variable to the fake home directory
+            cmd.env("HOME", PathBuf::from(env!("CARGO_MANIFEST_DIR")));
+            cmd.env("GITHUB_ACTIONS", "true");
+
+            // Run the command and capture the output
+            let assert = cmd.assert();
+
+            // Construct the expected output file paths
+            let mut expected_output_path_stderr = path.clone();
+            expected_output_path_stderr.set_extension("stderr");
+            let mut expected_output_path_stdout = path.clone();
+            expected_output_path_stdout.set_extension("stdout");
+            let mut expected_output_path_exitcode = path.clone();
+            expected_output_path_exitcode.set_extension("exitcode");
+
+            // Read the expected output
+            let output = assert.get_output();
+            let stderr_str = std::str::from_utf8(&output.stderr).unwrap();
+            let stdout_str = std::str::from_utf8(&output.stdout).unwrap();
+            let exit_code_str = output.status.code().unwrap().to_string();
+
+            let test_dir_str = lint_dir.to_string_lossy().to_string();
+            let stderr_normalized: String = stderr_str.replace(&test_dir_str, "tests/lint");
+            let stdout_normalized: String = stdout_str.replace(&test_dir_str, "tests/lint");
+
+            expect_file![expected_output_path_stderr].assert_eq(&stderr_normalized);
+            expect_file![expected_output_path_stdout].assert_eq(&stdout_normalized);
+            expect_file![expected_output_path_exitcode].assert_eq(&exit_code_str);
+        }
+    }
+}

crates/lib/src/cli.rs

@@ -1 +1,2 @@
 pub mod formatters;
+pub mod github_annotation_native_formatter;

crates/lib/src/cli/formatters.rs

@@ -1,6 +1,6 @@
 use std::borrow::Cow;
 use std::io::{IsTerminal, Stderr, Write};
-use std::sync::atomic::AtomicBool;
+use std::sync::atomic::{AtomicBool, AtomicUsize};
 
 use anstyle::{AnsiColor, Effects, Style};
 use itertools::enumerate;
@@ -64,6 +64,7 @@ pub struct OutputStreamFormatter {
     verbosity: i32,
     output_line_length: usize,
     pub has_fail: AtomicBool,
+    files_dispatched: AtomicUsize,
 }
 
 impl Formatter for OutputStreamFormatter {
@@ -78,13 +79,21 @@ impl Formatter for OutputStreamFormatter {
         );
 
         self.dispatch(&s);
+        self.files_dispatched
+            .fetch_add(1, std::sync::atomic::Ordering::SeqCst);
     }
 
     fn has_fail(&self) -> bool {
         self.has_fail.load(std::sync::atomic::Ordering::SeqCst)
     }
 
     fn completion_message(&self) {
+        let count = self
+            .files_dispatched
+            .load(std::sync::atomic::Ordering::SeqCst);
+        let message = format!("The linter processed {count} file(s).\n");
+        self.dispatch(&message);
+
         let message = if self.plain_output {
             "All Finished\n"
         } else {
@@ -112,6 +121,7 @@ impl OutputStreamFormatter {
             verbosity: 0,
             output_line_length: 80,
             has_fail: false.into(),
+            files_dispatched: 0.into(),
         }
     }
 

crates/lib/src/cli/github_annotation_native_formatter.rs

@@ -0,0 +1,77 @@
+use crate::core::config::FluffConfig;
+use crate::core::linter::linted_file::LintedFile;
+use std::io::{Stderr, Write};
+use std::sync::atomic::{AtomicBool, Ordering};
+
+use super::formatters::Formatter;
+
+#[derive(Debug)]
+pub struct GithubAnnotationNativeFormatter {
+    output_stream: Stderr,
+    pub has_fail: AtomicBool,
+}
+
+impl GithubAnnotationNativeFormatter {
+    pub fn new(stderr: Stderr) -> Self {
+        Self {
+            output_stream: stderr,
+            has_fail: AtomicBool::new(false),
+        }
+    }
+
+    fn dispatch(&self, s: &str) {
+        let _ignored = self.output_stream.lock().write(s.as_bytes()).unwrap();
+    }
+}
+
+impl Formatter for GithubAnnotationNativeFormatter {
+    fn dispatch_template_header(
+        &self,
+        _f_name: String,
+        _linter_config: FluffConfig,
+        _file_config: FluffConfig,
+    ) {
+        // No-op
+    }
+
+    fn dispatch_parse_header(&self, _f_name: String) {
+        // No-op
+    }
+
+    fn dispatch_file_violations(&self, linted_file: &LintedFile, _only_fixable: bool) {
+        let mut violations = linted_file.get_violations(None);
+
+        violations.sort_by(|a, b| {
+            a.line_no
+                .cmp(&b.line_no)
+                .then_with(|| a.line_pos.cmp(&b.line_pos))
+                .then_with(|| {
+                    let b = b.rule.as_ref().unwrap().code;
+                    a.rule.as_ref().unwrap().code.cmp(b)
+                })
+        });
+
+        for violation in violations {
+            let message = format!(
+                "::error title=sqruff,file={},line={},col={}::{}: {}\n",
+                linted_file.path,
+                violation.line_no,
+                violation.line_pos,
+                violation.rule.as_ref().unwrap().code,
+                violation.description
+            );
+            self.dispatch(&message);
+            if !violation.ignore && !violation.warning {
+                self.has_fail.store(true, Ordering::SeqCst);
+            }
+        }
+    }
+
+    fn has_fail(&self) -> bool {
+        self.has_fail.load(Ordering::SeqCst)
+    }
+
+    fn completion_message(&self) {
+        // No-op
+    }
+}