CodeFixService simplifications#59582
Conversation
| private readonly ImmutableArray<Lazy<CodeFixProvider, CodeChangeProviderMetadata>> _fixers; | ||
| private readonly Dictionary<string, List<Lazy<CodeFixProvider, CodeChangeProviderMetadata>>> _fixersPerLanguageMap; | ||
|
|
||
| private readonly Func<Workspace, ImmutableDictionary<LanguageKind, Lazy<ImmutableDictionary<DiagnosticId, ImmutableArray<CodeFixProvider>>>>> _getWorkspaceFixersMap; |
There was a problem hiding this comment.
remove unnecessary callbacks. These can just be normal methods in this type.
| new((d1, d2) => DiagnosticId.CompareOrdinal(d1.Id, d2.Id)); | ||
|
|
||
| private readonly IDiagnosticAnalyzerService _diagnosticService; | ||
| private readonly ImmutableArray<Lazy<CodeFixProvider, CodeChangeProviderMetadata>> _fixers; |
There was a problem hiding this comment.
don't store as an IEnumerable.
|
|
||
| private ImmutableDictionary<object, FixAllProviderInfo?> _fixAllProviderMap; | ||
| private ImmutableDictionary<CodeFixProvider, ImmutableArray<DiagnosticId>> _fixerToFixableIdsMap = ImmutableDictionary<CodeFixProvider, ImmutableArray<DiagnosticId>>.Empty; | ||
| private ImmutableDictionary<object, FixAllProviderInfo?> _fixAllProviderMap = ImmutableDictionary<object, FixAllProviderInfo?>.Empty; |
There was a problem hiding this comment.
move mutable fields to bottom.
a6f8838 to
25a439f
Compare
jasonmalinowski
left a comment
There was a problem hiding this comment.
My only real complaint is there's still code left after all this cleanup. 😛 
|
|
||
| // REVIEW: currently, fixer's priority is statically defined by the fixer itself. might considering making it more dynamic or configurable. | ||
| _getFixerPriorityMap = workspace => GetFixerPriorityPerLanguageMap(fixersPerLanguageMap, workspace); | ||
| _lazyFixerToMetadataMap = new(() => fixers.Where(service => service.IsValueCreated).ToImmutableDictionary(service => service.Value, service => service.Metadata)); |
There was a problem hiding this comment.
Out of curiosity, when this runs in practice are there any fixers already created or is this just initializing an empty map?
There was a problem hiding this comment.
(why this is stuffed in a Lazy of its own is terrifying...)
There was a problem hiding this comment.
@mavasani this is a bit scary. this lazy is exposed through:
private ImmutableDictionary<CodeFixProvider, CodeChangeProviderMetadata> FixerToMetadataMap => _lazyFixerToMetadataMap.Value;This means that we capture whatever the state of hte world is when this is computed. Meaning if a service hasn't been created, then it's never in this map. This seems fishy, esp in a multi-language world. How do we know that we won't capture a snapshot of things in a bad state?
I'm working in this area, and makign a bunch of a changes. This extracts out some general cleanup that comes with no behavior changes.