Skip to content

Commit

Permalink
Make DisplayParseError an error type
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Dec 30, 2023
1 parent 34d8bdd commit 8f10cd0
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 88 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions crates/ruff_cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ ruff_macros = { path = "../ruff_macros" }
ruff_python_ast = { path = "../ruff_python_ast" }
ruff_python_formatter = { path = "../ruff_python_formatter" }
ruff_source_file = { path = "../ruff_source_file" }
ruff_python_parser = { path = "../ruff_python_parser" }
ruff_python_trivia = { path = "../ruff_python_trivia" }
ruff_workspace = { path = "../ruff_workspace" }
ruff_text_size = { path = "../ruff_text_size" }
Expand Down
53 changes: 28 additions & 25 deletions crates/ruff_cli/src/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ 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, QuoteStyle};
use ruff_source_file::{LineIndex, SourceCode};
use ruff_text_size::{TextLen, TextRange, TextSize};
use ruff_workspace::resolver::{
match_exclusion, python_files_in_path, PyprojectConfig, ResolvedFile, Resolver,
Expand Down Expand Up @@ -327,7 +326,16 @@ pub(crate) fn format_source(
let options = settings.to_format_options(source_type, unformatted);

let formatted = format_module_source(unformatted, options).map_err(|err| {
FormatCommandError::Format(path.map(Path::to_path_buf), source_kind.clone(), err)
if let FormatModuleError::ParseError(err) = err {
DisplayParseError::from_source_kind(
err,
path.map(Path::to_path_buf),
source_kind,
)
.into()
} else {
FormatCommandError::Format(path.map(Path::to_path_buf), err)
}
})?;

let formatted = formatted.into_code();
Expand Down Expand Up @@ -356,11 +364,16 @@ pub(crate) fn format_source(
// Format the cell.
let formatted =
format_module_source(unformatted, options.clone()).map_err(|err| {
FormatCommandError::Format(
path.map(Path::to_path_buf),
source_kind.clone(),
err,
)
if let FormatModuleError::ParseError(err) = err {
DisplayParseError::from_source_kind(
err,
path.map(Path::to_path_buf),
source_kind,
)
.into()
} else {
FormatCommandError::Format(path.map(Path::to_path_buf), err)
}
})?;

// If the cell is unchanged, skip it.
Expand Down Expand Up @@ -571,9 +584,10 @@ impl<'a> FormatResults<'a> {
#[derive(Error, Debug)]
pub(crate) enum FormatCommandError {
Ignore(#[from] ignore::Error),
Parse(#[from] DisplayParseError),
Panic(Option<PathBuf>, PanicError),
Read(Option<PathBuf>, SourceError),
Format(Option<PathBuf>, SourceKind, FormatModuleError),
Format(Option<PathBuf>, FormatModuleError),
Write(Option<PathBuf>, SourceError),
Diff(Option<PathBuf>, io::Error),
}
Expand All @@ -588,9 +602,10 @@ impl FormatCommandError {
None
}
}
Self::Parse(err) => err.path(),
Self::Panic(path, _)
| Self::Read(path, _)
| Self::Format(path, _, _)
| Self::Format(path, _)
| Self::Write(path, _)
| Self::Diff(path, _) => path.as_deref(),
}
Expand Down Expand Up @@ -621,6 +636,9 @@ impl Display for FormatCommandError {
)
}
}
Self::Parse(err) => {
write!(f, "{err}")
}
Self::Read(path, err) => {
if let Some(path) = path {
write!(
Expand All @@ -647,22 +665,7 @@ impl Display for FormatCommandError {
write!(f, "{}{} {err}", "Failed to write".bold(), ":".bold())
}
}
Self::Format(path, source_kind, FormatModuleError::ParseError(err)) => {
write!(
f,
"{}",
DisplayParseError::new(
err,
&SourceCode::new(
source_kind.source_code(),
&LineIndex::from_source_text(source_kind.source_code()),
),
source_kind,
path.as_deref(),
)
)
}
Self::Format(path, _source_kind, err) => {
Self::Format(path, err) => {
if let Some(path) = path {
write!(
f,
Expand Down
17 changes: 5 additions & 12 deletions crates/ruff_cli/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use colored::Colorize;
use log::{debug, error, warn};
use rustc_hash::FxHashMap;

use crate::cache::{Cache, FileCacheKey, LintCacheData};
use ruff_diagnostics::Diagnostic;
use ruff_linter::linter::{lint_fix, lint_only, FixTable, FixerResult, LinterResult, ParseSource};
use ruff_linter::logging::DisplayParseError;
Expand All @@ -24,10 +23,12 @@ use ruff_linter::{fs, IOError, SyntaxError};
use ruff_notebook::{Notebook, NotebookError, NotebookIndex};
use ruff_python_ast::imports::ImportMap;
use ruff_python_ast::{PySourceType, SourceType, TomlSourceType};
use ruff_source_file::{LineIndex, SourceCode, SourceFileBuilder};
use ruff_source_file::SourceFileBuilder;
use ruff_text_size::{TextRange, TextSize};
use ruff_workspace::Settings;

use crate::cache::{Cache, FileCacheKey, LintCacheData};

#[derive(Debug, Default, PartialEq)]
pub(crate) struct Diagnostics {
pub(crate) messages: Vec<Message>,
Expand Down Expand Up @@ -356,18 +357,10 @@ pub(crate) fn lint_path(
}
}

if let Some(err) = parse_error {
if let Some(error) = parse_error {
error!(
"{}",
DisplayParseError::new(
&err,
&SourceCode::new(
source_kind.source_code(),
&LineIndex::from_source_text(source_kind.source_code())
),
&source_kind,
Some(path),
)
DisplayParseError::from_source_kind(error, Some(path.to_path_buf()), &source_kind,)
);
}

Expand Down
7 changes: 6 additions & 1 deletion crates/ruff_linter/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,12 @@ pub fn add_noqa_to_path(
if let Some(error) = error {
error!(
"{}",
DisplayParseError::new(&error, &locator.to_source_code(), source_kind, Some(path))
DisplayParseError::from_source_code(
error,
Some(path.to_path_buf()),
&locator.to_source_code(),
source_kind,
)
);
}

Expand Down
140 changes: 90 additions & 50 deletions crates/ruff_linter/src/logging.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::fmt::{Display, Formatter, Write};
use std::path::Path;
use std::path::{Path, PathBuf};
use std::sync::Mutex;

use anyhow::Result;
Expand All @@ -9,7 +9,7 @@ use log::Level;
use once_cell::sync::Lazy;
use ruff_python_parser::{ParseError, ParseErrorType};

use ruff_source_file::{OneIndexed, SourceCode, SourceLocation};
use ruff_source_file::{LineIndex, OneIndexed, SourceCode, SourceLocation};

use crate::fs;
use crate::source_kind::SourceKind;
Expand Down Expand Up @@ -138,32 +138,76 @@ pub fn set_up_logging(level: &LogLevel) -> Result<()> {

/// A wrapper around [`ParseError`] to translate byte offsets to user-facing
/// source code locations (typically, line and column numbers).
pub struct DisplayParseError<'a> {
error: &'a ParseError,
source_code: &'a SourceCode<'a, 'a>,
source_kind: &'a SourceKind,
path: Option<&'a Path>,
#[derive(Debug)]
pub struct DisplayParseError {
error: ParseError,
path: Option<PathBuf>,
location: ErrorLocation,
}

impl<'a> DisplayParseError<'a> {
pub fn new(
error: &'a ParseError,
source_code: &'a SourceCode<'a, 'a>,
source_kind: &'a SourceKind,
path: Option<&'a Path>,
impl DisplayParseError {
/// Create a [`DisplayParseError`] from a [`ParseError`] and a [`SourceKind`].
pub fn from_source_kind(
error: ParseError,
path: Option<PathBuf>,
source_kind: &SourceKind,
) -> Self {
Self {
Self::from_source_code(
error,
source_code,
path,
&SourceCode::new(
source_kind.source_code(),
&LineIndex::from_source_text(source_kind.source_code()),
),
source_kind,
)
}

/// Create a [`DisplayParseError`] from a [`ParseError`] and a [`SourceCode`].
pub fn from_source_code(
error: ParseError,
path: Option<PathBuf>,
source_code: &SourceCode,
source_kind: &SourceKind,
) -> Self {
// Translate the byte offset to a location in the originating source.
let location =
if let Some(jupyter_index) = source_kind.as_ipy_notebook().map(Notebook::index) {
let source_location = source_code.source_location(error.offset);

ErrorLocation::Cell(
jupyter_index
.cell(source_location.row)
.unwrap_or(OneIndexed::MIN),
SourceLocation {
row: jupyter_index
.cell_row(source_location.row)
.unwrap_or(OneIndexed::MIN),
column: source_location.column,
},
)
} else {
ErrorLocation::File(source_code.source_location(error.offset))
};

Self {
error,
path,
location,
}
}

/// Return the path of the file in which the error occurred.
pub fn path(&self) -> Option<&Path> {
self.path.as_deref()
}
}

impl Display for DisplayParseError<'_> {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
if let Some(path) = self.path {
impl std::error::Error for DisplayParseError {}

impl Display for DisplayParseError {
fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
if let Some(path) = self.path.as_ref() {
write!(
f,
"{header} {path}{colon}",
Expand All @@ -179,41 +223,29 @@ impl Display for DisplayParseError<'_> {
colon = ":".cyan(),
)?;
}

let source_location = self.source_code.source_location(self.error.offset);

// If we're working on a Jupyter notebook, translate the positions
// with respect to the cell and row in the cell. This is the same
// format as the `TextEmitter`.
let error_location =
if let Some(jupyter_index) = self.source_kind.as_ipy_notebook().map(Notebook::index) {
match &self.location {
ErrorLocation::File(location) => {
write!(
f,
"cell {cell}{colon}",
cell = jupyter_index
.cell(source_location.row)
.unwrap_or(OneIndexed::MIN),
"{row}{colon}{column}{colon} {inner}",
row = location.row,
column = location.column,
colon = ":".cyan(),
)?;

SourceLocation {
row: jupyter_index
.cell_row(source_location.row)
.unwrap_or(OneIndexed::MIN),
column: source_location.column,
}
} else {
source_location
};

write!(
f,
"{row}{colon}{column}{colon} {inner}",
row = error_location.row,
column = error_location.column,
colon = ":".cyan(),
inner = &DisplayParseErrorType(&self.error.error)
)
inner = &DisplayParseErrorType(&self.error.error)
)
}
ErrorLocation::Cell(cell, location) => {
write!(
f,
"{cell}{colon}{row}{colon}{column}{colon} {inner}",
cell = cell,
row = location.row,
column = location.column,
colon = ":".cyan(),
inner = &DisplayParseErrorType(&self.error.error)
)
}
}
}
}

Expand Down Expand Up @@ -251,6 +283,14 @@ impl Display for DisplayParseErrorType<'_> {
}
}

#[derive(Debug)]
enum ErrorLocation {
/// The error occurred in a Python file.
File(SourceLocation),
/// The error occurred in a Jupyter cell.
Cell(OneIndexed, SourceLocation),
}

/// Truncates the display text before the first newline character to avoid line breaks.
struct TruncateAtNewline<'a>(&'a dyn Display);

Expand Down

0 comments on commit 8f10cd0

Please sign in to comment.