Add analyzer/fixer to remove unnecessary 'unsafe' modifiers from members.#80794
Add analyzer/fixer to remove unnecessary 'unsafe' modifiers from members.#80794CyrusNajmabadi merged 18 commits intodotnet:mainfrom
Conversation
src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/WorkspaceExtensions.projitems
Outdated
Show resolved
Hide resolved
|
@dibarbet ptal :) |
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System.Net.Mime; |
There was a problem hiding this comment.
will remove.
| foreach (var linkedSpans in linkedSpansArray) | ||
| { | ||
| if (!linkedSpans.Contains(expr.Span)) | ||
| return WarningAnnotation.Create(CodeFixesResources.May_be_neccessary_in_other_project_this_document_is_linked_in); |
| } | ||
| """, | ||
| BatchFixedCode = """ | ||
| unsafe class C |
There was a problem hiding this comment.
I don't feel extremely strongly about this - but it doesn't seem ideal that the batch fixer may remove a different unsafe as compared to the regular fixer for this scenario.
There was a problem hiding this comment.
So that's not abnormal. We have lots of features where we may report an individual info on many different nodes, but we can't actually remove all of them in batch fixing.
…kspaceExtensions.projitems
835ee3e to
792d005
Compare
| (original, current) => WithoutUnsafeModifier(current).WithAdditionalAnnotations(nodeToAnnotation[original])), | ||
| originalTree.Options); | ||
| var updateRoot = updatedTree.GetRoot(cancellationToken); | ||
| var updatedCompilation = semanticModel.Compilation |
There was a problem hiding this comment.
Is it easier, possibly more efficient, to do this with TryGetSpeculativeSemanticModel?
i.e, get diagnostics of the original declaration. Skip analysis if there are any errors. Then get speculative semantic model with the unsafe keyword removed, and if it still doesn't have errors, then unsafe is unnecessary?
I haven't used speculative semantic model that much, so not sure if it's capable of doing the task here or not.
There was a problem hiding this comment.
Is it easier, possibly more efficient, to do this with TryGetSpeculativeSemanticModel?
No. Speculative models don't let you change actual top level symbols (like types and members). They allow you to speculative about different executable code within existing symbols.
Fixes #48031.