Decouple linter module from cache module

This commit is contained in:
Martin Fischer 2023-01-12 13:28:54 +01:00 committed by Charlie Marsh
parent 74f14182ea
commit 38f5e8f423
6 changed files with 162 additions and 150 deletions

View file

@ -1,3 +1,4 @@
#![cfg_attr(target_family = "wasm", allow(dead_code))]
use std::collections::hash_map::DefaultHasher; use std::collections::hash_map::DefaultHasher;
use std::fs; use std::fs;
use std::hash::{Hash, Hasher}; use std::hash::{Hash, Hasher};

View file

@ -17,8 +17,9 @@ use walkdir::WalkDir;
use crate::cache::CACHE_DIR_NAME; use crate::cache::CACHE_DIR_NAME;
use crate::cli::Overrides; use crate::cli::Overrides;
use crate::diagnostics::{lint_path, lint_stdin, Diagnostics};
use crate::iterators::par_iter; 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::logging::LogLevel;
use crate::message::Message; use crate::message::Message;
use crate::registry::RuleCode; use crate::registry::RuleCode;

154
src/diagnostics.rs Normal file
View file

@ -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<Message>,
pub fixed: usize,
}
impl Diagnostics {
pub fn new(messages: Vec<Message>) -> 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<Diagnostics> {
// 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<Diagnostics> {
// 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 })
}

View file

@ -18,6 +18,7 @@ mod cache;
mod checkers; mod checkers;
pub mod cli; pub mod cli;
mod cst; mod cst;
mod diagnostics;
mod directives; mod directives;
mod doc_lines; mod doc_lines;
mod docstrings; mod docstrings;

View file

@ -1,14 +1,8 @@
use std::fs::write;
use std::io;
use std::io::Write;
use std::ops::AddAssign;
use std::path::Path; use std::path::Path;
use anyhow::Result; use anyhow::Result;
use colored::Colorize; use colored::Colorize;
use log::debug;
use rustpython_parser::lexer::LexResult; use rustpython_parser::lexer::LexResult;
use similar::TextDiff;
use crate::ast::types::Range; use crate::ast::types::Range;
use crate::autofix::fix_file; use crate::autofix::fix_file;
@ -24,30 +18,11 @@ use crate::noqa::add_noqa;
use crate::registry::{Diagnostic, LintSource, RuleCode}; use crate::registry::{Diagnostic, LintSource, RuleCode};
use crate::settings::{flags, Settings}; use crate::settings::{flags, Settings};
use crate::source_code::{Locator, Stylist}; 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_NAME: &str = env!("CARGO_PKG_NAME");
const CARGO_PKG_REPOSITORY: &str = env!("CARGO_PKG_REPOSITORY"); const CARGO_PKG_REPOSITORY: &str = env!("CARGO_PKG_REPOSITORY");
#[derive(Debug, Default)]
pub struct Diagnostics {
pub messages: Vec<Message>,
pub fixed: usize,
}
impl Diagnostics {
pub fn new(messages: Vec<Message>) -> 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 /// Generate `Diagnostic`s from the source code contents at the
/// given `Path`. /// given `Path`.
#[allow(clippy::too_many_arguments)] #[allow(clippy::too_many_arguments)]
@ -192,70 +167,6 @@ pub(crate) fn check_path(
const MAX_ITERATIONS: usize = 100; 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<Diagnostics> {
// 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`. /// Add any missing `#noqa` pragmas to the source code at the given `Path`.
pub fn add_noqa_to_path(path: &Path, settings: &Settings) -> Result<usize> { pub fn add_noqa_to_path(path: &Path, settings: &Settings) -> Result<usize> {
// Validate the `Settings` and return any errors. // Validate the `Settings` and return any errors.
@ -301,65 +212,9 @@ pub fn add_noqa_to_path(path: &Path, settings: &Settings) -> Result<usize> {
) )
} }
/// 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<Diagnostics> {
// 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 /// Generate `Diagnostic`s (optionally including any autofix
/// patches) from source code content. /// patches) from source code content.
fn lint_only( pub fn lint_only(
contents: &str, contents: &str,
path: &Path, path: &Path,
package: Option<&Path>, package: Option<&Path>,
@ -410,7 +265,7 @@ fn lint_only(
/// Generate `Diagnostic`s from source code content, iteratively autofixing /// Generate `Diagnostic`s from source code content, iteratively autofixing
/// until stable. /// until stable.
fn lint_fix( pub fn lint_fix(
contents: &str, contents: &str,
path: &Path, path: &Path,
package: Option<&Path>, package: Option<&Path>,

View file

@ -10,8 +10,8 @@ use rustpython_parser::ast::Location;
use serde::Serialize; use serde::Serialize;
use serde_json::json; use serde_json::json;
use crate::diagnostics::Diagnostics;
use crate::fs::relativize_path; use crate::fs::relativize_path;
use crate::linter::Diagnostics;
use crate::logging::LogLevel; use crate::logging::LogLevel;
use crate::message::Message; use crate::message::Message;
use crate::registry::RuleCode; use crate::registry::RuleCode;