Skip to content

Commit

Permalink
chore: refactor linting CLI to use direct paths and improve output fo…
Browse files Browse the repository at this point in the history
…rmatting
  • Loading branch information
gvozdvmozgu authored and benfdking committed Mar 1, 2024
1 parent 2dd32d9 commit 6094408
Show file tree
Hide file tree
Showing 10 changed files with 99 additions and 74 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

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

1 change: 0 additions & 1 deletion crates/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ jemalloc = ["jemallocator"]
[dependencies]
sqruff-lib = { version = "0.1.4", path = "../lib" }
clap = { version = "4", features = ["derive"] }
glob = "0.3.1"

[target.'cfg(not(target_env = "msvc"))'.dependencies]
jemallocator = { version = "0.5", package = "tikv-jemallocator", optional = true }
Expand Down
5 changes: 3 additions & 2 deletions crates/cli/src/commands.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::path::PathBuf;

use clap::{Parser, Subcommand};

#[derive(Debug, Parser)]
Expand All @@ -18,8 +20,7 @@ pub enum Commands {

#[derive(Debug, Parser)]
pub struct LintArgs {
/// glob pattern to lint
pub file_path: String,
pub paths: Vec<PathBuf>,
}

#[derive(Debug, Parser)]
Expand Down
71 changes: 12 additions & 59 deletions crates/cli/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
use std::process::exit;

use clap::Parser;
use glob::{glob, Paths};
use sqruff_lib::api::simple::lint_with_formatter;
use clap::Parser as _;
use commands::LintArgs;
use sqruff_lib::cli::formatters::OutputStreamFormatter;
use sqruff_lib::core::config::FluffConfig;
use sqruff_lib::rules::layout;
use sqruff_lib::core::linter::linter::Linter;

use crate::commands::Cli;
use crate::commands::{Cli, Commands};

mod commands;

Expand All @@ -16,62 +13,18 @@ mod commands;
static ALLOC: jemallocator::Jemalloc = jemallocator::Jemalloc;

fn main() {
match main_wrapper() {
Ok(msg) => {
println!("{}", msg);
exit(0)
}
Err(e) => {
eprintln!("{}", e);
exit(1)
}
}
}
let formatter = OutputStreamFormatter::new(Box::new(std::io::stderr()), false);
let config = FluffConfig::default();

// TODO Handle the unwraps better
fn main_wrapper() -> Result<String, String> {
let cli = Cli::parse();
let mut has_errors = false;

let config = FluffConfig::from_root(None, false, None).unwrap();
let mut linter = Linter::new(config, formatter.into(), None);

match cli.command {
commands::Commands::Lint(lint_args) => {
let files = find_files(&lint_args.file_path)?;

let mut count = 0;
for file in files {
let formatter = OutputStreamFormatter::new(Box::new(std::io::stderr()), false);

let file = file.unwrap();
let file = file.to_str().unwrap();
let contents = std::fs::read_to_string(file).unwrap();
let linted = lint_with_formatter(
contents,
// TODO Make this a pointer
DEFAULT_DIALECT.to_string(),
layout::get_rules().into(),
None,
None,
formatter.into(),
)
.map_err(|e| format!("Error linting file '{}': {:?}", file, e))?;
if !linted.is_empty() {
has_errors = true;
}
count += 1;
}
}
commands::Commands::Fix(_) => {
unimplemented!();
Commands::Lint(LintArgs { paths }) => {
linter.lint_paths(paths);
}
};

if !has_errors { Ok(String::new()) } else { Err(String::new()) }
}

const DEFAULT_DIALECT: &str = "ansi";
Commands::Fix(_) => todo!(),
}

fn find_files(pattern: &str) -> Result<Paths, String> {
glob(pattern).map_err(|e| format!("Error finding files with pattern '{}': {:?}", pattern, e))
std::process::exit(if linter.formatter.unwrap().has_fail.get() { 1 } else { 0 })
}
3 changes: 1 addition & 2 deletions crates/cli/tests/ui/LT01_LT012.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
== [<string input>] FAIL
== [tests/ui/LT01_LT012.sql] FAIL
L: 1 | P: 7 | LT01 | Expected only single space before "1". Found " ". [layout.spacing]
L: 1 | P: 11 | LT12 | Files must end with a single trailing newline. [layout.end_of_file]

3 changes: 1 addition & 2 deletions crates/cli/tests/ui/test_fail_whitespace_before_comma.stderr
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
== [<string input>] FAIL
== [tests/ui/test_fail_whitespace_before_comma.sql] FAIL
L: 1 | P: 1 | LT09 | Select targets should be on a new line unless there is only one select target.
| [layout.select_targets]
L: 1 | P: 9 | LT01 | Unexpected whitespace before ",". [layout.spacing]
L: 1 | P: 11 | LT01 | Expected single whitespace between "," and "4". [layout.spacing]
L: 1 | P: 11 | LT12 | Files must end with a single trailing newline. [layout.end_of_file]

9 changes: 8 additions & 1 deletion crates/lib/src/cli/formatters.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::borrow::Cow;
use std::cell::Cell;
use std::io::{IsTerminal, Write};

use anstyle::{AnsiColor, Effects, Style};
Expand Down Expand Up @@ -56,6 +57,7 @@ pub struct OutputStreamFormatter {
filter_empty: bool,
verbosity: i32,
output_line_length: usize,
pub has_fail: Cell<bool>,
}

impl OutputStreamFormatter {
Expand All @@ -66,6 +68,7 @@ impl OutputStreamFormatter {
filter_empty: true,
verbosity: 0,
output_line_length: 80,
has_fail: false.into(),
}
}

Expand Down Expand Up @@ -149,6 +152,7 @@ impl OutputStreamFormatter {
fname,
linted_file.get_violations(only_fixable.then_some(true)),
);

self.dispatch(&s);
}

Expand All @@ -169,7 +173,10 @@ impl OutputStreamFormatter {

let color = match status {
Status::Pass | Status::Fixed => AnsiColor::Green,
Status::Fail | Status::Error => AnsiColor::Red,
Status::Fail | Status::Error => {
self.has_fail.set(true);
AnsiColor::Red
}
}
.on_default();

Expand Down
1 change: 1 addition & 0 deletions crates/lib/src/core/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ pub mod linted_dir;
pub mod linted_file;
pub mod linter;
pub mod linting_result;
mod runner;
46 changes: 40 additions & 6 deletions crates/lib/src/core/linter/linter.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::collections::{HashMap, HashSet};
use std::fs::File;
use std::io::{BufRead, BufReader};
use std::path::Path;
use std::path::{Path, PathBuf};
use std::time::Instant;

use itertools::Itertools;
Expand All @@ -10,6 +10,7 @@ use uuid::Uuid;
use walkdir::WalkDir;

use super::linted_dir::LintedDir;
use super::runner::RunnerContext;
use crate::cli::formatters::OutputStreamFormatter;
use crate::core::config::FluffConfig;
use crate::core::dialects::init::dialect_selector;
Expand All @@ -26,8 +27,9 @@ use crate::core::templaters::base::{RawTemplater, TemplatedFile, Templater};

pub struct Linter {
config: FluffConfig,
pub(crate) formatter: Option<OutputStreamFormatter>,
pub formatter: Option<OutputStreamFormatter>,
templater: Box<dyn Templater>,
rules: Vec<ErasedRule>,
}

impl Linter {
Expand All @@ -37,8 +39,15 @@ impl Linter {
templater: Option<Box<dyn Templater>>,
) -> Linter {
match templater {
Some(templater) => Linter { config, formatter, templater },
None => Linter { config, formatter, templater: Box::<RawTemplater>::default() },
Some(templater) => {
Linter { config, formatter, templater, rules: crate::rules::layout::get_rules() }
}
None => Linter {
config,
formatter,
templater: Box::<RawTemplater>::default(),
rules: crate::rules::layout::get_rules(),
},
}
}

Expand Down Expand Up @@ -143,6 +152,31 @@ impl Linter {
self.lint_parsed(parsed, rules, fix)
}

pub fn lint_paths(&mut self, mut paths: Vec<PathBuf>) {
if paths.is_empty() {
paths.push(std::env::current_dir().unwrap());
}

let mut expanded_paths = Vec::new();
for path in paths {
let paths = self.paths_from_path(path, None, None, None, None);
expanded_paths.extend(paths);
}

let mut runner = RunnerContext::sequential(self);
runner.run(expanded_paths);
}

pub fn render_file(&mut self, fname: String) -> RenderedFile {
let in_str = std::fs::read_to_string(&fname).unwrap();
self.render_string(in_str, fname, self.config.clone(), None).unwrap()
}

pub fn lint_rendered(&mut self, rendered: RenderedFile) {
let parsed = Self::parse_rendered(rendered, false);
self.lint_parsed(parsed, self.rules.clone(), false);
}

pub fn lint_parsed(
&mut self,
parsed_string: ParsedString,
Expand Down Expand Up @@ -326,7 +360,7 @@ impl Linter {
config,
time_dict: HashMap::new(),
f_name: f_name.to_owned(),
encoding: encoding.to_owned().unwrap(),
encoding: encoding.to_owned().unwrap_or_else(|| "UTF-8".into()),
source_str: f_name.to_owned(),
})
}
Expand Down Expand Up @@ -457,7 +491,7 @@ impl Linter {
// look for an ignore file in the direct parent of the file.
fn paths_from_path(
&self,
path: String,
path: PathBuf,
ignore_file_name: Option<String>,
ignore_non_existent_files: Option<bool>,
ignore_files: Option<bool>,
Expand Down
33 changes: 33 additions & 0 deletions crates/lib/src/core/linter/runner.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
use super::linter::Linter;

pub trait Runner: Sized {
fn run(&mut self, paths: Vec<String>, linter: &mut Linter);
}

pub struct RunnerContext<'me, R> {
linter: &'me mut Linter,
runner: R,
}

impl<'me> RunnerContext<'me, SequentialRunner> {
pub fn sequential(linter: &'me mut Linter) -> Self {
Self { linter, runner: SequentialRunner }
}
}

impl<R: Runner> RunnerContext<'_, R> {
pub fn run(&mut self, paths: Vec<String>) {
self.runner.run(paths, &mut self.linter);
}
}

pub struct SequentialRunner;

impl Runner for SequentialRunner {
fn run(&mut self, paths: Vec<String>, linter: &mut Linter) {
for path in paths {
let rendered = linter.render_file(path);
linter.lint_rendered(rendered);
}
}
}

0 comments on commit 6094408

Please sign in to comment.