From 087d92cbf44c8484b70cbcd83af3025c26852d2b Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Tue, 4 Mar 2025 17:00:31 +0000 Subject: [PATCH] Formatter: Fix syntax error location in notebooks (#16499) ## Summary Fixes https://github.com/astral-sh/ruff/issues/16476 fixes: #11453 We format notebooks cell by cell. That means, that offsets in parse errors are relative to the cell and not the entire document. We didn't account for this fact when emitting syntax errors for notebooks in the formatter. This PR ensures that we correctly offset parse errors by the cell location. ## Test Plan Added test (it panicked before) --- Cargo.lock | 1 + crates/ruff/Cargo.toml | 1 + crates/ruff/src/commands/format.rs | 7 ++- crates/ruff/tests/format.rs | 81 ++++++++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 56b2e3a0e0..d79795d463 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2693,6 +2693,7 @@ dependencies = [ "ruff_notebook", "ruff_python_ast", "ruff_python_formatter", + "ruff_python_parser", "ruff_server", "ruff_source_file", "ruff_text_size", diff --git a/crates/ruff/Cargo.toml b/crates/ruff/Cargo.toml index 30b2e74913..c9059a7eab 100644 --- a/crates/ruff/Cargo.toml +++ b/crates/ruff/Cargo.toml @@ -22,6 +22,7 @@ ruff_macros = { workspace = true } ruff_notebook = { workspace = true } ruff_python_ast = { workspace = true } ruff_python_formatter = { workspace = true } +ruff_python_parser = { workspace = true } ruff_server = { workspace = true } ruff_source_file = { workspace = true } ruff_text_size = { workspace = true } diff --git a/crates/ruff/src/commands/format.rs b/crates/ruff/src/commands/format.rs index 1702dcf6f8..1b78773b2a 100644 --- a/crates/ruff/src/commands/format.rs +++ b/crates/ruff/src/commands/format.rs @@ -11,6 +11,7 @@ use itertools::Itertools; use log::{error, warn}; use rayon::iter::Either::{Left, Right}; use rayon::iter::{IntoParallelRefIterator, ParallelIterator}; +use ruff_python_parser::ParseError; use rustc_hash::FxHashSet; use thiserror::Error; use tracing::debug; @@ -406,8 +407,12 @@ pub(crate) fn format_source( let formatted = format_module_source(unformatted, options.clone()).map_err(|err| { if let FormatModuleError::ParseError(err) = err { + // Offset the error by the start of the cell DisplayParseError::from_source_kind( - err, + ParseError { + error: err.error, + location: err.location.checked_add(*start).unwrap(), + }, path.map(Path::to_path_buf), source_kind, ) diff --git a/crates/ruff/tests/format.rs b/crates/ruff/tests/format.rs index 01839ebc8c..27e3db39e1 100644 --- a/crates/ruff/tests/format.rs +++ b/crates/ruff/tests/format.rs @@ -1699,6 +1699,87 @@ fn test_notebook_trailing_semicolon() { "##); } +#[test] +fn syntax_error_in_notebooks() -> Result<()> { + let tempdir = TempDir::new()?; + + let ruff_toml = tempdir.path().join("ruff.toml"); + fs::write( + &ruff_toml, + r#" +include = ["*.ipy"] +"#, + )?; + + fs::write( + tempdir.path().join("main.ipy"), + r#" +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "metadata": { + "id": "S6nTuMqGGqp2" + }, + "outputs": [], + "source": [ + "np.random.seed(RANDOM_STATE)\n", + "X = pd.DataFrame(data=X, columns=np.arange(0, X.shape[1]))\n", + "X[10] = X[6] + X[7] + np.random.random() * 0.01" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": { + "id": "fTZWxz1zpb9R" + }, + "outputs": [], + "source": [ + "for i in range(iterations):\n", + " # выберите случайный индекс в диапазон от 0 до len(X)-1 включительно при помощи функции random.randint\n", + " j = # ваш код здесь\n" + ] + } + ], + "metadata": { + "colab": { + "provenance": [] + }, + "kernelspec": { + "display_name": "ml", + "language": "python", + "name": "python3" + }, + "language_info": { + "name": "python", + "version": "3.12.9" + } + }, + "nbformat": 4, + "nbformat_minor": 0 + } +"#, + )?; + + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .current_dir(tempdir.path()) + .arg("format") + .arg("--no-cache") + .args(["--config", &ruff_toml.file_name().unwrap().to_string_lossy()]) + .args(["--extension", "ipy:ipynb"]) + .arg("."), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Failed to parse main.ipy:2:3:24: Expected an expression + "); + Ok(()) +} + #[test] fn extension() -> Result<()> { let tempdir = TempDir::new()?;