Pass CancellationTokens in the Visual Studio layer#52276
Conversation
| // TODO: Should we pass in cancellationToken here insead of CancellationToken.None? | ||
| return await ((IVsAsyncFileChangeEx)_fileChangeService).AdviseFileChangeAsync(_filePath, _fileChangeFlags, this, CancellationToken.None).ConfigureAwait(false); |
There was a problem hiding this comment.
Yeah, this is fine since if you look at the call to _fileChangeCookie.GetValueAsync it's passing in CancellationToken.None anyways. Passing in cancellationToken would have had the same effect since it would have been None in practice. That said any attempt to make this cancellation safe is going to require an audit of the entire type which is definitely out of scope.
There was a problem hiding this comment.
(I admit it's not entirely clear to me why this is using AsyncLazy in the first place...)
|
Your original PR had an editorconfig entry that was enforcing this analyzer at the root of the repo: . # CA2016: Forward the 'CancellationToken' parameter to methods
dotnet_diagnostic.CA2016.severity = warningGiven you are breaking the change into multiple PRs, each of your PR should add/update an editorconfig entry with the correct globbing section header enforcing the analyzer in that layer. [src/{VisualStudio}/**/*.{cs,vb}]
# CA2016: Forward the 'CancellationToken' parameter to methods
dotnet_diagnostic.CA2016.severity = warning |
As long as you are confident to get all those PRs in, sounds good to me. |
Redo of #52247
This cleans up passing cancellation tokens in the Visual Studio layer