-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Refactoring of EntityToString API #3635
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the entity-to-string conversion API by consolidating multiple entity-specific methods (MethodToString, FieldToString, PropertyToString, EventToString) into a unified EntityToString method. The refactoring introduces a new ILAmbience class that handles IL-style formatting of types and entities using ConversionFlags instead of multiple boolean parameters.
Key changes:
- Introduced
ILAmbienceclass withConvertSymbolandConvertTypemethods to handle formatting - Replaced boolean parameters in API methods with
ConversionFlagsenum for more granular control - Deprecated entity-specific methods in favor of unified
EntityToStringAPI - Migrated
EscapeNameutility methods fromLanguageclass toILAmbience
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ICSharpCode.Decompiler/IL/ILAmbience.cs | New file implementing the core IL formatting logic with ConvertSymbol/ConvertType methods |
| ILSpy/Languages/Language.cs | Updated TypeToString signature, added EntityToString, deprecated old entity-specific methods, removed TypeToStringVisitor inner class |
| ILSpy/Languages/CSharpLanguage.cs | Removed overrides of deprecated methods, updated TypeToString and added EntityToString override |
| ICSharpCode.ILSpyX/Abstractions/ILanguage.cs | Updated interface to replace deprecated methods with EntityToString |
| ILSpy/ViewModels/CompareViewModel.cs | Updated entity formatting calls to use new EntityToString API with appropriate ConversionFlags |
| ILSpy/TreeNodes/*.cs | Updated all tree node classes to use EntityToString with ConversionFlags instead of deprecated methods |
| ILSpy/Analyzers/TreeNodes/*.cs | Updated analyzer tree nodes to use EntityToString with appropriate flags |
| ILSpy/Search/SearchResultFactory.cs | Updated search result formatting to use new API |
| ICSharpCode.Decompiler/Disassembler/ReflectionDisassembler.cs | Changed EnumNameCollection visibility to internal static, converted from class to struct, updated WriteEnum/WriteFlags signatures |
| ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpAmbience.cs | Minor visibility and nullable annotation updates |
| ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj | Added ILAmbience.cs to project |
Comments suppressed due to low confidence (1)
ICSharpCode.Decompiler/Disassembler/ReflectionDisassembler.cs:2031
- Changing EnumNameCollection from a class to a struct causes issues with the collection initializer syntax used for static readonly fields (lines 102, 118, 127, 136, 143, 1270, 1279, 1418, 1492). When using collection initializers like "new EnumNameCollection() { ... }", the Add method is called on the struct instance. However, since these are static readonly fields, the struct instance is readonly, and calling Add (which modifies the struct) will only modify a temporary copy, leaving the actual field empty. This will cause all enum/flag name lookups to fail silently.
internal struct EnumNameCollection<T> : IEnumerable<KeyValuePair<long, string>> where T : struct
{
List<KeyValuePair<long, string>> names = new List<KeyValuePair<long, string>>();
public EnumNameCollection()
{
}
public void Add(T flag, string name)
{
this.names.Add(new KeyValuePair<long, string>(Convert.ToInt64(flag), name));
}
public IEnumerator<KeyValuePair<long, string>> GetEnumerator()
{
return names.GetEnumerator();
}
System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
{
return names.GetEnumerator();
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
26eed2b to
80ba0c3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
80ba0c3 to
77e0156
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5a2f188 to
8350006
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 33 out of 33 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dbba18d to
11dfe61
Compare
No description provided.