Skip to content

Commit

Permalink
Add row and column numbers to formatted parse errors (#9321)
Browse files Browse the repository at this point in the history
## Summary

We now render parse errors in the formatter identically to those in the
linter, e.g.:

```
❯ cargo run -p ruff_cli -- format foo.py
error: Failed to parse foo.py:1:17: Unexpected token '='
```

Closes #8338.

Closes #9311.
  • Loading branch information
charliermarsh authored Dec 31, 2023
1 parent e80260a commit 48e04cc
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 59 deletions.
43 changes: 33 additions & 10 deletions crates/ruff_cli/src/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ use tracing::debug;

use ruff_diagnostics::SourceMap;
use ruff_linter::fs;
use ruff_linter::logging::LogLevel;
use ruff_linter::logging::{DisplayParseError, LogLevel};
use ruff_linter::registry::Rule;
use ruff_linter::rules::flake8_quotes::settings::Quote;
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 @@ -244,7 +245,7 @@ pub(crate) fn format_path(
// Extract the sources from the file.
let unformatted = match SourceKind::from_path(path, source_type) {
Ok(Some(source_kind)) => source_kind,
// Non Python Jupyter notebook
// Non-Python Jupyter notebook.
Ok(None) => return Ok(FormatResult::Skipped),
Err(err) => {
return Err(FormatCommandError::Read(Some(path.to_path_buf()), err));
Expand Down Expand Up @@ -321,12 +322,13 @@ pub(crate) fn format_source(
path: Option<&Path>,
settings: &FormatterSettings,
) -> Result<FormattedSource, FormatCommandError> {
match source_kind {
match &source_kind {
SourceKind::Python(unformatted) => {
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), err))?;
let formatted = format_module_source(unformatted, options).map_err(|err| {
FormatCommandError::Format(path.map(Path::to_path_buf), source_kind.clone(), err)
})?;

let formatted = formatted.into_code();
if formatted.len() == unformatted.len() && formatted == *unformatted {
Expand All @@ -352,8 +354,14 @@ pub(crate) fn format_source(
let unformatted = &notebook.source_code()[range];

// Format the cell.
let formatted = format_module_source(unformatted, options.clone())
.map_err(|err| FormatCommandError::Format(path.map(Path::to_path_buf), err))?;
let formatted =
format_module_source(unformatted, options.clone()).map_err(|err| {
FormatCommandError::Format(
path.map(Path::to_path_buf),
source_kind.clone(),
err,
)
})?;

// If the cell is unchanged, skip it.
let formatted = formatted.as_code();
Expand Down Expand Up @@ -565,7 +573,7 @@ pub(crate) enum FormatCommandError {
Ignore(#[from] ignore::Error),
Panic(Option<PathBuf>, PanicError),
Read(Option<PathBuf>, SourceError),
Format(Option<PathBuf>, FormatModuleError),
Format(Option<PathBuf>, SourceKind, FormatModuleError),
Write(Option<PathBuf>, SourceError),
Diff(Option<PathBuf>, io::Error),
}
Expand All @@ -582,7 +590,7 @@ impl FormatCommandError {
}
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 @@ -639,7 +647,22 @@ impl Display for FormatCommandError {
write!(f, "{}{} {err}", "Failed to write".bold(), ":".bold())
}
}
Self::Format(path, err) => {
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) => {
if let Some(path) = path {
write!(
f,
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff_cli/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,13 +360,13 @@ pub(crate) fn lint_path(
error!(
"{}",
DisplayParseError::new(
err,
SourceCode::new(
&err,
&SourceCode::new(
source_kind.source_code(),
&LineIndex::from_source_text(source_kind.source_code())
),
&source_kind,
path,
Some(path),
)
);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_cli/tests/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ from module import =
----- stdout -----
----- stderr -----
error: Failed to format main.py: source contains syntax errors: invalid syntax. Got unexpected token '=' at byte offset 20
error: Failed to parse main.py:2:20: Unexpected token '='
"###);

Ok(())
Expand Down
9 changes: 5 additions & 4 deletions crates/ruff_dev/src/format_dev.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ use ruff_linter::settings::types::{FilePattern, FilePatternSet};
use ruff_python_formatter::{
format_module_source, FormatModuleError, MagicTrailingComma, PreviewMode, PyFormatOptions,
};
use ruff_python_parser::ParseError;
use ruff_workspace::resolver::{python_files_in_path, PyprojectConfig, ResolvedFile, Resolver};

/// Find files that ruff would check so we can format them. Adapted from `ruff_cli`.
Expand Down Expand Up @@ -742,11 +743,11 @@ enum CheckFileError {
reformatted: String,
},
/// The input file was already invalid (not a bug)
SyntaxErrorInInput(FormatModuleError),
SyntaxErrorInInput(ParseError),
/// The formatter introduced a syntax error
SyntaxErrorInOutput {
formatted: String,
error: FormatModuleError,
error: ParseError,
},
/// The formatter failed (bug)
FormatError(FormatError),
Expand Down Expand Up @@ -796,7 +797,7 @@ fn format_dev_file(
let start = Instant::now();
let printed = match format_module_source(&content, options.clone()) {
Ok(printed) => printed,
Err(err @ (FormatModuleError::LexError(_) | FormatModuleError::ParseError(_))) => {
Err(FormatModuleError::ParseError(err)) => {
return Err(CheckFileError::SyntaxErrorInInput(err));
}
Err(FormatModuleError::FormatError(err)) => {
Expand All @@ -823,7 +824,7 @@ fn format_dev_file(
if stability_check {
let reformatted = match format_module_source(formatted, options) {
Ok(reformatted) => reformatted,
Err(err @ (FormatModuleError::LexError(_) | FormatModuleError::ParseError(_))) => {
Err(FormatModuleError::ParseError(err)) => {
return Err(CheckFileError::SyntaxErrorInOutput {
formatted: formatted.to_string(),
error: err,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ pub fn add_noqa_to_path(
if let Some(error) = error {
error!(
"{}",
DisplayParseError::new(error, locator.to_source_code(), source_kind, path)
DisplayParseError::new(&error, &locator.to_source_code(), source_kind, Some(path))
);
}

Expand Down
37 changes: 24 additions & 13 deletions crates/ruff_linter/src/logging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,19 +136,21 @@ pub fn set_up_logging(level: &LogLevel) -> Result<()> {
Ok(())
}

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

impl<'a> DisplayParseError<'a> {
pub fn new(
error: ParseError,
source_code: SourceCode<'a, 'a>,
error: &'a ParseError,
source_code: &'a SourceCode<'a, 'a>,
source_kind: &'a SourceKind,
path: &'a Path,
path: Option<&'a Path>,
) -> Self {
Self {
error,
Expand All @@ -161,13 +163,22 @@ impl<'a> DisplayParseError<'a> {

impl Display for DisplayParseError<'_> {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
write!(
f,
"{header} {path}{colon}",
header = "Failed to parse".bold(),
path = fs::relativize_path(self.path).bold(),
colon = ":".cyan(),
)?;
if let Some(path) = self.path {
write!(
f,
"{header} {path}{colon}",
header = "Failed to parse".bold(),
path = fs::relativize_path(path).bold(),
colon = ":".cyan(),
)?;
} else {
write!(
f,
"{header}{colon}",
header = "Failed to parse".bold(),
colon = ":".cyan(),
)?;
}

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

Expand Down
28 changes: 8 additions & 20 deletions crates/ruff_python_formatter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ use ruff_formatter::{format, FormatError, Formatted, PrintError, Printed, Source
use ruff_python_ast::AstNode;
use ruff_python_ast::Mod;
use ruff_python_index::tokens_and_ranges;
use ruff_python_parser::lexer::LexicalError;
use ruff_python_parser::{parse_ok_tokens, AsMode, ParseError};
use ruff_python_parser::{parse_ok_tokens, AsMode, ParseError, ParseErrorType};
use ruff_python_trivia::CommentRanges;
use ruff_source_file::Locator;

Expand Down Expand Up @@ -108,35 +107,25 @@ where

#[derive(Error, Debug)]
pub enum FormatModuleError {
#[error("source contains syntax errors: {0}")]
LexError(LexicalError),
#[error("source contains syntax errors: {0}")]
ParseError(ParseError),
#[error(transparent)]
ParseError(#[from] ParseError),
#[error(transparent)]
FormatError(#[from] FormatError),
#[error(transparent)]
PrintError(#[from] PrintError),
}

impl From<LexicalError> for FormatModuleError {
fn from(value: LexicalError) -> Self {
Self::LexError(value)
}
}

impl From<ParseError> for FormatModuleError {
fn from(value: ParseError) -> Self {
Self::ParseError(value)
}
}

#[tracing::instrument(name = "format", level = Level::TRACE, skip_all)]
pub fn format_module_source(
source: &str,
options: PyFormatOptions,
) -> Result<Printed, FormatModuleError> {
let source_type = options.source_type();
let (tokens, comment_ranges) = tokens_and_ranges(source, source_type)?;
let (tokens, comment_ranges) =
tokens_and_ranges(source, source_type).map_err(|err| ParseError {
offset: err.location,
error: ParseErrorType::Lexical(err.error),
})?;
let module = parse_ok_tokens(tokens, source, source_type.as_mode())?;
let formatted = format_module_ast(&module, &comment_ranges, source, options)?;
Ok(formatted.print()?)
Expand Down Expand Up @@ -180,7 +169,6 @@ mod tests {

use ruff_python_ast::PySourceType;
use ruff_python_index::tokens_and_ranges;

use ruff_python_parser::{parse_ok_tokens, AsMode};

use crate::{format_module_ast, format_module_source, PyFormatOptions};
Expand Down
11 changes: 4 additions & 7 deletions crates/ruff_python_formatter/src/string/docstring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

use std::{borrow::Cow, collections::VecDeque};

use ruff_python_parser::ParseError;
use {once_cell::sync::Lazy, regex::Regex};

use {
ruff_formatter::{write, FormatOptions, IndentStyle, LineWidth, Printed},
ruff_python_trivia::{is_python_whitespace, PythonWhitespace},
Expand Down Expand Up @@ -499,11 +499,7 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> {
let printed = match docstring_format_source(options, self.quote_char, &codeblob) {
Ok(printed) => printed,
Err(FormatModuleError::FormatError(err)) => return Err(err),
Err(
FormatModuleError::LexError(_)
| FormatModuleError::ParseError(_)
| FormatModuleError::PrintError(_),
) => {
Err(FormatModuleError::ParseError(_) | FormatModuleError::PrintError(_)) => {
return Ok(None);
}
};
Expand Down Expand Up @@ -1518,7 +1514,8 @@ fn docstring_format_source(
use ruff_python_parser::AsMode;

let source_type = options.source_type();
let (tokens, comment_ranges) = ruff_python_index::tokens_and_ranges(source, source_type)?;
let (tokens, comment_ranges) =
ruff_python_index::tokens_and_ranges(source, source_type).map_err(ParseError::from)?;
let module = ruff_python_parser::parse_ok_tokens(tokens, source, source_type.as_mode())?;
let source_code = ruff_formatter::SourceCode::new(source);
let comments = crate::Comments::from_ast(&module, source_code, &comment_ranges);
Expand Down
9 changes: 9 additions & 0 deletions crates/ruff_python_parser/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,15 @@ impl ParseErrorType {
}
}

impl From<LexicalError> for ParseError {
fn from(error: LexicalError) -> Self {
ParseError {
error: ParseErrorType::Lexical(error.error),
offset: error.location,
}
}
}

/// An expression that may be parenthesized.
#[derive(Clone, Debug)]
pub(super) struct ParenthesizedExpr {
Expand Down

0 comments on commit 48e04cc

Please sign in to comment.