Skip to content

Commit

Permalink
Merge main to main-vs-deps (#57242)
Browse files Browse the repository at this point in the history
* Addresses the first bullet point in #56843.

Prior to this change, compiler diagnostic analyzer could only be run from the IDE for entire document span. This is primarily due to lack of an analyzer API that allows the analyzer to register a span-based semantic diagnostic callback. Though we can consider designing and adding such an analyzer API, it would be purely an IDE-only analyzer action, as analyzers are never executed for a span in batch compilation mode - this would make it difficult to justify adding such an API.

With this PR, we add a workaround in the analyzer driver to allow executing compiler analyzer's semantic model action scoped to a filter span. This should speed up executing the compiler analyzer for lightbulb scenarios in the IDE, which are always scoped to current line span.

Verified that both the compiler and IDE tests added with this PR fail prior to the product changes.

**NOTE:** This change would not give us any perceivable improvement in non-async lightbulb scenario, as we are extremely likely to have at least one other analyzer that needs to run on the entire document. The performance improvement should be perceivable in async lightbulb scenario as the high priority code fixes bucket (add usings and merge conflict resolution) only run the compiler analyzer.

* Update src/Compilers/Core/Portable/DiagnosticAnalyzer/DiagnosticAnalysisContext.cs

* Add test and fix filtering by analysis scope.

* Address further feedback

* Add work item attribute to test

* Remove GetAdjustedSpanForCompilerAnalyzerAsync workaround in the IDE

* Unncessary usings

* Revert "Unncessary usings"

This reverts commit 510edb8.

* Revert "Remove GetAdjustedSpanForCompilerAnalyzerAsync workaround in the IDE"

This reverts commit 23e3310.

* Update test to account for workaround in IDE DocumentAnalysisExecutor for #1557

* Address PR feedback

* Fix VisualStudioActiveDocumentTracker to handle VSSELELEMID.SEID_WindowFrame

Fixes #57203. Issue description has more details

* Update src/Compilers/Core/Portable/DiagnosticAnalyzer/CompilationUnitCompletedEvent.cs

* Apply arcade-powered source-build patches (#55823)

* Don't include desktop artifacts that don't exist in source-build.
Source-build doesn't have these artifacts available, even when we eventually will build desktop TFMs, because Roslyn is one of the first builds in source-build.  Instead Roslyn is picking up reference packages that don't have the `lib` directory which is causing a build failure.  This disables the attempt to grab these desktop artifacts so source-build just skips them instead.

* Honor suppression on switch expression (#57204)

* Merge pull request #57227 from dotnet/dev/rigibson/update-feature-status-2

Update Language Feature Status.md

Co-authored-by: Manish Vasani <mavasani@microsoft.com>
Co-authored-by: Chris Rummel <crummel@microsoft.com>
Co-authored-by: Julien Couvreur <jcouv@users.noreply.github.com>
Co-authored-by: Rikki Gibson <rigibson@microsoft.com>
  • Loading branch information
4 people authored Oct 19, 2021
2 parents b1c1f6e + 645f405 commit 07250d0
Show file tree
Hide file tree
Showing 15 changed files with 221 additions and 42 deletions.
2 changes: 1 addition & 1 deletion docs/Language Feature Status.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ efforts behind them.
| ------- | ------ | ----- | --------- | -------- | --------- |
| [nameof(parameter)](https://github.com/dotnet/csharplang/issues/373) | main | [In Progress](https://github.com/dotnet/roslyn/issues/40524) | [jcouv](https://github.com/jcouv) | TBD | [jcouv](https://github.com/jcouv) |
| [Relax ordering of `ref` and `partial` modifiers](https://github.com/dotnet/csharplang/issues/946) | [ref-partial](https://github.com/dotnet/roslyn/tree/features/ref-partial) | In Progress | [alrz](https://github.com/alrz) | [gafter](https://github.com/gafter) | [jcouv](https://github.com/jcouv) |
| [Parameter null-checking](https://github.com/dotnet/csharplang/issues/2145) | [param-nullchecking](https://github.com/dotnet/roslyn/tree/features/param-nullchecking) | [In Progress](https://github.com/dotnet/roslyn/issues/36024) | [fayrose](https://github.com/fayrose) | [agocke](https://github.com/agocke) | [jaredpar](https://github.com/jaredpar) |
| [Parameter null-checking](https://github.com/dotnet/csharplang/issues/2145) | [param-nullchecking](https://github.com/dotnet/roslyn/tree/features/param-nullchecking) | [In Progress](https://github.com/dotnet/roslyn/issues/36024) | [RikkiGibson](https://github.com/RikkiGibson), [fayrose](https://github.com/fayrose) | [cston](https://github.com/cston), [chsienki](https://github.com/chsienki) | [jaredpar](https://github.com/jaredpar) |
| [Generic attributes](https://github.com/dotnet/csharplang/issues/124) | [generic-attributes](https://github.com/dotnet/roslyn/tree/features/generic-attributes) | [Merged into 17.0p4 (preview langver)](https://github.com/dotnet/roslyn/issues/36285) | [AviAvni](https://github.com/AviAvni) | [RikkiGibson](https://github.com/RikkiGibson), [jcouv](https://github.com/jcouv) | [mattwar](https://github.com/mattwar) |
| [Default in deconstruction](https://github.com/dotnet/roslyn/pull/25562) | [decon-default](https://github.com/dotnet/roslyn/tree/features/decon-default) | [Implemented](https://github.com/dotnet/roslyn/issues/25559) | [jcouv](https://github.com/jcouv) | [gafter](https://github.com/gafter) | [jcouv](https://github.com/jcouv) |
| [List patterns](https://github.com/dotnet/csharplang/issues/3435) | [list-patterns](https://github.com/dotnet/roslyn/tree/features/list-patterns) | [In Progress](https://github.com/dotnet/roslyn/issues/51289) | [alrz](https://github.com/alrz) | [jcouv](https://github.com/jcouv), [333fred](https://github.com/333fred) | [333fred](https://github.com/333fred) |
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Conversions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ private BoundExpression ConvertSwitchExpression(BoundUnconvertedSwitchExpression
var newSwitchArms = builder.ToImmutableAndFree();
return new BoundConvertedSwitchExpression(
source.Syntax, source.Type, targetTyped, source.Expression, newSwitchArms, source.DecisionDag,
source.DefaultLabel, source.ReportedNotExhaustive, destination, hasErrors || source.HasErrors);
source.DefaultLabel, source.ReportedNotExhaustive, destination, hasErrors || source.HasErrors).WithSuppression(source.IsSuppressed);
}

private BoundExpression CreateUserDefinedConversion(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -733,5 +733,39 @@ void M2()
_ = await compilationWithAnalyzers.GetAnalysisResultAsync(semanticModel, filterSpan: null, CancellationToken.None);
Assert.True(eventQueue.IsCompleted);
}

[Fact, WorkItem(56843, "https://github.com/dotnet/roslyn/issues/56843")]
public async Task TestCompilerAnalyzerForSpanBasedSemanticDiagnostics()
{
var source = @"
class C
{
void M1()
{
int x1 = 0; // CS0219 (unused variable)
}
}";
var compilation = CreateCompilation(source);
var syntaxTree = compilation.SyntaxTrees[0];
var semanticModel = compilation.GetSemanticModel(syntaxTree);

// Get compiler analyzer diagnostics for a span within "M1".
var localDecl = syntaxTree.GetRoot().DescendantNodes().OfType<LocalDeclarationStatementSyntax>().First();
var span = localDecl.Span;
var compilerAnalyzer = new CSharpCompilerDiagnosticAnalyzer();
var compilationWithAnalyzers = compilation.WithAnalyzers(ImmutableArray.Create<DiagnosticAnalyzer>(compilerAnalyzer), AnalyzerOptions.Empty);
var result = await compilationWithAnalyzers.GetAnalysisResultAsync(semanticModel, span, CancellationToken.None);
var diagnostics = result.SemanticDiagnostics[syntaxTree][compilerAnalyzer];
diagnostics.Verify(
// (6,13): warning CS0219: The variable 'x1' is assigned but its value is never used
// int x1 = 0; // CS0219 (unused variable)
Diagnostic(ErrorCode.WRN_UnreferencedVarAssg, "x1").WithArguments("x1").WithLocation(6, 13));

// Verify no diagnostics with a span outside the local decl
span = localDecl.GetLastToken().GetNextToken().Span;
result = await compilationWithAnalyzers.GetAnalysisResultAsync(semanticModel, span, CancellationToken.None);
var diagnosticsByAnalyzerMap = result.SemanticDiagnostics[syntaxTree];
Assert.Empty(diagnosticsByAnalyzerMap);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -153293,5 +153293,24 @@ public void M()
Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "field").WithLocation(19, 17)
);
}

[Fact, WorkItem(52143, "https://github.com/dotnet/roslyn/issues/52143")]
public void SuppressedSwitchExpression()
{
var source = @"
#nullable enable
C? nullable = null;

C c2 = (args[0].Length switch
{
0 => nullable,
_ => null
})!;

class C { }
";
var comp = CreateCompilation(source);
comp.VerifyDiagnostics();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ private static void AddDiagnostics_NoLock(
}
}

internal AnalysisResult ToAnalysisResult(ImmutableArray<DiagnosticAnalyzer> analyzers, CancellationToken cancellationToken)
internal AnalysisResult ToAnalysisResult(ImmutableArray<DiagnosticAnalyzer> analyzers, AnalysisScope analysisScope, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();

Expand All @@ -362,12 +362,13 @@ internal AnalysisResult ToAnalysisResult(ImmutableArray<DiagnosticAnalyzer> anal
ImmutableDictionary<DiagnosticAnalyzer, ImmutableArray<Diagnostic>> nonLocalDiagnostics;

var analyzersSet = analyzers.ToImmutableHashSet();
Func<Diagnostic, bool> shouldInclude = analysisScope.ShouldInclude;
lock (_gate)
{
localSyntaxDiagnostics = GetImmutable(analyzersSet, _localSyntaxDiagnosticsOpt);
localSemanticDiagnostics = GetImmutable(analyzersSet, _localSemanticDiagnosticsOpt);
localAdditionalFileDiagnostics = GetImmutable(analyzersSet, _localAdditionalFileDiagnosticsOpt);
nonLocalDiagnostics = GetImmutable(analyzersSet, _nonLocalDiagnosticsOpt);
localSyntaxDiagnostics = GetImmutable(analyzersSet, shouldInclude, _localSyntaxDiagnosticsOpt);
localSemanticDiagnostics = GetImmutable(analyzersSet, shouldInclude, _localSemanticDiagnosticsOpt);
localAdditionalFileDiagnostics = GetImmutable(analyzersSet, shouldInclude, _localAdditionalFileDiagnosticsOpt);
nonLocalDiagnostics = GetImmutable(analyzersSet, shouldInclude, _nonLocalDiagnosticsOpt);
}

cancellationToken.ThrowIfCancellationRequested();
Expand All @@ -377,6 +378,7 @@ internal AnalysisResult ToAnalysisResult(ImmutableArray<DiagnosticAnalyzer> anal

private static ImmutableDictionary<TKey, ImmutableDictionary<DiagnosticAnalyzer, ImmutableArray<Diagnostic>>> GetImmutable<TKey>(
ImmutableHashSet<DiagnosticAnalyzer> analyzers,
Func<Diagnostic, bool> shouldInclude,
Dictionary<TKey, Dictionary<DiagnosticAnalyzer, ImmutableArray<Diagnostic>.Builder>>? localDiagnosticsOpt)
where TKey : class
{
Expand All @@ -395,7 +397,11 @@ private static ImmutableDictionary<TKey, ImmutableDictionary<DiagnosticAnalyzer,
{
if (analyzers.Contains(diagnosticsByAnalyzer.Key))
{
perTreeBuilder.Add(diagnosticsByAnalyzer.Key, diagnosticsByAnalyzer.Value.ToImmutable());
var diagnostics = diagnosticsByAnalyzer.Value.Where(shouldInclude).ToImmutableArray();
if (!diagnostics.IsEmpty)
{
perTreeBuilder.Add(diagnosticsByAnalyzer.Key, diagnostics);
}
}
}

Expand All @@ -408,6 +414,7 @@ private static ImmutableDictionary<TKey, ImmutableDictionary<DiagnosticAnalyzer,

private static ImmutableDictionary<DiagnosticAnalyzer, ImmutableArray<Diagnostic>> GetImmutable(
ImmutableHashSet<DiagnosticAnalyzer> analyzers,
Func<Diagnostic, bool> shouldInclude,
Dictionary<DiagnosticAnalyzer, ImmutableArray<Diagnostic>.Builder>? nonLocalDiagnosticsOpt)
{
if (nonLocalDiagnosticsOpt == null)
Expand All @@ -420,7 +427,11 @@ private static ImmutableDictionary<DiagnosticAnalyzer, ImmutableArray<Diagnostic
{
if (analyzers.Contains(diagnosticsByAnalyzer.Key))
{
builder.Add(diagnosticsByAnalyzer.Key, diagnosticsByAnalyzer.Value.ToImmutable());
var diagnostics = diagnosticsByAnalyzer.Value.Where(shouldInclude).ToImmutableArray();
if (!diagnostics.IsEmpty)
{
builder.Add(diagnosticsByAnalyzer.Key, diagnostics);
}
}
}

Expand Down
11 changes: 10 additions & 1 deletion src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerDriver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1516,7 +1516,10 @@ private async Task OnEventProcessedCoreAsync(CompilationEvent compilationEvent,

break;

case CompilationUnitCompletedEvent compilationUnitCompletedEvent:
case CompilationUnitCompletedEvent compilationUnitCompletedEvent when !compilationUnitCompletedEvent.FilterSpan.HasValue:
// Clear the semantic model cache only if we have completed analysis for the entire compilation unit,
// i.e. the event has a null filter span. Compilation unit completed event with a non-null filter span
// indicates a synthesized event for partial analysis of the tree and we avoid clearing the semantic model cache for that case.
SemanticModelProvider.ClearCache(compilationUnitCompletedEvent.CompilationUnit, compilationUnitCompletedEvent.Compilation);
break;

Expand Down Expand Up @@ -1800,6 +1803,12 @@ private bool TryProcessCompilationUnitCompleted(CompilationUnitCompletedEvent co
continue;
}

// Only compiler analyzer supports span-based semantic model action callbacks.
if (completedEvent.FilterSpan.HasValue && !IsCompilerAnalyzer(analyzer))
{
continue;
}

// Execute actions for a given analyzer sequentially.
if (!AnalyzerExecutor.TryExecuteSemanticModelActions(semanticModelActions, analyzer, semanticModel, completedEvent, analysisScope, analysisState, isGeneratedCode))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -676,18 +676,20 @@ public bool TryExecuteSemanticModelActions(
ImmutableArray<SemanticModelAnalyzerAction> semanticModelActions,
DiagnosticAnalyzer analyzer,
SemanticModel semanticModel,
CompilationEvent compilationUnitCompletedEvent,
CompilationUnitCompletedEvent compilationUnitCompletedEvent,
AnalysisScope analysisScope,
AnalysisState? analysisState,
bool isGeneratedCode)
{
Debug.Assert(!compilationUnitCompletedEvent.FilterSpan.HasValue || _isCompilerAnalyzer!(analyzer), "Only compiler analyzer supports span-based semantic model action callbacks");

AnalyzerStateData? analyzerState = null;

try
{
if (TryStartProcessingEvent(compilationUnitCompletedEvent, analyzer, analysisScope, analysisState, out analyzerState))
{
ExecuteSemanticModelActionsCore(semanticModelActions, analyzer, semanticModel, analyzerState, isGeneratedCode);
ExecuteSemanticModelActionsCore(semanticModelActions, analyzer, semanticModel, analyzerState, analysisScope, isGeneratedCode);
analysisState?.MarkEventComplete(compilationUnitCompletedEvent, analyzer);
return true;
}
Expand All @@ -705,6 +707,7 @@ private void ExecuteSemanticModelActionsCore(
DiagnosticAnalyzer analyzer,
SemanticModel semanticModel,
AnalyzerStateData? analyzerState,
AnalysisScope analysisScope,
bool isGeneratedCode)
{
if (isGeneratedCode && _shouldSkipAnalysisOnGeneratedCode(analyzer) ||
Expand All @@ -724,7 +727,7 @@ private void ExecuteSemanticModelActionsCore(
_cancellationToken.ThrowIfCancellationRequested();

var context = new SemanticModelAnalysisContext(semanticModel, AnalyzerOptions, diagReporter.AddDiagnosticAction,
isSupportedDiagnostic, _cancellationToken);
isSupportedDiagnostic, analysisScope.FilterSpanOpt, _cancellationToken);

// Catch Exception from action.
ExecuteAndCatchIfThrows(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +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 Microsoft.CodeAnalysis.Text;

namespace Microsoft.CodeAnalysis.Diagnostics
{
internal sealed class CompilationUnitCompletedEvent : CompilationEvent
{
public CompilationUnitCompletedEvent(Compilation compilation, SyntaxTree compilationUnit) : base(compilation)
public CompilationUnitCompletedEvent(Compilation compilation, SyntaxTree compilationUnit, TextSpan? filterSpan = null)
: base(compilation)
{
this.CompilationUnit = compilationUnit;
this.FilterSpan = filterSpan;
}

public SyntaxTree CompilationUnit { get; }

/// <summary>
/// Optional filter span for a synthesized CompilationUnitCompletedEvent generated for span-based semantic diagnostic computation.
/// Such synthesized events are used primarily for performance improvements when running compiler analyzer in span-based mode in the IDE,
/// such as computing diagnostics for the lightbulb for the current line.
/// Note that such a synthesized CompilationUnitCompletedEvent with non-null FilterSpan is not a true
/// compilation unit completed event, but just a stub event to drive span-based semantic model action callbacks
/// for analyzer execution. This event will eventually be followed by a true CompilationUnitCompletedEvent
/// with null FilterSpan when the entire compilation unit has actually completed.
/// See https://github.com/dotnet/roslyn/issues/56843 for details.
/// </summary>
public TextSpan? FilterSpan { get; }

public override string ToString()
{
return "CompilationUnitCompletedEvent(" + CompilationUnit.FilePath + ")";
}
=> $"CompilationUnitCompletedEvent({CompilationUnit.FilePath}){FilterSpan}";
}
}
Loading

0 comments on commit 07250d0

Please sign in to comment.