Skip to content

Commit 6789f00

Browse files
committed
[internal] Return Messages from check_path
Summary -- This PR updates `check_path` in the `ruff_linter` crate to return a `Vec<Message>` instead of a `Vec<Diagnostic>`. The main motivation for this is to make it easier to convert semantic syntax errors directly into `Message`s rather than `Diagnostic`s in #16106. However, this also has the benefit of keeping the preview check on unsupported syntax errors in `check_path`, as suggested in https://github.com/astral-sh/ruff/pull/16429/files#r1974748024. Test Plan -- Existing tests. I also tested the playground and server manually.
1 parent dcf31c9 commit 6789f00

File tree

11 files changed

+175
-214
lines changed

11 files changed

+175
-214
lines changed

crates/ruff/src/cache.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,7 @@ impl FileCache {
353353
fix: msg.fix.clone(),
354354
file: file.clone(),
355355
noqa_offset: msg.noqa_offset,
356+
parent: msg.parent,
356357
})
357358
})
358359
.collect()
@@ -445,6 +446,7 @@ impl LintCacheData {
445446
CacheMessage {
446447
kind: msg.kind.clone(),
447448
range: msg.range,
449+
parent: msg.parent,
448450
fix: msg.fix.clone(),
449451
noqa_offset: msg.noqa_offset,
450452
}
@@ -465,6 +467,7 @@ pub(super) struct CacheMessage {
465467
kind: DiagnosticKind,
466468
/// Range into the message's [`FileCache::source`].
467469
range: TextRange,
470+
parent: Option<TextSize>,
468471
fix: Option<Fix>,
469472
noqa_offset: TextSize,
470473
}
@@ -702,7 +705,10 @@ mod tests {
702705
.unwrap();
703706
}
704707

705-
assert_eq!(expected_diagnostics, got_diagnostics);
708+
assert_eq!(
709+
expected_diagnostics, got_diagnostics,
710+
"left == {expected_diagnostics:#?}, right == {got_diagnostics:#?}",
711+
);
706712
}
707713

708714
#[test]

crates/ruff_linter/src/fix/edits.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,7 @@ fn all_lines_fit(
592592
#[cfg(test)]
593593
mod tests {
594594
use anyhow::{anyhow, Result};
595+
use ruff_source_file::SourceFileBuilder;
595596
use test_case::test_case;
596597

597598
use ruff_diagnostics::{Diagnostic, Edit, Fix};
@@ -604,6 +605,7 @@ mod tests {
604605
use crate::fix::edits::{
605606
add_to_dunder_all, make_redundant_alias, next_stmt_break, trailing_semicolon,
606607
};
608+
use crate::message::DiagnosticMessage;
607609
use crate::Locator;
608610

609611
/// Parse the given source using [`Mode::Module`] and return the first statement.
@@ -735,14 +737,22 @@ x = 1 \
735737
let diag = {
736738
use crate::rules::pycodestyle::rules::MissingNewlineAtEndOfFile;
737739
let mut iter = edits.into_iter();
738-
Diagnostic::new(
740+
let diag = Diagnostic::new(
739741
MissingNewlineAtEndOfFile, // The choice of rule here is arbitrary.
740742
TextRange::default(),
741743
)
742744
.with_fix(Fix::safe_edits(
743745
iter.next().ok_or(anyhow!("expected edits nonempty"))?,
744746
iter,
745-
))
747+
));
748+
DiagnosticMessage {
749+
kind: diag.kind,
750+
range: diag.range,
751+
fix: diag.fix,
752+
parent: diag.parent,
753+
file: SourceFileBuilder::new("<filename>", "<code>").finish(),
754+
noqa_offset: TextSize::default(),
755+
}
746756
};
747757
assert_eq!(apply_fixes([diag].iter(), &locator).code, expect);
748758
Ok(())

crates/ruff_linter/src/fix/mod.rs

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@ use std::collections::BTreeSet;
33
use itertools::Itertools;
44
use rustc_hash::{FxHashMap, FxHashSet};
55

6-
use ruff_diagnostics::{Diagnostic, Edit, Fix, IsolationLevel, SourceMap};
6+
use ruff_diagnostics::{Edit, Fix, IsolationLevel, SourceMap};
77
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
88

99
use crate::linter::FixTable;
10+
use crate::message::{DiagnosticMessage, Message};
1011
use crate::registry::{AsRule, Rule};
1112
use crate::settings::types::UnsafeFixes;
1213
use crate::Locator;
@@ -26,14 +27,15 @@ pub(crate) struct FixResult {
2627

2728
/// Fix errors in a file, and write the fixed source code to disk.
2829
pub(crate) fn fix_file(
29-
diagnostics: &[Diagnostic],
30+
diagnostics: &[Message],
3031
locator: &Locator,
3132
unsafe_fixes: UnsafeFixes,
3233
) -> Option<FixResult> {
3334
let required_applicability = unsafe_fixes.required_applicability();
3435

3536
let mut with_fixes = diagnostics
3637
.iter()
38+
.filter_map(Message::as_diagnostic_message)
3739
.filter(|diagnostic| {
3840
diagnostic
3941
.fix
@@ -51,7 +53,7 @@ pub(crate) fn fix_file(
5153

5254
/// Apply a series of fixes.
5355
fn apply_fixes<'a>(
54-
diagnostics: impl Iterator<Item = &'a Diagnostic>,
56+
diagnostics: impl Iterator<Item = &'a DiagnosticMessage>,
5557
locator: &'a Locator<'a>,
5658
) -> FixResult {
5759
let mut output = String::with_capacity(locator.len());
@@ -161,22 +163,26 @@ fn cmp_fix(rule1: Rule, rule2: Rule, fix1: &Fix, fix2: &Fix) -> std::cmp::Orderi
161163

162164
#[cfg(test)]
163165
mod tests {
164-
use ruff_diagnostics::{Diagnostic, Edit, Fix, SourceMarker};
166+
use ruff_diagnostics::{Edit, Fix, SourceMarker};
167+
use ruff_source_file::SourceFileBuilder;
165168
use ruff_text_size::{Ranged, TextSize};
166169

167170
use crate::fix::{apply_fixes, FixResult};
171+
use crate::message::DiagnosticMessage;
168172
use crate::rules::pycodestyle::rules::MissingNewlineAtEndOfFile;
169173
use crate::Locator;
170174

171175
#[allow(deprecated)]
172-
fn create_diagnostics(edit: impl IntoIterator<Item = Edit>) -> Vec<Diagnostic> {
176+
fn create_diagnostics(edit: impl IntoIterator<Item = Edit>) -> Vec<DiagnosticMessage> {
173177
edit.into_iter()
174-
.map(|edit| Diagnostic {
178+
.map(|edit| DiagnosticMessage {
175179
// The choice of rule here is arbitrary.
176180
kind: MissingNewlineAtEndOfFile.into(),
177181
range: edit.range(),
178182
fix: Some(Fix::safe_edit(edit)),
179183
parent: None,
184+
file: SourceFileBuilder::new("<filename>", "<code>").finish(),
185+
noqa_offset: TextSize::default(),
180186
})
181187
.collect()
182188
}

crates/ruff_linter/src/linter.rs

Lines changed: 21 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ pub struct FixerResult<'a> {
5858
pub fixed: FixTable,
5959
}
6060

61-
/// Generate `Diagnostic`s from the source code contents at the
62-
/// given `Path`.
61+
/// Generate [`Message`]s from the source code contents at the given `Path`.
6362
#[allow(clippy::too_many_arguments)]
6463
pub fn check_path(
6564
path: &Path,
@@ -74,7 +73,7 @@ pub fn check_path(
7473
source_type: PySourceType,
7574
parsed: &Parsed<ModModule>,
7675
target_version: PythonVersion,
77-
) -> Vec<Diagnostic> {
76+
) -> Vec<Message> {
7877
// Aggregate all diagnostics.
7978
let mut diagnostics = vec![];
8079

@@ -322,7 +321,20 @@ pub fn check_path(
322321
}
323322
}
324323

325-
diagnostics
324+
let syntax_errors = if settings.preview.is_enabled() {
325+
parsed.unsupported_syntax_errors()
326+
} else {
327+
&[]
328+
};
329+
330+
diagnostics_to_messages(
331+
diagnostics,
332+
parsed.errors(),
333+
syntax_errors,
334+
path,
335+
locator,
336+
directives,
337+
)
326338
}
327339

328340
const MAX_ITERATIONS: usize = 100;
@@ -417,7 +429,7 @@ pub fn lint_only(
417429
);
418430

419431
// Generate diagnostics.
420-
let diagnostics = check_path(
432+
let messages = check_path(
421433
path,
422434
package,
423435
&locator,
@@ -432,21 +444,8 @@ pub fn lint_only(
432444
target_version,
433445
);
434446

435-
let syntax_errors = if settings.preview.is_enabled() {
436-
parsed.unsupported_syntax_errors()
437-
} else {
438-
&[]
439-
};
440-
441447
LinterResult {
442-
messages: diagnostics_to_messages(
443-
diagnostics,
444-
parsed.errors(),
445-
syntax_errors,
446-
path,
447-
&locator,
448-
&directives,
449-
),
448+
messages,
450449
has_syntax_error: parsed.has_syntax_errors(),
451450
}
452451
}
@@ -591,22 +590,9 @@ pub fn lint_fix<'a>(
591590
report_failed_to_converge_error(path, transformed.source_code(), &diagnostics);
592591
}
593592

594-
let syntax_errors = if settings.preview.is_enabled() {
595-
parsed.unsupported_syntax_errors()
596-
} else {
597-
&[]
598-
};
599-
600593
return Ok(FixerResult {
601594
result: LinterResult {
602-
messages: diagnostics_to_messages(
603-
diagnostics,
604-
parsed.errors(),
605-
syntax_errors,
606-
path,
607-
&locator,
608-
&directives,
609-
),
595+
messages: diagnostics,
610596
has_syntax_error: !is_valid_syntax,
611597
},
612598
transformed,
@@ -625,8 +611,8 @@ fn collect_rule_codes(rules: impl IntoIterator<Item = Rule>) -> String {
625611
}
626612

627613
#[allow(clippy::print_stderr)]
628-
fn report_failed_to_converge_error(path: &Path, transformed: &str, diagnostics: &[Diagnostic]) {
629-
let codes = collect_rule_codes(diagnostics.iter().map(|diagnostic| diagnostic.kind.rule()));
614+
fn report_failed_to_converge_error(path: &Path, transformed: &str, diagnostics: &[Message]) {
615+
let codes = collect_rule_codes(diagnostics.iter().filter_map(Message::rule));
630616
if cfg!(debug_assertions) {
631617
eprintln!(
632618
"{}{} Failed to converge after {} iterations in `{}` with rule codes {}:---\n{}\n---",

crates/ruff_linter/src/message/mod.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,24 +41,25 @@ mod text;
4141

4242
/// Message represents either a diagnostic message corresponding to a rule violation or a syntax
4343
/// error message raised by the parser.
44-
#[derive(Debug, PartialEq, Eq)]
44+
#[derive(Clone, Debug, PartialEq, Eq)]
4545
pub enum Message {
4646
Diagnostic(DiagnosticMessage),
4747
SyntaxError(SyntaxErrorMessage),
4848
}
4949

5050
/// A diagnostic message corresponding to a rule violation.
51-
#[derive(Debug, PartialEq, Eq)]
51+
#[derive(Clone, Debug, PartialEq, Eq)]
5252
pub struct DiagnosticMessage {
5353
pub kind: DiagnosticKind,
5454
pub range: TextRange,
5555
pub fix: Option<Fix>,
56+
pub parent: Option<TextSize>,
5657
pub file: SourceFile,
5758
pub noqa_offset: TextSize,
5859
}
5960

6061
/// A syntax error message raised by the parser.
61-
#[derive(Debug, PartialEq, Eq)]
62+
#[derive(Clone, Debug, PartialEq, Eq)]
6263
pub struct SyntaxErrorMessage {
6364
pub message: String,
6465
pub range: TextRange,
@@ -91,6 +92,7 @@ impl Message {
9192
range: diagnostic.range(),
9293
kind: diagnostic.kind,
9394
fix: diagnostic.fix,
95+
parent: diagnostic.parent,
9496
file,
9597
noqa_offset,
9698
})
@@ -140,6 +142,13 @@ impl Message {
140142
}
141143
}
142144

145+
pub fn into_diagnostic_message(self) -> Option<DiagnosticMessage> {
146+
match self {
147+
Message::Diagnostic(m) => Some(m),
148+
Message::SyntaxError(_) => None,
149+
}
150+
}
151+
143152
/// Returns `true` if `self` is a syntax error message.
144153
pub const fn is_syntax_error(&self) -> bool {
145154
matches!(self, Message::SyntaxError(_))

0 commit comments

Comments
 (0)