Fix trait message return type metadata

crates/e2e/src/client.rs

@@ -384,7 +384,7 @@ where
         Args: scale::Encode,
     {
         let salt = Self::salt();
-        let data = constructor_exec_input(constructor.into());
+        let data = constructor_exec_input(constructor);
 
         // dry run the instantiate to calculate the gas limit
         let dry_run = self

crates/ink/codegen/src/generator/metadata.rs

@@ -297,7 +297,7 @@ impl Metadata<'_> {
                         as #trait_path>::__ink_TraitInfo
                         as ::ink::reflect::TraitMessageInfo<#local_id>>::SELECTOR
                 }};
-                let ret_ty = Self::generate_return_type(message.output());
+                let ret_ty = Self::generate_return_type(Some(&message.wrapped_output()));
                 let label = [trait_ident.to_string(), message_ident.to_string()].join("::");
                 quote_spanned!(message_span=>
                     ::ink::metadata::MessageSpec::from_label(#label)

crates/ink/tests/return_type_metadata.rs

@@ -0,0 +1,144 @@
+// Copyright 2018-2022 Parity Technologies (UK) Ltd.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#![cfg_attr(not(feature = "std"), no_std)]
+#[ink::contract]
+mod contract {
+    #[ink::trait_definition]
+    pub trait TraitDefinition {
+        #[ink(message)]
+        fn get_value(&self) -> u32;
+    }
+
+    #[ink(storage)]
+    pub struct Contract {}
+
+    impl Contract {
+        #[ink(constructor)]
+        pub fn try_new() -> Result<Self, u8> {
+            Err(1)
+        }
+    }
+
+    impl TraitDefinition for Contract {
+        #[ink(message)]
+        fn get_value(&self) -> u32 {
+            42
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use scale_info::{
+        form::PortableForm,
+        Type,
+        TypeDef,
+        TypeDefPrimitive,
+        TypeDefTuple,
+    };
+
+    fn generate_metadata() -> ink_metadata::InkProject {
+        extern "Rust" {
+            fn __ink_generate_metadata() -> ink_metadata::InkProject;
+        }
+
+        unsafe { __ink_generate_metadata() }
+    }
+
+    /// Extract the type defs of the `Ok` and `Error` variants of a `Result` type.
+    ///
+    /// Panics if the type def is not a valid result
+    fn extract_result<'a>(
+        metadata: &'a ink_metadata::InkProject,
+        ty: &'a Type<PortableForm>,
+    ) -> (&'a Type<PortableForm>, &'a Type<PortableForm>) {
+        assert_eq!(
+            "Result",
+            format!("{}", ty.path()),
+            "Message return type should be a Result"
+        );
+        match ty.type_def() {
+            TypeDef::Variant(variant) => {
+                assert_eq!(2, variant.variants().len());
+                let ok_variant = &variant.variants()[0];
+                let ok_field = &ok_variant.fields()[0];
+                let ok_ty = resolve_type(metadata, ok_field.ty().id());
+                assert_eq!("Ok", ok_variant.name());
+
+                let err_variant = &variant.variants()[1];
+                let err_field = &err_variant.fields()[0];
+                let err_ty = resolve_type(metadata, err_field.ty().id());
+                assert_eq!("Err", err_variant.name());
+
+                (ok_ty, err_ty)
+            }
+            td => panic!("Expected a Variant type def enum, got {:?}", td),
+        }
+    }
+
+    /// Resolve a type with the given id from the type registry
+    fn resolve_type(
+        metadata: &ink_metadata::InkProject,
+        type_id: u32,
+    ) -> &Type<PortableForm> {
+        metadata
+            .registry()
+            .resolve(type_id)
+            .unwrap_or_else(|| panic!("No type found in registry with id {}", type_id))
+    }
+
+    #[test]
+    fn trait_message_metadata_return_value_is_result() {
+        let metadata = generate_metadata();
+
+        let message = metadata.spec().messages().iter().next().unwrap();
+        assert_eq!("TraitDefinition::get_value", message.label());
+
+        let type_spec = message.return_type().opt_type().unwrap();
+        let ty = resolve_type(&metadata, type_spec.ty().id());
+        let (ok_ty, _) = extract_result(&metadata, ty);
+
+        assert_eq!(&TypeDef::Primitive(TypeDefPrimitive::U32), ok_ty.type_def());
+    }
+
+    #[test]
+    fn fallible_constructor_metadata_is_nested_result() {
+        let metadata = generate_metadata();
+        let constructor = metadata.spec().constructors().iter().next().unwrap();
+
+        assert_eq!("try_new", constructor.label());
+        let type_spec = constructor.return_type().opt_type().unwrap();
+        assert_eq!(
+            "ink_primitives::ConstructorResult",
+            format!("{}", type_spec.display_name())
+        );
+
+        let outer_result_ty = resolve_type(&metadata, type_spec.ty().id());
+        let (outer_ok_ty, outer_err_ty) = extract_result(&metadata, outer_result_ty);
+        let (inner_ok_ty, _) = extract_result(&metadata, outer_ok_ty);
+
+        assert_eq!(
+            format!("{}", outer_err_ty.path()),
+            "ink_primitives::LangError"
+        );
+
+        let unit_ty = TypeDef::Tuple(TypeDefTuple::new_portable(vec![]));
+        assert_eq!(
+            &unit_ty,
+            inner_ok_ty.type_def(),
+            "Ok variant should be a unit `()` type"
+        );
+    }
+}

crates/ink/tests/ui/contract/pass/constructor-return-result-alias.rs

@@ -22,16 +22,6 @@ mod contract {
     }
 }
 
-use ink::metadata::InkProject;
-
-fn generate_metadata() -> InkProject {
-    extern "Rust" {
-        fn __ink_generate_metadata() -> InkProject;
-    }
-
-    unsafe { __ink_generate_metadata() }
-}
-
 fn main() {
     use contract::Contract;
     use std::any::TypeId;
@@ -50,69 +40,4 @@ fn main() {
         >(),
         TypeId::of::<contract::Error>(),
     );
-
-    let metadata = generate_metadata();
-
-    let constructor = metadata.spec().constructors().iter().next().unwrap();
-
-    assert_eq!("constructor", constructor.label());
-    let type_spec = constructor.return_type().opt_type().unwrap();
-    assert_eq!(
-        "ink_primitives::ConstructorResult",
-        format!("{}", type_spec.display_name())
-    );
-    let ty = metadata.registry().resolve(type_spec.ty().id()).unwrap();
-
-    assert_eq!("Result", format!("{}", ty.path()));
-    match ty.type_def() {
-        scale_info::TypeDef::Variant(variant) => {
-            assert_eq!(2, variant.variants().len());
-
-            // Outer Result
-            let outer_ok_variant = &variant.variants()[0];
-            let outer_ok_field = &outer_ok_variant.fields()[0];
-            let outer_ok_ty = metadata
-                .registry()
-                .resolve(outer_ok_field.ty().id())
-                .unwrap();
-            assert_eq!("Ok", outer_ok_variant.name());
-
-            // Inner Result
-            let inner_ok_ty = match outer_ok_ty.type_def() {
-                scale_info::TypeDef::Variant(variant) => {
-                    assert_eq!(2, variant.variants().len());
-
-                    let inner_ok_variant = &variant.variants()[0];
-                    assert_eq!("Ok", inner_ok_variant.name());
-
-                    let inner_ok_field = &inner_ok_variant.fields()[0];
-                    metadata
-                        .registry()
-                        .resolve(inner_ok_field.ty().id())
-                        .unwrap()
-                }
-                td => panic!("Expected a Variant type def enum, got {:?}", td),
-            };
-
-            let unit_ty = scale_info::TypeDef::Tuple(
-                scale_info::TypeDefTuple::new_portable(vec![]),
-            );
-
-            assert_eq!(
-                &unit_ty,
-                inner_ok_ty.type_def(),
-                "Ok variant should be a unit `()` type"
-            );
-
-            let err_variant = &variant.variants()[1];
-            let err_field = &err_variant.fields()[0];
-            let err_ty_result = metadata.registry().resolve(err_field.ty().id());
-            assert_eq!("Err", err_variant.name());
-            assert!(
-                err_ty_result.is_some(),
-                "Error variant must be encoded with SCALE"
-            );
-        }
-        td => panic!("Expected a Variant type def enum, got {:?}", td),
-    }
 }