jbevain / cecil

Cecil is a library to inspect, modify and create .NET programs and libraries.
MIT License
2.74k stars 624 forks source link

`ToString()` of `MethodDefinition` does not include generic parameters, making it non-unique #834

Closed Zastai closed 2 years ago

Zastai commented 2 years ago

I am processing assemblies, looking at the methods, first grouping them by their Name, then their string form, expecting the latter to be unique within a type.

However, the following methods

  public static IReadOnlyDictionary<string, T>? ReadDictionary<T>
    (this ref Utf8JsonReader reader, JsonSerializerOptions options)
  { ... }

  public static IReadOnlyDictionary<string, T>? ReadDictionary<T, TValue>
    (this ref Utf8JsonReader reader, JsonSerializerOptions options)
    where TValue : T
  { ... }

both give

System.Collections.Generic.IReadOnlyDictionary`2<System.String,T> MetaBrainz.Common.Json.JsonUtils::ReadDictionary(System.Text.Json.Utf8JsonReader&,System.Text.Json.JsonSerializerOptions)

as their ToString(). This omits the <T> and <T,TValue>, resulting in duplicate values.

Would it be possible to change this so that the generic parameters are included (the constraints aren't needed), making the string form usable as a unique signature?

Note that a SortName property (which omits the return type, and with a guarantee of uniqueness) would also be useful (although for my purposes it doesn't matter too much that I am sorting by return type first).

jbevain commented 2 years ago

Name simply shows what's in the metadata for this method, so that's never going to change.

Using a name-based solution is not advisable to distinguish the identity of method definitions, so I'm reluctant to add anything to that effect. You're better off writing your own comparers for sort scenarios.

For bigger scenarios we always suggest doing something like the Mono Linker is doing: controlling the set of loaded assemblies then you can use object identity.

Zastai commented 2 years ago

I wasn't suggesting changing Name, only ToString(); that's also useful for debugging so you can easily see what method you're working with.

My use case is generating a source with an assembly's public API, for easy text diffing. There having a stable sequence is rather important, and I can't use object identity because it's across multiple invocations.

I've adjusted my code to add the generic arguments to the sort key myself, so it's not a big deal. Would simply have been nice if ToString() had been suitable.