Auto merge of #14710 - weihanglo:rustfix, r=epage

refactor(rustfix): minor refactors

Author: bors

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "rustfix"
-version = "0.8.7"
+version = "0.8.8"
 authors = [
     "Pascal Hertleif <[email protected]>",
     "Oliver Schneider <[email protected]>",

crates/rustfix/Cargo.toml

@@ -167,8 +167,6 @@ pub fn collect_suggestions<S: ::std::hash::BuildHasher>(
         }
     }
 
-    let snippets = diagnostic.spans.iter().map(span_to_snippet).collect();
-
     let solutions: Vec<_> = diagnostic
         .children
         .iter()
@@ -204,7 +202,7 @@ pub fn collect_suggestions<S: ::std::hash::BuildHasher>(
     } else {
         Some(Suggestion {
             message: diagnostic.message.clone(),
-            snippets,
+            snippets: diagnostic.spans.iter().map(span_to_snippet).collect(),
             solutions,
         })
     }

crates/rustfix/src/lib.rs

@@ -24,7 +24,7 @@ impl State {
 }
 
 /// Span with a change [`State`].
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Clone, PartialEq, Eq)]
 struct Span {
     /// Start of this span in parent data
     start: usize,
@@ -34,6 +34,17 @@ struct Span {
     data: State,
 }
 
+impl std::fmt::Debug for Span {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let state = match self.data {
+            State::Initial => "initial",
+            State::Replaced(_) => "replaced",
+            State::Inserted(_) => "inserted",
+        };
+        write!(f, "({}, {}: {state})", self.start, self.end)
+    }
+}
+
 /// A container that allows easily replacing chunks of its data
 #[derive(Debug, Clone, Default)]
 pub struct Data {
@@ -97,102 +108,82 @@ impl Data {
         // [^empty]: Leading and trailing ones might be empty if we replace
         // the whole chunk. As an optimization and without loss of generality we
         // don't add empty parts.
-        let new_parts = {
-            let Some(index_of_part_to_split) = self.parts.iter().position(|p| {
-                !p.data.is_inserted() && p.start <= range.start && p.end >= range.end
-            }) else {
-                if tracing::enabled!(tracing::Level::DEBUG) {
-                    let slices = self
-                        .parts
-                        .iter()
-                        .map(|p| {
-                            (
-                                p.start,
-                                p.end,
-                                match p.data {
-                                    State::Initial => "initial",
-                                    State::Replaced(..) => "replaced",
-                                    State::Inserted(..) => "inserted",
-                                },
-                            )
-                        })
-                        .collect::<Vec<_>>();
-                    tracing::debug!(
-                        "no single slice covering {}..{}, current slices: {:?}",
-                        range.start,
-                        range.end,
-                        slices,
-                    );
-                }
+        let Some(index_of_part_to_split) = self
+            .parts
+            .iter()
+            .position(|p| !p.data.is_inserted() && p.start <= range.start && p.end >= range.end)
+        else {
+            tracing::debug!(
+                "no single slice covering {}..{}, current slices: {:?}",
+                range.start,
+                range.end,
+                self.parts,
+            );
+            return Err(Error::MaybeAlreadyReplaced(range));
+        };
 
-                return Err(Error::MaybeAlreadyReplaced(range));
-            };
+        let part_to_split = &self.parts[index_of_part_to_split];
 
-            let part_to_split = &self.parts[index_of_part_to_split];
-
-            // If this replacement matches exactly the part that we would
-            // otherwise split then we ignore this for now. This means that you
-            // can replace the exact same range with the exact same content
-            // multiple times and we'll process and allow it.
-            //
-            // This is currently done to alleviate issues like
-            // rust-lang/rust#51211 although this clause likely wants to be
-            // removed if that's fixed deeper in the compiler.
-            if part_to_split.start == range.start && part_to_split.end == range.end {
-                if let State::Replaced(ref replacement) = part_to_split.data {
-                    if &**replacement == data {
-                        return Ok(());
-                    }
+        // If this replacement matches exactly the part that we would
+        // otherwise split then we ignore this for now. This means that you
+        // can replace the exact same range with the exact same content
+        // multiple times and we'll process and allow it.
+        //
+        // This is currently done to alleviate issues like
+        // rust-lang/rust#51211 although this clause likely wants to be
+        // removed if that's fixed deeper in the compiler.
+        if part_to_split.start == range.start && part_to_split.end == range.end {
+            if let State::Replaced(ref replacement) = part_to_split.data {
+                if &**replacement == data {
+                    return Ok(());
                 }
             }
+        }
 
-            if part_to_split.data != State::Initial {
-                return Err(Error::AlreadyReplaced);
-            }
-
-            let mut new_parts = Vec::with_capacity(self.parts.len() + 2);
+        if part_to_split.data != State::Initial {
+            return Err(Error::AlreadyReplaced);
+        }
 
-            // Previous parts
-            if let Some(ps) = self.parts.get(..index_of_part_to_split) {
-                new_parts.extend_from_slice(ps);
-            }
+        let mut new_parts = Vec::with_capacity(self.parts.len() + 2);
 
-            // Keep initial data on left side of part
-            if range.start > part_to_split.start {
-                new_parts.push(Span {
-                    start: part_to_split.start,
-                    end: range.start,
-                    data: State::Initial,
-                });
-            }
+        // Previous parts
+        if let Some(ps) = self.parts.get(..index_of_part_to_split) {
+            new_parts.extend_from_slice(ps);
+        }
 
-            // New part
+        // Keep initial data on left side of part
+        if range.start > part_to_split.start {
             new_parts.push(Span {
-                start: range.start,
-                end: range.end,
-                data: if insert_only {
-                    State::Inserted(data.into())
-                } else {
-                    State::Replaced(data.into())
-                },
+                start: part_to_split.start,
+                end: range.start,
+                data: State::Initial,
             });
+        }
 
-            // Keep initial data on right side of part
-            if range.end < part_to_split.end {
-                new_parts.push(Span {
-                    start: range.end,
-                    end: part_to_split.end,
-                    data: State::Initial,
-                });
-            }
-
-            // Following parts
-            if let Some(ps) = self.parts.get(index_of_part_to_split + 1..) {
-                new_parts.extend_from_slice(ps);
-            }
+        // New part
+        new_parts.push(Span {
+            start: range.start,
+            end: range.end,
+            data: if insert_only {
+                State::Inserted(data.into())
+            } else {
+                State::Replaced(data.into())
+            },
+        });
+
+        // Keep initial data on right side of part
+        if range.end < part_to_split.end {
+            new_parts.push(Span {
+                start: range.end,
+                end: part_to_split.end,
+                data: State::Initial,
+            });
+        }
 
-            new_parts
-        };
+        // Following parts
+        if let Some(ps) = self.parts.get(index_of_part_to_split + 1..) {
+            new_parts.extend_from_slice(ps);
+        }
 
         self.parts = new_parts;