Add an early return + more tests for the expensive newtype or tuple c…

…heck for unwrapping newtype variants

src/de/mod.rs

@@ -17,7 +17,7 @@ use crate::{
     error::{Result, SpannedResult},
     extensions::Extensions,
     options::Options,
-    parse::{AnyNum, Bytes, ParsedStr, StructType, BASE64_ENGINE},
+    parse::{AnyNum, Bytes, NewtypeMode, ParsedStr, StructType, TupleMode, BASE64_ENGINE},
 };
 
 mod id;
@@ -154,7 +154,17 @@ impl<'de> Deserializer<'de> {
         let old_serde_content_newtype = self.serde_content_newtype;
         self.serde_content_newtype = false;
 
-        match (self.bytes.check_struct_type(false)?, ident) {
+        match (
+            self.bytes.check_struct_type(
+                NewtypeMode::NoParensMeanUnit,
+                if old_serde_content_newtype {
+                    TupleMode::DifferentiateNewtype // separate match on NewtypeOrTuple below
+                } else {
+                    TupleMode::ImpreciseTupleOrNewtype // Tuple and NewtypeOrTuple match equally
+                },
+            )?,
+            ident,
+        ) {
             (StructType::Unit, Some(ident)) if is_serde_content => {
                 // serde's Content type needs the ident for unit variants
                 visitor.visit_str(ident)
@@ -245,21 +255,27 @@ impl<'de, 'a> de::Deserializer<'de> for &'a mut Deserializer<'de> {
         V: Visitor<'de>,
     {
         if self.newtype_variant {
-            match self.bytes.check_struct_type(true)? {
-                StructType::Tuple => {
-                    // newtype variant wraps a tuple (struct)
-                    // first argument is technically incorrect, but ignored anyway
-                    return self.deserialize_tuple(0, visitor);
-                }
+            if let Some(b')') = self.bytes.peek() {
+                // newtype variant wraps the unit type / a unit struct without name
+                return self.deserialize_unit(visitor);
+            }
+
+            match self
+                .bytes
+                .check_struct_type(NewtypeMode::InsideNewtype, TupleMode::DifferentiateNewtype)?
+            {
                 StructType::Named => {
                     // newtype variant wraps a named struct
                     // giving no name results in worse errors but is necessary here
                     return self.handle_struct_after_name("", visitor);
                 }
-                StructType::NewtypeOrTuple if self.bytes.peek() == Some(b')') => {
-                    // newtype variant wraps the unit type / a unit struct without name
-                    return self.deserialize_unit(visitor);
+                StructType::Tuple => {
+                    // newtype variant wraps a tuple (struct)
+                    // first argument is technically incorrect, but ignored anyway
+                    return self.deserialize_tuple(0, visitor);
                 }
+                /* StructType::NewtypeOrTuple */
+                // StructType::Unit is impossible with NewtypeMode::InsideNewtype
                 _ => {
                     // continue as usual with the inner content of the newtype variant
                     self.newtype_variant = false;

src/parse.rs

@@ -435,16 +435,32 @@ impl<'a> Bytes<'a> {
             .map_or(false, |&b| is_ident_other_char(b))
     }
 
-    /// Check which type of struct we are currently parsing.
+    /// Check which type of struct we are currently parsing. The parsing state
+    ///  is only changed in case of an error, to provide a better position.
     ///
-    /// Setting `no_unit` to `true` skips an initial check for unit structs,
+    /// [`NewtypeMode::NoParensMeanUnit`] detects (tuple) structs by a leading
+    ///  opening bracket and reports a unit struct otherwise.
+    /// [`NewtypeMode::InsideNewtype`] skips an initial check for unit structs,
     ///  and means that any leading opening bracket is not considered to open
     ///  a (tuple) struct but to be part of the structs inner contents.
-    /// Setting `no_unit` to `false` detects (tuple) structs by a leading
-    ///  opening bracket and reports a unit struct otherwise.
-    pub fn check_struct_type(&mut self, no_unit: bool) -> Result<StructType> {
-        fn check_struct_type_inner(bytes: &mut Bytes, no_unit: bool) -> Result<StructType> {
-            if !no_unit && !bytes.consume("(") {
+    ///
+    /// [`TupleMode::ImpreciseTupleOrNewtype`] only performs a cheap, O(1),
+    ///  single-identifier lookahead check to distinguish tuple structs from
+    ///  non-tuple structs.
+    /// [`TupleMode::DifferentiateNewtype`] performs an expensive, O(N), look-
+    ///  ahead over the entire next value tree, which can span the entirety of
+    ///  the remaining document in the worst case.
+    pub fn check_struct_type(
+        &mut self,
+        newtype: NewtypeMode,
+        tuple: TupleMode,
+    ) -> Result<StructType> {
+        fn check_struct_type_inner(
+            bytes: &mut Bytes,
+            newtype: NewtypeMode,
+            tuple: TupleMode,
+        ) -> Result<StructType> {
+            if matches!(newtype, NewtypeMode::NoParensMeanUnit) && !bytes.consume("(") {
                 return Ok(StructType::Unit);
             }
 
@@ -465,6 +481,10 @@ impl<'a> Bytes<'a> {
                 };
             }
 
+            if matches!(tuple, TupleMode::ImpreciseTupleOrNewtype) {
+                return Ok(StructType::NewtypeOrTuple);
+            }
+
             let mut braces = 1_usize;
             let mut comma = false;
 
@@ -502,7 +522,7 @@ impl<'a> Bytes<'a> {
         // Create a temporary working copy
         let mut bytes = *self;
 
-        let result = check_struct_type_inner(&mut bytes, no_unit);
+        let result = check_struct_type_inner(&mut bytes, newtype, tuple);
 
         if result.is_err() {
             // Adjust the error span to fit the working copy
@@ -1075,6 +1095,16 @@ pub enum StructType {
     Unit,
 }
 
+pub enum NewtypeMode {
+    NoParensMeanUnit,
+    InsideNewtype,
+}
+
+pub enum TupleMode {
+    ImpreciseTupleOrNewtype,
+    DifferentiateNewtype,
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

tests/465_unwrap_some_newtype_variant_value.rs

@@ -18,4 +18,27 @@ fn deserialise_value_with_unwrap_some_newtype_variant() {
             .collect()
         ))))),
     );
+    assert_eq!(
+        ron::from_str("#![enable(unwrap_variant_newtypes)] Some(42, true)"),
+        Ok(ron::Value::Option(Some(Box::new(ron::Value::Seq(vec![
+            ron::Value::Number(42.into()),
+            ron::Value::Bool(true)
+        ]))))),
+    );
+    assert_eq!(
+        ron::from_str("#![enable(unwrap_variant_newtypes)] Some(42,)"),
+        Ok(ron::Value::Option(Some(Box::new(ron::Value::Seq(vec![
+            ron::Value::Number(42.into())
+        ]))))),
+    );
+    assert_eq!(
+        ron::from_str("#![enable(unwrap_variant_newtypes)] Some()"),
+        Ok(ron::Value::Option(Some(Box::new(ron::Value::Unit)))),
+    );
+    assert_eq!(
+        ron::from_str("#![enable(unwrap_variant_newtypes)] Some(42)"),
+        Ok(ron::Value::Option(Some(Box::new(ron::Value::Number(
+            42.into()
+        ))))),
+    );
 }