Fix 'Use Local Function' fixer in code style layer #68457
Conversation
ghost
added
Area-IDE
untriaged
| #if CODE_STYLE | ||
| var info = new CSharpCodeGenerationContextInfo( | ||
| CodeGenerationContext.Default, CSharpCodeGenerationOptions.Default, new CSharpCodeGenerationService(document.Project.Services), root.SyntaxTree.Options.LanguageVersion()); | ||
| CodeGenerationContext.Default, CSharpCodeGenerationOptions.Default, new CSharpCodeGenerationService(document.Project.GetExtendedLanguageServices().LanguageServices), root.SyntaxTree.Options.LanguageVersion()); |
There was a problem hiding this comment.
📝 This fixes the root cause of the bug
|
|
||
| internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace) | ||
| => (new CSharpUseLocalFunctionDiagnosticAnalyzer(), GetCSharpUseLocalFunctionCodeFixProvider()); | ||
| => (new CSharpUseLocalFunctionDiagnosticAnalyzer(), new CSharpUseLocalFunctionCodeFixProvider()); |
There was a problem hiding this comment.
📝 This fixes the failure to identify the bug in testing
There was a problem hiding this comment.
@sharwell I'm very surprised. The deleted GetCSharpUseLocalFunctionCodeFixProvider method exactly does new CSharpUseLocalFunctionCodeFixProvider(), how that would matter in identifying the bug in testing?
There was a problem hiding this comment.
➡️ There are two instances of this type: one in the code style layer, and one in the features layer. Previously, when tests compiled in the code style layer they were still using the fixer from the features layer. The relocated instantiation binds to the other type, so tests in the code style layer use the fix from the code style layer.
|
|
||
| internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace) | ||
| => (new MakeLocalFunctionStaticDiagnosticAnalyzer(), GetMakeLocalFunctionStaticCodeFixProvider()); | ||
| => (new MakeLocalFunctionStaticDiagnosticAnalyzer(), new MakeLocalFunctionStaticCodeFixProvider()); |
There was a problem hiding this comment.
📝 This is another code fixer which was not properly tested, but there were no test failures when it was corrected
| var info = new CSharpCodeGenerationContextInfo( | ||
| CodeGenerationContext.Default, CSharpCodeGenerationOptions.Default, new CSharpCodeGenerationService(document.Project.Services), root.SyntaxTree.Options.LanguageVersion()); | ||
| CodeGenerationContext.Default, CSharpCodeGenerationOptions.Default, new CSharpCodeGenerationService(document.Project.GetExtendedLanguageServices().LanguageServices), root.SyntaxTree.Options.LanguageVersion()); | ||
| #else |
There was a problem hiding this comment.
so, this seems like something we likely need an analyzer for, right? Like it's farrrrrr too easy to screw this up.
Another option seems to be (CSharpCodeGenarationService)document.GetRequiredLanguageService<ICodeGenerationService>() (or just make CSharpCodeGenerationContextInfo take an ICodeGenService, i dont' think it needs the subtype.
There was a problem hiding this comment.
➡️ Updated BannedSymbols.txt to account for the new Services property, which was not present at the time LanguageServices property was excluded.
...es/DiagnosticsTestUtilities/Diagnostics/AbstractDiagnosticProviderBasedUserDiagnosticTest.cs
Show resolved
Hide resolved
...es/DiagnosticsTestUtilities/Diagnostics/AbstractDiagnosticProviderBasedUserDiagnosticTest.cs
Show resolved
Hide resolved
CyrusNajmabadi
left a comment
CyrusNajmabadi
left a comment
There was a problem hiding this comment.
lgtm. i think we could make this slightly nicer though by avoidign the direct casll to GetExtendedLanguageServices here.
4 participants
Fixes #68440