Reject unsupported Soroban string returns

Author: mohamedbasuony

src/sema/functions.rs

@@ -1062,6 +1062,41 @@ pub fn resolve_returns(
                         diagnostics.push(Diagnostic::error(r.ty.loc(), message));
                         success = false
                     }
+
+                    if ns.target == Target::Soroban {
+                        let unsupported_type = match &ty {
+                            Type::String
+                                if !matches!(r.storage, Some(pt::StorageLocation::Storage(_))) =>
+                            {
+                                let storage = r
+                                    .storage
+                                    .as_ref()
+                                    .map_or("memory", pt::StorageLocation::as_str);
+                                Some(format!("string {storage}"))
+                            }
+                            Type::DynamicBytes
+                                if !matches!(r.storage, Some(pt::StorageLocation::Storage(_))) =>
+                            {
+                                let storage = r
+                                    .storage
+                                    .as_ref()
+                                    .map_or("memory", pt::StorageLocation::as_str);
+                                Some(format!("bytes {storage}"))
+                            }
+                            Type::Bytes(n) => Some(format!("bytes{n}")),
+                            _ => None,
+                        };
+
+                        if let Some(unsupported_type) = unsupported_type {
+                            diagnostics.push(Diagnostic::error(
+                                r.loc,
+                                format!(
+                                    "type '{unsupported_type}' is not supported as a Soroban external function return value"
+                                ),
+                            ));
+                            success = false;
+                        }
+                    }
                 }
                 let ty = if !ty.can_have_data_location() {
                     if let Some(storage) = &r.storage {

src/sema/variables.rs

@@ -497,6 +497,24 @@ pub fn variable_decl<'a>(
                 ns,
             )?;
 
+            if ns.target == Target::Soroban {
+                let unsupported_type = match &param.ty {
+                    Type::DynamicBytes => Some("bytes".to_string()),
+                    Type::Bytes(n) => Some(format!("bytes{n}")),
+                    _ => None,
+                };
+
+                if let Some(unsupported_type) = unsupported_type {
+                    ns.diagnostics.push(Diagnostic::error(
+                        def.ty.loc(),
+                        format!(
+                            "type '{unsupported_type}' is not supported as a Soroban public variable accessor return value"
+                        ),
+                    ));
+                    return ret;
+                }
+            }
+
             if param.ty.contains_mapping(ns) {
                 // we can't return a mapping
                 ns.diagnostics.push(Diagnostic::decl_error(

tests/soroban_testcases/unsupported_parameters.rs

@@ -71,8 +71,8 @@ fn dynamic_bytes_external_parameters_are_rejected() {
 fn static_bytes_external_parameters_are_rejected() {
     let ns = compile_soroban(
         r#"contract test {
-    function len(bytes32 data) public returns (bytes32) {
-        return data;
+    function len(bytes32 data) public returns (uint64) {
+        return uint64(data.length);
     }
 }"#,
     );
@@ -93,6 +93,104 @@ fn static_bytes_external_parameters_are_rejected() {
         .starts_with("2:"));
 }
 
+#[test]
+fn string_external_returns_are_rejected() {
+    let ns = compile_soroban(
+        r#"contract test {
+    function public_make() public returns (string memory) {
+        return "hello";
+    }
+
+    function external_make() external returns (string memory) {
+        return "hello";
+    }
+}"#,
+    );
+
+    let errors = ns
+        .diagnostics
+        .iter()
+        .filter(|diagnostic| diagnostic.level == Level::Error)
+        .collect::<Vec<_>>();
+
+    assert_eq!(errors.len(), 2);
+    assert!(errors.iter().all(|diagnostic| diagnostic.message
+        == "type 'string memory' is not supported as a Soroban external function return value"));
+
+    let locations = errors
+        .iter()
+        .map(|diagnostic| ns.loc_to_string(PathDisplay::None, &diagnostic.loc))
+        .collect::<Vec<_>>();
+
+    assert!(locations.iter().any(|loc| loc.starts_with("2:")));
+    assert!(locations.iter().any(|loc| loc.starts_with("6:")));
+}
+
+#[test]
+fn bytes_external_returns_are_rejected() {
+    let ns = compile_soroban(
+        r#"contract test {
+    function public_make() public returns (bytes memory) {
+        return hex"01";
+    }
+
+    function external_make() external returns (bytes32) {
+        return bytes32(uint256(1));
+    }
+}"#,
+    );
+
+    let errors = ns
+        .diagnostics
+        .iter()
+        .filter(|diagnostic| diagnostic.level == Level::Error)
+        .collect::<Vec<_>>();
+
+    assert_eq!(errors.len(), 2);
+    assert!(errors.iter().any(|diagnostic| diagnostic.message
+        == "type 'bytes memory' is not supported as a Soroban external function return value"));
+    assert!(errors.iter().any(|diagnostic| diagnostic.message
+        == "type 'bytes32' is not supported as a Soroban external function return value"));
+
+    let locations = errors
+        .iter()
+        .map(|diagnostic| ns.loc_to_string(PathDisplay::None, &diagnostic.loc))
+        .collect::<Vec<_>>();
+
+    assert!(locations.iter().any(|loc| loc.starts_with("2:")));
+    assert!(locations.iter().any(|loc| loc.starts_with("6:")));
+}
+
+#[test]
+fn bytes_public_accessors_are_rejected() {
+    let ns = compile_soroban(
+        r#"contract test {
+    bytes public dynamic_data;
+    bytes32 public fixed_data;
+}"#,
+    );
+
+    let errors = ns
+        .diagnostics
+        .iter()
+        .filter(|diagnostic| diagnostic.level == Level::Error)
+        .collect::<Vec<_>>();
+
+    assert_eq!(errors.len(), 2);
+    assert!(errors.iter().any(|diagnostic| diagnostic.message
+        == "type 'bytes' is not supported as a Soroban public variable accessor return value"));
+    assert!(errors.iter().any(|diagnostic| diagnostic.message
+        == "type 'bytes32' is not supported as a Soroban public variable accessor return value"));
+
+    let locations = errors
+        .iter()
+        .map(|diagnostic| ns.loc_to_string(PathDisplay::None, &diagnostic.loc))
+        .collect::<Vec<_>>();
+
+    assert!(locations.iter().any(|loc| loc.starts_with("2:")));
+    assert!(locations.iter().any(|loc| loc.starts_with("3:")));
+}
+
 #[test]
 fn internal_private_dynamic_bytes_parameters_are_allowed() {
     let ns = compile_soroban(