Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update cancellation token handling behavior for certain analyzer APIs #69108

Merged
merged 7 commits into from
Jul 26, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -3511,8 +3511,7 @@ public void M() { }
var compWithAnalyzers = new CompilationWithAnalyzers(
compilation,
analyzers.ToImmutableArray(),
new AnalyzerOptions(ImmutableArray<AdditionalText>.Empty),
CancellationToken.None);
new AnalyzerOptions(ImmutableArray<AdditionalText>.Empty));
var diagnostics = await compWithAnalyzers.GetAnalyzerSemanticDiagnosticsAsync(model, filterSpan: null, CancellationToken.None);
diagnostics.Verify(Diagnostic("ID0001", "M").WithLocation(4, 17));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public void GetAnalyzerTelemetry()
DiagnosticAnalyzer analyzer = new AnalyzerWithDisabledRules();
var analyzers = ImmutableArray.Create(analyzer);
var analyzerOptions = new AnalyzerOptions(ImmutableArray<AdditionalText>.Empty);
var compWithAnalyzers = new CompilationWithAnalyzers(compilation, analyzers, analyzerOptions, CancellationToken.None);
var compWithAnalyzers = new CompilationWithAnalyzers(compilation, analyzers, analyzerOptions);

var analysisResult = compWithAnalyzers.GetAnalysisResultAsync(CancellationToken.None).Result;
Assert.Empty(analysisResult.CompilationDiagnostics);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.ComponentModel;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
Expand All @@ -26,7 +27,6 @@ public class CompilationWithAnalyzers
private readonly ImmutableArray<DiagnosticAnalyzer> _analyzers;
private readonly ImmutableArray<DiagnosticSuppressor> _suppressors;
private readonly CompilationWithAnalyzersOptions _analysisOptions;
private readonly CancellationToken _cancellationToken;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bad captured CT.


/// <summary>
/// Builder for storing current, possibly partial, analysis results:
Expand Down Expand Up @@ -61,17 +61,26 @@ public class CompilationWithAnalyzers
/// An optional cancellation token which can be used to cancel analysis.
/// Note: This token is only used if the API invoked to get diagnostics doesn't provide a cancellation token.
/// </summary>
public CancellationToken CancellationToken => _cancellationToken;
[Obsolete("This CancellationToken is always 'None'", error: false)]
[EditorBrowsable(EditorBrowsableState.Never)]
public CancellationToken CancellationToken => CancellationToken.None;

/// <inheritdoc cref="CompilationWithAnalyzers(Compilation, ImmutableArray{DiagnosticAnalyzer}, AnalyzerOptions?)"/>
[Obsolete("Use constructor without a cancellation token")]
[EditorBrowsable(EditorBrowsableState.Never)]
public CompilationWithAnalyzers(Compilation compilation, ImmutableArray<DiagnosticAnalyzer> analyzers, AnalyzerOptions? options, CancellationToken cancellationToken)
: this(compilation, analyzers, options)
{
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kept for binary compat. if anyone is using this, they're warning to stop.


/// <summary>
/// Creates a new compilation by attaching diagnostic analyzers to an existing compilation.
/// </summary>
/// <param name="compilation">The original compilation.</param>
/// <param name="analyzers">The set of analyzers to include in future analyses.</param>
/// <param name="options">Options that are passed to analyzers.</param>
/// <param name="cancellationToken">A cancellation token that can be used to abort analysis.</param>
public CompilationWithAnalyzers(Compilation compilation, ImmutableArray<DiagnosticAnalyzer> analyzers, AnalyzerOptions? options, CancellationToken cancellationToken)
: this(compilation, analyzers, new CompilationWithAnalyzersOptions(options, onAnalyzerException: null, analyzerExceptionFilter: null, concurrentAnalysis: true, logAnalyzerExecutionTime: true, reportSuppressedDiagnostics: false), cancellationToken)
public CompilationWithAnalyzers(Compilation compilation, ImmutableArray<DiagnosticAnalyzer> analyzers, AnalyzerOptions? options)
: this(compilation, analyzers, new CompilationWithAnalyzersOptions(options, onAnalyzerException: null, analyzerExceptionFilter: null, concurrentAnalysis: true, logAnalyzerExecutionTime: true, reportSuppressedDiagnostics: false))
{
}

Expand All @@ -82,11 +91,6 @@ public CompilationWithAnalyzers(Compilation compilation, ImmutableArray<Diagnost
/// <param name="analyzers">The set of analyzers to include in future analyses.</param>
/// <param name="analysisOptions">Options to configure analyzer execution.</param>
public CompilationWithAnalyzers(Compilation compilation, ImmutableArray<DiagnosticAnalyzer> analyzers, CompilationWithAnalyzersOptions analysisOptions)
: this(compilation, analyzers, analysisOptions, cancellationToken: CancellationToken.None)
{
}

private CompilationWithAnalyzers(Compilation compilation, ImmutableArray<DiagnosticAnalyzer> analyzers, CompilationWithAnalyzersOptions analysisOptions, CancellationToken cancellationToken)
{
VerifyArguments(compilation, analyzers, analysisOptions);

Expand All @@ -98,7 +102,6 @@ private CompilationWithAnalyzers(Compilation compilation, ImmutableArray<Diagnos
_analyzers = analyzers;
_suppressors = analyzers.OfType<DiagnosticSuppressor>().ToImmutableArrayOrEmpty();
_analysisOptions = analysisOptions;
_cancellationToken = cancellationToken;

_analysisResultBuilder = new AnalysisResultBuilder(analysisOptions.LogAnalyzerExecutionTime, analyzers, _analysisOptions.Options?.AdditionalFiles ?? ImmutableArray<AdditionalText>.Empty);
_compilationAnalysisScope = AnalysisScope.Create(_compilation, _analyzers, this);
Expand Down Expand Up @@ -225,15 +228,18 @@ private void VerifyAdditionalFile(AdditionalText file)
/// <summary>
/// Returns diagnostics produced by all <see cref="Analyzers"/>.
/// </summary>
[EditorBrowsable(EditorBrowsableState.Never)]
public Task<ImmutableArray<Diagnostic>> GetAnalyzerDiagnosticsAsync()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't warn on this, as we often have APIs where user provided cancellationtokens are optional, and we want to allow users to call those without a CT for simple programming purposes. We do hide this though, so that the user only sees the method below, which follows our standard api pattern of an actually 'optional' parameter.

{
return GetAnalyzerDiagnosticsAsync(_cancellationToken);
return GetAnalyzerDiagnosticsAsync(CancellationToken.None);
}

/// <summary>
/// Returns diagnostics produced by all <see cref="Analyzers"/>.
/// </summary>
public async Task<ImmutableArray<Diagnostic>> GetAnalyzerDiagnosticsAsync(CancellationToken cancellationToken)
#pragma warning disable RS0027 // API with optional parameter(s) should have the most parameters amongst its public overloads
public async Task<ImmutableArray<Diagnostic>> GetAnalyzerDiagnosticsAsync(CancellationToken cancellationToken = default)
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
#pragma warning restore RS0027 // API with optional parameter(s) should have the most parameters amongst its public overloads
{
return await GetAnalyzerDiagnosticsCoreAsync(Analyzers, cancellationToken).ConfigureAwait(false);
}
Expand Down Expand Up @@ -273,15 +279,16 @@ public async Task<AnalysisResult> GetAnalysisResultAsync(ImmutableArray<Diagnost
/// <summary>
/// Returns all diagnostics produced by compilation and by all <see cref="Analyzers"/>.
/// </summary>
[EditorBrowsable(EditorBrowsableState.Never)]
public Task<ImmutableArray<Diagnostic>> GetAllDiagnosticsAsync()
{
return GetAllDiagnosticsAsync(_cancellationToken);
return GetAllDiagnosticsAsync(CancellationToken.None);
}

/// <summary>
/// Returns all diagnostics produced by compilation and by all <see cref="Analyzers"/>.
/// </summary>
public async Task<ImmutableArray<Diagnostic>> GetAllDiagnosticsAsync(CancellationToken cancellationToken)
public async Task<ImmutableArray<Diagnostic>> GetAllDiagnosticsAsync(CancellationToken cancellationToken = default)
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
{
var diagnostics = await getAllDiagnosticsWithoutStateTrackingAsync(Analyzers, cancellationToken: cancellationToken).ConfigureAwait(false);
return diagnostics.AddRange(_exceptionDiagnostics);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,34 @@
// 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;
using System.Collections.Immutable;
using System.ComponentModel;
using System.Threading;

namespace Microsoft.CodeAnalysis.Diagnostics
{
public static class DiagnosticAnalyzerExtensions
{
/// <inheritdoc cref="WithAnalyzers(Compilation, ImmutableArray{DiagnosticAnalyzer}, AnalyzerOptions?)"/>
[Obsolete("Use WithAnalyzers overload without a cancellation token", error: false)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're probably okay since this is just a warning, but let's confirm that we don't need to document this as a break

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tagging @333fred, but i'm virtually certain we don't need to document. There is no actual 'break' here. Both binary and source bulds will continue to be fine. Just that source builds may get a warning. We obsolete things like that all the time without documenting.

[EditorBrowsable(EditorBrowsableState.Never)]
public static CompilationWithAnalyzers WithAnalyzers(this Compilation compilation, ImmutableArray<DiagnosticAnalyzer> analyzers, AnalyzerOptions? options, CancellationToken cancellationToken)
Copy link
Member Author

@CyrusNajmabadi CyrusNajmabadi Jul 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kept for binary compat. if anyone is using this, they're warned to stop.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Consider adding this to the banned APIs in dotnet/roslyn-analyzers so it applies even to users who reference old versions of the compiler in their analyzer projects

{
return new CompilationWithAnalyzers(compilation, analyzers, options, cancellationToken);
}

/// <summary>
/// Returns a new compilation with attached diagnostic analyzers.
/// </summary>
/// <param name="compilation">Compilation to which analyzers are to be added.</param>
/// <param name="analyzers">The set of analyzers to include in future analyses.</param>
/// <param name="options">Options that are passed to analyzers.</param>
/// <param name="cancellationToken">A cancellation token that can be used to abort analysis.</param>
public static CompilationWithAnalyzers WithAnalyzers(this Compilation compilation, ImmutableArray<DiagnosticAnalyzer> analyzers, AnalyzerOptions? options = null, CancellationToken cancellationToken = default)
#pragma warning disable RS0027 // API with optional parameter(s) should have the most parameters amongst its public overloads
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Consider using SuppressMessage for this one since it has a space specifically for the justification

public static CompilationWithAnalyzers WithAnalyzers(this Compilation compilation, ImmutableArray<DiagnosticAnalyzer> analyzers, AnalyzerOptions? options = null)
#pragma warning restore RS0027 // API with optional parameter(s) should have the most parameters amongst its public overloads
{
return new CompilationWithAnalyzers(compilation, analyzers, options, cancellationToken);
return new CompilationWithAnalyzers(compilation, analyzers, options);
}

/// <summary>
Expand Down
8 changes: 8 additions & 0 deletions src/Compilers/Core/Portable/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
*REMOVED*static Microsoft.CodeAnalysis.SeparatedSyntaxList<TNode>.implicit operator Microsoft.CodeAnalysis.SeparatedSyntaxList<TNode!>(Microsoft.CodeAnalysis.SeparatedSyntaxList<Microsoft.CodeAnalysis.SyntaxNode!> nodes) -> Microsoft.CodeAnalysis.SeparatedSyntaxList<TNode!>
*REMOVED*static Microsoft.CodeAnalysis.SyntaxList<TNode>.implicit operator Microsoft.CodeAnalysis.SyntaxList<TNode!>(Microsoft.CodeAnalysis.SyntaxList<Microsoft.CodeAnalysis.SyntaxNode!> nodes) -> Microsoft.CodeAnalysis.SyntaxList<TNode!>
*REMOVED*Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers.GetAllDiagnosticsAsync(System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task<System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Diagnostic!>>!
*REMOVED*Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task<System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Diagnostic!>>!
*REMOVED*static Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzerExtensions.WithAnalyzers(this Microsoft.CodeAnalysis.Compilation! compilation, System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer!> analyzers, Microsoft.CodeAnalysis.Diagnostics.AnalyzerOptions? options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers!
Microsoft.CodeAnalysis.CommandLineArguments.ReportInternalsVisibleToAttributes.get -> bool
Microsoft.CodeAnalysis.Diagnostics.AdditionalTextValueProvider<TValue>
Microsoft.CodeAnalysis.Diagnostics.AdditionalTextValueProvider<TValue>.AdditionalTextValueProvider(System.Func<Microsoft.CodeAnalysis.AdditionalText!, TValue>! computeValue, System.Collections.Generic.IEqualityComparer<Microsoft.CodeAnalysis.AdditionalText!>? additionalTextComparer = null) -> void
Expand All @@ -11,10 +14,13 @@ Microsoft.CodeAnalysis.Diagnostics.CompilationStartAnalysisContext.TryGetValue<T
Microsoft.CodeAnalysis.Diagnostics.AdditionalFileAnalysisContext.FilterSpan.get -> Microsoft.CodeAnalysis.Text.TextSpan?
Microsoft.CodeAnalysis.Diagnostics.CodeBlockAnalysisContext.FilterSpan.get -> Microsoft.CodeAnalysis.Text.TextSpan?
Microsoft.CodeAnalysis.Diagnostics.CodeBlockStartAnalysisContext<TLanguageKindEnum>.FilterSpan.get -> Microsoft.CodeAnalysis.Text.TextSpan?
Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers.CompilationWithAnalyzers(Microsoft.CodeAnalysis.Compilation! compilation, System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer!> analyzers, Microsoft.CodeAnalysis.Diagnostics.AnalyzerOptions? options) -> void
Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers.GetAllDiagnosticsAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Diagnostic!>>!
Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers.GetAnalysisResultAsync(Microsoft.CodeAnalysis.AdditionalText! file, Microsoft.CodeAnalysis.Text.TextSpan? filterSpan, System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer!> analyzers, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task<Microsoft.CodeAnalysis.Diagnostics.AnalysisResult!>!
Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers.GetAnalysisResultAsync(Microsoft.CodeAnalysis.AdditionalText! file, Microsoft.CodeAnalysis.Text.TextSpan? filterSpan, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task<Microsoft.CodeAnalysis.Diagnostics.AnalysisResult!>!
Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers.GetAnalysisResultAsync(Microsoft.CodeAnalysis.SyntaxTree! tree, Microsoft.CodeAnalysis.Text.TextSpan? filterSpan, System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer!> analyzers, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task<Microsoft.CodeAnalysis.Diagnostics.AnalysisResult!>!
Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers.GetAnalysisResultAsync(Microsoft.CodeAnalysis.SyntaxTree! tree, Microsoft.CodeAnalysis.Text.TextSpan? filterSpan, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task<Microsoft.CodeAnalysis.Diagnostics.AnalysisResult!>!
Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers.GetAnalyzerDiagnosticsAsync(System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Diagnostic!>>!
Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers.GetAnalyzerSyntaxDiagnosticsAsync(Microsoft.CodeAnalysis.SyntaxTree! tree, Microsoft.CodeAnalysis.Text.TextSpan? filterSpan, System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer!> analyzers, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task<System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Diagnostic!>>!
Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers.GetAnalyzerSyntaxDiagnosticsAsync(Microsoft.CodeAnalysis.SyntaxTree! tree, Microsoft.CodeAnalysis.Text.TextSpan? filterSpan, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.Task<System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Diagnostic!>>!
Microsoft.CodeAnalysis.Diagnostics.OperationAnalysisContext.FilterSpan.get -> Microsoft.CodeAnalysis.Text.TextSpan?
Expand All @@ -32,6 +38,8 @@ Microsoft.CodeAnalysis.Diagnostics.SymbolStartAnalysisContext.FilterTree.get ->
Microsoft.CodeAnalysis.Diagnostics.SyntaxNodeAnalysisContext.FilterSpan.get -> Microsoft.CodeAnalysis.Text.TextSpan?
Microsoft.CodeAnalysis.Diagnostics.SyntaxNodeAnalysisContext.FilterTree.get -> Microsoft.CodeAnalysis.SyntaxTree!
Microsoft.CodeAnalysis.Diagnostics.SyntaxTreeAnalysisContext.FilterSpan.get -> Microsoft.CodeAnalysis.Text.TextSpan?
static Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzerExtensions.WithAnalyzers(this Microsoft.CodeAnalysis.Compilation! compilation, System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer!> analyzers, Microsoft.CodeAnalysis.Diagnostics.AnalyzerOptions? options = null) -> Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers!
static Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzerExtensions.WithAnalyzers(this Microsoft.CodeAnalysis.Compilation! compilation, System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.Diagnostics.DiagnosticAnalyzer!> analyzers, Microsoft.CodeAnalysis.Diagnostics.AnalyzerOptions? options, System.Threading.CancellationToken cancellationToken) -> Microsoft.CodeAnalysis.Diagnostics.CompilationWithAnalyzers!
Microsoft.CodeAnalysis.RefKind.RefReadOnlyParameter = 4 -> Microsoft.CodeAnalysis.RefKind
static Microsoft.CodeAnalysis.SeparatedSyntaxList<TNode>.explicit operator Microsoft.CodeAnalysis.SeparatedSyntaxList<TNode!>(Microsoft.CodeAnalysis.SeparatedSyntaxList<Microsoft.CodeAnalysis.SyntaxNode!> nodes) -> Microsoft.CodeAnalysis.SeparatedSyntaxList<TNode!>
static Microsoft.CodeAnalysis.SeparatedSyntaxList<TNode>.op_Implicit(Microsoft.CodeAnalysis.SeparatedSyntaxList<Microsoft.CodeAnalysis.SyntaxNode!> nodes) -> Microsoft.CodeAnalysis.SeparatedSyntaxList<TNode!>
Expand Down