Skip to content

Commit

Permalink
Switch to multipart suggestions.
Browse files Browse the repository at this point in the history
This commit changes the suggestion so that it is split into multiple
parts in an effort to reduce the impact the applied suggestion could
have on formatting.
  • Loading branch information
davidtwco committed Apr 11, 2019
1 parent 137ffa1 commit 5158063
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 208 deletions.
57 changes: 28 additions & 29 deletions src/librustc_resolve/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,14 +617,12 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
_ => format!("{}", ident),
};

// Assume this is the easy case of `use issue_59764::foo::makro;` and just remove
// intermediate segments.
let (mut span, mut correction) = (directive.span,
format!("{}::{}", module_name, import));

if directive.is_nested() {
span = directive.use_span;

let mut corrections: Vec<(Span, String)> = Vec::new();
if !directive.is_nested() {
// Assume this is the easy case of `use issue_59764::foo::makro;` and just remove
// intermediate segments.
corrections.push((directive.span, format!("{}::{}", module_name, import)));
} else {
// Find the binding span (and any trailing commas and spaces).
// ie. `use a::b::{c, d, e};`
// ^^^
Expand Down Expand Up @@ -652,6 +650,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
}
debug!("check_for_module_export_macro: removal_span={:?}", removal_span);

// Remove the `removal_span`.
corrections.push((removal_span, "".to_string()));

// Find the span after the crate name and if it has nested imports immediatately
// after the crate name already.
// ie. `use a::b::{c, d};`
Expand All @@ -666,34 +667,32 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {

let source_map = self.resolver.session.source_map();

// Remove two bytes at the end to keep all but the `};` characters.
// ie. `{b::{c, d}, e::{f, g}};`
// ^^^^^^^^^^^^^^^^^^^^^
let end_bytes = BytePos(if has_nested { 2 } else { 1 });
let mut remaining_span = after_crate_name.with_hi(
after_crate_name.hi() - end_bytes);
if has_nested {
// Remove two bytes at the start to keep all but the initial `{` character.
// ie. `{b::{c, d}, e::{f, g}`
// ^^^^^^^^^^^^^^^^^^^^
remaining_span = remaining_span.with_lo(after_crate_name.lo() + BytePos(1));
// Add the import to the start, with a `{` if required.
let start_point = source_map.start_point(after_crate_name);
if let Ok(start_snippet) = source_map.span_to_snippet(start_point) {
corrections.push((
start_point,
if has_nested {
// In this case, `start_snippet` must equal '{'.
format!("{}{}, ", start_snippet, import)
} else {
// In this case, add a `{`, then the moved import, then whatever
// was there before.
format!("{{{}, {}", import, start_snippet)
}
));
}

// Calculate the number of characters into a snippet to remove the removal
// span.
let lo = removal_span.lo() - remaining_span.lo();
let hi = lo + (removal_span.hi() - removal_span.lo());
if let Ok(mut remaining) = source_map.span_to_snippet(remaining_span) {
// Remove the original location of the binding.
remaining.replace_range((lo.0 as usize)..(hi.0 as usize), "");
correction = format!("use {}::{{{}, {}}};", module_name, import, remaining);
// Add a `};` to the end if nested, matching the `{` added at the start.
if !has_nested {
corrections.push((source_map.end_point(after_crate_name),
"};".to_string()));
}
}

let suggestion = Some((
span,
corrections,
String::from("a macro with this name exists at the root of the crate"),
correction,
Applicability::MaybeIncorrect,
));
let note = vec![
Expand Down
11 changes: 5 additions & 6 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,8 @@ impl Ord for BindingError {
}
}

/// A span, message, replacement text, and applicability.
type Suggestion = (Span, String, String, Applicability);
/// A vector of spans and replacements, a message and applicability.
type Suggestion = (Vec<(Span, String)>, String, Applicability);

enum ResolutionError<'a> {
/// Error E0401: can't use type or const parameters from outer function.
Expand Down Expand Up @@ -390,8 +390,8 @@ fn resolve_struct_error<'sess, 'a>(resolver: &'sess Resolver<'_>,
"failed to resolve: {}", &label);
err.span_label(span, label);

if let Some((span, msg, suggestion, applicability)) = suggestion {
err.span_suggestion(span, &msg, suggestion, applicability);
if let Some((suggestions, msg, applicability)) = suggestion {
err.multipart_suggestion(&msg, suggestions, applicability);
}

err
Expand Down Expand Up @@ -3774,9 +3774,8 @@ impl<'a> Resolver<'a> {
(
String::from("unresolved import"),
Some((
ident.span,
vec![(ident.span, candidate.path.to_string())],
String::from("a similar path exists"),
candidate.path.to_string(),
Applicability::MaybeIncorrect,
)),
)
Expand Down
12 changes: 6 additions & 6 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -793,8 +793,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
diag.span_label(err.span, label);
}

if let Some((span, msg, suggestion, applicability)) = err.suggestion {
diag.span_suggestion(span, &msg, suggestion, applicability);
if let Some((suggestions, msg, applicability)) = err.suggestion {
diag.multipart_suggestion(&msg, suggestions, applicability);
}
}

Expand Down Expand Up @@ -947,9 +947,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
label: None,
note,
suggestion: Some((
span,
vec![(span, Segment::names_to_string(&suggestion))],
String::from("a similar path exists"),
Segment::names_to_string(&suggestion),
Applicability::MaybeIncorrect,
)),
}
Expand Down Expand Up @@ -1113,8 +1112,9 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {

let lev_suggestion = find_best_match_for_name(names, &ident.as_str(), None)
.map(|suggestion|
(ident.span, String::from("a similar name exists in the module"),
suggestion.to_string(), Applicability::MaybeIncorrect)
(vec![(ident.span, suggestion.to_string())],
String::from("a similar name exists in the module"),
Applicability::MaybeIncorrect)
);

let (suggestion, note) = match self.check_for_module_export_macro(
Expand Down
134 changes: 0 additions & 134 deletions src/test/ui/issue-59764.fixed

This file was deleted.

1 change: 0 additions & 1 deletion src/test/ui/issue-59764.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// aux-build:issue-59764.rs
// compile-flags:--extern issue_59764
// edition:2018
// run-rustfix

#![allow(warnings)]

Expand Down
Loading

0 comments on commit 5158063

Please sign in to comment.