Addrange on immutableArray creates a new ImmutableArray #69331
Conversation
The buildonlydiagnostics don't get evaluated because of this.
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
ghost
added
the
Community
| @@ -113,7 +113,7 @@ internal partial class CodeFixService : ICodeFixService | |||
| } | |||
|
|
|||
| var buildOnlyDiagnosticsService = document.Project.Solution.Services.GetRequiredService<IBuildOnlyDiagnosticsService>(); | |||
| allDiagnostics.AddRange(buildOnlyDiagnosticsService.GetBuildOnlyDiagnostics(document.Id)); | |||
| allDiagnostics = allDiagnostics.AddRange(buildOnlyDiagnosticsService.GetBuildOnlyDiagnostics(document.Id)); | |||
There was a problem hiding this comment.
It would be good to add a unit test or an integration test for this.
|
The fix looks good to me, but this shouldn’t impact the code fix path for light bulb invocation as we already do the right thing for that: This fix will resolve the issue that if a line only has a fixable build-only diagnostic and no other code fix or refactoring on the same line, you would have not seen the light bulb in the margin from background analysis. I don’t believe that part can be tested thought, so I am good to sign off on this change without adding a test |
| @@ -113,7 +113,7 @@ internal partial class CodeFixService : ICodeFixService | |||
| } | |||
|
|
|||
| var buildOnlyDiagnosticsService = document.Project.Solution.Services.GetRequiredService<IBuildOnlyDiagnosticsService>(); | |||
| allDiagnostics.AddRange(buildOnlyDiagnosticsService.GetBuildOnlyDiagnostics(document.Id)); | |||
| allDiagnostics = allDiagnostics.AddRange(buildOnlyDiagnosticsService.GetBuildOnlyDiagnostics(document.Id)); | |||
There was a problem hiding this comment.
@jasonmalinowski dotnet/runtime#34098 tracks adding such an analyzer driven by a new attribute in the framework. This is supposed to be lot more refined than all our existing analyzers in same space (IDE0058, IDE0059, etc.).
Tagging @eerhardt @buyaa-n as the primary champions for the design of that analyzer. Not sure if this will make it into .NET8 as it seems to have large cost estimate.
There was a problem hiding this comment.
Note that we did have CA1806 which used to fire for the Collections.Immutable methods, but the [Pure] attribute was removed on these APIs in dotnet/runtime#35118. The CA1806 analyzer no longer fires because there are no [Pure] attributes on the APIs.
Not sure if this will make it into .NET8 as it seems to have large cost estimate.
No, dotnet/runtime#34098 has been pushed out of .NET 8.
cc @jeffhandley
There was a problem hiding this comment.
In the interim, it might make sense to modify CA1806 to explicitly include items we know have return values that shouldn't be dropped.
The buildonlydiagnostics don't get evaluated because of this.