Simplify LinterResult, avoid cloning ParseError

crates/ruff/src/diagnostics.rs

@@ -264,8 +264,8 @@ pub(crate) fn lint_path(
  // Lint the file.
  let (
  LinterResult {
- data: messages,
- error: parse_error,
+ messages,
+ has_syntax_error: has_error,
  },
  transformed,
  fixed,
@@ -334,7 +334,7 @@ pub(crate) fn lint_path(
 
  if let Some((cache, relative_path, key)) = caching {
  // We don't cache parsing errors.
- if parse_error.is_none() {
+ if !has_error {
  // `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.
@@ -400,7 +400,7 @@ pub(crate) fn lint_stdin(
  };
 
  // Lint the inputs.
- let (LinterResult { data: messages, .. }, transformed, fixed) =
+ let (LinterResult { messages, .. }, transformed, fixed) =
  if matches!(fix_mode, flags::FixMode::Apply | flags::FixMode::Diff) {
  if let Ok(FixerResult {
  result,

crates/ruff_benchmark/benches/linter.rs

@@ -73,7 +73,7 @@ fn benchmark_linter(mut group: BenchmarkGroup, settings: &LinterSettings) {
  );
 
  // Assert that file contains no parse errors
- assert_eq!(result.error, None);
+ assert!(!result.has_syntax_error);
  },
  criterion::BatchSize::SmallInput,
  );

crates/ruff_linter/src/linter.rs

@@ -35,29 +35,19 @@ use crate::settings::{flags, LinterSettings};
 use crate::source_kind::SourceKind;
 use crate::{directives, fs};
 
-/// A [`Result`]-like type that returns both data and an error. Used to return
-/// diagnostics even in the face of parse errors, since many diagnostics can be
-/// generated without a full AST.
-pub struct LinterResult<T> {
- pub data: T,
- pub error: Option<ParseError>,
-}
-
-impl<T> LinterResult<T> {
- const fn new(data: T, error: Option<ParseError>) -> Self {
- Self { data, error }
- }
-
- fn map<U, F: FnOnce(T) -> U>(self, f: F) -> LinterResult<U> {
- LinterResult::new(f(self.data), self.error)
- }
+pub struct LinterResult {
+ /// A collection of diagnostic messages generated by the linter.
+ pub messages: Vec<Message>,
+ /// A flag indicating the presence of syntax errors in the source file.
+ /// If `true`, at least one syntax error was detected in the source file.
+ pub has_syntax_error: bool,
 }
 
 pub type FixTable = FxHashMap<Rule, usize>;
 
 pub struct FixerResult<'a> {
  /// The result returned by the linter, after applying any fixes.
- pub result: LinterResult<Vec<Message>>,
+ pub result: LinterResult,
  /// The resulting source code, after applying any fixes.
  pub transformed: Cow<'a, SourceKind>,
  /// The number of fixes applied for each [`Rule`].
@@ -79,7 +69,7 @@ pub fn check_path(
  source_kind: &SourceKind,
  source_type: PySourceType,
  parsed: &Parsed<ModModule>,
-) -> LinterResult<Vec<Diagnostic>> {
+) -> Vec<Diagnostic> {
  // Aggregate all diagnostics.
  let mut diagnostics = vec![];
 
@@ -317,7 +307,7 @@ pub fn check_path(
  }
  }
 
- LinterResult::new(diagnostics, parsed.errors().iter().next().cloned())
+ diagnostics
 }
 
 const MAX_ITERATIONS: usize = 100;
@@ -351,9 +341,7 @@ pub fn add_noqa_to_path(
  );
 
  // Generate diagnostics, ignoring any existing `noqa` directives.
- let LinterResult {
- data: diagnostics, ..
- } = check_path(
+ let diagnostics = check_path(
  path,
  package,
  &locator,
@@ -390,7 +378,7 @@ pub fn lint_only(
  source_kind: &SourceKind,
  source_type: PySourceType,
  source: ParseSource,
-) -> LinterResult<Vec<Message>> {
+) -> LinterResult {
  let parsed = source.into_parsed(source_kind, source_type);
 
  // Map row and column locations to byte slices (lazily).
@@ -411,7 +399,7 @@ pub fn lint_only(
  );
 
  // Generate diagnostics.
- let result = check_path(
+ let diagnostics = check_path(
  path,
  package,
  &locator,
@@ -425,9 +413,16 @@ pub fn lint_only(
  &parsed,
  );
 
- result.map(|diagnostics| {
- diagnostics_to_messages(diagnostics, parsed.errors(), path, &locator, &directives)
- })
+ LinterResult {
+ messages: diagnostics_to_messages(
+ diagnostics,
+ parsed.errors(),
+ path,
+ &locator,
+ &directives,
+ ),
+ has_syntax_error: !parsed.is_valid(),
+ }
 }
 
 /// Convert from diagnostics to messages.
@@ -479,8 +474,8 @@ pub fn lint_fix<'a>(
  // As an escape hatch, bail after 100 iterations.
  let mut iterations = 0;
 
- // Track whether the _initial_ source code was parseable.
- let mut parseable = false;
+ // Track whether the _initial_ source code is valid syntax.
+ let mut is_valid_syntax = false;
 
  // Continuously fix until the source code stabilizes.
  loop {
@@ -506,7 +501,7 @@ pub fn lint_fix<'a>(
  );
 
  // Generate diagnostics.
- let result = check_path(
+ let diagnostics = check_path(
  path,
  package,
  &locator,
@@ -521,19 +516,21 @@ pub fn lint_fix<'a>(
  );
 
  if iterations == 0 {
- parseable = result.error.is_none();
+ is_valid_syntax = parsed.is_valid();
  } else {
  // If the source code was parseable on the first pass, but is no
  // longer parseable on a subsequent pass, then we've introduced a
  // syntax error. Return the original code.
- if parseable && result.error.is_some() {
- report_fix_syntax_error(
- path,
- transformed.source_code(),
- &result.error.unwrap(),
- fixed.keys().copied(),
- );
- return Err(anyhow!("Fix introduced a syntax error"));
+ if is_valid_syntax {
+ if let Some(error) = parsed.errors().first() {
+ report_fix_syntax_error(
+ path,
+ transformed.source_code(),
+ error,
+ fixed.keys().copied(),
+ );
+ return Err(anyhow!("Fix introduced a syntax error"));
+ }
  }
  }
 
@@ -542,7 +539,7 @@ pub fn lint_fix<'a>(
  code: fixed_contents,
  fixes: applied,
  source_map,
- }) = fix_file(&result.data, &locator, unsafe_fixes)
+ }) = fix_file(&diagnostics, &locator, unsafe_fixes)
  {
  if iterations < MAX_ITERATIONS {
  // Count the number of fixed errors.
@@ -559,13 +556,20 @@ pub fn lint_fix<'a>(
  continue;
  }
 
- report_failed_to_converge_error(path, transformed.source_code(), &result.data);
+ report_failed_to_converge_error(path, transformed.source_code(), &diagnostics);
  }
 
  return Ok(FixerResult {
- result: result.map(|diagnostics| {
- diagnostics_to_messages(diagnostics, parsed.errors(), path, &locator, &directives)
- }),
+ result: LinterResult {
+ messages: diagnostics_to_messages(
+ diagnostics,
+ parsed.errors(),
+ path,
+ &locator,
+ &directives,
+ ),
+ has_syntax_error: !is_valid_syntax,
+ },
  transformed,
  fixed,
  });

crates/ruff_linter/src/rules/pyflakes/mod.rs

@@ -22,7 +22,7 @@ mod tests {
  use ruff_source_file::Locator;
  use ruff_text_size::Ranged;
 
- use crate::linter::{check_path, LinterResult};
+ use crate::linter::check_path;
  use crate::registry::{AsRule, Linter, Rule};
  use crate::rules::pyflakes;
  use crate::settings::types::PreviewMode;
@@ -650,10 +650,7 @@ mod tests {
  &locator,
  &indexer,
  );
- let LinterResult {
- data: mut diagnostics,
- ..
- } = check_path(
+ let mut diagnostics = check_path(
  Path::new("<filename>"),
  None,
  &locator,

crates/ruff_linter/src/test.rs

@@ -23,7 +23,7 @@ use ruff_text_size::Ranged;
 
 use crate::directives;
 use crate::fix::{fix_file, FixResult};
-use crate::linter::{check_path, LinterResult};
+use crate::linter::check_path;
 use crate::message::{Emitter, EmitterContext, Message, TextEmitter};
 use crate::packaging::detect_package_root;
 use crate::registry::AsRule;
@@ -119,10 +119,7 @@ pub(crate) fn test_contents<'a>(
  &locator,
  &indexer,
  );
- let LinterResult {
- data: diagnostics,
- error,
- } = check_path(
+ let diagnostics = check_path(
  path,
  path.parent()
  .and_then(|parent| detect_package_root(parent, &settings.namespace_packages)),
@@ -137,7 +134,7 @@ pub(crate) fn test_contents<'a>(
  &parsed,
  );
 
- let source_has_errors = error.is_some();
+ let source_has_errors = !parsed.is_valid();
 
  // Detect fixes that don't converge after multiple iterations.
  let mut iterations = 0;
@@ -186,10 +183,7 @@ pub(crate) fn test_contents<'a>(
  &indexer,
  );
 
- let LinterResult {
- data: fixed_diagnostics,
- ..
- } = check_path(
+ let fixed_diagnostics = check_path(
  path,
  None,
  &locator,

crates/ruff_server/src/fix.rs

@@ -68,7 +68,10 @@ pub(crate) fn fix_all(
  // which is inconsistent with how `ruff check --fix` works.
  let FixerResult {
  transformed,
- result: LinterResult { error, .. },
+ result: LinterResult {
+ has_syntax_error: has_error,
+ ..
+ },
  ..
  } = ruff_linter::linter::lint_fix(
  &query.virtual_file_path(),
@@ -80,11 +83,9 @@ pub(crate) fn fix_all(
  source_type,
  )?;
 
- if let Some(error) = error {
+ if has_error {
  // abort early if a parsing error occurred
- return Err(anyhow::anyhow!(
- "A parsing error occurred during `fix_all`: {error}"
- ));
+ return Err(anyhow::anyhow!("A parsing error occurred during `fix_all`"));
  }
 
  // fast path: if `transformed` is still borrowed, no changes were made and we can return early