[v2] fix(ast): spell storage layout types as solc labels them

crates/solidity-v2/outputs/cargo/ast/src/abi/node_extensions/contract_definition.rs

@@ -96,7 +96,7 @@ impl ContractDefinitionStruct {
             let position = builder.allocate(variable_size)?;
 
             let label = state_variable.ir_node.name.unparse().to_string();
-            let type_name = self.semantic.type_internal_name(variable_type_id);
+            let type_name = self.semantic.type_abi_internal_name(variable_type_id);
             storage_layout.push(StorageItem {
                 node_id,
                 label,

crates/solidity-v2/outputs/cargo/semantic/src/context/mod.rs

@@ -1,3 +1,4 @@
+use std::fmt::Write;
 use std::ops::Range;
 
 pub use contract_data::ContractReference;
@@ -285,19 +286,23 @@ impl SemanticContext {
     /// Qualifies a nested definition with its enclosing scope, as solc's
     /// `canonicalName` does: `L.S`, `C.E`.
     pub(crate) fn definition_canonical_name(&self, definition_id: NodeId) -> String {
-        let name = self
-            .binder
-            .find_definition_by_id(definition_id)
-            .unwrap()
-            .identifier()
-            .unparse();
-        match self.binder.enclosing_definition_node_id(definition_id) {
-            Some(enclosing) => format!(
-                "{scope}.{name}",
-                scope = self.definition_canonical_name(enclosing)
-            ),
-            None => name.to_string(),
+        let mut name = String::new();
+        self.write_definition_canonical_name(definition_id, &mut name);
+        name
+    }
+
+    fn write_definition_canonical_name(&self, definition_id: NodeId, out: &mut String) {
+        if let Some(enclosing) = self.binder.enclosing_definition_node_id(definition_id) {
+            self.write_definition_canonical_name(enclosing, out);
+            out.push('.');
         }
+        out.push_str(
+            self.binder
+                .find_definition_by_id(definition_id)
+                .unwrap()
+                .identifier()
+                .unparse(),
+        );
     }
 
     pub fn type_internal_name(&self, type_id: TypeId) -> String {
@@ -379,81 +384,102 @@ impl SemanticContext {
         }
     }
 
-    /// The type as solc's `Type::toString(true)` spells it, which is the JSON ABI `internalType`:
-    /// a kind prefix on user-defined types (`struct C.S`, `enum C.E`, `contract I`),
-    /// `address payable`, and function types with their parameters, mutability, `external` and
-    /// returns; never a data location. Everything else spells as [`Self::type_internal_name`].
+    /// The type as solc's `Type::toString(true)` spells it, which is the JSON ABI `internalType`
+    /// and the storage layout label: a kind prefix on user-defined types (`struct C.S`,
+    /// `enum C.E`, `contract I`), `address payable`, and function types with their parameters,
+    /// mutability, `external` and returns; never a data location. Everything else spells as
+    /// [`Self::type_internal_name`].
     pub fn type_abi_internal_name(&self, type_id: TypeId) -> String {
+        // Sized on the ABI benchmarks: most names fit without the buffer regrowing.
+        let mut name = String::with_capacity(32);
+        self.write_type_abi_internal_name(type_id, &mut name);
+        name
+    }
+
+    fn write_type_abi_internal_name(&self, type_id: TypeId, out: &mut String) {
         match self.types.get_type_by_id(type_id) {
-            Type::Address(address) if address.is_payable => "address payable".to_string(),
+            Type::Address(address) if address.is_payable => out.push_str("address payable"),
             Type::Array(ArrayType { element_type, .. }) => {
-                format!(
-                    "{element}[]",
-                    element = self.type_abi_internal_name(*element_type)
-                )
+                self.write_type_abi_internal_name(*element_type, out);
+                out.push_str("[]");
             }
             Type::Contract(ContractType { definition_id })
             | Type::Interface(InterfaceType { definition_id }) => {
-                format!(
-                    "contract {}",
-                    self.definition_canonical_name(*definition_id)
-                )
+                out.push_str("contract ");
+                self.write_definition_canonical_name(*definition_id, out);
             }
             Type::Enum(EnumType { definition_id }) => {
-                format!("enum {}", self.definition_canonical_name(*definition_id))
+                out.push_str("enum ");
+                self.write_definition_canonical_name(*definition_id, out);
             }
             Type::FixedSizeArray(FixedSizeArrayType {
                 element_type, size, ..
             }) => {
-                format!(
-                    "{element}[{size}]",
-                    element = self.type_abi_internal_name(*element_type),
-                )
+                self.write_type_abi_internal_name(*element_type, out);
+                write!(out, "[{size}]").unwrap();
+            }
+            Type::Function(function_type) => {
+                self.write_function_type_abi_internal_name(function_type, out);
+            }
+            Type::Mapping(MappingType {
+                key_type_id,
+                value_type_id,
+            }) => {
+                out.push_str("mapping(");
+                self.write_type_abi_internal_name(*key_type_id, out);
+                out.push_str(" => ");
+                self.write_type_abi_internal_name(*value_type_id, out);
+                out.push(')');
             }
-            Type::Function(function_type) => self.function_type_abi_internal_name(function_type),
             Type::Struct(StructType { definition_id, .. }) => {
-                format!("struct {}", self.definition_canonical_name(*definition_id))
+                out.push_str("struct ");
+                self.write_definition_canonical_name(*definition_id, out);
             }
-            _ => self.type_internal_name(type_id),
+            _ => out.push_str(&self.type_internal_name(type_id)),
+        }
+    }
+
+    fn write_type_abi_internal_names(&self, type_ids: &[TypeId], out: &mut String) {
+        for (index, type_id) in type_ids.iter().enumerate() {
+            if index > 0 {
+                out.push(',');
+            }
+            self.write_type_abi_internal_name(*type_id, out);
         }
     }
 
     /// `function (T1,T2) [pure|view|payable] [external] [returns (R1,R2)]`: `nonpayable` and
     /// `internal` are implied by their absence, as in solc.
-    fn function_type_abi_internal_name(&self, function_type: &FunctionType) -> String {
-        let parameters = function_type
-            .parameter_types
-            .iter()
-            .map(|type_id| self.type_abi_internal_name(*type_id))
-            .collect::<Vec<_>>()
-            .join(",");
-        let mutability = match function_type.mutability {
+    fn write_function_type_abi_internal_name(
+        &self,
+        function_type: &FunctionType,
+        out: &mut String,
+    ) {
+        out.push_str("function (");
+        self.write_type_abi_internal_names(&function_type.parameter_types, out);
+        out.push(')');
+        out.push_str(match function_type.mutability {
             FunctionTypeMutability::Pure => " pure",
             FunctionTypeMutability::View => " view",
             FunctionTypeMutability::Payable => " payable",
             FunctionTypeMutability::NonPayable => "",
-        };
-        let visibility = if function_type.is_externally_visible() {
-            " external"
-        } else {
-            ""
-        };
-        let returns = match self.types.get_type_by_id(function_type.return_type) {
-            Type::Void => String::new(),
-            Type::Tuple(TupleType { types }) => format!(
-                " returns ({})",
-                types
-                    .iter()
-                    .map(|type_id| self.type_abi_internal_name(*type_id))
-                    .collect::<Vec<_>>()
-                    .join(",")
-            ),
-            _ => format!(
-                " returns ({})",
-                self.type_abi_internal_name(function_type.return_type)
-            ),
-        };
-        format!("function ({parameters}){mutability}{visibility}{returns}")
+        });
+        if function_type.is_externally_visible() {
+            out.push_str(" external");
+        }
+        match self.types.get_type_by_id(function_type.return_type) {
+            Type::Void => {}
+            Type::Tuple(TupleType { types }) => {
+                out.push_str(" returns (");
+                self.write_type_abi_internal_names(types, out);
+                out.push(')');
+            }
+            _ => {
+                out.push_str(" returns (");
+                self.write_type_abi_internal_name(function_type.return_type, out);
+                out.push(')');
+            }
+        }
     }
 
     pub fn type_library_name(&self, type_id: TypeId) -> Option<String> {

crates/solidity-v2/outputs/cargo/slang_solidity/src/tests/abi/storage_layout.rs

@@ -83,15 +83,21 @@ fn test_storage_layout() {
 
     assert_layout_item_eq!(layout[0], "a", uint!(0_U256), 0, "uint256");
     assert_layout_item_eq!(layout[1], "e", uint!(1_U256), 0, "uint8[]");
-    assert_layout_item_eq!(layout[2], "f", uint!(2_U256), 0, "mapping(uint256 => S)");
+    assert_layout_item_eq!(
+        layout[2],
+        "f",
+        uint!(2_U256),
+        0,
+        "mapping(uint256 => struct S)"
+    );
     assert_layout_item_eq!(layout[3], "g", uint!(3_U256), 0, "uint16");
     assert_layout_item_eq!(layout[4], "h", uint!(3_U256), 2, "uint16");
-    assert_layout_item_eq!(layout[5], "s", uint!(4_U256), 0, "S");
+    assert_layout_item_eq!(layout[5], "s", uint!(4_U256), 0, "struct S");
     assert_layout_item_eq!(layout[6], "k", uint!(5_U256), 0, "int8");
     assert_layout_item_eq!(layout[7], "l", uint!(5_U256), 1, "bytes21");
     assert_layout_item_eq!(layout[8], "m", uint!(6_U256), 0, "uint8[10]");
     assert_layout_item_eq!(layout[9], "n", uint!(7_U256), 0, "bytes5[8]");
-    assert_layout_item_eq!(layout[10], "t", uint!(9_U256), 0, "T[2]");
+    assert_layout_item_eq!(layout[10], "t", uint!(9_U256), 0, "struct T[2]");
     assert_layout_item_eq!(layout[11], "o", uint!(13_U256), 0, "bytes5");
 
     let transient_layout = counter_abi.transient_storage_layout();
@@ -165,7 +171,7 @@ fn test_struct_members_packing_into_a_full_slot_occupy_one_slot() {
     let layout = g_abi.storage_layout();
 
     assert_eq!(layout.len(), 2);
-    assert_layout_item_eq!(layout[0], "s", uint!(0_U256), 0, "PerfectFit");
+    assert_layout_item_eq!(layout[0], "s", uint!(0_U256), 0, "struct PerfectFit");
     assert_layout_item_eq!(layout[1], "tail", uint!(1_U256), 0, "uint256");
 }
 
@@ -332,7 +338,7 @@ fn test_oversized_base_slot_has_no_layout() {
 }
 
 // A struct declared inside a contract is laid out under its scope-qualified
-// name, the same spelling a library selector hashes.
+// name.
 define_fixture!(
     NestedStructLayout,
     file: "main.sol", r#"
@@ -358,5 +364,76 @@ fn test_nested_struct_lays_out_under_its_qualified_name() {
     let layout = contract_abi.storage_layout();
 
     assert_eq!(layout.len(), 1);
-    assert_layout_item_eq!(layout[0], "nested", uint!(0_U256), 0, "C.Inner");
+    assert_layout_item_eq!(layout[0], "nested", uint!(0_U256), 0, "struct C.Inner");
+}
+
+// Expected type names are solc 0.8.35's `storageLayout` labels for this source.
+define_fixture!(
+    TypeLabels,
+    file: "main.sol", r#"
+pragma solidity *;
+interface I {}
+struct S { uint256 a; }
+type U is uint64;
+
+contract Store {
+    struct TokenData { uint256 id; }
+    enum E { A }
+
+    function (uint256, TokenData memory) view returns (string memory) internal renderer;
+    mapping(bytes4 => function (bytes memory)) internal handlers;
+    function (uint256) external returns (bool) ext;
+    function () pure internal noargs;
+    function (address payable) payable external pay;
+    mapping(uint256 => S) structs;
+    S[] structArray;
+    TokenData[2] fixedStructs;
+    E e;
+    mapping(E => I[]) enumKeys;
+    I i;
+    address payable owner;
+    address plain;
+    U u;
+    mapping(address => mapping(uint256 => S)) nested;
+}
+"#,
+);
+
+#[test]
+fn test_type_names_match_solc_labels() {
+    let unit = TypeLabels::build_compilation_unit();
+    let contract = unit
+        .find_contract_by_name("Store")
+        .next()
+        .expect("contract can be found");
+    let contract_abi = contract.compute_abi().expect("can compute ABI");
+    let type_names: Vec<_> = contract_abi
+        .storage_layout()
+        .iter()
+        .map(|item| (item.label(), item.type_name()))
+        .collect();
+
+    assert_eq!(
+        type_names,
+        [
+            (
+                "renderer",
+                "function (uint256,struct Store.TokenData) view returns (string)"
+            ),
+            ("handlers", "mapping(bytes4 => function (bytes))"),
+            ("ext", "function (uint256) external returns (bool)"),
+            ("noargs", "function () pure"),
+            ("pay", "function (address payable) payable external"),
+            ("structs", "mapping(uint256 => struct S)"),
+            ("structArray", "struct S[]"),
+            ("fixedStructs", "struct Store.TokenData[2]"),
+            ("e", "enum Store.E"),
+            ("enumKeys", "mapping(enum Store.E => contract I[])"),
+            ("i", "contract I"),
+            ("owner", "address payable"),
+            ("plain", "address"),
+            ("u", "U"),
+            ("nested", "mapping(address => mapping(uint256 => struct S))"),
+        ]
+    );
 }