Add CodeFix and Refactoring Name to Razor ExternalAccess#51917
Add CodeFix and Refactoring Name to Razor ExternalAccess#51917JoeRobich merged 8 commits intodotnet:mainfrom
Conversation
|
@TanayParikh I kept the external access changes separate. |
Thanks @JoeRobich! |
ac2ead6 to
fd34ce9
Compare
|
|
||
| namespace Microsoft.CodeAnalysis.ExternalAccess.Razor | ||
| { | ||
| internal static class RazorPredefinedCodeFixProviderNames |
There was a problem hiding this comment.
💡 Only include the ones Razor needs to use. Otherwise, any change to the internal name of any code fix means we have to coordinate with another project.
There was a problem hiding this comment.
There is no issue with leaving Names in the Razor class that are no longer in the Roslyn class.
|
|
||
| namespace Microsoft.CodeAnalysis.ExternalAccess.Razor | ||
| { | ||
| internal static class RazorPredefinedCodeRefactoringProviderNames |
There was a problem hiding this comment.
💡 Only include the ones Razor needs to use. Otherwise, any change to the internal name of any refactoring means we have to coordinate with another project.
There was a problem hiding this comment.
Otherwise, any change to the internal name of any refactoring means we have to coordinate with another project.
If the internal name change is for a refactoring not supported by razor, would we still need to co-ordinate?
I'd prefer to have all these accessible if possible to simplify the process of adding support for new razor C# code fix/refactoring providers. Without these in place we'd have to make Roslyn changes / package upgrades for every time we'd like to add a code fix/refactoring.
src/Tools/ExternalAccess/Razor/RazorPredefinedCodeFixProviderNames.cs
Outdated
Show resolved
Hide resolved
| // Validates that all names from Roslyn's PredefinedCode*ProviderNames class exist in the RazorPredefinedCode*ProviderNames class | ||
| // and that they have the same value. It does not fail if the Razor class contains names no longer in the Roslyn class as they may |
There was a problem hiding this comment.
Validates that all names from Roslyn's PredefinedCode*ProviderNames class exist in the RazorPredefinedCode*ProviderNames
This is not a desirable characteristic (see prior comments).
and that they have the same value
This is not a desirable characteristic. The property definitions explicitly allow these to decouple if/when necessary.
There was a problem hiding this comment.
This is not a desirable characteristic (see prior comments).
I feel like the PR when a name is added is a good time to ensure Razor will be able to consume it. Razor still gets to choose when they make use of the new Name.
This is not a desirable characteristic. The property definitions explicitly allow these to decouple if/when necessary.
I think it is useful to help Razor reason about which Provider is which.
Talked with Sam offline and he believes we can address any issue with the Tests when they come up.
davidwengier
left a comment
There was a problem hiding this comment.
If only there was a "linked file but with different namespace" feature of somethingorother.
Based on the changes in #51456 since provider names were added. Changes for this PR are in last three commits.