better checking of tag duplicates, avoid discarding invalid variant e…

…rrors
tokio-rs · Nov 25, 2023 · 8c9856a · 8c9856a
1 parent 907e9f6
commit 8c9856a
Showing 1 changed file with 27 additions and 26 deletions.
diff --git a/prost-derive/src/lib.rs b/prost-derive/src/lib.rs
@@ -88,19 +88,18 @@ fn try_message(input: TokenStream) -> Result<TokenStream, Error> {
     // TODO: This encodes oneof fields in the position of their lowest tag,
     // regardless of the currently occupied variant, is that consequential?
     // See: https://developers.google.com/protocol-buffers/docs/encoding#order
-    fields.sort_by_key(|&(_, ref field)| field.tags().into_iter().min().unwrap());
+    fields.sort_by_key(|(_, field)| field.tags().into_iter().min().unwrap());
     let fields = fields;
 
-    let mut tags = fields
+    if let Some((duplicate_tag, _)) = fields
         .iter()
-        .flat_map(|&(_, ref field)| field.tags())
-        .collect::<Vec<_>>();
-    let num_tags = tags.len();
-    tags.sort_unstable();
-    tags.dedup();
-    if tags.len() != num_tags {
-        bail!("message {} has fields with duplicate tags", ident);
-    }
+        .flat_map(|(_, field)| field.tags())
+        .sorted_unstable()
+        .tuple_windows()
+        .find(|(a, b)| a == b)
+    {
+        bail!("message {} has duplicate tag {}", ident, duplicate_tag)
+    };
 
     let encoded_len = fields
         .iter()
@@ -414,23 +413,25 @@ fn try_oneof(input: TokenStream) -> Result<TokenStream, Error> {
         }
     }
 
-    let mut tags = fields
+    if let Some((invalid_variant, _)) = fields.iter().find(|(_, field)| field.tags().len() > 1) {
+        bail!(
+            "invalid oneof variant {}::{}: oneof variants may only have a single tag",
+            ident,
+            invalid_variant
+        );
+    }
+    if let Some((duplicate_tag, _)) = fields
         .iter()
-        .flat_map(|&(ref variant_ident, ref field)| -> Result<u32, Error> {
-            if field.tags().len() > 1 {
-                bail!(
-                    "invalid oneof variant {}::{}: oneof variants may only have a single tag",
-                    ident,
-                    variant_ident
-                );
-            }
-            Ok(field.tags()[0])
-        })
-        .collect::<Vec<_>>();
-    tags.sort_unstable();
-    tags.dedup();
-    if tags.len() != fields.len() {
-        panic!("invalid oneof {}: variants have duplicate tags", ident);
+        .flat_map(|(_, field)| field.tags())
+        .sorted_unstable()
+        .tuple_windows()
+        .find(|(a, b)| a == b)
+    {
+        bail!(
+            "invalid oneof {}: multiple variants have tag {}",
+            ident,
+            duplicate_tag
+        );
     }
 
     let encode = fields.iter().map(|&(ref variant_ident, ref field)| {