derive PartialOrd to avoid incorrect implementation

turbopack/crates/turbo-tasks-fs/src/lib.rs

@@ -26,7 +26,7 @@ mod watcher;
 
 use std::{
     borrow::Cow,
-    cmp::min,
+    cmp::{min, Ordering},
     collections::HashSet,
     fmt::{self, Debug, Display, Formatter, Write as _},
     fs::FileType,
@@ -1518,7 +1518,7 @@ impl RealPathResult {
     }
 }
 
-#[derive(Clone, Copy, Debug, DeterministicHash)]
+#[derive(Clone, Copy, Debug, DeterministicHash, PartialOrd, Ord)]
 #[turbo_tasks::value(shared)]
 pub enum Permissions {
     Readable,
@@ -1571,7 +1571,7 @@ impl From<std::fs::Permissions> for Permissions {
 }
 
 #[turbo_tasks::value(shared)]
-#[derive(Clone, Debug, DeterministicHash)]
+#[derive(Clone, Debug, DeterministicHash, PartialOrd, Ord)]
 pub enum FileContent {
     Content(File),
     NotFound,
@@ -1676,11 +1676,11 @@ pub enum LinkContent {
 }
 
 #[turbo_tasks::value(shared)]
-#[derive(Clone, DeterministicHash)]
+#[derive(Clone, DeterministicHash, PartialOrd, Ord)]
 pub struct File {
-    meta: FileMeta,
     #[turbo_tasks(debug_ignore)]
     content: Rope,
+    meta: FileMeta,
 }
 
 impl File {
@@ -1866,6 +1866,20 @@ pub struct FileMeta {
     content_type: Option<Mime>,
 }
 
+impl Ord for FileMeta {
+    fn cmp(&self, other: &Self) -> Ordering {
+        self.permissions
+            .cmp(&other.permissions)
+            .then_with(|| self.content_type.as_ref().cmp(&other.content_type.as_ref()))
+    }
+}
+
+impl PartialOrd for FileMeta {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
 impl From<std::fs::Metadata> for FileMeta {
     fn from(meta: std::fs::Metadata) -> Self {
         let permissions = meta.permissions().into();

turbopack/crates/turbo-tasks-fs/src/rope.rs

@@ -1,6 +1,6 @@
 use std::{
     borrow::Cow,
-    cmp::min,
+    cmp::{min, Ordering},
     fmt,
     io::{BufRead, Read, Result as IoResult, Write},
     mem,
@@ -375,12 +375,20 @@ pub mod ser_as_string {
 impl PartialEq for Rope {
     // Ropes with similar contents are equals, regardless of their structure.
     fn eq(&self, other: &Self) -> bool {
-        if Arc::ptr_eq(&self.data, &other.data) {
-            return true;
-        }
         if self.len() != other.len() {
             return false;
         }
+        self.cmp(other) == Ordering::Equal
+    }
+}
+
+impl Eq for Rope {}
+
+impl Ord for Rope {
+    fn cmp(&self, other: &Self) -> Ordering {
+        if Arc::ptr_eq(&self.data, &other.data) {
+            return Ordering::Equal;
+        }
 
         // Fast path for structurally equal Ropes. With this, we can do memory reference
         // checks and skip some contents equality.
@@ -392,11 +400,11 @@ impl PartialEq for Rope {
             let a = &left[index];
             let b = &right[index];
 
-            match a.maybe_eq(b) {
+            match a.maybe_cmp(b) {
                 // Bytes or InnerRope point to the same memory, or Bytes are contents equal.
-                Some(true) => index += 1,
+                Some(Ordering::Equal) => index += 1,
                 // Bytes are not contents equal.
-                Some(false) => return false,
+                Some(ordering) => return ordering,
                 // InnerRopes point to different memory, or the Ropes weren't structurally equal.
                 None => break,
             }
@@ -406,7 +414,7 @@ impl PartialEq for Rope {
         if index == len {
             // We know that any remaining RopeElem in the InnerRope must contain content, so
             // if either one contains more RopeElem than they cannot be equal.
-            return left.len() == right.len();
+            return left.len().cmp(&right.len());
         }
 
         // At this point, we need to do slower contents equality. It's possible we'll
@@ -422,26 +430,31 @@ impl PartialEq for Rope {
 
                     // When one buffer is consumed, both must be consumed.
                     if len == 0 {
-                        return a.len() == b.len();
+                        return a.len().cmp(&b.len());
                     }
 
-                    if a[0..len] != b[0..len] {
-                        return false;
+                    match a[0..len].cmp(&b[0..len]) {
+                        Ordering::Equal => {
+                            left.consume(len);
+                            right.consume(len);
+                        }
+                        ordering => return ordering,
                     }
-
-                    left.consume(len);
-                    right.consume(len);
                 }
 
                 // If an error is ever returned (which shouldn't happen for us) for either/both,
                 // then we can't prove equality.
-                _ => return false,
+                _ => unreachable!(),
             }
         }
     }
 }
 
-impl Eq for Rope {}
+impl PartialOrd for Rope {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(self.cmp(other))
+    }
+}
 
 impl From<Vec<u8>> for Uncommitted {
     fn from(bytes: Vec<u8>) -> Self {
@@ -538,11 +551,11 @@ impl Deref for InnerRope {
 }
 
 impl RopeElem {
-    fn maybe_eq(&self, other: &Self) -> Option<bool> {
+    fn maybe_cmp(&self, other: &Self) -> Option<Ordering> {
         match (self, other) {
             (Local(a), Local(b)) => {
                 if a.len() == b.len() {
-                    return Some(a == b);
+                    return Some(a.cmp(b));
                 }
 
                 // But if not, the rope may still be contents equal if a following section
@@ -551,7 +564,7 @@ impl RopeElem {
             }
             (Shared(a), Shared(b)) => {
                 if Arc::ptr_eq(&a.0, &b.0) {
-                    return Some(true);
+                    return Some(Ordering::Equal);
                 }
 
                 // But if not, they might still be equal and we need to fallback to slower

turbopack/crates/turbopack-core/src/issue/mod.rs

@@ -685,14 +685,14 @@ impl Display for IssueStage {
 }
 
 #[turbo_tasks::value(serialization = "none")]
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, PartialOrd, Ord)]
 pub struct PlainIssue {
     pub severity: IssueSeverity,
-    pub file_path: RcStr,
-
     pub stage: IssueStage,
 
     pub title: StyledString,
+    pub file_path: RcStr,
+
     pub description: Option<StyledString>,
     pub detail: Option<StyledString>,
     pub documentation_link: RcStr,
@@ -702,35 +702,6 @@ pub struct PlainIssue {
     pub processing_path: ReadRef<PlainIssueProcessingPath>,
 }
 
-impl Ord for PlainIssue {
-    fn cmp(&self, other: &Self) -> Ordering {
-        macro_rules! cmp {
-            ($a:expr, $b:expr) => {
-                match $a.cmp(&$b) {
-                    Ordering::Equal => {}
-                    other => return other,
-                }
-            };
-        }
-
-        cmp!(self.severity, other.severity);
-        cmp!(self.stage, other.stage);
-        cmp!(self.title, other.title);
-        cmp!(self.file_path, other.file_path);
-        cmp!(self.description, other.description);
-        cmp!(self.detail, other.detail);
-        cmp!(self.documentation_link, other.documentation_link);
-        cmp!(self.source, other.source);
-        Ordering::Equal
-    }
-}
-
-impl PartialOrd for PlainIssue {
-    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        Some(self.cmp(other))
-    }
-}
-
 fn hash_plain_issue(issue: &PlainIssue, hasher: &mut Xxh3Hash64Hasher, full: bool) {
     hasher.write_ref(&issue.severity);
     hasher.write_ref(&issue.file_path);
@@ -827,7 +798,7 @@ impl IssueSource {
 }
 
 #[turbo_tasks::value(serialization = "none")]
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, PartialOrd, Ord)]
 pub struct PlainSource {
     pub ident: ReadRef<RcStr>,
     #[turbo_tasks(debug_ignore)]
@@ -852,24 +823,12 @@ impl PlainSource {
     }
 }
 
-impl Ord for PlainSource {
-    fn cmp(&self, other: &Self) -> Ordering {
-        self.ident.cmp(&other.ident)
-    }
-}
-
-impl PartialOrd for PlainSource {
-    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        Some(self.cmp(other))
-    }
-}
-
 #[turbo_tasks::value(transparent, serialization = "none")]
-#[derive(Clone, Debug, DeterministicHash)]
+#[derive(Clone, Debug, DeterministicHash, PartialOrd, Ord)]
 pub struct PlainIssueProcessingPath(Option<Vec<ReadRef<PlainIssueProcessingPathItem>>>);
 
 #[turbo_tasks::value(serialization = "none")]
-#[derive(Clone, Debug, DeterministicHash)]
+#[derive(Clone, Debug, DeterministicHash, PartialOrd, Ord)]
 pub struct PlainIssueProcessingPathItem {
     pub file_path: Option<ReadRef<RcStr>>,
     pub description: ReadRef<RcStr>,