THRIFT-5360 Remove deprecated Error::description() methods (#2342)

Client: rs

compiler/cpp/src/thrift/generate/t_rs_generator.cc

@@ -1087,15 +1087,7 @@ void t_rs_generator::render_struct_definition(
 
 void t_rs_generator::render_exception_struct_error_trait_impls(const string& struct_name, t_struct* tstruct) {
   // error::Error trait
-  f_gen_ << "impl Error for " << struct_name << " {" << endl;
-  indent_up();
-  f_gen_ << indent() << "fn description(&self) -> &str {" << endl;
-  indent_up();
-  f_gen_ << indent() << "\"" << "remote service threw " << tstruct->get_name() << "\"" << endl; // use *original* name
-  indent_down();
-  f_gen_ << indent() << "}" << endl;
-  indent_down();
-  f_gen_ << "}" << endl;
+  f_gen_ << "impl Error for " << struct_name << " {}" << endl;
   f_gen_ << endl;
 
   // convert::From trait
@@ -1115,7 +1107,12 @@ void t_rs_generator::render_exception_struct_error_trait_impls(const string& str
   indent_up();
   f_gen_ << indent() << "fn fmt(&self, f: &mut Formatter) -> fmt::Result {" << endl;
   indent_up();
-  f_gen_ << indent() << "self.description().fmt(f)" << endl;
+  f_gen_
+      << indent()
+      << "write!(f, "
+      << "\"remote service threw " << tstruct->get_name() << "\"" // use *original* name
+      << ")"
+      << endl;
   indent_down();
   f_gen_ << indent() << "}" << endl;
   indent_down();
@@ -2896,7 +2893,7 @@ void t_rs_generator::render_sync_handler_failed_user_exception_branch(t_function
 
   f_gen_ << indent() << "let ret_err = {" << endl;
   indent_up();
-  render_thrift_error_struct("ApplicationError", "ApplicationErrorKind::Unknown", "usr_err.description()");
+  render_thrift_error_struct("ApplicationError", "ApplicationErrorKind::Unknown", "usr_err.to_string()");
   indent_down();
   f_gen_ << indent() << "};" << endl;
   render_sync_handler_send_exception_response(tfunc, "ret_err");
@@ -2919,7 +2916,7 @@ void t_rs_generator::render_sync_handler_failed_application_exception_branch(
 void t_rs_generator::render_sync_handler_failed_default_exception_branch(t_function *tfunc) {
   f_gen_ << indent() << "let ret_err = {" << endl;
   indent_up();
-  render_thrift_error_struct("ApplicationError", "ApplicationErrorKind::Unknown", "e.description()");
+  render_thrift_error_struct("ApplicationError", "ApplicationErrorKind::Unknown", "e.to_string()");
   indent_down();
   f_gen_ << indent() << "};" << endl;
   if (tfunc->is_oneway()) {

lib/rs/README.md

@@ -48,6 +48,90 @@ Breaking changes are minimized. When they are made they will be outlined below w
 
 ##### Thrift 0.15.0
 
+* **[THRIFT-5360]** - No longer define OR generate `description()` methods for `Error` types.
+
+  `Error.description()` was soft-deprecated in 1.27, and deprecated as of 1.41.
+  Library error types also do not implement `Error.description()`. Also, as a result of this change
+  the generated Rust representation of an Error no longer implements the `Error.description()` method.
+  Instead, it generates a `Display` impl with the same information.
+
+  For example:
+
+  ```thrift
+  exception Xception {
+    1: i32 errorCode,
+    2: string message
+  }
+  ```
+
+  used to generate:
+
+  ```rust
+  use std::error::Error;
+  use std::fmt;
+  use std::fmt::{Display, Formatter};
+
+  // auto-generated by the Thrift compiler
+  #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
+  pub struct Xception {
+    pub error_code: Option<i32>,
+    pub message: Option<String>,
+  }
+
+  // auto-generated by the Thrift compiler
+  impl Error for Xception {
+    fn description(&self) -> &str {
+      "remote service threw Xception"
+    }
+  }
+
+  // auto-generated by the Thrift compiler
+  impl From<Xception> for thrift::Error {
+    fn from(e: Xception) -> Self {
+      thrift::Error::User(Box::new(e))
+    }
+  }
+
+  // auto-generated by the Thrift compiler
+  impl Display for Xception {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+      self.description().format(f)
+    }
+  }
+  ```
+
+  It *now* generates:
+
+  ```rust
+  use std::error::Error;
+  use std::fmt;
+  use std::fmt::{Display, Formatter};
+
+  // auto-generated by the Thrift compiler
+  #[derive(Clone, Debug, Eq, Ord, PartialEq, PartialOrd)]
+  pub struct Xception {
+    pub error_code: Option<i32>,
+    pub message: Option<String>,
+  }
+
+  // auto-generated by the Thrift compiler
+  impl Error for Xception { }
+
+  // auto-generated by the Thrift compiler
+  impl From<Xception> for thrift::Error {
+    fn from(e: Xception) -> Self {
+      thrift::Error::User(Box::new(e))
+    }
+  }
+
+  // auto-generated by the Thrift compiler
+  impl Display for Xception {
+    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
+      write!(f, "remote service threw Xception")
+    }
+  }
+  ```
+
 * **[THRIFT-5314]** - Generate enums implementations that are forward compatible (i.e. don't error on unknown values)
 
   As a result of this change the Rust representation of an enum changes from a standard

lib/rs/src/errors.rs

@@ -17,7 +17,6 @@
 
 use std::convert::TryFrom;
 use std::convert::{From, Into};
-use std::error::Error as StdError;
 use std::fmt::{Debug, Display, Formatter};
 use std::{error, fmt, io, string};
 
@@ -146,11 +145,7 @@ use crate::protocol::{
 /// }
 ///
 /// // auto-generated by the Thrift compiler
-/// impl Error for Xception {
-///   fn description(&self) -> &str {
-///     "remote service threw Xception"
-///   }
-/// }
+/// impl Error for Xception { }
 ///
 /// // auto-generated by the Thrift compiler
 /// impl From<Xception> for thrift::Error {
@@ -162,7 +157,7 @@ use crate::protocol::{
 /// // auto-generated by the Thrift compiler
 /// impl Display for Xception {
 ///   fn fmt(&self, f: &mut Formatter) -> fmt::Result {
-///     self.description().fmt(f)
+///     write!(f, "remote service threw Xception")
 ///   }
 /// }
 ///
@@ -270,16 +265,7 @@ impl Error {
     }
 }
 
-impl error::Error for Error {
-    fn description(&self) -> &str {
-        match *self {
-            Error::Transport(ref e) => TransportError::description(e),
-            Error::Protocol(ref e) => ProtocolError::description(e),
-            Error::Application(ref e) => ApplicationError::description(e),
-            Error::User(ref e) => e.description(),
-        }
-    }
-}
+impl error::Error for Error {}
 
 impl Debug for Error {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
@@ -370,6 +356,7 @@ impl TransportError {
 /// I/O error categories.
 ///
 /// This list may grow, and it is not recommended to match against it.
+#[non_exhaustive]
 #[derive(Clone, Copy, Eq, Debug, PartialEq)]
 pub enum TransportErrorKind {
     /// Catch-all I/O error.
@@ -388,9 +375,9 @@ pub enum TransportErrorKind {
     SizeLimit = 6,
 }
 
-impl TransportError {
-    fn description(&self) -> &str {
-        match self.kind {
+impl Display for TransportError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        let error_text = match self.kind {
             TransportErrorKind::Unknown => "transport error",
             TransportErrorKind::NotOpen => "not open",
             TransportErrorKind::AlreadyOpen => "already open",
@@ -398,13 +385,9 @@ impl TransportError {
             TransportErrorKind::EndOfFile => "end of file",
             TransportErrorKind::NegativeSize => "negative size message",
             TransportErrorKind::SizeLimit => "message too long",
-        }
-    }
-}
+        };
 
-impl Display for TransportError {
-    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.description())
+        write!(f, "{}", error_text)
     }
 }
 
@@ -434,24 +417,24 @@ impl From<io::Error> for Error {
             | io::ErrorKind::ConnectionRefused
             | io::ErrorKind::NotConnected => Error::Transport(TransportError {
                 kind: TransportErrorKind::NotOpen,
-                message: err.description().to_owned(),
+                message: err.to_string(),
             }),
             io::ErrorKind::AlreadyExists => Error::Transport(TransportError {
                 kind: TransportErrorKind::AlreadyOpen,
-                message: err.description().to_owned(),
+                message: err.to_string(),
             }),
             io::ErrorKind::TimedOut => Error::Transport(TransportError {
                 kind: TransportErrorKind::TimedOut,
-                message: err.description().to_owned(),
+                message: err.to_string(),
             }),
             io::ErrorKind::UnexpectedEof => Error::Transport(TransportError {
                 kind: TransportErrorKind::EndOfFile,
-                message: err.description().to_owned(),
+                message: err.to_string(),
             }),
             _ => {
                 Error::Transport(TransportError {
                     kind: TransportErrorKind::Unknown,
-                    message: err.description().to_owned(), // FIXME: use io error's debug string
+                    message: err.to_string(), // FIXME: use io error's debug string
                 })
             }
         }
@@ -462,7 +445,7 @@ impl From<string::FromUtf8Error> for Error {
     fn from(err: string::FromUtf8Error) -> Self {
         Error::Protocol(ProtocolError {
             kind: ProtocolErrorKind::InvalidData,
-            message: err.description().to_owned(), // FIXME: use fmt::Error's debug string
+            message: err.to_string(), // FIXME: use fmt::Error's debug string
         })
     }
 }
@@ -498,6 +481,7 @@ impl ProtocolError {
 /// Runtime library error categories.
 ///
 /// This list may grow, and it is not recommended to match against it.
+#[non_exhaustive]
 #[derive(Clone, Copy, Eq, Debug, PartialEq)]
 pub enum ProtocolErrorKind {
     /// Catch-all runtime-library error.
@@ -518,9 +502,9 @@ pub enum ProtocolErrorKind {
     DepthLimit = 6,
 }
 
-impl ProtocolError {
-    fn description(&self) -> &str {
-        match self.kind {
+impl Display for ProtocolError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        let error_text = match self.kind {
             ProtocolErrorKind::Unknown => "protocol error",
             ProtocolErrorKind::InvalidData => "bad data",
             ProtocolErrorKind::NegativeSize => "negative message size",
@@ -528,13 +512,9 @@ impl ProtocolError {
             ProtocolErrorKind::BadVersion => "invalid thrift version",
             ProtocolErrorKind::NotImplemented => "not implemented",
             ProtocolErrorKind::DepthLimit => "maximum skip depth reached",
-        }
-    }
-}
+        };
 
-impl Display for ProtocolError {
-    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.description())
+        write!(f, "{}", error_text)
     }
 }
 
@@ -589,6 +569,7 @@ impl ApplicationError {
 /// Auto-generated or user-implemented code error categories.
 ///
 /// This list may grow, and it is not recommended to match against it.
+#[non_exhaustive]
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
 pub enum ApplicationErrorKind {
     /// Catch-all application error.
@@ -618,9 +599,9 @@ pub enum ApplicationErrorKind {
     UnsupportedClientType = 10, // ??
 }
 
-impl ApplicationError {
-    fn description(&self) -> &str {
-        match self.kind {
+impl Display for ApplicationError {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        let error_text = match self.kind {
             ApplicationErrorKind::Unknown => "service error",
             ApplicationErrorKind::UnknownMethod => "unknown service method",
             ApplicationErrorKind::InvalidMessageType => "wrong message type received",
@@ -632,13 +613,9 @@ impl ApplicationError {
             ApplicationErrorKind::InvalidTransform => "invalid transform",
             ApplicationErrorKind::InvalidProtocol => "invalid protocol requested",
             ApplicationErrorKind::UnsupportedClientType => "unsupported protocol client",
-        }
-    }
-}
+        };
 
-impl Display for ApplicationError {
-    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", self.description())
+        write!(f, "{}", error_text)
     }
 }