Ensure unique types in codegen (#967)

* Ensure unique types in codegen

* tweak comment

* Test the duplicate type codegen stuff

* appease clippy

* fix import

* add another test for generics being de-duplicated

* cargo fmt

---------

Co-authored-by: Niklas Adolfsson <niklasadolfsson1@gmail.com>

codegen/src/api/mod.rs

@@ -27,7 +27,7 @@ use frame_metadata::{RuntimeMetadata, RuntimeMetadataPrefixed};
 use heck::ToSnakeCase as _;
 use proc_macro2::TokenStream as TokenStream2;
 use quote::{format_ident, quote};
-use std::{fs, io::Read, path, string::ToString};
+use std::{collections::HashMap, fs, io::Read, path, string::ToString};
 use syn::parse_quote;
 
 /// Generates the API for interacting with a Substrate runtime.
@@ -175,15 +175,56 @@ impl RuntimeGenerator {
     ///
     /// Supported versions: v14 and v15.
     pub fn new(metadata: RuntimeMetadataPrefixed) -> Self {
-        let metadata = match metadata.1 {
+        let mut metadata = match metadata.1 {
             RuntimeMetadata::V14(v14) => metadata_v14_to_latest(v14),
             RuntimeMetadata::V15(v15) => v15,
             _ => panic!("Unsupported metadata version {:?}", metadata.1),
         };
 
+        Self::ensure_unique_type_paths(&mut metadata);
+
         RuntimeGenerator { metadata }
     }
 
+    /// Ensure that every unique type we'll be generating or referring to also has a
+    /// unique path, so that types with matching paths don't end up overwriting each other
+    /// in the codegen. We ignore any types with generics; Subxt actually endeavours to
+    /// de-duplicate those into single types with a generic.
+    fn ensure_unique_type_paths(metadata: &mut RuntimeMetadataV15) {
+        let mut visited_path_counts = HashMap::<Vec<String>, usize>::new();
+        for ty in &mut metadata.types.types {
+            // Ignore types without a path (ie prelude types).
+            if ty.ty.path.namespace().is_empty() {
+                continue;
+            }
+
+            let has_valid_type_params = ty.ty.type_params.iter().any(|tp| tp.ty.is_some());
+
+            // Ignore types which have generic params that the type generation will use.
+            // Ordinarily we'd expect that any two types with identical paths must be parameterized
+            // in order to share the path. However scale-info doesn't understand all forms of generics
+            // properly I think (eg generics that have associated types that can differ), and so in
+            // those cases we need to fix the paths for Subxt to generate correct code.
+            if has_valid_type_params {
+                continue;
+            }
+
+            // Count how many times we've seen the same path already.
+            let visited_count = visited_path_counts
+                .entry(ty.ty.path.segments.clone())
+                .or_default();
+            *visited_count += 1;
+
+            // alter the type so that if it's been seen more than once, we append a number to
+            // its name to ensure that every unique type has a unique path, too.
+            if *visited_count > 1 {
+                if let Some(name) = ty.ty.path.segments.last_mut() {
+                    *name = format!("{name}{visited_count}");
+                }
+            }
+        }
+    }
+
     /// Generate the API for interacting with a Substrate runtime.
     ///
     /// # Arguments

testing/integration-tests/src/codegen/codegen_tests.rs

@@ -0,0 +1,143 @@
+// Copyright 2019-2023 Parity Technologies (UK) Ltd.
+// This file is dual-licensed as Apache-2.0 or GPL-3.0.
+// see LICENSE for license details.
+
+use frame_metadata::{
+    v15::{ExtrinsicMetadata, RuntimeMetadataV15},
+    RuntimeMetadataPrefixed,
+};
+use scale_info::{meta_type, IntoPortable, TypeInfo};
+use subxt_codegen::{CratePath, DerivesRegistry, RuntimeGenerator, TypeSubstitutes};
+
+fn generate_runtime_interface_from_metadata(metadata: RuntimeMetadataPrefixed) -> String {
+    // Generate a runtime interface from the provided metadata.
+    let generator = RuntimeGenerator::new(metadata);
+    let item_mod = syn::parse_quote!(
+        pub mod api {}
+    );
+    let crate_path = CratePath::default();
+    let derives = DerivesRegistry::with_default_derives(&crate_path);
+    let type_substitutes = TypeSubstitutes::with_default_substitutes(&crate_path);
+    generator
+        .generate_runtime(item_mod, derives, type_substitutes, crate_path, false)
+        .expect("API generation must be valid")
+        .to_string()
+}
+
+fn generate_runtime_interface_with_type_registry<F>(f: F) -> String
+where
+    F: Fn(&mut scale_info::Registry),
+{
+    #[derive(TypeInfo)]
+    struct Runtime;
+    #[derive(TypeInfo)]
+    enum RuntimeCall {}
+    #[derive(TypeInfo)]
+    enum RuntimeEvent {}
+    #[derive(TypeInfo)]
+    pub enum DispatchError {}
+
+    // We need these types for codegen to work:
+    let mut registry = scale_info::Registry::new();
+    let ty = registry.register_type(&meta_type::<Runtime>());
+    registry.register_type(&meta_type::<RuntimeCall>());
+    registry.register_type(&meta_type::<RuntimeEvent>());
+    registry.register_type(&meta_type::<DispatchError>());
+
+    // Allow custom types to be added for testing:
+    f(&mut registry);
+
+    let extrinsic = ExtrinsicMetadata {
+        ty: meta_type::<()>(),
+        version: 0,
+        signed_extensions: vec![],
+    }
+    .into_portable(&mut registry);
+    let metadata = RuntimeMetadataV15 {
+        types: registry.into(),
+        pallets: Vec::new(),
+        extrinsic,
+        ty,
+        apis: vec![],
+    };
+
+    let metadata = RuntimeMetadataPrefixed::from(metadata);
+    generate_runtime_interface_from_metadata(metadata)
+}
+
+#[test]
+fn dupe_types_do_not_overwrite_each_other() {
+    let interface = generate_runtime_interface_with_type_registry(|registry| {
+        // Now we duplicate some types with same type info. We need two unique types here,
+        // and can't just add one type to the registry twice, because the registry knows if
+        // type IDs are the same.
+        enum Foo {}
+        impl TypeInfo for Foo {
+            type Identity = Self;
+            fn type_info() -> scale_info::Type {
+                scale_info::Type::builder()
+                    .path(scale_info::Path::new("DuplicateType", "dupe_mod"))
+                    .variant(
+                        scale_info::build::Variants::new()
+                            .variant("FirstDupeTypeVariant", |builder| builder.index(0)),
+                    )
+            }
+        }
+        enum Bar {}
+        impl TypeInfo for Bar {
+            type Identity = Self;
+            fn type_info() -> scale_info::Type {
+                scale_info::Type::builder()
+                    .path(scale_info::Path::new("DuplicateType", "dupe_mod"))
+                    .variant(
+                        scale_info::build::Variants::new()
+                            .variant("SecondDupeTypeVariant", |builder| builder.index(0)),
+                    )
+            }
+        }
+
+        registry.register_type(&meta_type::<Foo>());
+        registry.register_type(&meta_type::<Bar>());
+    });
+
+    assert!(interface.contains("DuplicateType"));
+    assert!(interface.contains("FirstDupeTypeVariant"));
+
+    assert!(interface.contains("DuplicateType2"));
+    assert!(interface.contains("SecondDupeTypeVariant"));
+}
+
+#[test]
+fn generic_types_overwrite_each_other() {
+    let interface = generate_runtime_interface_with_type_registry(|registry| {
+        // If we have two types mentioned in the registry that have generic params,
+        // only one type will be output (the codegen assumes that the generic param will disambiguate)
+        enum Foo {}
+        impl TypeInfo for Foo {
+            type Identity = Self;
+            fn type_info() -> scale_info::Type {
+                scale_info::Type::builder()
+                    .path(scale_info::Path::new("DuplicateType", "dupe_mod"))
+                    .type_params([scale_info::TypeParameter::new("T", Some(meta_type::<u8>()))])
+                    .variant(scale_info::build::Variants::new())
+            }
+        }
+        enum Bar {}
+        impl TypeInfo for Bar {
+            type Identity = Self;
+            fn type_info() -> scale_info::Type {
+                scale_info::Type::builder()
+                    .path(scale_info::Path::new("DuplicateType", "dupe_mod"))
+                    .type_params([scale_info::TypeParameter::new("T", Some(meta_type::<u8>()))])
+                    .variant(scale_info::build::Variants::new())
+            }
+        }
+
+        registry.register_type(&meta_type::<Foo>());
+        registry.register_type(&meta_type::<Bar>());
+    });
+
+    assert!(interface.contains("DuplicateType"));
+    // We do _not_ expect this to exist, since a generic is present on the type:
+    assert!(!interface.contains("DuplicateType2"));
+}

testing/integration-tests/src/codegen/mod.rs

@@ -14,4 +14,5 @@
 #[allow(clippy::all)]
 mod polkadot;
 
-mod codegen_documentation;
+mod codegen_tests;
+mod documentation;