Skip to content

Commit

Permalink
Use transformed source code for diagnostic locations
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jan 6, 2024
1 parent 59078c5 commit 9a1917e
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 16 deletions.
73 changes: 73 additions & 0 deletions Untitled.ipynb
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
{
"cells": [
{
"cell_type": "code",
"execution_count": 6,
"id": "54a242d2-f4cf-40dd-8a48-8c08856f9716",
"metadata": {},
"outputs": [
{
"name": "stdout",
"output_type": "stream",
"text": [
"1\n"
]
}
],
"source": [
"print(1)"
]
},
{
"cell_type": "code",
"execution_count": 7,
"id": "8492efd7-5c96-40ab-8afb-8e0a6fff4bc7",
"metadata": {},
"outputs": [
{
"ename": "NameError",
"evalue": "name 'x' is not defined",
"output_type": "error",
"traceback": [
"\u001b[0;31m---------------------------------------------------------------------------\u001b[0m",
"\u001b[0;31mNameError\u001b[0m Traceback (most recent call last)",
"Cell \u001b[0;32mIn[7], line 2\u001b[0m\n\u001b[1;32m 1\u001b[0m \u001b[38;5;28;01mimport\u001b[39;00m \u001b[38;5;21;01msys\u001b[39;00m\n\u001b[0;32m----> 2\u001b[0m \u001b[38;5;28mprint\u001b[39m(\u001b[43mx\u001b[49m)\n",
"\u001b[0;31mNameError\u001b[0m: name 'x' is not defined"
]
}
],
"source": [
"print(x)"
]
},
{
"cell_type": "code",
"execution_count": null,
"id": "25e31239-d707-4096-b9be-c55b46966c02",
"metadata": {},
"outputs": [],
"source": []
}
],
"metadata": {
"kernelspec": {
"display_name": "Python 3 (ipykernel)",
"language": "python",
"name": "python3"
},
"language_info": {
"codemirror_mode": {
"name": "ipython",
"version": 3
},
"file_extension": ".py",
"mimetype": "text/x-python",
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.12.0"
}
},
"nbformat": 4,
"nbformat_minor": 5
}
42 changes: 29 additions & 13 deletions crates/ruff_cli/src/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![cfg_attr(target_family = "wasm", allow(dead_code))]

use std::borrow::Cow;
use std::fs::File;
use std::io;
use std::ops::{Add, AddAssign};
Expand Down Expand Up @@ -273,6 +274,7 @@ pub(crate) fn lint_path(
data: (messages, imports),
error: parse_error,
},
transformed,
fixed,
) = if matches!(fix_mode, flags::FixMode::Apply | flags::FixMode::Diff) {
if let Ok(FixerResult {
Expand Down Expand Up @@ -301,7 +303,12 @@ pub(crate) fn lint_path(
flags::FixMode::Generate => {}
}
}
(result, fixed)
let transformed = if let Cow::Owned(transformed) = transformed {
transformed
} else {
source_kind
};
(result, transformed, fixed)
} else {
// If we fail to fix, lint the original source code.
let result = lint_only(
Expand All @@ -313,8 +320,9 @@ pub(crate) fn lint_path(
source_type,
ParseSource::None,
);
let transformed = source_kind;
let fixed = FxHashMap::default();
(result, fixed)
(result, transformed, fixed)
}
} else {
let result = lint_only(
Expand All @@ -326,16 +334,17 @@ pub(crate) fn lint_path(
source_type,
ParseSource::None,
);
let transformed = source_kind;
let fixed = FxHashMap::default();
(result, fixed)
(result, transformed, fixed)
};

let imports = imports.unwrap_or_default();

if let Some((cache, relative_path, key)) = caching {
// We don't cache parsing errors.
if parse_error.is_none() {
// `FixMode::Generate` and `FixMode::Diff` rely on side-effects (writing to disk,
// `FixMode::Apply` and `FixMode::Diff` rely on side-effects (writing to disk,
// and writing the diff to stdout, respectively). If a file has diagnostics, we
// need to avoid reading from and writing to the cache in these modes.
if match fix_mode {
Expand All @@ -350,7 +359,7 @@ pub(crate) fn lint_path(
LintCacheData::from_messages(
&messages,
imports.clone(),
source_kind.as_ipy_notebook().map(Notebook::index).cloned(),
transformed.as_ipy_notebook().map(Notebook::index).cloned(),
),
);
}
Expand All @@ -360,11 +369,11 @@ pub(crate) fn lint_path(
if let Some(error) = parse_error {
error!(
"{}",
DisplayParseError::from_source_kind(error, Some(path.to_path_buf()), &source_kind,)
DisplayParseError::from_source_kind(error, Some(path.to_path_buf()), &transformed)
);
}

let notebook_indexes = if let SourceKind::IpyNotebook(notebook) = source_kind {
let notebook_indexes = if let SourceKind::IpyNotebook(notebook) = transformed {
FxHashMap::from_iter([(path.to_string_lossy().to_string(), notebook.into_index())])
} else {
FxHashMap::default()
Expand Down Expand Up @@ -415,6 +424,7 @@ pub(crate) fn lint_stdin(
data: (messages, imports),
error: parse_error,
},
transformed,
fixed,
) = if matches!(fix_mode, flags::FixMode::Apply | flags::FixMode::Diff) {
if let Ok(FixerResult {
Expand Down Expand Up @@ -443,8 +453,12 @@ pub(crate) fn lint_stdin(
}
flags::FixMode::Generate => {}
}

(result, fixed)
let transformed = if let Cow::Owned(transformed) = transformed {
transformed
} else {
source_kind
};
(result, transformed, fixed)
} else {
// If we fail to fix, lint the original source code.
let result = lint_only(
Expand All @@ -456,14 +470,15 @@ pub(crate) fn lint_stdin(
source_type,
ParseSource::None,
);
let fixed = FxHashMap::default();

// Write the contents to stdout anyway.
if fix_mode.is_apply() {
source_kind.write(&mut io::stdout().lock())?;
}

(result, fixed)
let transformed = source_kind;
let fixed = FxHashMap::default();
(result, transformed, fixed)
}
} else {
let result = lint_only(
Expand All @@ -475,8 +490,9 @@ pub(crate) fn lint_stdin(
source_type,
ParseSource::None,
);
let transformed = source_kind;
let fixed = FxHashMap::default();
(result, fixed)
(result, transformed, fixed)
};

let imports = imports.unwrap_or_default();
Expand All @@ -488,7 +504,7 @@ pub(crate) fn lint_stdin(
);
}

let notebook_indexes = if let SourceKind::IpyNotebook(notebook) = source_kind {
let notebook_indexes = if let SourceKind::IpyNotebook(notebook) = transformed {
FxHashMap::from_iter([(
path.map_or_else(|| "-".into(), |path| path.to_string_lossy().to_string()),
notebook.into_index(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ pub(crate) fn no_newline_at_end_of_file(
}

if !source.ends_with(['\n', '\r']) {
// Note: if `lines.last()` is `None`, then `contents` is empty (and so we don't
// want to raise W292 anyway).
// Both locations are at the end of the file (and thus the same).
let range = TextRange::empty(locator.contents().text_len());

let mut diagnostic = Diagnostic::new(MissingNewlineAtEndOfFile, range);
Expand All @@ -60,5 +57,6 @@ pub(crate) fn no_newline_at_end_of_file(
)));
return Some(diagnostic);
}

None
}

0 comments on commit 9a1917e

Please sign in to comment.