From c59207d4afcf7d55abb974d89c7f7a73d914a67d Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 1 Oct 2023 20:02:51 -0400 Subject: [PATCH] Remove LintSource --- crates/ruff_cli/src/commands/add_noqa.rs | 6 +- crates/ruff_cli/src/commands/format.rs | 15 ++- crates/ruff_cli/src/commands/format_stdin.rs | 16 ++- crates/ruff_cli/src/diagnostics.rs | 134 ++++++------------- crates/ruff_linter/src/source_kind.rs | 46 ++++--- 5 files changed, 88 insertions(+), 129 deletions(-) diff --git a/crates/ruff_cli/src/commands/add_noqa.rs b/crates/ruff_cli/src/commands/add_noqa.rs index 513f4ba2487ca5..1fd8f5425f979f 100644 --- a/crates/ruff_cli/src/commands/add_noqa.rs +++ b/crates/ruff_cli/src/commands/add_noqa.rs @@ -7,12 +7,12 @@ use log::{debug, error}; use rayon::prelude::*; use ruff_linter::linter::add_noqa_to_path; +use ruff_linter::source_kind::SourceKind; use ruff_linter::warn_user_once; use ruff_python_ast::{PySourceType, SourceType}; use ruff_workspace::resolver::{python_files_in_path, PyprojectConfig}; use crate::args::CliOverrides; -use crate::diagnostics::LintSource; /// Add `noqa` directives to a collection of files. pub(crate) fn add_noqa( @@ -57,8 +57,8 @@ pub(crate) fn add_noqa( .and_then(|parent| package_roots.get(parent)) .and_then(|package| *package); let settings = resolver.resolve(path, pyproject_config); - let LintSource(source_kind) = match LintSource::try_from_path(path, source_type) { - Ok(Some(source)) => source, + let source_kind = match SourceKind::from_path(path, source_type) { + Ok(Some(source_kind)) => source_kind, Ok(None) => return None, Err(e) => { error!("Failed to extract source from {}: {e}", path.display()); diff --git a/crates/ruff_cli/src/commands/format.rs b/crates/ruff_cli/src/commands/format.rs index 7d2ad4144f0560..05787743dc31c0 100644 --- a/crates/ruff_cli/src/commands/format.rs +++ b/crates/ruff_cli/src/commands/format.rs @@ -15,7 +15,7 @@ use tracing::debug; use ruff_diagnostics::SourceMap; use ruff_linter::fs; use ruff_linter::logging::LogLevel; -use ruff_linter::source_kind::{SourceKind, SourceReadError, SourceWriteError}; +use ruff_linter::source_kind::{SourceError, SourceKind}; use ruff_linter::warn_user_once; use ruff_python_ast::{PySourceType, SourceType}; use ruff_python_formatter::{format_module_source, FormatModuleError}; @@ -147,8 +147,13 @@ fn format_path( mode: FormatMode, ) -> Result { // Extract the sources from the file. - let source_kind = SourceKind::from_path(path, source_type) - .map_err(|err| FormatCommandError::Read(Some(path.to_path_buf()), err))?; + let source_kind = match SourceKind::from_path(path, source_type) { + Ok(Some(source_kind)) => source_kind, + Ok(None) => return Ok(FormatCommandResult::Unchanged), + Err(err) => { + return Err(FormatCommandError::Read(Some(path.to_path_buf()), err)); + } + }; // Format the source. if let Some(source_kind) = format_source(&source_kind, source_type, Some(path), settings)? { @@ -328,9 +333,9 @@ impl Display for FormatResultSummary { pub(crate) enum FormatCommandError { Ignore(#[from] ignore::Error), Panic(Option, PanicError), - Read(Option, SourceReadError), + Read(Option, SourceError), Format(Option, FormatModuleError), - Write(Option, SourceWriteError), + Write(Option, SourceError), } impl Display for FormatCommandError { diff --git a/crates/ruff_cli/src/commands/format_stdin.rs b/crates/ruff_cli/src/commands/format_stdin.rs index d7c34684be1852..b7d5a617864a6b 100644 --- a/crates/ruff_cli/src/commands/format_stdin.rs +++ b/crates/ruff_cli/src/commands/format_stdin.rs @@ -74,9 +74,15 @@ fn format_source_code( ) -> Result { // Read the source from stdin. let source_code = read_from_stdin() - .map_err(|err| FormatCommandError::Read(path.map(Into::into), err.into()))?; - let source_kind = SourceKind::from_source_code(source_code, source_type) - .map_err(|err| FormatCommandError::Read(path.map(Into::into), err))?; + .map_err(|err| FormatCommandError::Read(path.map(Path::to_path_buf), err.into()))?; + + let source_kind = match SourceKind::from_source_code(source_code, source_type) { + Ok(Some(source_kind)) => source_kind, + Ok(None) => return Ok(FormatCommandResult::Unchanged), + Err(err) => { + return Err(FormatCommandError::Read(path.map(Path::to_path_buf), err)); + } + }; // Format the source, and write to stdout regardless of the mode. if let Some(formatted) = format_source(&source_kind, source_type, path, settings)? { @@ -84,7 +90,7 @@ fn format_source_code( let mut writer = stdout().lock(); formatted .write(&mut writer) - .map_err(|err| FormatCommandError::Write(path.map(Into::into), err))?; + .map_err(|err| FormatCommandError::Write(path.map(Path::to_path_buf), err))?; } Ok(FormatCommandResult::Formatted) @@ -93,7 +99,7 @@ fn format_source_code( let mut writer = stdout().lock(); source_kind .write(&mut writer) - .map_err(|err| FormatCommandError::Write(path.map(Into::into), err))?; + .map_err(|err| FormatCommandError::Write(path.map(Path::to_path_buf), err))?; } Ok(FormatCommandResult::Unchanged) diff --git a/crates/ruff_cli/src/diagnostics.rs b/crates/ruff_cli/src/diagnostics.rs index fc49b0bfe648be..d3dea9f56aeba7 100644 --- a/crates/ruff_cli/src/diagnostics.rs +++ b/crates/ruff_cli/src/diagnostics.rs @@ -14,7 +14,6 @@ use filetime::FileTime; use log::{debug, error, warn}; use rustc_hash::FxHashMap; use similar::TextDiff; -use thiserror::Error; use ruff_diagnostics::Diagnostic; use ruff_linter::linter::{lint_fix, lint_only, FixTable, FixerResult, LinterResult}; @@ -23,12 +22,12 @@ use ruff_linter::message::Message; use ruff_linter::pyproject_toml::lint_pyproject_toml; use ruff_linter::registry::AsRule; use ruff_linter::settings::{flags, LinterSettings}; -use ruff_linter::source_kind::SourceKind; +use ruff_linter::source_kind::{SourceError, SourceKind}; use ruff_linter::{fs, IOError, SyntaxError}; use ruff_macros::CacheKey; use ruff_notebook::{Cell, Notebook, NotebookError, NotebookIndex}; use ruff_python_ast::imports::ImportMap; -use ruff_python_ast::{PySourceType, SourceType, TomlSourceType}; +use ruff_python_ast::{SourceType, TomlSourceType}; use ruff_source_file::{LineIndex, SourceCode, SourceFileBuilder}; use ruff_text_size::{TextRange, TextSize}; use ruff_workspace::Settings; @@ -82,15 +81,38 @@ impl Diagnostics { } } - /// Generate [`Diagnostics`] based on a [`SourceExtractionError`]. + /// Generate [`Diagnostics`] based on a [`SourceError`]. pub(crate) fn from_source_error( - err: &SourceExtractionError, + err: &SourceError, path: Option<&Path>, settings: &LinterSettings, ) -> Self { - let diagnostic = Diagnostic::from(err); + let diagnostic = match err { + // IO errors. + SourceError::Io(_) + | SourceError::Notebook(NotebookError::Io(_) | NotebookError::Json(_)) => { + Diagnostic::new( + IOError { + message: err.to_string(), + }, + TextRange::default(), + ) + } + // Syntax errors. + SourceError::Notebook( + NotebookError::InvalidJson(_) + | NotebookError::InvalidSchema(_) + | NotebookError::InvalidFormat(_), + ) => Diagnostic::new( + SyntaxError { + message: err.to_string(), + }, + TextRange::default(), + ), + }; + if settings.rules.enabled(diagnostic.kind.rule()) { - let name = path.map_or_else(|| "-".into(), std::path::Path::to_string_lossy); + let name = path.map_or_else(|| "-".into(), Path::to_string_lossy); let dummy = SourceFileBuilder::new(name, "").finish(); Self::new( vec![Message::from_diagnostic( @@ -183,13 +205,12 @@ pub(crate) fn lint_path( .iter_enabled() .any(|rule_code| rule_code.lint_source().is_pyproject_toml()) { - let contents = - match std::fs::read_to_string(path).map_err(SourceExtractionError::Io) { - Ok(contents) => contents, - Err(err) => { - return Ok(Diagnostics::from_source_error(&err, Some(path), settings)); - } - }; + let contents = match std::fs::read_to_string(path).map_err(SourceError::from) { + Ok(contents) => contents, + Err(err) => { + return Ok(Diagnostics::from_source_error(&err, Some(path), settings)); + } + }; let source_file = SourceFileBuilder::new(path.to_string_lossy(), contents).finish(); lint_pyproject_toml(source_file, settings) } else { @@ -205,8 +226,8 @@ pub(crate) fn lint_path( }; // Extract the sources from the file. - let LintSource(source_kind) = match LintSource::try_from_path(path, source_type) { - Ok(Some(sources)) => sources, + let source_kind = match SourceKind::from_path(path, source_type) { + Ok(Some(source_kind)) => source_kind, Ok(None) => return Ok(Diagnostics::default()), Err(err) => { return Ok(Diagnostics::from_source_error(&err, Some(path), settings)); @@ -371,8 +392,8 @@ pub(crate) fn lint_stdin( }; // Extract the sources from the file. - let LintSource(source_kind) = match LintSource::try_from_source_code(contents, source_type) { - Ok(Some(sources)) => sources, + let source_kind = match SourceKind::from_source_code(contents, source_type) { + Ok(Some(source_kind)) => source_kind, Ok(None) => return Ok(Diagnostics::default()), Err(err) => { return Ok(Diagnostics::from_source_error(&err, path, &settings.linter)); @@ -487,80 +508,3 @@ pub(crate) fn lint_stdin( notebook_indexes, }) } - -#[derive(Debug)] -pub(crate) struct LintSource(pub(crate) SourceKind); - -impl LintSource { - /// Extract the lint [`LintSource`] from the given file path. - pub(crate) fn try_from_path( - path: &Path, - source_type: PySourceType, - ) -> Result, SourceExtractionError> { - if source_type.is_ipynb() { - let notebook = Notebook::from_path(path)?; - Ok(notebook - .is_python_notebook() - .then_some(LintSource(SourceKind::IpyNotebook(notebook)))) - } else { - // This is tested by ruff_cli integration test `unreadable_file` - let contents = std::fs::read_to_string(path)?; - Ok(Some(LintSource(SourceKind::Python(contents)))) - } - } - - /// Extract the lint [`LintSource`] from the raw string contents, optionally accompanied by a - /// file path indicating the path to the file from which the contents were read. If provided, - /// the file path should be used for diagnostics, but not for reading the file from disk. - pub(crate) fn try_from_source_code( - source_code: String, - source_type: PySourceType, - ) -> Result, SourceExtractionError> { - if source_type.is_ipynb() { - let notebook = Notebook::from_source_code(&source_code)?; - Ok(notebook - .is_python_notebook() - .then_some(LintSource(SourceKind::IpyNotebook(notebook)))) - } else { - Ok(Some(LintSource(SourceKind::Python(source_code)))) - } - } -} - -#[derive(Error, Debug)] -pub(crate) enum SourceExtractionError { - /// The extraction failed due to an [`io::Error`]. - #[error(transparent)] - Io(#[from] io::Error), - /// The extraction failed due to a [`NotebookError`]. - #[error(transparent)] - Notebook(#[from] NotebookError), -} - -impl From<&SourceExtractionError> for Diagnostic { - fn from(err: &SourceExtractionError) -> Self { - match err { - // IO errors. - SourceExtractionError::Io(_) - | SourceExtractionError::Notebook(NotebookError::Io(_) | NotebookError::Json(_)) => { - Diagnostic::new( - IOError { - message: err.to_string(), - }, - TextRange::default(), - ) - } - // Syntax errors. - SourceExtractionError::Notebook( - NotebookError::InvalidJson(_) - | NotebookError::InvalidSchema(_) - | NotebookError::InvalidFormat(_), - ) => Diagnostic::new( - SyntaxError { - message: err.to_string(), - }, - TextRange::default(), - ), - } - } -} diff --git a/crates/ruff_linter/src/source_kind.rs b/crates/ruff_linter/src/source_kind.rs index cd0984bdd04dfb..cc7887c4e78853 100644 --- a/crates/ruff_linter/src/source_kind.rs +++ b/crates/ruff_linter/src/source_kind.rs @@ -38,49 +38,53 @@ impl SourceKind { } } - /// Read the source kind from the given path. - pub fn from_path(path: &Path, source_type: PySourceType) -> Result { + /// Read the [`SourceKind`] from the given path. Returns `None` if the source is not a Python + /// source file. + pub fn from_path(path: &Path, source_type: PySourceType) -> Result, SourceError> { if source_type.is_ipynb() { let notebook = Notebook::from_path(path)?; - Ok(Self::IpyNotebook(notebook)) + Ok(notebook + .is_python_notebook() + .then_some(Self::IpyNotebook(notebook))) } else { let contents = std::fs::read_to_string(path)?; - Ok(Self::Python(contents)) + Ok(Some(Self::Python(contents))) } } - /// Read the source kind from the given source code. + /// Read the [`SourceKind`] from the given source code. Returns `None` if the source is not + /// Python source code. pub fn from_source_code( source_code: String, source_type: PySourceType, - ) -> Result { + ) -> Result, SourceError> { if source_type.is_ipynb() { let notebook = Notebook::from_source_code(&source_code)?; - Ok(Self::IpyNotebook(notebook)) + Ok(notebook + .is_python_notebook() + .then_some(Self::IpyNotebook(notebook))) } else { - Ok(Self::Python(source_code)) + Ok(Some(Self::Python(source_code))) } } - /// Write the source kind to the given writer. - pub fn write(&self, writer: &mut dyn Write) -> Result<(), SourceWriteError> { + /// Write the [`SourceKind`] to the given writer. + pub fn write(&self, writer: &mut dyn Write) -> Result<(), SourceError> { match self { - SourceKind::Python(source) => writer.write_all(source.as_bytes()).map_err(Into::into), - SourceKind::IpyNotebook(notebook) => notebook.write(writer).map_err(Into::into), + SourceKind::Python(source) => { + writer.write_all(source.as_bytes())?; + Ok(()) + } + SourceKind::IpyNotebook(notebook) => { + notebook.write(writer)?; + Ok(()) + } } } } #[derive(Error, Debug)] -pub enum SourceReadError { - #[error(transparent)] - Io(#[from] io::Error), - #[error(transparent)] - Notebook(#[from] NotebookError), -} - -#[derive(Error, Debug)] -pub enum SourceWriteError { +pub enum SourceError { #[error(transparent)] Io(#[from] io::Error), #[error(transparent)]