Skip to content

Commit f91db73

Browse files
committed
refactor(linter): add CompositeFix::merge_fixes_fallible method (#14093)
Add a method `CompositeFix::merge_fixes_fallible` which returns `Result::Err` in case of overlapping fixes or other illegal states, instead of panicking in debug mode. `CompositeFix::merge_fixes` does the same as it did before, but uses `merge_fixes_fallible` internally. Use an error type `MergeFixesError` instead of `String` to avoid adding string formatting overhead when the primary use case in release mode is to ignore the errors anyway. `merge_fixes_fallible` will be used in JS plugins where we don't want to panic on these illegal states, but output a diagnostic instead.
1 parent 8b7c784 commit f91db73

File tree

1 file changed

+58
-20
lines changed
  • crates/oxc_linter/src/fixer

1 file changed

+58
-20
lines changed

crates/oxc_linter/src/fixer/fix.rs

Lines changed: 58 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
use std::{borrow::Cow, ops::Deref};
1+
use std::{
2+
borrow::Cow,
3+
fmt::{self, Display},
4+
ops::Deref,
5+
};
26

37
use bitflags::bitflags;
48

@@ -525,20 +529,43 @@ impl<'a> CompositeFix<'a> {
525529
}
526530
}
527531

528-
/// Merges multiple fixes to one, returns an [`Fix::empty`] (which will not fix anything) if:
532+
/// Merges multiple fixes to one.
529533
///
530-
/// 1. `fixes` is empty
531-
/// 2. contains overlapped ranges
532-
/// 3. contains negative ranges (span.start > span.end)
534+
/// Returns a [`Fix::empty`] (which will not fix anything) if any of:
535+
/// * `fixes` is empty.
536+
/// * Overlapped ranges.
537+
/// * Negative ranges (`span.start` > `span.end`).
538+
/// * Ranges are out of bounds of `source_text`.
533539
///
534540
/// <https://github.com/eslint/eslint/blob/v9.9.1/lib/linter/report-translator.js#L147-L179>
541+
///
542+
/// # Panics
543+
/// In debug mode, panics if merging fails.
535544
pub fn merge_fixes(fixes: Vec<Fix<'a>>, source_text: &str) -> Fix<'a> {
545+
Self::merge_fixes_fallible(fixes, source_text).unwrap_or_else(|err| {
546+
debug_assert!(false, "{err}");
547+
Fix::empty()
548+
})
549+
}
550+
551+
/// Merges multiple fixes to one.
552+
///
553+
/// # Errors
554+
///
555+
/// Returns a [`MergeFixesError`] error if any of:
556+
/// * Overlapped ranges.
557+
/// * Negative ranges (`span.start` > `span.end`).
558+
/// * Ranges are out of bounds of `source_text`.
559+
pub fn merge_fixes_fallible(
560+
fixes: Vec<Fix<'a>>,
561+
source_text: &str,
562+
) -> Result<Fix<'a>, MergeFixesError> {
536563
let mut fixes = fixes;
537564
if fixes.is_empty() {
538565
// Do nothing
539-
return Fix::empty();
566+
return Ok(Fix::empty());
540567
} else if fixes.len() == 1 {
541-
return fixes.pop().unwrap();
568+
return Ok(fixes.pop().unwrap());
542569
}
543570

544571
fixes.sort_unstable_by(|a, b| a.span.cmp(&b.span));
@@ -558,21 +585,14 @@ impl<'a> CompositeFix<'a> {
558585

559586
// negative range or overlapping ranges is invalid
560587
if span.start > span.end {
561-
debug_assert!(false, "Negative range is invalid: {span:?}");
562-
return Fix::empty();
588+
return Err(MergeFixesError::NegativeRange(span));
563589
}
564590
if last_pos > span.start {
565-
debug_assert!(
566-
false,
567-
"Fix must not be overlapped, last_pos: {}, span.start: {}",
568-
last_pos, span.start
569-
);
570-
return Fix::empty();
591+
return Err(MergeFixesError::Overlap(last_pos, span.start));
571592
}
572593

573594
let Some(before) = source_text.get((last_pos) as usize..span.start as usize) else {
574-
debug_assert!(false, "Invalid range: {}, {}", last_pos, span.start);
575-
return Fix::empty();
595+
return Err(MergeFixesError::InvalidRange(last_pos, span.start));
576596
};
577597

578598
output.reserve(before.len() + content.len());
@@ -582,8 +602,7 @@ impl<'a> CompositeFix<'a> {
582602
}
583603

584604
let Some(after) = source_text.get(last_pos as usize..end as usize) else {
585-
debug_assert!(false, "Invalid range: {:?}", last_pos as usize..end as usize);
586-
return Fix::empty();
605+
return Err(MergeFixesError::InvalidRange(last_pos, end));
587606
};
588607

589608
output.push_str(after);
@@ -592,7 +611,26 @@ impl<'a> CompositeFix<'a> {
592611
if let Some(message) = merged_fix_message {
593612
fix = fix.with_message(message);
594613
}
595-
fix
614+
Ok(fix)
615+
}
616+
}
617+
618+
/// Error returned by [`CompositeFix::merge_fixes_fallible`].
619+
pub enum MergeFixesError {
620+
NegativeRange(Span),
621+
Overlap(u32, u32),
622+
InvalidRange(u32, u32),
623+
}
624+
625+
impl Display for MergeFixesError {
626+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
627+
match self {
628+
Self::NegativeRange(span) => write!(f, "Negative range is invalid: {span:?}"),
629+
Self::Overlap(last_pos, start) => {
630+
write!(f, "Fix must not be overlapped, last_pos: {last_pos}, span.start: {start}")
631+
}
632+
Self::InvalidRange(start, end) => write!(f, "Invalid range: {:?}", start..end),
633+
}
596634
}
597635
}
598636

0 commit comments

Comments
 (0)