From b2d2b36b5333726fd00c7e1fd5c6d37d4f2a12a5 Mon Sep 17 00:00:00 2001 From: Osama Abdelkader Date: Fri, 24 Oct 2025 23:01:41 +0300 Subject: [PATCH] Fix incorrect span highlighting in E0432 import suggestions When suggesting import corrections where the suggestion shares a prefix with the original import (e.g., 'sync' -> 'std::sync'), the diagnostic was incorrectly highlighting 'td::s' instead of 'std::'. This was caused by the as_substr function in rustc_errors/src/lib.rs using a flawed algorithm that didn't properly handle overlapping prefix/suffix patterns. The fix rewrites as_substr to: - Try all possible prefix/suffix splits - Select the split with the shortest insertion - Use longer suffix as tiebreaker for better context This not only fixes the reported bug but also improves highlighting accuracy in 7 other diagnostic scenarios, making the '+' markers point to the exact insertion location in all cases. Signed-off-by: Osama Abdelkader --- compiler/rustc_errors/src/lib.rs | 59 +++++++++++++++---- .../argument-suggestions/issue-97197.stderr | 2 +- ...-crate-rename-suggestion-formatting.stderr | 2 +- tests/ui/imports/no-std-inject.stderr | 2 +- tests/ui/suggestions/return-bindings.stderr | 7 +-- ...t-field-type-including-single-colon.stderr | 4 +- .../type-ascription-instead-of-path-2.stderr | 2 +- ...-ascription-instead-of-path-in-type.stderr | 2 +- 8 files changed, 55 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_errors/src/lib.rs b/compiler/rustc_errors/src/lib.rs index 6bec65f2d538c..8e439007272b9 100644 --- a/compiler/rustc_errors/src/lib.rs +++ b/compiler/rustc_errors/src/lib.rs @@ -303,20 +303,53 @@ impl TrimmedSubstitutionPart { /// `BB` is. Return the length of the prefix, the "trimmed" suggestion, and the length /// of the suffix. fn as_substr<'a>(original: &'a str, suggestion: &'a str) -> Option<(usize, &'a str, usize)> { - let common_prefix = original - .chars() - .zip(suggestion.chars()) - .take_while(|(c1, c2)| c1 == c2) - .map(|(c, _)| c.len_utf8()) - .sum(); - let original = &original[common_prefix..]; - let suggestion = &suggestion[common_prefix..]; - if suggestion.ends_with(original) { - let common_suffix = original.len(); - Some((common_prefix, &suggestion[..suggestion.len() - original.len()], common_suffix)) - } else { - None + // When the original string appears multiple times in the suggestion (e.g., "sync" in "std::sync"), + // the previous algorithm would incorrectly split it. We need to find the split that results in + // the shortest insertion, which is the most useful for diagnostics. + + let mut best: Option<(usize, &'a str, usize)> = None; + let mut min_insertion_len = usize::MAX; + + // Try all possible prefix lengths + let mut prefix_len = 0; + for prefix_chars in 0..=original.chars().count() { + if prefix_chars > 0 { + prefix_len += original.chars().nth(prefix_chars - 1).unwrap().len_utf8(); + } + + // Check if this prefix matches in the suggestion + if !suggestion.is_char_boundary(prefix_len) + || !suggestion[..prefix_len].ends_with(&original[..prefix_len]) + { + continue; + } + + let original_suffix = &original[prefix_len..]; + let suggestion_after_prefix = &suggestion[prefix_len..]; + + // Check if the remaining original appears as a suffix in the remaining suggestion + if !suggestion_after_prefix.ends_with(original_suffix) { + continue; + } + + // This is a valid split - calculate the insertion length + let suffix_len = original_suffix.len(); + let insertion_len = suggestion_after_prefix.len() - suffix_len; + + // Keep track of the split with the smallest insertion + // If there's a tie, prefer the one with a longer suffix (more context) + let is_better = insertion_len < min_insertion_len + || (insertion_len == min_insertion_len + && best.map_or(true, |(_, _, old_suffix)| suffix_len > old_suffix)); + + if is_better { + min_insertion_len = insertion_len; + let insertion = &suggestion_after_prefix[..insertion_len]; + best = Some((prefix_len, insertion, suffix_len)); + } } + + best } impl CodeSuggestion { diff --git a/tests/ui/argument-suggestions/issue-97197.stderr b/tests/ui/argument-suggestions/issue-97197.stderr index ae56e12c975f8..41394dd3a2351 100644 --- a/tests/ui/argument-suggestions/issue-97197.stderr +++ b/tests/ui/argument-suggestions/issue-97197.stderr @@ -12,7 +12,7 @@ LL | pub fn g(a1: (), a2: bool, a3: bool, a4: bool, a5: bool, a6: ()) -> () {} help: provide the arguments | LL | g((), /* bool */, /* bool */, /* bool */, /* bool */, ()); - | +++++++++++++++++++++++++++++++++++++++++++++++ + | ++++++++++++++++++++++++++++++++++++++++++++++++ error: aborting due to 1 previous error diff --git a/tests/ui/imports/issue-45799-bad-extern-crate-rename-suggestion-formatting.stderr b/tests/ui/imports/issue-45799-bad-extern-crate-rename-suggestion-formatting.stderr index 2621f913186ee..4d25a60a5b091 100644 --- a/tests/ui/imports/issue-45799-bad-extern-crate-rename-suggestion-formatting.stderr +++ b/tests/ui/imports/issue-45799-bad-extern-crate-rename-suggestion-formatting.stderr @@ -8,7 +8,7 @@ LL | extern crate std; help: you can use `as` to change the binding name of the import | LL | extern crate std as other_std; - | ++++++++++++ + | +++++++++++++ error: aborting due to 1 previous error diff --git a/tests/ui/imports/no-std-inject.stderr b/tests/ui/imports/no-std-inject.stderr index 7299c2e8a9f39..9924d2c56809a 100644 --- a/tests/ui/imports/no-std-inject.stderr +++ b/tests/ui/imports/no-std-inject.stderr @@ -8,7 +8,7 @@ LL | extern crate core; help: you can use `as` to change the binding name of the import | LL | extern crate core as other_core; - | +++++++++++++ + | ++++++++++++++ error: aborting due to 1 previous error diff --git a/tests/ui/suggestions/return-bindings.stderr b/tests/ui/suggestions/return-bindings.stderr index 651998043e108..0f486845cc366 100644 --- a/tests/ui/suggestions/return-bindings.stderr +++ b/tests/ui/suggestions/return-bindings.stderr @@ -24,7 +24,6 @@ help: consider returning the local binding `s` | LL | let s: String = if let Some(s) = opt_str { LL ~ s -LL ~ | error[E0308]: mismatched types @@ -56,7 +55,6 @@ help: consider returning the local binding `s` | LL | let s: String = if let Some(s) = opt_str { LL ~ s -LL ~ | error[E0308]: `if` and `else` have incompatible types @@ -75,9 +73,8 @@ LL | | }; | help: consider returning the local binding `s` | -LL | let s = if let Some(s) = opt_str { -LL ~ s -LL ~ } else { +LL ~ let s = if let Some(s) = opt_str { +LL + s | error[E0308]: mismatched types diff --git a/tests/ui/suggestions/struct-field-type-including-single-colon.stderr b/tests/ui/suggestions/struct-field-type-including-single-colon.stderr index 5ffc5b40849b6..fecbc7af8bb9f 100644 --- a/tests/ui/suggestions/struct-field-type-including-single-colon.stderr +++ b/tests/ui/suggestions/struct-field-type-including-single-colon.stderr @@ -7,7 +7,7 @@ LL | a: foo:A, help: write a path separator here | LL | a: foo::A, - | + + | + error: expected `,`, or `}`, found `:` --> $DIR/struct-field-type-including-single-colon.rs:9:11 @@ -26,7 +26,7 @@ LL | b: foo::bar:B, help: write a path separator here | LL | b: foo::bar::B, - | + + | + error: expected `,`, or `}`, found `:` --> $DIR/struct-field-type-including-single-colon.rs:15:16 diff --git a/tests/ui/suggestions/type-ascription-instead-of-path-2.stderr b/tests/ui/suggestions/type-ascription-instead-of-path-2.stderr index 79dffc0cf9b08..06a2c7357830a 100644 --- a/tests/ui/suggestions/type-ascription-instead-of-path-2.stderr +++ b/tests/ui/suggestions/type-ascription-instead-of-path-2.stderr @@ -7,7 +7,7 @@ LL | let _ = vec![Ok(2)].into_iter().collect:,_>>()?; help: maybe write a path separator here | LL | let _ = vec![Ok(2)].into_iter().collect::,_>>()?; - | + + | + error: aborting due to 1 previous error diff --git a/tests/ui/suggestions/type-ascription-instead-of-path-in-type.stderr b/tests/ui/suggestions/type-ascription-instead-of-path-in-type.stderr index a424bc7e72452..ee9b2e2c9a7ac 100644 --- a/tests/ui/suggestions/type-ascription-instead-of-path-in-type.stderr +++ b/tests/ui/suggestions/type-ascription-instead-of-path-in-type.stderr @@ -7,7 +7,7 @@ LL | let _: Vec = A::B; help: you might have meant to write a path instead of an associated type bound | LL | let _: Vec = A::B; - | + + | + error[E0107]: struct takes at least 1 generic argument but 0 generic arguments were supplied --> $DIR/type-ascription-instead-of-path-in-type.rs:6:12