From 38f5e8f42325067b45ca133bb9cccce4faa6083b Mon Sep 17 00:00:00 2001 From: Martin Fischer Date: Thu, 12 Jan 2023 13:28:54 +0100 Subject: [PATCH] Decouple linter module from cache module --- src/cache.rs | 1 + src/commands.rs | 3 +- src/diagnostics.rs | 154 +++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + src/linter.rs | 151 +------------------------------------------- src/printer.rs | 2 +- 6 files changed, 162 insertions(+), 150 deletions(-) create mode 100644 src/diagnostics.rs diff --git a/src/cache.rs b/src/cache.rs index 1ec0caa21e..f484f86e60 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -1,3 +1,4 @@ +#![cfg_attr(target_family = "wasm", allow(dead_code))] use std::collections::hash_map::DefaultHasher; use std::fs; use std::hash::{Hash, Hasher}; diff --git a/src/commands.rs b/src/commands.rs index ad3de3b4f3..de2f8fe906 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -17,8 +17,9 @@ use walkdir::WalkDir; use crate::cache::CACHE_DIR_NAME; use crate::cli::Overrides; +use crate::diagnostics::{lint_path, lint_stdin, Diagnostics}; use crate::iterators::par_iter; -use crate::linter::{add_noqa_to_path, lint_path, lint_stdin, Diagnostics}; +use crate::linter::add_noqa_to_path; use crate::logging::LogLevel; use crate::message::Message; use crate::registry::RuleCode; diff --git a/src/diagnostics.rs b/src/diagnostics.rs new file mode 100644 index 0000000000..943c4d557a --- /dev/null +++ b/src/diagnostics.rs @@ -0,0 +1,154 @@ +#![cfg_attr(target_family = "wasm", allow(dead_code))] +use std::fs::write; +use std::io; +use std::io::Write; +use std::ops::AddAssign; +use std::path::Path; + +use anyhow::Result; +use log::debug; +use similar::TextDiff; + +use crate::linter::{lint_fix, lint_only}; +use crate::message::Message; +use crate::settings::{flags, Settings}; +use crate::{cache, fix, fs}; + +#[derive(Debug, Default)] +pub struct Diagnostics { + pub messages: Vec, + pub fixed: usize, +} + +impl Diagnostics { + pub fn new(messages: Vec) -> Self { + Self { messages, fixed: 0 } + } +} + +impl AddAssign for Diagnostics { + fn add_assign(&mut self, other: Self) { + self.messages.extend(other.messages); + self.fixed += other.fixed; + } +} + +/// Lint the source code at the given `Path`. +pub fn lint_path( + path: &Path, + package: Option<&Path>, + settings: &Settings, + cache: flags::Cache, + autofix: fix::FixMode, +) -> Result { + // Validate the `Settings` and return any errors. + settings.validate()?; + + // Check the cache. + // TODO(charlie): `fixer::Mode::Apply` and `fixer::Mode::Diff` both have + // side-effects that aren't captured in the cache. (In practice, it's fine + // to cache `fixer::Mode::Apply`, since a file either has no fixes, or we'll + // write the fixes to disk, thus invalidating the cache. But it's a bit hard + // to reason about. We need to come up with a better solution here.) + let metadata = if matches!(cache, flags::Cache::Enabled) + && matches!(autofix, fix::FixMode::None | fix::FixMode::Generate) + { + let metadata = path.metadata()?; + if let Some(messages) = cache::get(path, &metadata, settings, autofix.into()) { + debug!("Cache hit for: {}", path.to_string_lossy()); + return Ok(Diagnostics::new(messages)); + } + Some(metadata) + } else { + None + }; + + // Read the file from disk. + let contents = fs::read_file(path)?; + + // Lint the file. + let (messages, fixed) = if matches!(autofix, fix::FixMode::Apply | fix::FixMode::Diff) { + let (transformed, fixed, messages) = lint_fix(&contents, path, package, settings)?; + if fixed > 0 { + if matches!(autofix, fix::FixMode::Apply) { + write(path, transformed)?; + } else if matches!(autofix, fix::FixMode::Diff) { + let mut stdout = io::stdout().lock(); + TextDiff::from_lines(&contents, &transformed) + .unified_diff() + .header(&fs::relativize_path(path), &fs::relativize_path(path)) + .to_writer(&mut stdout)?; + stdout.write_all(b"\n")?; + stdout.flush()?; + } + } + (messages, fixed) + } else { + let messages = lint_only(&contents, path, package, settings, autofix.into())?; + let fixed = 0; + (messages, fixed) + }; + + // Re-populate the cache. + if let Some(metadata) = metadata { + cache::set(path, &metadata, settings, autofix.into(), &messages); + } + + Ok(Diagnostics { messages, fixed }) +} + +/// Generate `Diagnostic`s from source code content derived from +/// stdin. +pub fn lint_stdin( + path: Option<&Path>, + package: Option<&Path>, + contents: &str, + settings: &Settings, + autofix: fix::FixMode, +) -> Result { + // Validate the `Settings` and return any errors. + settings.validate()?; + + // Lint the inputs. + let (messages, fixed) = if matches!(autofix, fix::FixMode::Apply | fix::FixMode::Diff) { + let (transformed, fixed, messages) = lint_fix( + contents, + path.unwrap_or_else(|| Path::new("-")), + package, + settings, + )?; + + if matches!(autofix, fix::FixMode::Apply) { + // Write the contents to stdout, regardless of whether any errors were fixed. + io::stdout().write_all(transformed.as_bytes())?; + } else if matches!(autofix, fix::FixMode::Diff) { + // But only write a diff if it's non-empty. + if fixed > 0 { + let text_diff = TextDiff::from_lines(contents, &transformed); + let mut unified_diff = text_diff.unified_diff(); + if let Some(path) = path { + unified_diff.header(&fs::relativize_path(path), &fs::relativize_path(path)); + } + + let mut stdout = io::stdout().lock(); + unified_diff.to_writer(&mut stdout)?; + stdout.write_all(b"\n")?; + stdout.flush()?; + } + } + + (messages, fixed) + } else { + let messages = lint_only( + contents, + path.unwrap_or_else(|| Path::new("-")), + package, + settings, + autofix.into(), + )?; + let fixed = 0; + (messages, fixed) + }; + + Ok(Diagnostics { messages, fixed }) +} diff --git a/src/lib.rs b/src/lib.rs index 5bb7dfb2f6..93319fc0cd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,6 +18,7 @@ mod cache; mod checkers; pub mod cli; mod cst; +mod diagnostics; mod directives; mod doc_lines; mod docstrings; diff --git a/src/linter.rs b/src/linter.rs index 29478eafed..3c4c373a80 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -1,14 +1,8 @@ -use std::fs::write; -use std::io; -use std::io::Write; -use std::ops::AddAssign; use std::path::Path; use anyhow::Result; use colored::Colorize; -use log::debug; use rustpython_parser::lexer::LexResult; -use similar::TextDiff; use crate::ast::types::Range; use crate::autofix::fix_file; @@ -24,30 +18,11 @@ use crate::noqa::add_noqa; use crate::registry::{Diagnostic, LintSource, RuleCode}; use crate::settings::{flags, Settings}; use crate::source_code::{Locator, Stylist}; -use crate::{cache, directives, fix, fs, rustpython_helpers, violations}; +use crate::{directives, fs, rustpython_helpers, violations}; const CARGO_PKG_NAME: &str = env!("CARGO_PKG_NAME"); const CARGO_PKG_REPOSITORY: &str = env!("CARGO_PKG_REPOSITORY"); -#[derive(Debug, Default)] -pub struct Diagnostics { - pub messages: Vec, - pub fixed: usize, -} - -impl Diagnostics { - pub fn new(messages: Vec) -> Self { - Self { messages, fixed: 0 } - } -} - -impl AddAssign for Diagnostics { - fn add_assign(&mut self, other: Self) { - self.messages.extend(other.messages); - self.fixed += other.fixed; - } -} - /// Generate `Diagnostic`s from the source code contents at the /// given `Path`. #[allow(clippy::too_many_arguments)] @@ -192,70 +167,6 @@ pub(crate) fn check_path( const MAX_ITERATIONS: usize = 100; -/// Lint the source code at the given `Path`. -pub fn lint_path( - path: &Path, - package: Option<&Path>, - settings: &Settings, - cache: flags::Cache, - autofix: fix::FixMode, -) -> Result { - // Validate the `Settings` and return any errors. - settings.validate()?; - - // Check the cache. - // TODO(charlie): `fixer::Mode::Apply` and `fixer::Mode::Diff` both have - // side-effects that aren't captured in the cache. (In practice, it's fine - // to cache `fixer::Mode::Apply`, since a file either has no fixes, or we'll - // write the fixes to disk, thus invalidating the cache. But it's a bit hard - // to reason about. We need to come up with a better solution here.) - let metadata = if matches!(cache, flags::Cache::Enabled) - && matches!(autofix, fix::FixMode::None | fix::FixMode::Generate) - { - let metadata = path.metadata()?; - if let Some(messages) = cache::get(path, &metadata, settings, autofix.into()) { - debug!("Cache hit for: {}", path.to_string_lossy()); - return Ok(Diagnostics::new(messages)); - } - Some(metadata) - } else { - None - }; - - // Read the file from disk. - let contents = fs::read_file(path)?; - - // Lint the file. - let (messages, fixed) = if matches!(autofix, fix::FixMode::Apply | fix::FixMode::Diff) { - let (transformed, fixed, messages) = lint_fix(&contents, path, package, settings)?; - if fixed > 0 { - if matches!(autofix, fix::FixMode::Apply) { - write(path, transformed)?; - } else if matches!(autofix, fix::FixMode::Diff) { - let mut stdout = io::stdout().lock(); - TextDiff::from_lines(&contents, &transformed) - .unified_diff() - .header(&fs::relativize_path(path), &fs::relativize_path(path)) - .to_writer(&mut stdout)?; - stdout.write_all(b"\n")?; - stdout.flush()?; - } - } - (messages, fixed) - } else { - let messages = lint_only(&contents, path, package, settings, autofix.into())?; - let fixed = 0; - (messages, fixed) - }; - - // Re-populate the cache. - if let Some(metadata) = metadata { - cache::set(path, &metadata, settings, autofix.into(), &messages); - } - - Ok(Diagnostics { messages, fixed }) -} - /// Add any missing `#noqa` pragmas to the source code at the given `Path`. pub fn add_noqa_to_path(path: &Path, settings: &Settings) -> Result { // Validate the `Settings` and return any errors. @@ -301,65 +212,9 @@ pub fn add_noqa_to_path(path: &Path, settings: &Settings) -> Result { ) } -/// Generate `Diagnostic`s from source code content derived from -/// stdin. -pub fn lint_stdin( - path: Option<&Path>, - package: Option<&Path>, - contents: &str, - settings: &Settings, - autofix: fix::FixMode, -) -> Result { - // Validate the `Settings` and return any errors. - settings.validate()?; - - // Lint the inputs. - let (messages, fixed) = if matches!(autofix, fix::FixMode::Apply | fix::FixMode::Diff) { - let (transformed, fixed, messages) = lint_fix( - contents, - path.unwrap_or_else(|| Path::new("-")), - package, - settings, - )?; - - if matches!(autofix, fix::FixMode::Apply) { - // Write the contents to stdout, regardless of whether any errors were fixed. - io::stdout().write_all(transformed.as_bytes())?; - } else if matches!(autofix, fix::FixMode::Diff) { - // But only write a diff if it's non-empty. - if fixed > 0 { - let text_diff = TextDiff::from_lines(contents, &transformed); - let mut unified_diff = text_diff.unified_diff(); - if let Some(path) = path { - unified_diff.header(&fs::relativize_path(path), &fs::relativize_path(path)); - } - - let mut stdout = io::stdout().lock(); - unified_diff.to_writer(&mut stdout)?; - stdout.write_all(b"\n")?; - stdout.flush()?; - } - } - - (messages, fixed) - } else { - let messages = lint_only( - contents, - path.unwrap_or_else(|| Path::new("-")), - package, - settings, - autofix.into(), - )?; - let fixed = 0; - (messages, fixed) - }; - - Ok(Diagnostics { messages, fixed }) -} - /// Generate `Diagnostic`s (optionally including any autofix /// patches) from source code content. -fn lint_only( +pub fn lint_only( contents: &str, path: &Path, package: Option<&Path>, @@ -410,7 +265,7 @@ fn lint_only( /// Generate `Diagnostic`s from source code content, iteratively autofixing /// until stable. -fn lint_fix( +pub fn lint_fix( contents: &str, path: &Path, package: Option<&Path>, diff --git a/src/printer.rs b/src/printer.rs index a3455ff72c..64aa06558a 100644 --- a/src/printer.rs +++ b/src/printer.rs @@ -10,8 +10,8 @@ use rustpython_parser::ast::Location; use serde::Serialize; use serde_json::json; +use crate::diagnostics::Diagnostics; use crate::fs::relativize_path; -use crate::linter::Diagnostics; use crate::logging::LogLevel; use crate::message::Message; use crate::registry::RuleCode;