Asked to post this by @sharwell .
I'm concerned with some of the recent work i've seen go in around IDE analzyers, fixers and refactorings. Specifically the work around:
- the 'code cleanup' feature.
- generating a .editorconfig from feature options
- updating an .editorconfig to suppress a feature
All of these recent pieces of work have shared a similar design flaw that I strongly believe needs to be addressed. Specifically, all of them operate by literal hardcoding information about certain features and how they behave into a different layer in the stack. For example:
|
private static ImmutableArray<(string description, PerLanguageOption<bool> option, ImmutableArray<string> diagnosticIds)> _optionDiagnosticsMappings = |
|
ImmutableArray.Create( |
|
(CSharpFeaturesResources.Apply_implicit_explicit_type_preferences, |
|
CodeCleanupOptions.ApplyImplicitExplicitTypePreferences, |
|
ImmutableArray.Create(IDEDiagnosticIds.UseImplicitTypeDiagnosticId, |
|
IDEDiagnosticIds.UseExplicitTypeDiagnosticId)), |
Here, Code-Cleanup hardcodes in knowledge of how use implicit/explicit type works. Another example is here:
|
// TODO: Just get the first fix for now until we have a way to config user's preferred fix |
|
// https://github.com/dotnet/roslyn/issues/27066 |
|
return result.ToImmutableAndFree().FirstOrDefault(); |
Where we assume that the first fix offered is the right way to fix any issue.
These sorts of hardcoding make our system very brittle and hard to maintain. It means that one cannot just go update a single feature in its appropriate location. Instead, one has to know the entire set of points in the entire IDE that need to be updated to make the feature properly work.
Similarly, this also means that the only way to provide IDE features that work properly with these other systems is to ship them out of roslyn itself. Say, for example, a third party had written the "add accessibility modifiers" feature. There would have been no way for them to integrate properly with these IDE subsystems.
Finally, because of this hard coding, we've effectively made it much harder to do things like light-up VB support. Instead of VB just lighting up naturally (if this had only involved changes at the individual feature level), we now need to specifically redo all this hard code knowledge for VB as well. This leaves VB falling further behind and increases the likelihood of it not getting these features, whihc increases hte likelihood of future designs becoming even more brittle since we won't do the work to properly design them to work across languages.
--
I think these approaches are acceptable in feature-branches, but should not become part of the main project. We've seen far too often that once integrated, resources are rarely spent to address these issues. When we want to actually roll this into the product, we should go and produce a design without tight coupling and brittleness. For example, instead of the way we did things here, we could instead have a new interface that analyzers/fixers could implement. For example, something like ISupportCodeCleanup. We would encode into that interface all the information the underlying system needs to run properly, and we would then implement that interface where appropriate. This would now mean that we could keep all feature logic in a specific location, and we would not be shutting out third parties from participating. IT would also mean that VB woudl light up naturally with C# since they so often use the same Analyzers/Providers.
Asked to post this by @sharwell .
I'm concerned with some of the recent work i've seen go in around IDE analzyers, fixers and refactorings. Specifically the work around:
All of these recent pieces of work have shared a similar design flaw that I strongly believe needs to be addressed. Specifically, all of them operate by literal hardcoding information about certain features and how they behave into a different layer in the stack. For example:
roslyn/src/Features/CSharp/Portable/CodeCleanup/CSharpCodeCleanupService.cs
Lines 43 to 48 in 0c884ee
Here, Code-Cleanup hardcodes in knowledge of how use implicit/explicit type works. Another example is here:
roslyn/src/EditorFeatures/Core/Implementation/CodeFixes/CodeFixService.cs
Lines 229 to 231 in 0c884ee
Where we assume that the first fix offered is the right way to fix any issue.
These sorts of hardcoding make our system very brittle and hard to maintain. It means that one cannot just go update a single feature in its appropriate location. Instead, one has to know the entire set of points in the entire IDE that need to be updated to make the feature properly work.
Similarly, this also means that the only way to provide IDE features that work properly with these other systems is to ship them out of roslyn itself. Say, for example, a third party had written the "add accessibility modifiers" feature. There would have been no way for them to integrate properly with these IDE subsystems.
Finally, because of this hard coding, we've effectively made it much harder to do things like light-up VB support. Instead of VB just lighting up naturally (if this had only involved changes at the individual feature level), we now need to specifically redo all this hard code knowledge for VB as well. This leaves VB falling further behind and increases the likelihood of it not getting these features, whihc increases hte likelihood of future designs becoming even more brittle since we won't do the work to properly design them to work across languages.
--
I think these approaches are acceptable in feature-branches, but should not become part of the main project. We've seen far too often that once integrated, resources are rarely spent to address these issues. When we want to actually roll this into the product, we should go and produce a design without tight coupling and brittleness. For example, instead of the way we did things here, we could instead have a new interface that analyzers/fixers could implement. For example, something like
ISupportCodeCleanup. We would encode into that interface all the information the underlying system needs to run properly, and we would then implement that interface where appropriate. This would now mean that we could keep all feature logic in a specific location, and we would not be shutting out third parties from participating. IT would also mean that VB woudl light up naturally with C# since they so often use the same Analyzers/Providers.