From 35b0a04d1d99da97886986f765e01fb7e53bcc5b Mon Sep 17 00:00:00 2001 From: akhera99 Date: Fri, 17 Dec 2021 14:05:07 -0800 Subject: [PATCH 01/41] use the end of the span instead of the start to determine line position --- .../InlineDiagnostics/InlineDiagnosticsAdornmentManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core.Wpf/InlineDiagnostics/InlineDiagnosticsAdornmentManager.cs b/src/EditorFeatures/Core.Wpf/InlineDiagnostics/InlineDiagnosticsAdornmentManager.cs index 97ce478c2c1e3..929e599b576bb 100644 --- a/src/EditorFeatures/Core.Wpf/InlineDiagnostics/InlineDiagnosticsAdornmentManager.cs +++ b/src/EditorFeatures/Core.Wpf/InlineDiagnostics/InlineDiagnosticsAdornmentManager.cs @@ -189,7 +189,7 @@ protected override void AddAdornmentsToAdornmentLayer_CallOnlyOnUIThread(Normali } // Need to get the SnapshotPoint to be able to get the IWpfTextViewLine - var point = tagMappingSpan.Span.Start.GetPoint(TextView.TextSnapshot, PositionAffinity.Predecessor); + var point = tagMappingSpan.Span.End.GetPoint(TextView.TextSnapshot, PositionAffinity.Predecessor); if (point == null) { continue; From 8df82f3f44f27ab55ed3ba76d3d4b05ab6f0297d Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Fri, 17 Dec 2021 23:40:30 -0800 Subject: [PATCH 02/41] Fix leak in _hostAnalyzerStateMap Fixes [AB#1449967](https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1449967) #57648 fixed a race condition in computing the host analyzer references, but introduced a leak in _hostAnalyzerStateMap, wherein each new solution opened ends up creating a new entry in this map, even though the underlying set of host analyzer references doesn't change. This is due to the fact that each new SolutionState instance creates a new instance of HostDiagnosticAnalyzers, whose constructor creates a new map from analyzer references ID to the analyzer reference. This map is used as part of the key for _hostAnalyzerStateMap, leading to the leak. This change fixes the leak by using the underlying list of host analyzer references as part of the key for _hostAnalyzerStateMap, which is reference equals across all solution instances opened in the VS session. Verified manually that after this fix, repeatedly opening and closing a solution in VS no longer leads to unbounded growth of _hostAnalyzerStateMap. --- .../Test2/Diagnostics/DiagnosticServiceTests.vb | 3 +-- ...agnosticIncrementalAnalyzer.StateManager.HostStates.cs | 6 +++--- .../DiagnosticIncrementalAnalyzer.StateManager.cs | 4 ++-- .../Core/Portable/Diagnostics/HostDiagnosticAnalyzers.cs | 8 ++++---- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb b/src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb index 91c3d346c47ec..5a47f2cc52cb4 100644 --- a/src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb +++ b/src/EditorFeatures/Test2/Diagnostics/DiagnosticServiceTests.vb @@ -149,8 +149,7 @@ Namespace Microsoft.CodeAnalysis.Editor.Implementation.Diagnostics.UnitTests Assert.Equal(workspaceDiagnosticAnalyzer.DiagDescriptor.Id, descriptors(0).Id) ' Add an existing workspace analyzer to the project, ensure no duplicate diagnostics. - Dim duplicateProjectAnalyzersReference = hostAnalyzers.GetHostAnalyzerReferencesMap().First().Value - project = project.WithAnalyzerReferences({duplicateProjectAnalyzersReference}) + project = project.WithAnalyzerReferences(hostAnalyzers.HostAnalyzerReferences) ' Verify duplicate descriptors or diagnostics. descriptorsMap = hostAnalyzers.GetDiagnosticDescriptorsPerReference(diagnosticService.AnalyzerInfoCache, project) diff --git a/src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs b/src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs index af1fe1edd92ab..fe8be59046e32 100644 --- a/src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs +++ b/src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.HostStates.cs @@ -16,10 +16,10 @@ private partial class StateManager { public IEnumerable GetAllHostStateSets() { - var analyzerReferencesMap = _workspace.CurrentSolution.State.Analyzers.GetHostAnalyzerReferencesMap(); + var analyzerReferences = _workspace.CurrentSolution.State.Analyzers.HostAnalyzerReferences; foreach (var (key, value) in _hostAnalyzerStateMap) { - if (key.AnalyzerReferences == analyzerReferencesMap) + if (key.AnalyzerReferences == analyzerReferences) { foreach (var stateSet in value.OrderedStateSets) { @@ -31,7 +31,7 @@ public IEnumerable GetAllHostStateSets() private HostAnalyzerStateSets GetOrCreateHostStateSets(Project project, ProjectAnalyzerStateSets projectStateSets) { - var key = new HostAnalyzerStateSetKey(project.Language, project.Solution.State.Analyzers.GetHostAnalyzerReferencesMap()); + var key = new HostAnalyzerStateSetKey(project.Language, project.Solution.State.Analyzers.HostAnalyzerReferences); var hostStateSets = ImmutableInterlocked.GetOrAdd(ref _hostAnalyzerStateMap, key, CreateLanguageSpecificAnalyzerMap, project.Solution.State.Analyzers); return hostStateSets.WithExcludedAnalyzers(projectStateSets.SkippedAnalyzersInfo.SkippedAnalyzers); diff --git a/src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.cs b/src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.cs index 1cd6f957c6a29..8cd11d6a45737 100644 --- a/src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.cs +++ b/src/Features/Core/Portable/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.StateManager.cs @@ -314,14 +314,14 @@ private void VerifyProjectDiagnosticStates(IEnumerable stateSets) private readonly struct HostAnalyzerStateSetKey : IEquatable { - public HostAnalyzerStateSetKey(string language, ImmutableDictionary analyzerReferences) + public HostAnalyzerStateSetKey(string language, IReadOnlyList analyzerReferences) { Language = language; AnalyzerReferences = analyzerReferences; } public string Language { get; } - public ImmutableDictionary AnalyzerReferences { get; } + public IReadOnlyList AnalyzerReferences { get; } public bool Equals(HostAnalyzerStateSetKey other) => Language == other.Language && AnalyzerReferences == other.AnalyzerReferences; diff --git a/src/Workspaces/Core/Portable/Diagnostics/HostDiagnosticAnalyzers.cs b/src/Workspaces/Core/Portable/Diagnostics/HostDiagnosticAnalyzers.cs index ea6e57536778b..70d8c63279ca1 100644 --- a/src/Workspaces/Core/Portable/Diagnostics/HostDiagnosticAnalyzers.cs +++ b/src/Workspaces/Core/Portable/Diagnostics/HostDiagnosticAnalyzers.cs @@ -53,8 +53,9 @@ internal sealed class HostDiagnosticAnalyzers /// private readonly ConditionalWeakTable, StrongBox>> _skippedHostAnalyzers; - internal HostDiagnosticAnalyzers(IEnumerable hostAnalyzerReferences) + internal HostDiagnosticAnalyzers(IReadOnlyList hostAnalyzerReferences) { + HostAnalyzerReferences = hostAnalyzerReferences; _hostAnalyzerReferencesMap = CreateAnalyzerReferencesMap(hostAnalyzerReferences); _hostDiagnosticAnalyzersPerLanguageMap = new ConcurrentDictionary>>(concurrencyLevel: 2, capacity: 2); _lazyHostDiagnosticAnalyzersPerReferenceMap = new Lazy>>(() => CreateDiagnosticAnalyzersPerReferenceMap(_hostAnalyzerReferencesMap), isThreadSafe: true); @@ -64,10 +65,9 @@ internal HostDiagnosticAnalyzers(IEnumerable hostAnalyzerRefe } /// - /// It returns a map with as key and as value + /// List of host s /// - public ImmutableDictionary GetHostAnalyzerReferencesMap() - => _hostAnalyzerReferencesMap; + public IReadOnlyList HostAnalyzerReferences { get; } /// /// Get identity and s map for given From 55c4ba96fc6be78d6a946c465940ac8601452840 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 08:07:47 -0800 Subject: [PATCH 03/41] Remove unnecessary weakref valuessources in compilation tracker. --- ...pilationTracker.CompilationTrackerState.cs | 28 ++++++------------- .../SolutionState.CompilationTracker.cs | 10 +++---- .../WorkspaceConfigurationOptions.cs | 10 +------ 3 files changed, 15 insertions(+), 33 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs index 7da382f9dc134..53203f228348c 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs @@ -167,18 +167,9 @@ public static CompilationTrackerState Create( // DeclarationState now. We'll pass false for generatedDocumentsAreFinal because this is being called // if our referenced projects are changing, so we'll have to rerun to consume changes. return intermediateProjects.Length == 0 - ? new AllSyntaxTreesParsedState(solutionServices, compilation, generatorInfo.WithDocumentsAreFinal(false)) + ? new AllSyntaxTreesParsedState(compilation, generatorInfo.WithDocumentsAreFinal(false)) : new InProgressState(compilation, generatorInfo, compilationWithGeneratedDocuments, intermediateProjects); } - - public static ValueSource> CreateValueSource( - Compilation compilation, - SolutionServices services) - { - return services.SupportsCachingRecoverableObjects && !services.Workspace.Options.GetOption(WorkspaceConfigurationOptions.DisableCompilationTrackerWeakCompilationReferences) - ? new WeakValueSource(compilation) - : new ConstantValueSource>(compilation); - } } /// @@ -236,10 +227,9 @@ public InProgressState( private sealed class AllSyntaxTreesParsedState : CompilationTrackerState { public AllSyntaxTreesParsedState( - SolutionServices solutionServices, Compilation declarationCompilation, CompilationTrackerGeneratorInfo generatorInfo) - : base(CreateValueSource(declarationCompilation, solutionServices), + : base(new ConstantValueSource>(declarationCompilation), generatorInfo) { } @@ -277,24 +267,24 @@ private sealed class FinalState : CompilationTrackerState public override ValueSource> FinalCompilationWithGeneratedDocuments { get; } private FinalState( - ValueSource> finalCompilationSource, - ValueSource> compilationWithoutGeneratedFilesSource, + Compilation finalCompilationSource, + Compilation compilationWithoutGeneratedFilesSource, Compilation compilationWithoutGeneratedFiles, bool hasSuccessfullyLoaded, CompilationTrackerGeneratorInfo generatorInfo, UnrootedSymbolSet unrootedSymbolSet) - : base(compilationWithoutGeneratedFilesSource, + : base(new ConstantValueSource>(compilationWithoutGeneratedFilesSource), generatorInfo.WithDocumentsAreFinal(true)) // when we're in a final state, we've ran generators and should not run again { HasSuccessfullyLoaded = hasSuccessfullyLoaded; - FinalCompilationWithGeneratedDocuments = finalCompilationSource; + FinalCompilationWithGeneratedDocuments = new ConstantValueSource>(finalCompilationSource); UnrootedSymbolSet = unrootedSymbolSet; if (this.GeneratorInfo.Documents.IsEmpty) { // In this case, the finalCompilationSource and compilationWithoutGeneratedFilesSource should point to the // same Compilation, which should be compilationWithoutGeneratedFiles itself - Debug.Assert(finalCompilationSource.TryGetValue(out var finalCompilationVal)); + Debug.Assert(FinalCompilationWithGeneratedDocuments.TryGetValue(out var finalCompilationVal)); Debug.Assert(object.ReferenceEquals(finalCompilationVal.Value, compilationWithoutGeneratedFiles)); } } @@ -303,8 +293,8 @@ private FinalState( /// Not held onto /// Not held onto public static FinalState Create( - ValueSource> finalCompilationSource, - ValueSource> compilationWithoutGeneratedFilesSource, + Compilation finalCompilationSource, + Compilation compilationWithoutGeneratedFilesSource, Compilation compilationWithoutGeneratedFiles, bool hasSuccessfullyLoaded, CompilationTrackerGeneratorInfo generatorInfo, diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs index 2597b6e1883af..e6306074b3bdc 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs @@ -205,8 +205,8 @@ public ICompilationTracker FreezePartialStateWithTree(SolutionState solution, Do // have the compilation immediately disappear. So we force it to stay around with a ConstantValueSource. // As a policy, all partial-state projects are said to have incomplete references, since the state has no guarantees. var finalState = FinalState.Create( - finalCompilationSource: new ConstantValueSource>(compilationPair.CompilationWithGeneratedDocuments), - compilationWithoutGeneratedFilesSource: new ConstantValueSource>(compilationPair.CompilationWithoutGeneratedDocuments), + finalCompilationSource: compilationPair.CompilationWithGeneratedDocuments, + compilationWithoutGeneratedFilesSource: compilationPair.CompilationWithoutGeneratedDocuments, compilationWithoutGeneratedFiles: compilationPair.CompilationWithoutGeneratedDocuments, hasSuccessfullyLoaded: false, generatorInfo, @@ -581,7 +581,7 @@ private async Task BuildDeclarationCompilationFromScratchAsync( } compilation = compilation.AddSyntaxTrees(trees); - WriteState(new AllSyntaxTreesParsedState(solutionServices, compilation, generatorInfo), solutionServices); + WriteState(new AllSyntaxTreesParsedState(compilation, generatorInfo), solutionServices); return compilation; } catch (Exception e) when (FatalError.ReportAndPropagateUnlessCanceled(e, cancellationToken)) @@ -922,8 +922,8 @@ private async Task FinalizeCompilationAsync( } var finalState = FinalState.Create( - CompilationTrackerState.CreateValueSource(compilationWithGenerators, solution.Services), - CompilationTrackerState.CreateValueSource(compilationWithoutGenerators, solution.Services), + compilationWithGenerators, + compilationWithoutGenerators, compilationWithoutGenerators, hasSuccessfullyLoaded, generatorInfo, diff --git a/src/Workspaces/Core/Portable/Workspace/WorkspaceConfigurationOptions.cs b/src/Workspaces/Core/Portable/Workspace/WorkspaceConfigurationOptions.cs index a865ce4a4c85d..d70cef1949bfb 100644 --- a/src/Workspaces/Core/Portable/Workspace/WorkspaceConfigurationOptions.cs +++ b/src/Workspaces/Core/Portable/Workspace/WorkspaceConfigurationOptions.cs @@ -26,17 +26,9 @@ internal class WorkspaceConfigurationOptions : IOptionProvider nameof(WorkspaceConfigurationOptions), nameof(DisableProjectCacheService), defaultValue: false, new FeatureFlagStorageLocation("Roslyn.DisableProjectCacheService")); - /// - /// Disables holding onto the assembly references for runtime (not user/nuget/etc.) dlls weakly. - /// - public static readonly Option DisableCompilationTrackerWeakCompilationReferences = new( - nameof(WorkspaceConfigurationOptions), nameof(DisableCompilationTrackerWeakCompilationReferences), defaultValue: false, - new FeatureFlagStorageLocation("Roslyn.DisableCompilationTrackerWeakCompilationReferences")); - ImmutableArray IOptionProvider.Options { get; } = ImmutableArray.Create( DisableRecoverableTrees, - DisableProjectCacheService, - DisableCompilationTrackerWeakCompilationReferences); + DisableProjectCacheService); [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] From 36beb71827cb58769525e1aacda35adc8966c39e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 08:12:50 -0800 Subject: [PATCH 04/41] REmove value-source as everything in constant --- ...pilationTracker.CompilationTrackerState.cs | 24 +++++++------------ .../SolutionState.CompilationTracker.cs | 20 +++++++--------- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs index 53203f228348c..ea113b4c82ab5 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs @@ -109,12 +109,10 @@ private abstract class CompilationTrackerState documentsAreFinal: false)); /// - /// The best compilation that is available that source generators have not ran on. May be an in-progress, - /// full declaration, a final compilation, or . - /// The value is an to represent the - /// possibility of the compilation already having been garabage collected. + /// The best compilation that is available that source generators have not ran on. May be an + /// in-progress, full declaration, a final compilation, or . /// - public ValueSource>? CompilationWithoutGeneratedDocuments { get; } + public Compilation? CompilationWithoutGeneratedDocuments { get; } public CompilationTrackerGeneratorInfo GeneratorInfo { get; } @@ -132,7 +130,7 @@ private abstract class CompilationTrackerState public virtual ValueSource>? FinalCompilationWithGeneratedDocuments => null; protected CompilationTrackerState( - ValueSource>? compilationWithoutGeneratedDocuments, + Compilation? compilationWithoutGeneratedDocuments, CompilationTrackerGeneratorInfo generatorInfo) { CompilationWithoutGeneratedDocuments = compilationWithoutGeneratedDocuments; @@ -155,7 +153,6 @@ protected CompilationTrackerState( } public static CompilationTrackerState Create( - SolutionServices solutionServices, Compilation compilation, CompilationTrackerGeneratorInfo generatorInfo, Compilation? compilationWithGeneratedDocuments, @@ -209,7 +206,7 @@ public InProgressState( CompilationTrackerGeneratorInfo generatorInfo, Compilation? compilationWithGeneratedDocuments, ImmutableArray<(ProjectState state, CompilationAndGeneratorDriverTranslationAction action)> intermediateProjects) - : base(compilationWithoutGeneratedDocuments: new ConstantValueSource>(inProgressCompilation), + : base(compilationWithoutGeneratedDocuments: inProgressCompilation, generatorInfo.WithDocumentsAreFinal(false)) // since we have a set of transformations to make, we'll always have to run generators again { Contract.ThrowIfTrue(intermediateProjects.IsDefault); @@ -226,11 +223,8 @@ public InProgressState( /// private sealed class AllSyntaxTreesParsedState : CompilationTrackerState { - public AllSyntaxTreesParsedState( - Compilation declarationCompilation, - CompilationTrackerGeneratorInfo generatorInfo) - : base(new ConstantValueSource>(declarationCompilation), - generatorInfo) + public AllSyntaxTreesParsedState(Compilation declarationCompilation, CompilationTrackerGeneratorInfo generatorInfo) + : base(declarationCompilation, generatorInfo) { } } @@ -273,8 +267,8 @@ private FinalState( bool hasSuccessfullyLoaded, CompilationTrackerGeneratorInfo generatorInfo, UnrootedSymbolSet unrootedSymbolSet) - : base(new ConstantValueSource>(compilationWithoutGeneratedFilesSource), - generatorInfo.WithDocumentsAreFinal(true)) // when we're in a final state, we've ran generators and should not run again + : base(compilationWithoutGeneratedFilesSource, + generatorInfo.WithDocumentsAreFinal(true)) // when we're in a final state, we've ran generators and should not run again { HasSuccessfullyLoaded = hasSuccessfullyLoaded; FinalCompilationWithGeneratedDocuments = new ConstantValueSource>(finalCompilationSource); diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs index e6306074b3bdc..1c2460399a0ae 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs @@ -76,7 +76,7 @@ private void WriteState(CompilationTrackerState state, SolutionServices solution { // Allow the cache service to create a strong reference to the compilation. We'll get the "furthest along" compilation we have // and hold onto that. - var compilationToCache = state.FinalCompilationWithGeneratedDocuments?.GetValueOrNull() ?? state.CompilationWithoutGeneratedDocuments?.GetValueOrNull(); + var compilationToCache = state.FinalCompilationWithGeneratedDocuments?.GetValueOrNull() ?? state.CompilationWithoutGeneratedDocuments; solutionServices.CacheService.CacheObjectIfCachingEnabledForKey(ProjectState.Id, state, compilationToCache); } @@ -122,7 +122,7 @@ public ICompilationTracker Fork( { var state = ReadState(); - var baseCompilation = state.CompilationWithoutGeneratedDocuments?.GetValueOrNull(cancellationToken); + var baseCompilation = state.CompilationWithoutGeneratedDocuments; if (baseCompilation != null) { var intermediateProjects = state is InProgressState inProgressState @@ -154,7 +154,7 @@ public ICompilationTracker Fork( } var newState = CompilationTrackerState.Create( - solutionServices, baseCompilation, state.GeneratorInfo, state.FinalCompilationWithGeneratedDocuments?.GetValueOrNull(cancellationToken), intermediateProjects); + baseCompilation, state.GeneratorInfo, state.FinalCompilationWithGeneratedDocuments?.GetValueOrNull(cancellationToken), intermediateProjects); return new CompilationTracker(newProject, newState, this.SkeletonReferenceCache.Clone()); } @@ -236,7 +236,7 @@ private void GetPartialCompilationState( CancellationToken cancellationToken) { var state = ReadState(); - var compilationWithoutGeneratedDocuments = state.CompilationWithoutGeneratedDocuments?.GetValueOrNull(cancellationToken); + var compilationWithoutGeneratedDocuments = state.CompilationWithoutGeneratedDocuments; // check whether we can bail out quickly for typing case var inProgressState = state as InProgressState; @@ -414,7 +414,7 @@ private async Task GetOrBuildDeclarationCompilationAsync(SolutionSe return compilation; } - compilation = state.CompilationWithoutGeneratedDocuments?.GetValueOrNull(cancellationToken); + compilation = state.CompilationWithoutGeneratedDocuments; if (compilation == null) { // We've got nothing. Build it from scratch :( @@ -506,7 +506,7 @@ private async Task BuildCompilationInfoAsync( return new CompilationInfo(compilation, state.HasSuccessfullyLoaded.Value, state.GeneratorInfo.Documents); } - compilation = state.CompilationWithoutGeneratedDocuments?.GetValueOrNull(cancellationToken); + compilation = state.CompilationWithoutGeneratedDocuments; if (compilation == null) { @@ -682,7 +682,7 @@ private async Task BuildFinalStateFromInProgressStateAsync( intermediateProjects = intermediateProjects.RemoveAt(0); this.WriteState(CompilationTrackerState.Create( - solutionServices, compilationWithoutGenerators, state.GeneratorInfo.WithDriver(generatorDriver), compilationWithGenerators, intermediateProjects), solutionServices); + compilationWithoutGenerators, state.GeneratorInfo.WithDriver(generatorDriver), compilationWithGenerators, intermediateProjects), solutionServices); } return (compilationWithoutGenerators, compilationWithGenerators, generatorDriver); @@ -989,13 +989,11 @@ private async Task FinalizeCompilationAsync( var state = ReadState(); // get compilation in any state it happens to be in right now. - if (state.CompilationWithoutGeneratedDocuments != null && - state.CompilationWithoutGeneratedDocuments.TryGetValue(out var compilationOpt) && - compilationOpt.HasValue && + if (state.CompilationWithoutGeneratedDocuments is { } compilationOpt && ProjectState.LanguageServices == fromProject.LanguageServices) { // if we have a compilation and its the correct language, use a simple compilation reference - return compilationOpt.Value.ToMetadataReference(projectReference.Aliases, projectReference.EmbedInteropTypes); + return compilationOpt.ToMetadataReference(projectReference.Aliases, projectReference.EmbedInteropTypes); } return null; From f62f5740201232cf60b456a27181d1180bc54331 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Wed, 15 Dec 2021 17:44:42 -0800 Subject: [PATCH 05/41] Remove unnecessary assertion that CancellationSeries is disposed This type functions correctly whether or not it is explicitly disposed. --- .../Portable/Utilities/CancellationSeries.cs | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/src/Workspaces/Core/Portable/Utilities/CancellationSeries.cs b/src/Workspaces/Core/Portable/Utilities/CancellationSeries.cs index 0dd0e7e17714c..f5072e2033b34 100644 --- a/src/Workspaces/Core/Portable/Utilities/CancellationSeries.cs +++ b/src/Workspaces/Core/Portable/Utilities/CancellationSeries.cs @@ -9,9 +9,6 @@ // reference implementation. using System; -#if DEBUG -using System.Diagnostics; -#endif using System.Threading; namespace Roslyn.Utilities @@ -42,22 +39,7 @@ public CancellationSeries(CancellationToken token = default) _cts.Cancel(); _superToken = token; - -#if DEBUG - _ctorStack = new StackTrace(); -#endif - } - -#if DEBUG - private readonly StackTrace _ctorStack; - - ~CancellationSeries() - { - Contract.ThrowIfFalse( - Environment.HasShutdownStarted || _cts == null, - $"Instance of CancellationSeries not disposed before being finalized{Environment.NewLine}Stack at construction:{Environment.NewLine}{_ctorStack}"); } -#endif /// /// Determines if the cancellation series has an active token which has not been cancelled. @@ -131,10 +113,6 @@ public CancellationToken CreateNext(CancellationToken token = default) public void Dispose() { -#if DEBUG - GC.SuppressFinalize(this); -#endif - var source = Interlocked.Exchange(ref _cts, null); if (source == null) From 999ff1ea085adde8dbade01d98ca351b4f2703e9 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 08:25:33 -0800 Subject: [PATCH 06/41] Inline another value always known to be a constnat value source. --- ...pilationTracker.CompilationTrackerState.cs | 22 +++++----- .../SolutionState.CompilationTracker.cs | 44 ++++++++----------- 2 files changed, 28 insertions(+), 38 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs index ea113b4c82ab5..4f1747a68f027 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs @@ -124,10 +124,8 @@ private abstract class CompilationTrackerState /// /// The final compilation is potentially available, otherwise . - /// The value is an to represent the - /// possibility of the compilation already having been garabage collected. /// - public virtual ValueSource>? FinalCompilationWithGeneratedDocuments => null; + public virtual Compilation? FinalCompilationWithGeneratedDocuments => null; protected CompilationTrackerState( Compilation? compilationWithoutGeneratedDocuments, @@ -252,13 +250,14 @@ private sealed class FinalState : CompilationTrackerState public readonly UnrootedSymbolSet UnrootedSymbolSet; /// - /// The final compilation, with all references and source generators run. This is distinct from - /// , which in the case will be the compilation - /// before any source generators were ran. This ensures that a later invocation of the source generators - /// consumes which will avoid generators being ran a second time on a compilation that - /// already contains the output of other generators. If source generators are not active, this is equal to . + /// The final compilation, with all references and source generators run. This is distinct from , which in the case will be the compilation before any + /// source generators were ran. This ensures that a later invocation of the source generators consumes + /// which will avoid generators being ran a second time on a compilation that + /// already contains the output of other generators. If source generators are not active, this is equal + /// to . /// - public override ValueSource> FinalCompilationWithGeneratedDocuments { get; } + public override Compilation FinalCompilationWithGeneratedDocuments { get; } private FinalState( Compilation finalCompilationSource, @@ -271,15 +270,14 @@ private FinalState( generatorInfo.WithDocumentsAreFinal(true)) // when we're in a final state, we've ran generators and should not run again { HasSuccessfullyLoaded = hasSuccessfullyLoaded; - FinalCompilationWithGeneratedDocuments = new ConstantValueSource>(finalCompilationSource); + FinalCompilationWithGeneratedDocuments = finalCompilationSource; UnrootedSymbolSet = unrootedSymbolSet; if (this.GeneratorInfo.Documents.IsEmpty) { // In this case, the finalCompilationSource and compilationWithoutGeneratedFilesSource should point to the // same Compilation, which should be compilationWithoutGeneratedFiles itself - Debug.Assert(FinalCompilationWithGeneratedDocuments.TryGetValue(out var finalCompilationVal)); - Debug.Assert(object.ReferenceEquals(finalCompilationVal.Value, compilationWithoutGeneratedFiles)); + Debug.Assert(object.ReferenceEquals(finalCompilationSource, compilationWithoutGeneratedFiles)); } } diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs index 1c2460399a0ae..a6ea2170d53f9 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs @@ -76,7 +76,7 @@ private void WriteState(CompilationTrackerState state, SolutionServices solution { // Allow the cache service to create a strong reference to the compilation. We'll get the "furthest along" compilation we have // and hold onto that. - var compilationToCache = state.FinalCompilationWithGeneratedDocuments?.GetValueOrNull() ?? state.CompilationWithoutGeneratedDocuments; + var compilationToCache = state.FinalCompilationWithGeneratedDocuments ?? state.CompilationWithoutGeneratedDocuments; solutionServices.CacheService.CacheObjectIfCachingEnabledForKey(ProjectState.Id, state, compilationToCache); } @@ -154,7 +154,7 @@ public ICompilationTracker Fork( } var newState = CompilationTrackerState.Create( - baseCompilation, state.GeneratorInfo, state.FinalCompilationWithGeneratedDocuments?.GetValueOrNull(cancellationToken), intermediateProjects); + baseCompilation, state.GeneratorInfo, state.FinalCompilationWithGeneratedDocuments, intermediateProjects); return new CompilationTracker(newProject, newState, this.SkeletonReferenceCache.Clone()); } @@ -269,20 +269,18 @@ private void GetPartialCompilationState( // if we already have a final compilation we are done. if (compilationWithoutGeneratedDocuments != null && state is FinalState finalState) { - var finalCompilation = finalState.FinalCompilationWithGeneratedDocuments.GetValueOrNull(cancellationToken); + var finalCompilation = finalState.FinalCompilationWithGeneratedDocuments; + Contract.ThrowIfNull(finalCompilation, "We have a FinalState, so we must have a non-null final compilation"); - if (finalCompilation != null) - { - compilations = new CompilationPair(compilationWithoutGeneratedDocuments, finalCompilation); - - // This should hopefully be safe to return as null. Because we already reached the 'FinalState' - // before, we should have already recorded the assembly symbols for it. So not recording them - // again is likely ok (as long as compilations continue to return the same IAssemblySymbols for - // the same references across source edits). - metadataReferenceToProjectId = null; - SolutionLogger.UseExistingFullProjectState(); - return; - } + compilations = new CompilationPair(compilationWithoutGeneratedDocuments, finalCompilation); + + // This should hopefully be safe to return as null. Because we already reached the 'FinalState' + // before, we should have already recorded the assembly symbols for it. So not recording them + // again is likely ok (as long as compilations continue to return the same IAssemblySymbols for + // the same references across source edits). + metadataReferenceToProjectId = null; + SolutionLogger.UseExistingFullProjectState(); + return; } // 1) if we have an in-progress compilation use it. @@ -365,14 +363,8 @@ private static bool IsTouchDocumentActionForDocument(CompilationAndGeneratorDriv public bool TryGetCompilation([NotNullWhen(true)] out Compilation? compilation) { var state = ReadState(); - if (state.FinalCompilationWithGeneratedDocuments != null && state.FinalCompilationWithGeneratedDocuments.TryGetValue(out var compilationOpt) && compilationOpt.HasValue) - { - compilation = compilationOpt.Value; - return true; - } - - compilation = null; - return false; + compilation = state.FinalCompilationWithGeneratedDocuments; + return compilation != null; } public Task GetCompilationAsync(SolutionState solution, CancellationToken cancellationToken) @@ -408,7 +400,7 @@ private async Task GetOrBuildDeclarationCompilationAsync(SolutionSe var state = ReadState(); // we are already in the final stage. just return it. - var compilation = state.FinalCompilationWithGeneratedDocuments?.GetValueOrNull(cancellationToken); + var compilation = state.FinalCompilationWithGeneratedDocuments; if (compilation != null) { return compilation; @@ -457,7 +449,7 @@ private async Task GetOrBuildCompilationInfoAsync( var state = ReadState(); // Try to get the built compilation. If it exists, then we can just return that. - var finalCompilation = state.FinalCompilationWithGeneratedDocuments?.GetValueOrNull(cancellationToken); + var finalCompilation = state.FinalCompilationWithGeneratedDocuments; if (finalCompilation != null) { RoslynDebug.Assert(state.HasSuccessfullyLoaded.HasValue); @@ -499,7 +491,7 @@ private async Task BuildCompilationInfoAsync( // if we already have a compilation, we must be already done! This can happen if two // threads were waiting to build, and we came in after the other succeeded. - var compilation = state.FinalCompilationWithGeneratedDocuments?.GetValueOrNull(cancellationToken); + var compilation = state.FinalCompilationWithGeneratedDocuments; if (compilation != null) { RoslynDebug.Assert(state.HasSuccessfullyLoaded.HasValue); From 24bd3ebfe8471ea13e3e560665118fa0a806c312 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 08:26:08 -0800 Subject: [PATCH 07/41] Add contract --- ...tionState.CompilationTracker.CompilationTrackerState.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs index 4f1747a68f027..679d2f109c71a 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs @@ -260,7 +260,7 @@ private sealed class FinalState : CompilationTrackerState public override Compilation FinalCompilationWithGeneratedDocuments { get; } private FinalState( - Compilation finalCompilationSource, + Compilation finalCompilation, Compilation compilationWithoutGeneratedFilesSource, Compilation compilationWithoutGeneratedFiles, bool hasSuccessfullyLoaded, @@ -269,15 +269,16 @@ private FinalState( : base(compilationWithoutGeneratedFilesSource, generatorInfo.WithDocumentsAreFinal(true)) // when we're in a final state, we've ran generators and should not run again { + Contract.ThrowIfNull(finalCompilation); HasSuccessfullyLoaded = hasSuccessfullyLoaded; - FinalCompilationWithGeneratedDocuments = finalCompilationSource; + FinalCompilationWithGeneratedDocuments = finalCompilation; UnrootedSymbolSet = unrootedSymbolSet; if (this.GeneratorInfo.Documents.IsEmpty) { // In this case, the finalCompilationSource and compilationWithoutGeneratedFilesSource should point to the // same Compilation, which should be compilationWithoutGeneratedFiles itself - Debug.Assert(object.ReferenceEquals(finalCompilationSource, compilationWithoutGeneratedFiles)); + Debug.Assert(object.ReferenceEquals(finalCompilation, compilationWithoutGeneratedFiles)); } } From 8316b81bae7384e6d24decf502af2383ffa36815 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 08:28:34 -0800 Subject: [PATCH 08/41] Remove specific ValueSource subclass that is no longer used --- .../Core/CompilerExtensions.projitems | 1 - .../ValuesSources/WeakValueSource.cs | 48 ------------------- 2 files changed, 49 deletions(-) delete mode 100644 src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/ValuesSources/WeakValueSource.cs diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems index 896e22e8878b3..f079edaaf9ae6 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems @@ -512,7 +512,6 @@ - diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/ValuesSources/WeakValueSource.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/ValuesSources/WeakValueSource.cs deleted file mode 100644 index 530ea06680e17..0000000000000 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/ValuesSources/WeakValueSource.cs +++ /dev/null @@ -1,48 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// 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.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis; - -namespace Roslyn.Utilities -{ - /// - /// A that keeps a weak reference to a value. - /// - internal sealed class WeakValueSource : ValueSource> - where T : class - { - private readonly WeakReference _weakValue; - - public WeakValueSource(T value) - => _weakValue = new WeakReference(value); - - public override bool TryGetValue(out Optional value) - { - if (_weakValue.TryGetTarget(out var target)) - { - value = target; - return true; - } - - value = default; - return false; - } - - public override Optional GetValue(CancellationToken cancellationToken) - { - if (_weakValue.TryGetTarget(out var target)) - { - return target; - } - - return default; - } - - public override Task> GetValueAsync(CancellationToken cancellationToken) - => Task.FromResult(GetValue(cancellationToken)); - } -} From d19870c2c9d38573eaabc51cd82018ad47986cd7 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Mon, 20 Dec 2021 08:19:11 -0800 Subject: [PATCH 09/41] Add initial test for inline diagnostics tagger --- .../InlineDiagnosticsTaggerProviderTests.cs | 47 +++++++++++++++++++ .../Squiggles/ErrorSquiggleProducerTests.cs | 20 ++++---- .../SuggestionTagProducerTests.cs | 2 +- .../Test/Preview/PreviewWorkspaceTests.cs | 2 +- .../Diagnostics/DiagnosticTaggerWrapper.cs | 4 +- .../Squiggles/SquiggleUtilities.cs | 12 +++-- .../Squiggles/TestDiagnosticTagProducer.cs | 15 +++--- .../Squiggles/ErrorSquiggleProducerTests.vb | 6 +-- 8 files changed, 81 insertions(+), 27 deletions(-) create mode 100644 src/EditorFeatures/CSharpTest/InlineDiagnostics/InlineDiagnosticsTaggerProviderTests.cs diff --git a/src/EditorFeatures/CSharpTest/InlineDiagnostics/InlineDiagnosticsTaggerProviderTests.cs b/src/EditorFeatures/CSharpTest/InlineDiagnostics/InlineDiagnosticsTaggerProviderTests.cs new file mode 100644 index 0000000000000..fcbb5b673911a --- /dev/null +++ b/src/EditorFeatures/CSharpTest/InlineDiagnostics/InlineDiagnosticsTaggerProviderTests.cs @@ -0,0 +1,47 @@ +// Licensed to the .NET Foundation under one or more agreements. +// 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.Collections.Immutable; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Editor.InlineDiagnostics; +using Microsoft.CodeAnalysis.Editor.UnitTests.Extensions; +using Microsoft.CodeAnalysis.Editor.UnitTests.Squiggles; +using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces; +using Microsoft.CodeAnalysis.Options; +using Microsoft.CodeAnalysis.Test.Utilities; +using Microsoft.VisualStudio.Text.Adornments; +using Microsoft.VisualStudio.Text.Tagging; +using Roslyn.Test.Utilities; +using Roslyn.Utilities; +using Xunit; + +namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.InlineDiagnostics +{ + [UseExportProvider] + public class InlineDiagnosticsTaggerProviderTests + { + [WpfFact, Trait(Traits.Feature, Traits.Features.ErrorSquiggles)] + public async Task ErrorTagGeneratedForError() + { + var spans = await GetTagSpansAsync("class C {"); + Assert.Equal(1, spans.Count()); + + var firstSpan = spans.First(); + Assert.Equal(PredefinedErrorTypeNames.SyntaxError, firstSpan.Tag.ErrorType); + } + + private static async Task>> GetTagSpansAsync(string content) + { + using var workspace = TestWorkspace.CreateCSharp(content, composition: SquiggleUtilities.WpfCompositionWithSolutionCrawler); + return await GetTagSpansAsync(workspace); + } + + private static async Task>> GetTagSpansAsync(TestWorkspace workspace) + { + workspace.ApplyOptions(new[] { KeyValuePairUtil.Create(new OptionKey2(InlineDiagnosticsOptions.EnableInlineDiagnostics, LanguageNames.CSharp), (object)true) }); + return (await TestDiagnosticTagProducer.GetDiagnosticsAndErrorSpans(workspace)).Item2; + } + } +} diff --git a/src/EditorFeatures/CSharpTest/Squiggles/ErrorSquiggleProducerTests.cs b/src/EditorFeatures/CSharpTest/Squiggles/ErrorSquiggleProducerTests.cs index cbdbef1636087..599213a3cc41b 100644 --- a/src/EditorFeatures/CSharpTest/Squiggles/ErrorSquiggleProducerTests.cs +++ b/src/EditorFeatures/CSharpTest/Squiggles/ErrorSquiggleProducerTests.cs @@ -75,7 +75,7 @@ void Test() "; using var workspace = TestWorkspace.Create(workspaceXml); - var spans = (await TestDiagnosticTagProducer.GetDiagnosticsAndErrorSpans(workspace)).Item2; + var spans = (await TestDiagnosticTagProducer.GetDiagnosticsAndErrorSpans(workspace)).Item2; Assert.Equal(1, spans.Count()); Assert.Equal(PredefinedErrorTypeNames.SyntaxError, spans.First().Tag.ErrorType); @@ -126,7 +126,7 @@ void Test() } }; - var diagnosticsAndSpans = await TestDiagnosticTagProducer.GetDiagnosticsAndErrorSpans(workspace, analyzerMap); + var diagnosticsAndSpans = await TestDiagnosticTagProducer.GetDiagnosticsAndErrorSpans(workspace, analyzerMap); var spans = diagnosticsAndSpans.Item1 @@ -204,7 +204,7 @@ public async Task SemanticErrorReported() { using var workspace = TestWorkspace.CreateCSharp("class C : Bar { }", composition: SquiggleUtilities.CompositionWithSolutionCrawler); - var spans = await TestDiagnosticTagProducer.GetDiagnosticsAndErrorSpans(workspace); + var spans = await TestDiagnosticTagProducer.GetDiagnosticsAndErrorSpans(workspace); Assert.Equal(1, spans.Item2.Count()); @@ -296,10 +296,10 @@ class Test var updateArgs = DiagnosticsUpdatedArgs.DiagnosticsCreated( new object(), workspace, workspace.CurrentSolution, document.Project.Id, document.Id, ImmutableArray.Create( - TestDiagnosticTagProducer.CreateDiagnosticData(document, new TextSpan(0, 0)), - TestDiagnosticTagProducer.CreateDiagnosticData(document, new TextSpan(0, 1)))); + TestDiagnosticTagProducer.CreateDiagnosticData(document, new TextSpan(0, 0)), + TestDiagnosticTagProducer.CreateDiagnosticData(document, new TextSpan(0, 1)))); - var spans = await TestDiagnosticTagProducer.GetErrorsFromUpdateSource(workspace, updateArgs); + var spans = await TestDiagnosticTagProducer.GetErrorsFromUpdateSource(workspace, updateArgs); Assert.Equal(1, spans.Count()); var first = spans.First(); @@ -327,10 +327,10 @@ class Test var updateArgs = DiagnosticsUpdatedArgs.DiagnosticsCreated( new LiveId(), workspace, workspace.CurrentSolution, document.Project.Id, document.Id, ImmutableArray.Create( - TestDiagnosticTagProducer.CreateDiagnosticData(document, new TextSpan(0, 0)), - TestDiagnosticTagProducer.CreateDiagnosticData(document, new TextSpan(0, 1)))); + TestDiagnosticTagProducer.CreateDiagnosticData(document, new TextSpan(0, 0)), + TestDiagnosticTagProducer.CreateDiagnosticData(document, new TextSpan(0, 1)))); - var spans = await TestDiagnosticTagProducer.GetErrorsFromUpdateSource(workspace, updateArgs); + var spans = await TestDiagnosticTagProducer.GetErrorsFromUpdateSource(workspace, updateArgs); Assert.Equal(2, spans.Count()); var first = spans.First(); @@ -350,7 +350,7 @@ public LiveId() private static async Task>> GetTagSpansAsync(string content) { using var workspace = TestWorkspace.CreateCSharp(content, composition: SquiggleUtilities.CompositionWithSolutionCrawler); - return (await TestDiagnosticTagProducer.GetDiagnosticsAndErrorSpans(workspace)).Item2; + return (await TestDiagnosticTagProducer.GetDiagnosticsAndErrorSpans(workspace)).Item2; } private sealed class ReportOnClassWithLink : DiagnosticAnalyzer diff --git a/src/EditorFeatures/CSharpTest/SuggestionTags/SuggestionTagProducerTests.cs b/src/EditorFeatures/CSharpTest/SuggestionTags/SuggestionTagProducerTests.cs index 7d236a28c1f68..60a800bee0936 100644 --- a/src/EditorFeatures/CSharpTest/SuggestionTags/SuggestionTagProducerTests.cs +++ b/src/EditorFeatures/CSharpTest/SuggestionTags/SuggestionTagProducerTests.cs @@ -47,7 +47,7 @@ void M() { { LanguageNames.CSharp, ImmutableArray.Create(new CSharpUseObjectInitializerDiagnosticAnalyzer()) } }; - var spans = (await TestDiagnosticTagProducer.GetDiagnosticsAndErrorSpans(workspace, analyzerMap)).Item2; + var spans = (await TestDiagnosticTagProducer.GetDiagnosticsAndErrorSpans(workspace, analyzerMap)).Item2; return (spans, workspace.Documents.Single().SelectedSpans.Single()); } } diff --git a/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs b/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs index a089e84008738..9ad9af6c9bc95 100644 --- a/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs +++ b/src/EditorFeatures/Test/Preview/PreviewWorkspaceTests.cs @@ -185,7 +185,7 @@ public async Task TestPreviewDiagnosticTagger() // enable preview diagnostics previewWorkspace.EnableDiagnostic(); - var diagnosticsAndErrorsSpans = await SquiggleUtilities.GetDiagnosticsAndErrorSpansAsync(workspace); + var diagnosticsAndErrorsSpans = await SquiggleUtilities.GetDiagnosticsAndErrorSpansAsync(workspace); const string AnalyzerCount = "Analyzer Count: "; Assert.Equal(AnalyzerCount + 1, AnalyzerCount + diagnosticsAndErrorsSpans.Item1.Length); diff --git a/src/EditorFeatures/TestUtilities/Diagnostics/DiagnosticTaggerWrapper.cs b/src/EditorFeatures/TestUtilities/Diagnostics/DiagnosticTaggerWrapper.cs index 9b228efd368af..bd97c0ed0d506 100644 --- a/src/EditorFeatures/TestUtilities/Diagnostics/DiagnosticTaggerWrapper.cs +++ b/src/EditorFeatures/TestUtilities/Diagnostics/DiagnosticTaggerWrapper.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editor.Implementation.Diagnostics; +using Microsoft.CodeAnalysis.Editor.InlineDiagnostics; using Microsoft.CodeAnalysis.Editor.Shared.Utilities; using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces; using Microsoft.CodeAnalysis.Shared.TestHooks; @@ -76,7 +77,8 @@ public ITaggerProvider TaggerProvider if (typeof(TProvider) == typeof(DiagnosticsSquiggleTaggerProvider) || typeof(TProvider) == typeof(DiagnosticsSuggestionTaggerProvider) - || typeof(TProvider) == typeof(DiagnosticsClassificationTaggerProvider)) + || typeof(TProvider) == typeof(DiagnosticsClassificationTaggerProvider) + || typeof(TProvider) == typeof(InlineDiagnosticsTaggerProvider)) { _taggerProvider = _workspace.ExportProvider.GetExportedValues() .OfType() diff --git a/src/EditorFeatures/TestUtilities/Squiggles/SquiggleUtilities.cs b/src/EditorFeatures/TestUtilities/Squiggles/SquiggleUtilities.cs index 421c387d4c951..1df8d05f47a29 100644 --- a/src/EditorFeatures/TestUtilities/Squiggles/SquiggleUtilities.cs +++ b/src/EditorFeatures/TestUtilities/Squiggles/SquiggleUtilities.cs @@ -26,13 +26,17 @@ public static class SquiggleUtilities internal static TestComposition CompositionWithSolutionCrawler = EditorTestCompositions.EditorFeatures .RemoveParts(typeof(MockWorkspaceEventListenerProvider)); - internal static async Task<(ImmutableArray, ImmutableArray>)> GetDiagnosticsAndErrorSpansAsync( + internal static TestComposition WpfCompositionWithSolutionCrawler = EditorTestCompositions.EditorFeaturesWpf + .RemoveParts(typeof(MockWorkspaceEventListenerProvider)); + + internal static async Task<(ImmutableArray, ImmutableArray>)> GetDiagnosticsAndErrorSpansAsync( TestWorkspace workspace, IReadOnlyDictionary> analyzerMap = null) - where TProvider : AbstractDiagnosticsAdornmentTaggerProvider + where TProvider : AbstractDiagnosticsAdornmentTaggerProvider + where TTag : class, ITag { - using var wrapper = new DiagnosticTaggerWrapper(workspace, analyzerMap); - var tagger = wrapper.TaggerProvider.CreateTagger(workspace.Documents.First().GetTextBuffer()); + using var wrapper = new DiagnosticTaggerWrapper(workspace, analyzerMap); + var tagger = wrapper.TaggerProvider.CreateTagger(workspace.Documents.First().GetTextBuffer()); using var disposable = tagger as IDisposable; await wrapper.WaitForTags(); diff --git a/src/EditorFeatures/TestUtilities/Squiggles/TestDiagnosticTagProducer.cs b/src/EditorFeatures/TestUtilities/Squiggles/TestDiagnosticTagProducer.cs index 18e8a2a490cdf..bcf78b9d02685 100644 --- a/src/EditorFeatures/TestUtilities/Squiggles/TestDiagnosticTagProducer.cs +++ b/src/EditorFeatures/TestUtilities/Squiggles/TestDiagnosticTagProducer.cs @@ -22,24 +22,25 @@ namespace Microsoft.CodeAnalysis.Editor.UnitTests.Squiggles { - internal sealed class TestDiagnosticTagProducer - where TProvider : AbstractDiagnosticsAdornmentTaggerProvider + internal sealed class TestDiagnosticTagProducer + where TProvider : AbstractDiagnosticsAdornmentTaggerProvider + where TTag : class, ITag { - internal static Task<(ImmutableArray, ImmutableArray>)> GetDiagnosticsAndErrorSpans( + internal static Task<(ImmutableArray, ImmutableArray>)> GetDiagnosticsAndErrorSpans( TestWorkspace workspace, IReadOnlyDictionary> analyzerMap = null) { - return SquiggleUtilities.GetDiagnosticsAndErrorSpansAsync(workspace, analyzerMap); + return SquiggleUtilities.GetDiagnosticsAndErrorSpansAsync(workspace, analyzerMap); } - internal static async Task>> GetErrorsFromUpdateSource(TestWorkspace workspace, DiagnosticsUpdatedArgs updateArgs) + internal static async Task>> GetErrorsFromUpdateSource(TestWorkspace workspace, DiagnosticsUpdatedArgs updateArgs) { var globalOptions = workspace.GetService(); var source = new TestDiagnosticUpdateSource(globalOptions); - using var wrapper = new DiagnosticTaggerWrapper(workspace, updateSource: source); + using var wrapper = new DiagnosticTaggerWrapper(workspace, updateSource: source); - var tagger = wrapper.TaggerProvider.CreateTagger(workspace.Documents.First().GetTextBuffer()); + var tagger = wrapper.TaggerProvider.CreateTagger(workspace.Documents.First().GetTextBuffer()); using var disposable = (IDisposable)tagger; source.RaiseDiagnosticsUpdated(updateArgs); diff --git a/src/EditorFeatures/VisualBasicTest/Squiggles/ErrorSquiggleProducerTests.vb b/src/EditorFeatures/VisualBasicTest/Squiggles/ErrorSquiggleProducerTests.vb index cb2703bb6094b..7987e1167cd42 100644 --- a/src/EditorFeatures/VisualBasicTest/Squiggles/ErrorSquiggleProducerTests.vb +++ b/src/EditorFeatures/VisualBasicTest/Squiggles/ErrorSquiggleProducerTests.vb @@ -25,7 +25,7 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.Squiggles Private Shared Async Function ProduceSquiggles(content As String) As Task(Of ImmutableArray(Of ITagSpan(Of IErrorTag))) Using workspace = TestWorkspace.CreateVisualBasic(content) - Return (Await TestDiagnosticTagProducer(Of DiagnosticsSquiggleTaggerProvider).GetDiagnosticsAndErrorSpans(workspace)).Item2 + Return (Await TestDiagnosticTagProducer(Of DiagnosticsSquiggleTaggerProvider, IErrorTag).GetDiagnosticsAndErrorSpans(workspace)).Item2 End Using End Function @@ -68,7 +68,7 @@ End Class") End Sub End Class") - Dim diagnosticsAndSpans = Await TestDiagnosticTagProducer(Of DiagnosticsSquiggleTaggerProvider).GetDiagnosticsAndErrorSpans(workspace) + Dim diagnosticsAndSpans = Await TestDiagnosticTagProducer(Of DiagnosticsSquiggleTaggerProvider, IErrorTag).GetDiagnosticsAndErrorSpans(workspace) Dim spans = diagnosticsAndSpans.Item1.Zip(diagnosticsAndSpans.Item2, Function(diagostic, span) (diagostic, span)).OrderBy(Function(s) s.span.Span.Span.Start).ToImmutableArray() Assert.Equal(1, spans.Count()) @@ -122,7 +122,7 @@ End Class" options.Add(preferIntrinsicPredefinedTypeOption, preferIntrinsicPredefinedTypeOptionValue) workspace.ApplyOptions(options) - Dim diagnosticsAndSpans = Await TestDiagnosticTagProducer(Of DiagnosticsSquiggleTaggerProvider).GetDiagnosticsAndErrorSpans(workspace, analyzerMap) + Dim diagnosticsAndSpans = Await TestDiagnosticTagProducer(Of DiagnosticsSquiggleTaggerProvider, IErrorTag).GetDiagnosticsAndErrorSpans(workspace, analyzerMap) Dim spans = diagnosticsAndSpans.Item1.Zip(diagnosticsAndSpans.Item2, Function(diagostic, span) (diagostic, span)).OrderBy(Function(s) s.span.Span.Span.Start).ToImmutableArray() Assert.Equal(2, spans.Length) From e3fd8e27534b35ff64f5a96b754c3c212999e9ba Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 09:44:06 -0800 Subject: [PATCH 10/41] Update underling-reassigned-variable to understand suppressed variables as well --- .../CSharpReassignedVariableTests.cs | 34 +++++++++++++++++++ .../Extensions/ExpressionSyntaxExtensions.cs | 27 ++++++++++++--- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/ReassignedVariable/CSharpReassignedVariableTests.cs b/src/EditorFeatures/CSharpTest/ReassignedVariable/CSharpReassignedVariableTests.cs index 4860b46b1673a..f7fe02f04df99 100644 --- a/src/EditorFeatures/CSharpTest/ReassignedVariable/CSharpReassignedVariableTests.cs +++ b/src/EditorFeatures/CSharpTest/ReassignedVariable/CSharpReassignedVariableTests.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Editor.UnitTests.ReassignedVariable; using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces; +using Roslyn.Test.Utilities; using Xunit; namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.ReassignedVariable @@ -652,6 +653,23 @@ void M() }"); } + [Fact] + public async Task TestReadonlyRefLocalWithNoReassignment1() + { + await TestAsync( +@" +using System; +class C +{ + void M() + { + int p = 0; + ref readonly int refP = ref p!; + Console.WriteLine(p); + } +}"); + } + [Fact] public async Task TestPointerCausingPossibleReassignment() { @@ -994,6 +1012,22 @@ void M(int p, int p) { p = 1; } +}"); + } + + [Fact, WorkItem(58161, "https://github.com/dotnet/roslyn/issues/58161")] + public async Task TestRefToSuppression1() + { + await TestAsync( +@"#nullable enable + +using System.Diagnostics.CodeAnalysis; +using System.Threading; + +class C +{ + public static T EnsureInitialized([NotNull] ref T? [|target|]) where T : class + => Volatile.Read(ref [|target|]!); }"); } } diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/ExpressionSyntaxExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/ExpressionSyntaxExtensions.cs index 86b38a51a44be..ac112a463f720 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/ExpressionSyntaxExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/ExpressionSyntaxExtensions.cs @@ -197,8 +197,27 @@ public static bool IsInOutContext(this ExpressionSyntax expression) } public static bool IsInRefContext(this ExpressionSyntax expression) - => expression.IsParentKind(SyntaxKind.RefExpression) || - (expression?.Parent as ArgumentSyntax)?.RefOrOutKeyword.Kind() == SyntaxKind.RefKeyword; + => IsInRefContext(expression, out _); + + /// + /// Returns true if this expression is in some ref keyword context. If then + /// will be the node containing the keyword. + /// + public static bool IsInRefContext(this ExpressionSyntax expression, [NotNullWhen(true)] out SyntaxNode? refParent) + { + while (expression.Parent is ParenthesizedExpressionSyntax or PostfixUnaryExpressionSyntax(SyntaxKind.SuppressNullableWarningExpression)) + expression = (ExpressionSyntax)expression.Parent; + + if (expression.Parent is RefExpressionSyntax or + ArgumentSyntax { RefOrOutKeyword.RawKind: (int)SyntaxKind.RefKeyword }) + { + refParent = expression.Parent; + return true; + } + + refParent = null; + return false; + } public static bool IsInInContext(this ExpressionSyntax expression) => (expression?.Parent as ArgumentSyntax)?.RefKindKeyword.Kind() == SyntaxKind.InKeyword; @@ -311,12 +330,12 @@ public static bool IsWrittenTo(this ExpressionSyntax expression, SemanticModel s if (expression.IsOnlyWrittenTo()) return true; - if (expression.IsInRefContext()) + if (expression.IsInRefContext(out var refParent)) { // most cases of `ref x` will count as a potential write of `x`. An important exception is: // `ref readonly y = ref x`. In that case, because 'y' can't be written to, this would not // be a write of 'x'. - if (expression is { Parent: { RawKind: (int)SyntaxKind.RefExpression, Parent: EqualsValueClauseSyntax { Parent: VariableDeclaratorSyntax { Parent: VariableDeclarationSyntax { Type: RefTypeSyntax refType } } } } } + if (refParent.Parent is EqualsValueClauseSyntax { Parent: VariableDeclaratorSyntax { Parent: VariableDeclarationSyntax { Type: RefTypeSyntax refType } } } && refType.ReadOnlyKeyword != default) { return false; From dd7d3de8cd89de59237694f0343b119857f164bb Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 13:35:53 -0800 Subject: [PATCH 11/41] Allow null args --- .../CSharp/Extensions/ExpressionSyntaxExtensions.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/ExpressionSyntaxExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/ExpressionSyntaxExtensions.cs index ac112a463f720..094b84b4f7d81 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/ExpressionSyntaxExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/ExpressionSyntaxExtensions.cs @@ -203,13 +203,13 @@ public static bool IsInRefContext(this ExpressionSyntax expression) /// Returns true if this expression is in some ref keyword context. If then /// will be the node containing the keyword. /// - public static bool IsInRefContext(this ExpressionSyntax expression, [NotNullWhen(true)] out SyntaxNode? refParent) + public static bool IsInRefContext(this ExpressionSyntax? expression, [NotNullWhen(true)] out SyntaxNode? refParent) { - while (expression.Parent is ParenthesizedExpressionSyntax or PostfixUnaryExpressionSyntax(SyntaxKind.SuppressNullableWarningExpression)) + while (expression?.Parent is ParenthesizedExpressionSyntax or PostfixUnaryExpressionSyntax(SyntaxKind.SuppressNullableWarningExpression)) expression = (ExpressionSyntax)expression.Parent; - if (expression.Parent is RefExpressionSyntax or - ArgumentSyntax { RefOrOutKeyword.RawKind: (int)SyntaxKind.RefKeyword }) + if (expression?.Parent is RefExpressionSyntax or + ArgumentSyntax { RefOrOutKeyword.RawKind: (int)SyntaxKind.RefKeyword }) { refParent = expression.Parent; return true; From 49fe3355d5f1d8d24a28b42c3b4f6126e4bb51d0 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 13:36:22 -0800 Subject: [PATCH 12/41] NRT --- .../Compiler/CSharp/Extensions/ExpressionSyntaxExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/ExpressionSyntaxExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/ExpressionSyntaxExtensions.cs index 094b84b4f7d81..43a5fdbc698d4 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/ExpressionSyntaxExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Extensions/ExpressionSyntaxExtensions.cs @@ -203,7 +203,7 @@ public static bool IsInRefContext(this ExpressionSyntax expression) /// Returns true if this expression is in some ref keyword context. If then /// will be the node containing the keyword. /// - public static bool IsInRefContext(this ExpressionSyntax? expression, [NotNullWhen(true)] out SyntaxNode? refParent) + public static bool IsInRefContext([NotNullWhen(true)] this ExpressionSyntax? expression, [NotNullWhen(true)] out SyntaxNode? refParent) { while (expression?.Parent is ParenthesizedExpressionSyntax or PostfixUnaryExpressionSyntax(SyntaxKind.SuppressNullableWarningExpression)) expression = (ExpressionSyntax)expression.Parent; From 288782a960dee67d13d8fa4daeb0a798b961be33 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 14:31:38 -0800 Subject: [PATCH 13/41] Initial trial --- .../ConvertNamespaceTransform.cs | 118 ++++++++++++++++-- .../Wrapping/AbstractCodeActionComputer.cs | 9 +- .../Indentation/IIndentationService.cs | 15 +++ 3 files changed, 128 insertions(+), 14 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs b/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs index 3050716634ac4..51262b097164e 100644 --- a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs +++ b/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs @@ -2,9 +2,11 @@ // 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; using System.Collections.Generic; using System.Collections.Immutable; +using System.IO.Pipes; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -14,8 +16,11 @@ using Microsoft.CodeAnalysis.CSharp.Formatting; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Indentation; using Microsoft.CodeAnalysis.Options; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; #if CODE_STYLE @@ -26,15 +31,114 @@ namespace Microsoft.CodeAnalysis.CSharp.ConvertNamespace { internal static class ConvertNamespaceTransform { - public static async Task ConvertAsync( - Document document, BaseNamespaceDeclarationSyntax baseNamespace, CancellationToken cancellationToken) + public static Task ConvertAsync(Document document, BaseNamespaceDeclarationSyntax baseNamespace, CancellationToken cancellationToken) + => baseNamespace switch + { + FileScopedNamespaceDeclarationSyntax fileScopedNamespace => ConvertFileScopedNamespaceAsync(document, fileScopedNamespace, cancellationToken), + NamespaceDeclarationSyntax namespaceDeclaration => ConvertNamespaceDeclarationAsync(document, namespaceDeclaration, cancellationToken), + _ => throw ExceptionUtilities.UnexpectedValue(baseNamespace.Kind()), + }; + + private static async Task ConvertNamespaceDeclarationAsync(Document document, NamespaceDeclarationSyntax namespaceDeclaration, CancellationToken cancellationToken) + { + // First, determine how much indentation we had inside the original block namespace. We'll attempt to remove + // that much indentation from each applicable line after we conver the block namespace to a file scoped + // namespace. + var indentation = await GetIndentationAsync(document, namespaceDeclaration, cancellationToken).ConfigureAwait(false); + + // Next, actually replace the block namespace with the file scoped namespace. + var annotation = new SyntaxAnnotation(); + document = await ReplaceWithFileScopedNamespaceAsync(document, namespaceDeclaration, annotation, cancellationToken).ConfigureAwait(false); + + // Now, find the file scoped namespace in the updated doc and go and dedent every line if applicable. + if (indentation == null) + return document; + + return await DedentNamespaceAsync(document, indentation, annotation, cancellationToken).ConfigureAwait(false); + } + + private static async Task ReplaceWithFileScopedNamespaceAsync( + Document document, NamespaceDeclarationSyntax namespaceDeclaration, SyntaxAnnotation annotation, CancellationToken cancellationToken) { var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - return document.WithSyntaxRoot(root.ReplaceNode(baseNamespace, Convert(baseNamespace))); + var converted = ConvertNamespaceDeclaration(namespaceDeclaration); + var updatedRoot = root.ReplaceNode( + namespaceDeclaration, + converted.WithAdditionalAnnotations(annotation)); + return document.WithSyntaxRoot(updatedRoot); + } + + private static async Task GetIndentationAsync(Document document, NamespaceDeclarationSyntax namespaceDeclaration, CancellationToken cancellationToken) + { + var indentationService = document.GetRequiredLanguageService(); + var sourceText = await document.GetTextAsync(cancellationToken).ConfigureAwait(false); + + var openBraceLine = sourceText.Lines.GetLineFromPosition(namespaceDeclaration.OpenBraceToken.SpanStart).LineNumber; + var closeBraceLine = sourceText.Lines.GetLineFromPosition(namespaceDeclaration.CloseBraceToken.SpanStart).LineNumber; + if (openBraceLine == closeBraceLine) + return null; + + var options = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false); + var style = options.GetOption(FormattingOptions.SmartIndent, document.Project.Language); + + var indentation = indentationService.GetIndentation(document, openBraceLine + 1, style, cancellationToken); + + var useTabs = options.GetOption(FormattingOptions.UseTabs); + var tabSize = options.GetOption(FormattingOptions.TabSize); + + return indentation.GetIndentationString(sourceText, useTabs, tabSize); + } + + private static async Task DedentNamespaceAsync( + Document document, string indentation, SyntaxAnnotation annotation, CancellationToken cancellationToken) + { + var syntaxTree = await document.GetRequiredSyntaxTreeAsync(cancellationToken).ConfigureAwait(false); + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + var text = await root.SyntaxTree.GetTextAsync(cancellationToken).ConfigureAwait(false); + + var fileScopedNamespace = (FileScopedNamespaceDeclarationSyntax)root.GetAnnotatedNodes(annotation).Single(); + var semicolonLine = text.Lines.GetLineFromPosition(fileScopedNamespace.SemicolonToken.SpanStart).LineNumber; + + using var _ = ArrayBuilder.GetInstance(out var changes); + for (var line = semicolonLine + 1; line < text.Lines.Count; line++) + changes.AddIfNotNull(TryDedentLine(syntaxTree, text, indentation, text.Lines[line], cancellationToken)); + + var dedentedText = text.WithChanges(changes); + return document.WithText(dedentedText); + } + + private static TextChange? TryDedentLine( + SyntaxTree tree, SourceText text, string indentation, TextLine textLine, CancellationToken cancellationToken) + { + // if this line is inside a string-literal or interpolated-text-content, then we definitely do not want to + // touch what is inside there. Note: this will not apply to raw-string literals, which can potentially be + // dedented safely depending on the position of their close terminator. + if (tree.IsEntirelyWithinStringLiteral(textLine.Span.Start, cancellationToken)) + return null; + + // Determine the amount of indentation this text line starts with. + var commonIndentation = 0; + while (commonIndentation < indentation.Length && commonIndentation < textLine.Span.Length) + { + if (indentation[commonIndentation] != text[textLine.Start + commonIndentation]) + break; + + commonIndentation++; + } + + return new TextChange(new TextSpan(textLine.Start, commonIndentation), newText: ""); + } + + private static async Task ConvertFileScopedNamespaceAsync( + Document document, FileScopedNamespaceDeclarationSyntax fileScopedNamespace, CancellationToken cancellationToken) + { + var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); + return document.WithSyntaxRoot(root.ReplaceNode(fileScopedNamespace, ConvertFileScopedNamespace(fileScopedNamespace))); } public static BaseNamespaceDeclarationSyntax Convert(BaseNamespaceDeclarationSyntax baseNamespace) { + return baseNamespace switch { FileScopedNamespaceDeclarationSyntax fileScopedNamespace => ConvertFileScopedNamespace(fileScopedNamespace), @@ -68,7 +172,7 @@ private static FileScopedNamespaceDeclarationSyntax ConvertNamespaceDeclaration( { // We move leading and trailing trivia on the open brace to just be trailing trivia on the semicolon, so we preserve // comments etc. logically at the top of the file. - var semiColon = SyntaxFactory.Token(SyntaxKind.SemicolonToken); + var semiColon = SyntaxFactory.Token(SyntaxKind.SemicolonToken).WithoutTrivia(); if (namespaceDeclaration.Name.GetTrailingTrivia().Any(t => t.IsSingleOrMultiLineComment())) { @@ -90,7 +194,7 @@ private static FileScopedNamespaceDeclarationSyntax ConvertNamespaceDeclaration( semiColon, namespaceDeclaration.Externs, namespaceDeclaration.Usings, - namespaceDeclaration.Members).WithAdditionalAnnotations(Formatter.Annotation); + namespaceDeclaration.Members); // Ensure there's a blank line between the namespace line and the first body member. var firstBodyToken = fileScopedNamespace.SemicolonToken.GetNextToken(); @@ -99,7 +203,7 @@ private static FileScopedNamespaceDeclarationSyntax ConvertNamespaceDeclaration( { fileScopedNamespace = fileScopedNamespace.ReplaceToken( firstBodyToken, - firstBodyToken.WithPrependedLeadingTrivia(SyntaxFactory.ElasticCarriageReturnLineFeed)); + firstBodyToken.WithPrependedLeadingTrivia(SyntaxFactory.CarriageReturnLineFeed)); } // Copy leading trivia from the close brace to the end of the file scoped namespace (which means after all of the members) @@ -107,7 +211,7 @@ private static FileScopedNamespaceDeclarationSyntax ConvertNamespaceDeclaration( { // Ensure there is one blank line before the trivia fileScopedNamespace = fileScopedNamespace - .WithAppendedTrailingTrivia(SyntaxFactory.ElasticCarriageReturnLineFeed) + .WithAppendedTrailingTrivia(SyntaxFactory.CarriageReturnLineFeed) .WithAppendedTrailingTrivia(namespaceDeclaration.CloseBraceToken.LeadingTrivia.WithoutLeadingBlankLines()); } diff --git a/src/Features/Core/Portable/Wrapping/AbstractCodeActionComputer.cs b/src/Features/Core/Portable/Wrapping/AbstractCodeActionComputer.cs index c4bd517cb51cf..ae5877e62ff21 100644 --- a/src/Features/Core/Portable/Wrapping/AbstractCodeActionComputer.cs +++ b/src/Features/Core/Portable/Wrapping/AbstractCodeActionComputer.cs @@ -12,6 +12,7 @@ using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.Editing; using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.Indentation; using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; @@ -104,13 +105,7 @@ protected string GetSmartIndentationAfter(SyntaxNodeOrToken nodeOrToken) FormattingOptions.IndentStyle.Smart, CancellationToken); - var baseLine = newSourceText.Lines.GetLineFromPosition(desiredIndentation.BasePosition); - var baseOffsetInLine = desiredIndentation.BasePosition - baseLine.Start; - - var indent = baseOffsetInLine + desiredIndentation.Offset; - - var indentString = indent.CreateIndentationString(UseTabs, TabSize); - return indentString; + return desiredIndentation.GetIndentationString(newSourceText, UseTabs, TabSize); } /// diff --git a/src/Workspaces/Core/Portable/Indentation/IIndentationService.cs b/src/Workspaces/Core/Portable/Indentation/IIndentationService.cs index 0de1e4a7535d2..f8b6460499ef4 100644 --- a/src/Workspaces/Core/Portable/Indentation/IIndentationService.cs +++ b/src/Workspaces/Core/Portable/Indentation/IIndentationService.cs @@ -5,6 +5,7 @@ using System.Threading; using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.Host; +using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Indentation @@ -63,4 +64,18 @@ public static IndentationResult GetIndentation( return service.GetIndentation(document, lineNumber, style, cancellationToken); } } + + internal static class IndentationResultExtensions + { + public static string GetIndentationString(this IndentationResult indentationResult, SourceText sourceText, bool useTabs, int tabSize) + { + var baseLine = sourceText.Lines.GetLineFromPosition(indentationResult.BasePosition); + var baseOffsetInLine = indentationResult.BasePosition - baseLine.Start; + + var indent = baseOffsetInLine + indentationResult.Offset; + + var indentString = indent.CreateIndentationString(useTabs, tabSize); + return indentString; + } + } } From 0ec375fa8a4ccfe4ee5ed0dc59c014d3a4bb3185 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 14:42:11 -0800 Subject: [PATCH 14/41] Further work --- .../ConvertNamespace/ConvertNamespaceCodeFixProvider.cs | 9 +++++---- .../ConvertNamespace/ConvertNamespaceTransform.cs | 1 - .../ConvertNamespace/ConvertNamespaceRefactoringTests.cs | 8 ++++---- .../Indentation/AbstractIndentationService.Indenter.cs | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceCodeFixProvider.cs index f0777a097d2c4..23f3c9dd11369 100644 --- a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceCodeFixProvider.cs @@ -55,17 +55,18 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context) return Task.CompletedTask; } - protected override Task FixAllAsync( + protected override async Task FixAllAsync( Document document, ImmutableArray diagnostics, SyntaxEditor editor, CancellationToken cancellationToken) { var diagnostic = diagnostics.First(); var namespaceDecl = (BaseNamespaceDeclarationSyntax)diagnostic.AdditionalLocations[0].FindNode(cancellationToken); - var converted = Convert(namespaceDecl); + var converted = await ConvertAsync(document, namespaceDecl, cancellationToken).ConfigureAwait(false); - editor.ReplaceNode(namespaceDecl, converted); - return Task.CompletedTask; + editor.ReplaceNode( + editor.OriginalRoot, + await converted.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false)); } private class MyCodeAction : CustomCodeActions.DocumentChangeAction diff --git a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs b/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs index 51262b097164e..a55ccaea2c3a5 100644 --- a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs +++ b/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs @@ -138,7 +138,6 @@ private static async Task ConvertFileScopedNamespaceAsync( public static BaseNamespaceDeclarationSyntax Convert(BaseNamespaceDeclarationSyntax baseNamespace) { - return baseNamespace switch { FileScopedNamespaceDeclarationSyntax fileScopedNamespace => ConvertFileScopedNamespace(fileScopedNamespace), diff --git a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceRefactoringTests.cs b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceRefactoringTests.cs index feae3626badc2..dd270fb20db51 100644 --- a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceRefactoringTests.cs +++ b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceRefactoringTests.cs @@ -478,9 +478,9 @@ public async Task TextConvertToFileScopedWithCommentedOutContents() FixedCode = @" namespace N; - // public class C - // { - // } +// public class C +// { +// } ", LanguageVersion = LanguageVersion.CSharp10, Options = @@ -513,7 +513,7 @@ public class C { } - // I'll probably write some more code here later +// I'll probably write some more code here later ", LanguageVersion = LanguageVersion.CSharp10, Options = diff --git a/src/Workspaces/Core/Portable/Indentation/AbstractIndentationService.Indenter.cs b/src/Workspaces/Core/Portable/Indentation/AbstractIndentationService.Indenter.cs index e98912c2a2f39..796e7905ca082 100644 --- a/src/Workspaces/Core/Portable/Indentation/AbstractIndentationService.Indenter.cs +++ b/src/Workspaces/Core/Portable/Indentation/AbstractIndentationService.Indenter.cs @@ -171,7 +171,7 @@ public bool TryGetSmartTokenIndentation(out IndentationResult indentationResult) var formatter = _service.CreateSmartTokenFormatter(this); var changes = formatter.FormatTokenAsync(Document.Project.Solution.Workspace, token, CancellationToken) - .WaitAndGetResult(CancellationToken); + .WaitAndGetResult_CanCallOnBackground(CancellationToken); var updatedSourceText = sourceText.WithChanges(changes); if (LineToBeIndented.LineNumber < updatedSourceText.Lines.Count) From a8553a0ec6536dbf71b7a6467fb48cd1af6e6dcb Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 15:25:26 -0800 Subject: [PATCH 15/41] Before trivia moving --- .../ConvertNamespaceTransform.cs | 37 ++++++++----- .../ConvertNamespaceCommandHandler.cs | 54 +++++++------------ ...eclarationNewDocumentFormattingProvider.cs | 7 ++- 3 files changed, 47 insertions(+), 51 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs b/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs index a55ccaea2c3a5..ad5c2539be7dc 100644 --- a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs +++ b/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs @@ -31,15 +31,23 @@ namespace Microsoft.CodeAnalysis.CSharp.ConvertNamespace { internal static class ConvertNamespaceTransform { - public static Task ConvertAsync(Document document, BaseNamespaceDeclarationSyntax baseNamespace, CancellationToken cancellationToken) - => baseNamespace switch + public static async Task ConvertAsync(Document document, BaseNamespaceDeclarationSyntax baseNamespace, CancellationToken cancellationToken) + { + switch (baseNamespace) { - FileScopedNamespaceDeclarationSyntax fileScopedNamespace => ConvertFileScopedNamespaceAsync(document, fileScopedNamespace, cancellationToken), - NamespaceDeclarationSyntax namespaceDeclaration => ConvertNamespaceDeclarationAsync(document, namespaceDeclaration, cancellationToken), - _ => throw ExceptionUtilities.UnexpectedValue(baseNamespace.Kind()), + case FileScopedNamespaceDeclarationSyntax fileScopedNamespace: + return await ConvertFileScopedNamespaceAsync(document, fileScopedNamespace, cancellationToken).ConfigureAwait(false); + + case NamespaceDeclarationSyntax namespaceDeclaration: + var (doc, _) = await ConvertNamespaceDeclarationAsync(document, namespaceDeclaration, cancellationToken).ConfigureAwait(false); + return doc; + + default: + throw ExceptionUtilities.UnexpectedValue(baseNamespace.Kind()); }; + } - private static async Task ConvertNamespaceDeclarationAsync(Document document, NamespaceDeclarationSyntax namespaceDeclaration, CancellationToken cancellationToken) + public static async Task<(Document document, TextSpan semicolonSpan)> ConvertNamespaceDeclarationAsync(Document document, NamespaceDeclarationSyntax namespaceDeclaration, CancellationToken cancellationToken) { // First, determine how much indentation we had inside the original block namespace. We'll attempt to remove // that much indentation from each applicable line after we conver the block namespace to a file scoped @@ -48,16 +56,16 @@ private static async Task ConvertNamespaceDeclarationAsync(Document do // Next, actually replace the block namespace with the file scoped namespace. var annotation = new SyntaxAnnotation(); - document = await ReplaceWithFileScopedNamespaceAsync(document, namespaceDeclaration, annotation, cancellationToken).ConfigureAwait(false); + var (updatedDocument, semicolonSpan) = await ReplaceWithFileScopedNamespaceAsync(document, namespaceDeclaration, annotation, cancellationToken).ConfigureAwait(false); // Now, find the file scoped namespace in the updated doc and go and dedent every line if applicable. if (indentation == null) - return document; + return (updatedDocument, semicolonSpan); - return await DedentNamespaceAsync(document, indentation, annotation, cancellationToken).ConfigureAwait(false); + return await DedentNamespaceAsync(updatedDocument, indentation, annotation, cancellationToken).ConfigureAwait(false); } - private static async Task ReplaceWithFileScopedNamespaceAsync( + private static async Task<(Document document, TextSpan semicolonSpan)> ReplaceWithFileScopedNamespaceAsync( Document document, NamespaceDeclarationSyntax namespaceDeclaration, SyntaxAnnotation annotation, CancellationToken cancellationToken) { var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); @@ -65,7 +73,8 @@ private static async Task ReplaceWithFileScopedNamespaceAsync( var updatedRoot = root.ReplaceNode( namespaceDeclaration, converted.WithAdditionalAnnotations(annotation)); - return document.WithSyntaxRoot(updatedRoot); + var fileScopedNamespace = (FileScopedNamespaceDeclarationSyntax)updatedRoot.GetAnnotatedNodes(annotation).Single(); + return (document.WithSyntaxRoot(updatedRoot), fileScopedNamespace.SemicolonToken.Span); } private static async Task GetIndentationAsync(Document document, NamespaceDeclarationSyntax namespaceDeclaration, CancellationToken cancellationToken) @@ -89,7 +98,7 @@ private static async Task ReplaceWithFileScopedNamespaceAsync( return indentation.GetIndentationString(sourceText, useTabs, tabSize); } - private static async Task DedentNamespaceAsync( + private static async Task<(Document document, TextSpan semicolonSpan)> DedentNamespaceAsync( Document document, string indentation, SyntaxAnnotation annotation, CancellationToken cancellationToken) { var syntaxTree = await document.GetRequiredSyntaxTreeAsync(cancellationToken).ConfigureAwait(false); @@ -104,7 +113,7 @@ private static async Task DedentNamespaceAsync( changes.AddIfNotNull(TryDedentLine(syntaxTree, text, indentation, text.Lines[line], cancellationToken)); var dedentedText = text.WithChanges(changes); - return document.WithText(dedentedText); + return (document.WithText(dedentedText), fileScopedNamespace.SemicolonToken.Span); } private static TextChange? TryDedentLine( @@ -136,7 +145,7 @@ private static async Task ConvertFileScopedNamespaceAsync( return document.WithSyntaxRoot(root.ReplaceNode(fileScopedNamespace, ConvertFileScopedNamespace(fileScopedNamespace))); } - public static BaseNamespaceDeclarationSyntax Convert(BaseNamespaceDeclarationSyntax baseNamespace) + private static BaseNamespaceDeclarationSyntax Convert(BaseNamespaceDeclarationSyntax baseNamespace) { return baseNamespace switch { diff --git a/src/EditorFeatures/CSharp/ConvertNamespace/ConvertNamespaceCommandHandler.cs b/src/EditorFeatures/CSharp/ConvertNamespace/ConvertNamespaceCommandHandler.cs index f18338c3709f3..3b0cba7fa8cf0 100644 --- a/src/EditorFeatures/CSharp/ConvertNamespace/ConvertNamespaceCommandHandler.cs +++ b/src/EditorFeatures/CSharp/ConvertNamespace/ConvertNamespaceCommandHandler.cs @@ -26,6 +26,7 @@ using Microsoft.VisualStudio.Text.Editor.Commanding.Commands; using Microsoft.VisualStudio.Text.Operations; using Microsoft.VisualStudio.Utilities; +using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Editor.CSharp.CompleteStatement { @@ -39,11 +40,6 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.CompleteStatement [Order(After = PredefinedCompletionNames.CompletionCommandHandler)] internal sealed class ConvertNamespaceCommandHandler : IChainedCommandHandler { - /// - /// Annotation used so we can find the semicolon after formatting so that we can properly place the caret. - /// - private static readonly SyntaxAnnotation s_annotation = new(); - /// /// A fake option set where the 'use file scoped' namespace option is on. That way we can call into the helpers /// and have the results come back positive for converting to file-scoped regardless of the current option @@ -79,7 +75,7 @@ public CommandState GetCommandState(TypeCharCommandArgs args, Func public void ExecuteCommand(TypeCharCommandArgs args, Action nextCommandHandler, CommandExecutionContext executionContext) { // Attempt to convert the block-namespace to a file-scoped namespace if we're at the right location. - var convertedRoot = ConvertNamespace(args, executionContext); + var (convertedText, semicolonSpan) = ConvertNamespace(args, executionContext); // No matter if we succeeded or not, insert the semicolon. This way, when we convert, the user can still // hit ctrl-z to get back to the code with just the semicolon inserted. @@ -87,7 +83,7 @@ public void ExecuteCommand(TypeCharCommandArgs args, Action nextCommandHandler, // If we weren't on a block namespace (or couldn't convert it for some reason), then bail out after // inserting the semicolon. - if (convertedRoot == null) + if (convertedText == null) return; // Otherwise, make a transaction for the edit and replace the buffer with the final text. @@ -95,14 +91,12 @@ public void ExecuteCommand(TypeCharCommandArgs args, Action nextCommandHandler, this.DisplayName, args.TextView, _textUndoHistoryRegistry, _editorOperationsFactoryService); var edit = args.SubjectBuffer.CreateEdit(EditOptions.DefaultMinimalChange, reiteratedVersionNumber: null, editTag: null); - edit.Replace(new Span(0, args.SubjectBuffer.CurrentSnapshot.Length), convertedRoot.ToFullString()); + edit.Replace(new Span(0, args.SubjectBuffer.CurrentSnapshot.Length), convertedText.ToString()); edit.Apply(); - // Attempt to place the caret right after the semicolon of the file-scoped namespace. - var annotatedToken = convertedRoot.GetAnnotatedTokens(s_annotation).FirstOrDefault(); - if (annotatedToken != default) - args.TextView.Caret.MoveTo(new SnapshotPoint(args.SubjectBuffer.CurrentSnapshot, annotatedToken.Span.End)); + // Place the caret right after the semicolon of the file-scoped namespace. + args.TextView.Caret.MoveTo(new SnapshotPoint(args.SubjectBuffer.CurrentSnapshot, semicolonSpan.End)); transaction?.Complete(); } @@ -111,25 +105,25 @@ public void ExecuteCommand(TypeCharCommandArgs args, Action nextCommandHandler, /// Returns the updated file contents if semicolon is typed after a block-scoped namespace name that can be /// converted. /// - private CompilationUnitSyntax? ConvertNamespace( + private (SourceText? convertedText, TextSpan semicolonSpan) ConvertNamespace( TypeCharCommandArgs args, CommandExecutionContext executionContext) { if (args.TypedChar != ';' || !args.TextView.Selection.IsEmpty) - return null; + return default; if (!_globalOptions.GetOption(FeatureOnOffOptions.AutomaticallyCompleteStatementOnSemicolon)) - return null; + return default; var subjectBuffer = args.SubjectBuffer; var caretOpt = args.TextView.GetCaretPoint(subjectBuffer); if (!caretOpt.HasValue) - return null; + return default; var caret = caretOpt.Value.Position; var document = subjectBuffer.CurrentSnapshot.GetOpenDocumentInCurrentContextWithChanges(); if (document == null) - return null; + return default; var cancellationToken = executionContext.OperationContext.UserCancellationToken; var root = (CompilationUnitSyntax)document.GetRequiredSyntaxRootSynchronously(cancellationToken); @@ -137,39 +131,29 @@ public void ExecuteCommand(TypeCharCommandArgs args, Action nextCommandHandler, // User has to be *after* an identifier token. var token = root.FindToken(caret); if (token.Kind() != SyntaxKind.IdentifierToken) - return null; + return default; if (caret < token.Span.End || caret >= token.FullSpan.End) { - return null; + return default; } var namespaceDecl = token.GetRequiredParent().GetAncestor(); if (namespaceDecl == null) - return null; + return default; // That identifier token has to be the last part of a namespace name. if (namespaceDecl.Name.GetLastToken() != token) - return null; + return default; // Pass in our special options, and C#10 so that if we can convert this to file-scoped, we will. if (!ConvertNamespaceAnalysis.CanOfferUseFileScoped(s_optionSet, root, namespaceDecl, forAnalyzer: true, LanguageVersion.CSharp10)) - return null; - - var fileScopedNamespace = (FileScopedNamespaceDeclarationSyntax)ConvertNamespaceTransform.Convert(namespaceDecl); - - // Place an annotation on the semicolon so that we can find it post-formatting to place the caret. - fileScopedNamespace = fileScopedNamespace.WithSemicolonToken( - fileScopedNamespace.SemicolonToken.WithAdditionalAnnotations(s_annotation)); - - var convertedRoot = root.ReplaceNode(namespaceDecl, fileScopedNamespace); - var formattedRoot = (CompilationUnitSyntax)Formatter.Format( - convertedRoot, Formatter.Annotation, - document.Project.Solution.Workspace, - options: null, rules: null, cancellationToken); + return default; - return formattedRoot; + var (converted, semicolonSpan) = ConvertNamespaceTransform.ConvertNamespaceDeclarationAsync(document, namespaceDecl, cancellationToken).WaitAndGetResult(cancellationToken); + var text = converted.GetTextSynchronously(cancellationToken); + return (text, semicolonSpan); } } } diff --git a/src/Features/CSharp/Portable/Formatting/CSharpNamespaceDeclarationNewDocumentFormattingProvider.cs b/src/Features/CSharp/Portable/Formatting/CSharpNamespaceDeclarationNewDocumentFormattingProvider.cs index 8350fb94e6d1f..d2d8d33740221 100644 --- a/src/Features/CSharp/Portable/Formatting/CSharpNamespaceDeclarationNewDocumentFormattingProvider.cs +++ b/src/Features/CSharp/Portable/Formatting/CSharpNamespaceDeclarationNewDocumentFormattingProvider.cs @@ -32,8 +32,11 @@ public async Task FormatNewDocumentAsync(Document document, Document? var optionSet = await document.GetOptionsAsync(cancellationToken).ConfigureAwait(false); var root = (CompilationUnitSyntax)await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var namespaces = GetNamespacesToReplace(document, root, optionSet); - return await document.ReplaceNodesAsync(namespaces, (oldNode, newNode) => ConvertNamespaceTransform.Convert((BaseNamespaceDeclarationSyntax)newNode), cancellationToken).ConfigureAwait(false); + var namespaces = GetNamespacesToReplace(document, root, optionSet).ToList(); + if (namespaces.Count != 1) + return document; + + return await ConvertNamespaceTransform.ConvertAsync(document, namespaces[0], cancellationToken).ConfigureAwait(false); } private static IEnumerable GetNamespacesToReplace(Document document, CompilationUnitSyntax root, DocumentOptionSet optionSet) From 7fe6edd3281fed836fe2f5df28be7449b97750b1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 15:38:29 -0800 Subject: [PATCH 16/41] Update formatting --- .../ConvertNamespaceTransform.cs | 49 ++++++------------- .../ConvertNamespaceCommandHandlerTests.cs | 2 +- .../ConvertNamespaceRefactoringTests.cs | 5 +- ...nvertToFileScopedNamespaceAnalyzerTests.cs | 4 +- 4 files changed, 21 insertions(+), 39 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs b/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs index ad5c2539be7dc..b4f5cda640536 100644 --- a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs +++ b/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs @@ -145,16 +145,6 @@ private static async Task ConvertFileScopedNamespaceAsync( return document.WithSyntaxRoot(root.ReplaceNode(fileScopedNamespace, ConvertFileScopedNamespace(fileScopedNamespace))); } - private static BaseNamespaceDeclarationSyntax Convert(BaseNamespaceDeclarationSyntax baseNamespace) - { - return baseNamespace switch - { - FileScopedNamespaceDeclarationSyntax fileScopedNamespace => ConvertFileScopedNamespace(fileScopedNamespace), - NamespaceDeclarationSyntax namespaceDeclaration => ConvertNamespaceDeclaration(namespaceDeclaration), - _ => throw ExceptionUtilities.UnexpectedValue(baseNamespace.Kind()), - }; - } - private static bool HasLeadingBlankLine( SyntaxToken token, out SyntaxToken withoutBlankLine) { @@ -178,37 +168,35 @@ private static bool HasLeadingBlankLine( private static FileScopedNamespaceDeclarationSyntax ConvertNamespaceDeclaration(NamespaceDeclarationSyntax namespaceDeclaration) { - // We move leading and trailing trivia on the open brace to just be trailing trivia on the semicolon, so we preserve - // comments etc. logically at the top of the file. var semiColon = SyntaxFactory.Token(SyntaxKind.SemicolonToken).WithoutTrivia(); - if (namespaceDeclaration.Name.GetTrailingTrivia().Any(t => t.IsSingleOrMultiLineComment())) - { - semiColon = semiColon.WithTrailingTrivia(namespaceDeclaration.Name.GetTrailingTrivia()) - .WithAppendedTrailingTrivia(namespaceDeclaration.OpenBraceToken.LeadingTrivia); - } - else - { - semiColon = semiColon.WithTrailingTrivia(namespaceDeclaration.OpenBraceToken.LeadingTrivia); - } - - semiColon = semiColon.WithAppendedTrailingTrivia(namespaceDeclaration.OpenBraceToken.TrailingTrivia); - + // Move trivia after the original name token to now be after the new semicolon token. var fileScopedNamespace = SyntaxFactory.FileScopedNamespaceDeclaration( namespaceDeclaration.AttributeLists, namespaceDeclaration.Modifiers, namespaceDeclaration.NamespaceKeyword, namespaceDeclaration.Name.WithoutTrailingTrivia(), - semiColon, + semiColon.WithTrailingTrivia(namespaceDeclaration.Name.GetTrailingTrivia()), namespaceDeclaration.Externs, namespaceDeclaration.Usings, namespaceDeclaration.Members); - // Ensure there's a blank line between the namespace line and the first body member. var firstBodyToken = fileScopedNamespace.SemicolonToken.GetNextToken(); - if (firstBodyToken.Kind() != SyntaxKind.EndOfFileToken && + + // If the open-brace token has any special trivia, then move them to before the first member in the namespace. + if (namespaceDeclaration.OpenBraceToken.LeadingTrivia.Any(t => t.IsSingleOrMultiLineComment() || t.IsDirective) || + namespaceDeclaration.OpenBraceToken.TrailingTrivia.Any(t => t.IsSingleOrMultiLineComment() || t.IsDirective)) + { + fileScopedNamespace = fileScopedNamespace.ReplaceToken( + firstBodyToken, + firstBodyToken.WithPrependedLeadingTrivia(namespaceDeclaration.OpenBraceToken.GetAllTrivia())); + + } + else if (firstBodyToken.Kind() != SyntaxKind.EndOfFileToken && !HasLeadingBlankLine(firstBodyToken, out _)) { + // Otherwise, ensure there's a blank line between the namespace line and the first body member. + fileScopedNamespace = fileScopedNamespace.ReplaceToken( firstBodyToken, firstBodyToken.WithPrependedLeadingTrivia(SyntaxFactory.CarriageReturnLineFeed)); @@ -216,12 +204,7 @@ private static FileScopedNamespaceDeclarationSyntax ConvertNamespaceDeclaration( // Copy leading trivia from the close brace to the end of the file scoped namespace (which means after all of the members) if (namespaceDeclaration.CloseBraceToken.HasLeadingTrivia) - { - // Ensure there is one blank line before the trivia - fileScopedNamespace = fileScopedNamespace - .WithAppendedTrailingTrivia(SyntaxFactory.CarriageReturnLineFeed) - .WithAppendedTrailingTrivia(namespaceDeclaration.CloseBraceToken.LeadingTrivia.WithoutLeadingBlankLines()); - } + fileScopedNamespace = fileScopedNamespace.WithAppendedTrailingTrivia(namespaceDeclaration.CloseBraceToken.LeadingTrivia); return fileScopedNamespace; } diff --git a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceCommandHandlerTests.cs b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceCommandHandlerTests.cs index 738bbb1a11c82..1b64a0d0d23ed 100644 --- a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceCommandHandlerTests.cs +++ b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceCommandHandlerTests.cs @@ -193,7 +193,7 @@ class C testState.SendTypeChar(';'); testState.AssertCodeIs( -@"namespace A.B; +@"namespace A.B; class C { diff --git a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceRefactoringTests.cs b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceRefactoringTests.cs index dd270fb20db51..a9b442fcb4125 100644 --- a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceRefactoringTests.cs +++ b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceRefactoringTests.cs @@ -417,8 +417,8 @@ class C } ", FixedCode = @" -namespace $$N; // comment - +namespace $$N; +// comment class C { } @@ -477,7 +477,6 @@ public async Task TextConvertToFileScopedWithCommentedOutContents() ", FixedCode = @" namespace N; - // public class C // { // } diff --git a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertToFileScopedNamespaceAnalyzerTests.cs b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertToFileScopedNamespaceAnalyzerTests.cs index 532c4c132d8b1..1d29d044b7cbd 100644 --- a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertToFileScopedNamespaceAnalyzerTests.cs +++ b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertToFileScopedNamespaceAnalyzerTests.cs @@ -347,8 +347,8 @@ class C } ", FixedCode = @" -namespace $$N; // comment - +namespace $$N; +// comment class C { } From 9897eb2c828348d3d4bde1aee82090d0675cc507 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 15:55:30 -0800 Subject: [PATCH 17/41] Update tests --- .../ConvertNamespaceTransform.cs | 15 +- .../ConvertNamespaceCommandHandlerTests.cs | 30 +- .../ConvertNamespaceRefactoringTests.cs | 1 + ...nvertToFileScopedNamespaceAnalyzerTests.cs | 285 ++++++++++++++++++ 4 files changed, 311 insertions(+), 20 deletions(-) diff --git a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs b/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs index b4f5cda640536..75a837fc73eb8 100644 --- a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs +++ b/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs @@ -190,21 +190,24 @@ private static FileScopedNamespaceDeclarationSyntax ConvertNamespaceDeclaration( fileScopedNamespace = fileScopedNamespace.ReplaceToken( firstBodyToken, firstBodyToken.WithPrependedLeadingTrivia(namespaceDeclaration.OpenBraceToken.GetAllTrivia())); + firstBodyToken = fileScopedNamespace.SemicolonToken.GetNextNonZeroWidthTokenOrEndOfFile(); } - else if (firstBodyToken.Kind() != SyntaxKind.EndOfFileToken && - !HasLeadingBlankLine(firstBodyToken, out _)) - { - // Otherwise, ensure there's a blank line between the namespace line and the first body member. + // Otherwise, ensure there's a blank line between the namespace line and the first body member. Don't bother + // with this though if we already separated things by moving a pp directive (like a #else) from the open brace + // to the first token. + if (firstBodyToken.Kind() != SyntaxKind.EndOfFileToken && + !HasLeadingBlankLine(firstBodyToken, out _) && + !namespaceDeclaration.OpenBraceToken.LeadingTrivia.Any(t => t.IsDirective)) + { fileScopedNamespace = fileScopedNamespace.ReplaceToken( firstBodyToken, firstBodyToken.WithPrependedLeadingTrivia(SyntaxFactory.CarriageReturnLineFeed)); } // Copy leading trivia from the close brace to the end of the file scoped namespace (which means after all of the members) - if (namespaceDeclaration.CloseBraceToken.HasLeadingTrivia) - fileScopedNamespace = fileScopedNamespace.WithAppendedTrailingTrivia(namespaceDeclaration.CloseBraceToken.LeadingTrivia); + fileScopedNamespace = fileScopedNamespace.WithAppendedTrailingTrivia(namespaceDeclaration.CloseBraceToken.LeadingTrivia); return fileScopedNamespace; } diff --git a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceCommandHandlerTests.cs b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceCommandHandlerTests.cs index 1b64a0d0d23ed..42e05bdf1de6d 100644 --- a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceCommandHandlerTests.cs +++ b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceCommandHandlerTests.cs @@ -44,7 +44,9 @@ public static XElement GetWorkspaceXml(string markup) internal void AssertCodeIs(string expectedCode) { - Assert.Equal(expectedCode, TextView.TextSnapshot.GetText()); + MarkupTestFile.GetPosition(expectedCode, out var massaged, out int caretPosition); + Assert.Equal(massaged, TextView.TextSnapshot.GetText()); + Assert.Equal(caretPosition, TextView.Caret.Position.BufferPosition.Position); } public void SendTypeChar(char ch) @@ -64,7 +66,7 @@ class C testState.SendTypeChar(';'); testState.AssertCodeIs( -@"namespace N; +@"namespace N;$$ class C { @@ -88,7 +90,7 @@ class C testState.SendTypeChar(';'); testState.AssertCodeIs( -@"namespace N; +@"namespace N;$$ { class C { @@ -109,7 +111,7 @@ class C testState.SendTypeChar(';'); testState.AssertCodeIs( -@"namespace A.B; +@"namespace A.B;$$ class C { @@ -130,7 +132,7 @@ class C testState.SendTypeChar(';'); testState.AssertCodeIs( -@"namespace A.;B +@"namespace A.;$$B { class C { @@ -151,7 +153,7 @@ class C testState.SendTypeChar(';'); testState.AssertCodeIs( -@"namespace A;.B +@"namespace A;$$.B { class C { @@ -172,7 +174,7 @@ class C testState.SendTypeChar(';'); testState.AssertCodeIs( -@"namespace ;A.B +@"namespace ;$$A.B { class C { @@ -193,7 +195,7 @@ class C testState.SendTypeChar(';'); testState.AssertCodeIs( -@"namespace A.B; +@"namespace A.B;$$ class C { @@ -214,7 +216,7 @@ class C testState.SendTypeChar(';'); testState.AssertCodeIs( -@"namespace ;N +@"namespace ;$$N { class C { @@ -238,7 +240,7 @@ class C testState.SendTypeChar(';'); testState.AssertCodeIs( -@"namespace N; +@"namespace N;$$ { namespace N2 { @@ -266,7 +268,7 @@ class C testState.SendTypeChar(';'); testState.AssertCodeIs( -@"namespace N; +@"namespace N;$$ { } @@ -299,7 +301,7 @@ class C using A; using B; -namespace N; +namespace N;$$ class C { @@ -325,7 +327,7 @@ class C testState.SendTypeChar(';'); testState.AssertCodeIs( @" -namespace N; +namespace N;$$ using A; using B; @@ -349,7 +351,7 @@ class C testState.SendTypeChar(';'); testState.AssertCodeIs( -@"namespace N; // Goo +@"namespace N;$$ // Goo class C { diff --git a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceRefactoringTests.cs b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceRefactoringTests.cs index a9b442fcb4125..45f83851ce2c2 100644 --- a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceRefactoringTests.cs +++ b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertNamespaceRefactoringTests.cs @@ -418,6 +418,7 @@ class C ", FixedCode = @" namespace $$N; + // comment class C { diff --git a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertToFileScopedNamespaceAnalyzerTests.cs b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertToFileScopedNamespaceAnalyzerTests.cs index 1d29d044b7cbd..9af05a7576de7 100644 --- a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertToFileScopedNamespaceAnalyzerTests.cs +++ b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertToFileScopedNamespaceAnalyzerTests.cs @@ -348,6 +348,7 @@ class C ", FixedCode = @" namespace $$N; + // comment class C { @@ -391,6 +392,290 @@ class C }.RunAsync(); } + [Fact] + public async Task TestConvertToFileScopedWithDocComment() + { + await new VerifyCS.Test + { + TestCode = @" +[|namespace N|] +{ + /// + class C + { + } +} +", + FixedCode = @" +namespace $$N; + +/// +class C +{ +} +", + LanguageVersion = LanguageVersion.CSharp10, + Options = + { + { CSharpCodeStyleOptions.NamespaceDeclarations, NamespaceDeclarationPreference.FileScoped } + } + }.RunAsync(); + } + + [Fact] + public async Task TestConvertToFileScopedWithPPDirective1() + { + await new VerifyCS.Test + { + TestCode = @" +[|namespace N|] +{ +#if X + class C + { + } +#else + class C + { + } +#endif +} +", + FixedCode = @" +namespace $$N; + +#if X +class C +{ +} +#else +class C +{ +} +#endif +", + LanguageVersion = LanguageVersion.CSharp10, + Options = + { + { CSharpCodeStyleOptions.NamespaceDeclarations, NamespaceDeclarationPreference.FileScoped } + } + }.RunAsync(); + } + + [Fact] + public async Task TestConvertToFileScopedWithBlockComment() + { + await new VerifyCS.Test + { + TestCode = @" +[|namespace N|] +{ + /* x + * x + */ + class C + { + } +} +", + FixedCode = @" +namespace $$N; + +/* x + * x + */ +class C +{ +} +", + LanguageVersion = LanguageVersion.CSharp10, + Options = + { + { CSharpCodeStyleOptions.NamespaceDeclarations, NamespaceDeclarationPreference.FileScoped } + } + }.RunAsync(); + } + + [Fact] + public async Task TestConvertToFileScopedWithBlockComment2() + { + await new VerifyCS.Test + { + TestCode = @" +[|namespace N|] +{ + /* x + x + */ + class C + { + } +} +", + FixedCode = @" +namespace $$N; + +/* x + x + */ +class C +{ +} +", + LanguageVersion = LanguageVersion.CSharp10, + Options = + { + { CSharpCodeStyleOptions.NamespaceDeclarations, NamespaceDeclarationPreference.FileScoped } + } + }.RunAsync(); + } + + [Fact] + public async Task TestConvertToFileScopedWithMultilineString() + { + await new VerifyCS.Test + { + TestCode = @" +[|namespace N|] +{ + class C + { + void M() + { + System.Console.WriteLine(@"" + a + b + c + d + e + ""); + } + } +} +", + FixedCode = @" +namespace $$N; + +class C +{ + void M() + { + System.Console.WriteLine(@"" + a + b + c + d + e + ""); + } +} +", + LanguageVersion = LanguageVersion.CSharp10, + Options = + { + { CSharpCodeStyleOptions.NamespaceDeclarations, NamespaceDeclarationPreference.FileScoped } + } + }.RunAsync(); + } + + [Fact] + public async Task TestConvertToFileScopedWithMultilineString2() + { + await new VerifyCS.Test + { + TestCode = @" +[|namespace N|] +{ + class C + { + void M() + { + System.Console.WriteLine($@"" + a + b + c{1 + 1} + d + e + ""); + } + } +} +", + FixedCode = @" +namespace $$N; + +class C +{ + void M() + { + System.Console.WriteLine($@"" + a + b + c{1 + 1} + d + e + ""); + } +} +", + LanguageVersion = LanguageVersion.CSharp10, + Options = + { + { CSharpCodeStyleOptions.NamespaceDeclarations, NamespaceDeclarationPreference.FileScoped } + } + }.RunAsync(); + } + + [Fact] + public async Task TestConvertToFileScopedWithMultilineString3() + { + await new VerifyCS.Test + { + TestCode = @" +[|namespace N|] +{ + class C + { + void M() + { + System.Console.WriteLine($@"" + a + b + c{ + 1 + 1 + }d + e + ""); + } + } +} +", + FixedCode = @" +namespace $$N; + +class C +{ + void M() + { + System.Console.WriteLine($@"" + a + b + c{ + 1 + 1 + }d + e + ""); + } +} +", + LanguageVersion = LanguageVersion.CSharp10, + Options = + { + { CSharpCodeStyleOptions.NamespaceDeclarations, NamespaceDeclarationPreference.FileScoped } + } + }.RunAsync(); + } + #endregion } } From 0cc31fa29a8a633aa1ba92bbab34e86aa96ee8d8 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 16:03:32 -0800 Subject: [PATCH 18/41] Add tests --- ...nvertToFileScopedNamespaceAnalyzerTests.cs | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertToFileScopedNamespaceAnalyzerTests.cs b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertToFileScopedNamespaceAnalyzerTests.cs index 9af05a7576de7..2e1d7d2ee8b01 100644 --- a/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertToFileScopedNamespaceAnalyzerTests.cs +++ b/src/EditorFeatures/CSharpTest/ConvertNamespace/ConvertToFileScopedNamespaceAnalyzerTests.cs @@ -676,6 +676,46 @@ void M() }.RunAsync(); } + [Fact] + public async Task TestConvertToFileScopedSingleLineNamespace1() + { + await new VerifyCS.Test + { + TestCode = @" +[|namespace N|] { class C { } } +", + FixedCode = @" +namespace $$N; +class C { } ", + LanguageVersion = LanguageVersion.CSharp10, + Options = + { + { CSharpCodeStyleOptions.NamespaceDeclarations, NamespaceDeclarationPreference.FileScoped } + } + }.RunAsync(); + } + + [Fact] + public async Task TestConvertToFileScopedSingleLineNamespace2() + { + await new VerifyCS.Test + { + TestCode = @" +[|namespace N|] +{ class C { } } +", + FixedCode = @" +namespace $$N; + +class C { } ", + LanguageVersion = LanguageVersion.CSharp10, + Options = + { + { CSharpCodeStyleOptions.NamespaceDeclarations, NamespaceDeclarationPreference.FileScoped } + } + }.RunAsync(); + } + #endregion } } From 6b8bf3bfebd03e5936a4c7929355592ae9c8de1b Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Mon, 20 Dec 2021 19:09:34 -0500 Subject: [PATCH 19/41] Update src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs --- .../CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs b/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs index 75a837fc73eb8..b8f4dc29bfe80 100644 --- a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs +++ b/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs @@ -191,7 +191,6 @@ private static FileScopedNamespaceDeclarationSyntax ConvertNamespaceDeclaration( firstBodyToken, firstBodyToken.WithPrependedLeadingTrivia(namespaceDeclaration.OpenBraceToken.GetAllTrivia())); firstBodyToken = fileScopedNamespace.SemicolonToken.GetNextNonZeroWidthTokenOrEndOfFile(); - } // Otherwise, ensure there's a blank line between the namespace line and the first body member. Don't bother From e01c2352135e06927be8bf3bcbd737251e991729 Mon Sep 17 00:00:00 2001 From: Fred Silberberg Date: Mon, 20 Dec 2021 17:04:43 -0800 Subject: [PATCH 20/41] Remove unused message argument (#58432) While adjusting this code in the required-members branch I noticed the parameter is unused by the error message, so I did a small refactoring in main to clean that up. --- .../Source/SourceMemberContainerSymbol.cs | 2 +- .../Test/Semantic/Semantics/RecordTests.cs | 50 +++++++++---------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs index b2d1803e41124..df51a3425bc4f 100644 --- a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs +++ b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol.cs @@ -453,7 +453,7 @@ internal static void ReportReservedTypeName(string? name, CSharpCompilation comp { if (compilation.LanguageVersion >= MessageID.IDS_FeatureRecords.RequiredVersion()) { - diagnostics.Add(ErrorCode.WRN_RecordNamedDisallowed, location, name); + diagnostics.Add(ErrorCode.WRN_RecordNamedDisallowed, location); } } else if (IsReservedTypeName(name)) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs index 45b519df6a5bd..1e78065f9d519 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs @@ -2854,7 +2854,7 @@ record M(record r) => r; comp.VerifyDiagnostics( // (2,7): warning CS8860: Types and aliases should not be named 'record'. // class record { } - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(2, 7), + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(2, 7), // (6,24): error CS1514: { expected // record M(record r) => r; Diagnostic(ErrorCode.ERR_LbraceExpected, "=>").WithLocation(6, 24), @@ -2886,7 +2886,7 @@ struct record { } comp.VerifyDiagnostics( // (2,8): warning CS8860: Types and aliases should not be named 'record'. // struct record { } - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(2, 8) + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(2, 8) ); } @@ -2900,7 +2900,7 @@ interface record { } comp.VerifyDiagnostics( // (2,11): warning CS8860: Types and aliases should not be named 'record'. // interface record { } - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(2, 11) + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(2, 11) ); } @@ -2914,7 +2914,7 @@ enum record { } comp.VerifyDiagnostics( // (2,6): warning CS8860: Types and aliases should not be named 'record'. // enum record { } - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(2, 6) + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(2, 6) ); } @@ -2928,7 +2928,7 @@ public void TypeNamedRecord_Delegate() comp.VerifyDiagnostics( // (2,15): warning CS8860: Types and aliases should not be named 'record'. // delegate void record(); - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(2, 15) + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(2, 15) ); } @@ -2955,7 +2955,7 @@ public void TypeNamedRecord_Alias() Diagnostic(ErrorCode.HDN_UnusedUsingDirective, "using record = System.Console;").WithLocation(2, 1), // (2,7): warning CS8860: Types and aliases should not be named 'record'. // using record = System.Console; - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(2, 7) + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(2, 7) ); } @@ -2999,16 +2999,16 @@ void local() // 4 comp.VerifyDiagnostics( // (2,9): warning CS8860: Types and aliases should not be named 'record'. // class C { } // 1 - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(2, 9), + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(2, 9), // (5,18): warning CS8860: Types and aliases should not be named 'record'. // class Nested { } // 2 - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(5, 18), + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(5, 18), // (9,17): warning CS8860: Types and aliases should not be named 'record'. // void Method() { } // 3 - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(9, 17), + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(9, 17), // (13,20): warning CS8860: Types and aliases should not be named 'record'. // void local() // 4 - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(13, 20) + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(13, 20) ); } @@ -3078,31 +3078,31 @@ partial void Method4() { } // 9 comp.VerifyDiagnostics( // (3,17): warning CS8860: Types and aliases should not be named 'record'. // partial class C { } // 1 - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(3, 17), + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(3, 17), // (5,17): warning CS8860: Types and aliases should not be named 'record'. // partial class D { } // 2 - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(5, 17), + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(5, 17), // (8,17): warning CS8860: Types and aliases should not be named 'record'. // partial class D { } // 3 - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(8, 17), + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(8, 17), // (9,17): warning CS8860: Types and aliases should not be named 'record'. // partial class D { } // 4 - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(9, 17), + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(9, 17), // (16,26): warning CS8860: Types and aliases should not be named 'record'. // partial class Nested { } // 5 - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(16, 26), + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(16, 26), // (22,25): warning CS8860: Types and aliases should not be named 'record'. // partial void Method() { } // 6 - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(22, 25), + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(22, 25), // (25,26): warning CS8860: Types and aliases should not be named 'record'. // partial void Method2(); // 7 - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(25, 26), + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(25, 26), // (27,26): warning CS8860: Types and aliases should not be named 'record'. // partial void Method3(); // 8 - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(27, 26), + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(27, 26), // (30,26): warning CS8860: Types and aliases should not be named 'record'. // partial void Method4() { } // 9 - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(30, 26) + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(30, 26) ); } @@ -3116,7 +3116,7 @@ record record { } comp.VerifyDiagnostics( // (2,8): warning CS8860: Types and aliases should not be named 'record'. // record record { } - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(2, 8) + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(2, 8) ); } @@ -3131,10 +3131,10 @@ partial class record { } comp.VerifyDiagnostics( // (2,15): warning CS8860: Types and aliases should not be named 'record'. // partial class record { } - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(2, 15), + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(2, 15), // (3,15): warning CS8860: Types and aliases should not be named 'record'. // partial class record { } - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(3, 15) + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(3, 15) ); } @@ -3159,7 +3159,7 @@ partial class record { } comp.VerifyDiagnostics( // (3,15): warning CS8860: Types and aliases should not be named 'record'. // partial class record { } - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(3, 15) + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(3, 15) ); } @@ -3174,7 +3174,7 @@ partial class @record { } comp.VerifyDiagnostics( // (2,15): warning CS8860: Types and aliases should not be named 'record'. // partial class record { } - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(2, 15) + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(2, 15) ); } @@ -3213,7 +3213,7 @@ public static void Main() comp.VerifyEmitDiagnostics( // (2,7): warning CS8860: Types and aliases should not be named 'record'. // class record { } - Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithArguments("record").WithLocation(2, 7) + Diagnostic(ErrorCode.WRN_RecordNamedDisallowed, "record").WithLocation(2, 7) ); CompileAndVerify(comp, expectedOutput: "RAN"); } From 6d4210e8b1dde3f0a1c85fefd96964597a01c389 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 17:38:56 -0800 Subject: [PATCH 21/41] Fix --- .../SolutionState.CompilationTracker.CompilationTrackerState.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs index 679d2f109c71a..e5ccf212e9807 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs @@ -138,7 +138,7 @@ protected CompilationTrackerState( // As a sanity check, we should never see the generated trees inside of the compilation that should not // have generated trees. - var compilation = compilationWithoutGeneratedDocuments?.GetValueOrNull(); + var compilation = compilationWithoutGeneratedDocuments; if (compilation != null) { From 6d52178b543a3ff71dab518493734e0f13163008 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 17:41:45 -0800 Subject: [PATCH 22/41] Simplify further --- ...lutionState.CompilationTracker.CompilationTrackerState.cs | 5 +---- .../Workspace/Solution/SolutionState.CompilationTracker.cs | 2 -- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs index e5ccf212e9807..9c2d1f4aabe49 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.CompilationTrackerState.cs @@ -261,12 +261,11 @@ private sealed class FinalState : CompilationTrackerState private FinalState( Compilation finalCompilation, - Compilation compilationWithoutGeneratedFilesSource, Compilation compilationWithoutGeneratedFiles, bool hasSuccessfullyLoaded, CompilationTrackerGeneratorInfo generatorInfo, UnrootedSymbolSet unrootedSymbolSet) - : base(compilationWithoutGeneratedFilesSource, + : base(compilationWithoutGeneratedFiles, generatorInfo.WithDocumentsAreFinal(true)) // when we're in a final state, we've ran generators and should not run again { Contract.ThrowIfNull(finalCompilation); @@ -287,7 +286,6 @@ private FinalState( /// Not held onto public static FinalState Create( Compilation finalCompilationSource, - Compilation compilationWithoutGeneratedFilesSource, Compilation compilationWithoutGeneratedFiles, bool hasSuccessfullyLoaded, CompilationTrackerGeneratorInfo generatorInfo, @@ -303,7 +301,6 @@ public static FinalState Create( return new FinalState( finalCompilationSource, - compilationWithoutGeneratedFilesSource, compilationWithoutGeneratedFiles, hasSuccessfullyLoaded, generatorInfo, diff --git a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs index a6ea2170d53f9..6eafed98ca82b 100644 --- a/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs +++ b/src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs @@ -206,7 +206,6 @@ public ICompilationTracker FreezePartialStateWithTree(SolutionState solution, Do // As a policy, all partial-state projects are said to have incomplete references, since the state has no guarantees. var finalState = FinalState.Create( finalCompilationSource: compilationPair.CompilationWithGeneratedDocuments, - compilationWithoutGeneratedFilesSource: compilationPair.CompilationWithoutGeneratedDocuments, compilationWithoutGeneratedFiles: compilationPair.CompilationWithoutGeneratedDocuments, hasSuccessfullyLoaded: false, generatorInfo, @@ -916,7 +915,6 @@ private async Task FinalizeCompilationAsync( var finalState = FinalState.Create( compilationWithGenerators, compilationWithoutGenerators, - compilationWithoutGenerators, hasSuccessfullyLoaded, generatorInfo, compilationWithGenerators, From f4d26789ea0d2ef370ed8dc20c6115d389c6f98d Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 18:13:28 -0800 Subject: [PATCH 23/41] ove files --- src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems | 5 +++-- .../ConvertNamespace/ConvertNamespaceCodeFixProvider.cs | 0 .../Portable}/ConvertNamespace/ConvertNamespaceTransform.cs | 0 3 files changed, 3 insertions(+), 2 deletions(-) rename src/{Analyzers/CSharp/CodeFixes => Features/CSharp/Portable}/ConvertNamespace/ConvertNamespaceCodeFixProvider.cs (100%) rename src/{Analyzers/CSharp/CodeFixes => Features/CSharp/Portable}/ConvertNamespace/ConvertNamespaceTransform.cs (100%) diff --git a/src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems b/src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems index bfdd9f0689a23..c58db92c96184 100644 --- a/src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems +++ b/src/Analyzers/CSharp/CodeFixes/CSharpCodeFixes.projitems @@ -16,8 +16,6 @@ - - @@ -78,4 +76,7 @@ + + + \ No newline at end of file diff --git a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceCodeFixProvider.cs b/src/Features/CSharp/Portable/ConvertNamespace/ConvertNamespaceCodeFixProvider.cs similarity index 100% rename from src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceCodeFixProvider.cs rename to src/Features/CSharp/Portable/ConvertNamespace/ConvertNamespaceCodeFixProvider.cs diff --git a/src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs b/src/Features/CSharp/Portable/ConvertNamespace/ConvertNamespaceTransform.cs similarity index 100% rename from src/Analyzers/CSharp/CodeFixes/ConvertNamespace/ConvertNamespaceTransform.cs rename to src/Features/CSharp/Portable/ConvertNamespace/ConvertNamespaceTransform.cs From f6d8885211633acef230c4e7e2b577286c7017d7 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 18:16:28 -0800 Subject: [PATCH 24/41] Move file --- .../Portable}/Formatting/FormattingOptions.IndentStyle.cs | 4 ---- .../Compiler/Core/CompilerExtensions.projitems | 1 - 2 files changed, 5 deletions(-) rename src/Workspaces/{SharedUtilitiesAndExtensions/Compiler/Core => Core/Portable}/Formatting/FormattingOptions.IndentStyle.cs (85%) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Formatting/FormattingOptions.IndentStyle.cs b/src/Workspaces/Core/Portable/Formatting/FormattingOptions.IndentStyle.cs similarity index 85% rename from src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Formatting/FormattingOptions.IndentStyle.cs rename to src/Workspaces/Core/Portable/Formatting/FormattingOptions.IndentStyle.cs index c381b219d88ae..110cf8f24dcf8 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Formatting/FormattingOptions.IndentStyle.cs +++ b/src/Workspaces/Core/Portable/Formatting/FormattingOptions.IndentStyle.cs @@ -4,11 +4,7 @@ namespace Microsoft.CodeAnalysis.Formatting { -#if CODE_STYLE - internal static class FormattingOptions -#else public static partial class FormattingOptions -#endif { public enum IndentStyle { diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems index 896e22e8878b3..31df451a34feb 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems @@ -300,7 +300,6 @@ - From c064f9efdf0211bd8acdb89bec1ee2a3196d0849 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 18:49:41 -0800 Subject: [PATCH 25/41] Fix tests --- .../SolutionTests/SolutionWithSourceGeneratorTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Workspaces/CoreTest/SolutionTests/SolutionWithSourceGeneratorTests.cs b/src/Workspaces/CoreTest/SolutionTests/SolutionWithSourceGeneratorTests.cs index d91bc55c6c2d5..4eb65cbf75144 100644 --- a/src/Workspaces/CoreTest/SolutionTests/SolutionWithSourceGeneratorTests.cs +++ b/src/Workspaces/CoreTest/SolutionTests/SolutionWithSourceGeneratorTests.cs @@ -343,7 +343,7 @@ public async Task RequestingGeneratedDocumentsTwiceGivesSameInstance() // finalizing a compilation more than once doesn't recreate things incorrectly or run the generator more than once. generatorRan = false; var compilationReference = ObjectReference.CreateFromFactory(() => project.GetCompilationAsync().Result); - compilationReference.AssertReleased(); + compilationReference.AssertHeld(); var secondCompilation = await project.GetRequiredCompilationAsync(CancellationToken.None); var generatedDocumentSecondTime = Assert.Single(await project.GetSourceGeneratedDocumentsAsync()); @@ -705,7 +705,7 @@ public async Task ForkAfterFreezeNoLongerRunsGeneratorsEvenIfCompilationFallsAwa Assert.True(generatorRan); generatorRan = false; - compilationReference.AssertReleased(); + compilationReference.AssertHeld(); var document = project.Documents.Single().WithFrozenPartialSemantics(CancellationToken.None); @@ -729,7 +729,7 @@ public async Task ChangesToAdditionalFilesCorrectlyAppliedEvenIfCompilationFalls .AddAdditionalDocument("Test.txt", "Hello, world!").Project; var compilationReference = ObjectReference.CreateFromFactory(() => project.GetRequiredCompilationAsync(CancellationToken.None).Result); - compilationReference.AssertReleased(); + compilationReference.AssertHeld(); var projectWithoutAdditionalFiles = project.RemoveAdditionalDocument(project.AdditionalDocumentIds.Single()); Assert.Empty((await projectWithoutAdditionalFiles.GetRequiredCompilationAsync(CancellationToken.None)).SyntaxTrees); From 63d6adb956f9dfcfd4d06cf7bbe528860ad100d4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 20 Dec 2021 19:15:55 -0800 Subject: [PATCH 26/41] lint --- .../Portable/ConvertNamespace/ConvertNamespaceTransform.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Features/CSharp/Portable/ConvertNamespace/ConvertNamespaceTransform.cs b/src/Features/CSharp/Portable/ConvertNamespace/ConvertNamespaceTransform.cs index b8f4dc29bfe80..ca9ee3864ee40 100644 --- a/src/Features/CSharp/Portable/ConvertNamespace/ConvertNamespaceTransform.cs +++ b/src/Features/CSharp/Portable/ConvertNamespace/ConvertNamespaceTransform.cs @@ -44,7 +44,7 @@ public static async Task ConvertAsync(Document document, BaseNamespace default: throw ExceptionUtilities.UnexpectedValue(baseNamespace.Kind()); - }; + } } public static async Task<(Document document, TextSpan semicolonSpan)> ConvertNamespaceDeclarationAsync(Document document, NamespaceDeclarationSyntax namespaceDeclaration, CancellationToken cancellationToken) From 2dab552234983b459d61af26f3871ec57ba46967 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Wed, 22 Dec 2021 08:44:57 +1100 Subject: [PATCH 27/41] Use ElasticSpace instead of ElasticZeroSpace (#58152) --- ...veUnnecessaryExpressionParenthesesTests.cs | 28 ++++++++++++++++++- .../AbstractChangeSignatureService.cs | 7 ----- .../CodeGeneration/CSharpSyntaxGenerator.cs | 9 ++---- .../Core/Portable/Editing/SyntaxGenerator.cs | 2 +- .../TriviaDataFactory.CodeShapeAnalyzer.cs | 12 ++++++++ .../VisualBasicSyntaxGenerator.vb | 8 ++---- 6 files changed, 44 insertions(+), 22 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/RemoveUnnecessaryParentheses/RemoveUnnecessaryExpressionParenthesesTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnnecessaryParentheses/RemoveUnnecessaryExpressionParenthesesTests.cs index 834a99db1e4e4..35ab81a00acf0 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnnecessaryParentheses/RemoveUnnecessaryExpressionParenthesesTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnnecessaryParentheses/RemoveUnnecessaryExpressionParenthesesTests.cs @@ -2120,7 +2120,7 @@ void M() { void M() { -#ifA || B +#if A || B #endif } }", @@ -2151,6 +2151,32 @@ void M() offeredWhenRequireForClarityIsEnabled: true, index: 1); } + [WorkItem(57768, "https://github.com/dotnet/roslyn/issues/57768")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryParentheses)] + public async Task TestParensAroundPPDirective3() + { + await TestAsync( +@"class C +{ + void M() + { +#if C +#elif$$(A || B) +#endif + } +}", +@"class C +{ + void M() + { +#if C +#elif A || B +#endif + } +}", +offeredWhenRequireForClarityIsEnabled: true); + } + [WorkItem(29454, "https://github.com/dotnet/roslyn/issues/29454")] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnnecessaryParentheses)] public async Task TestMissingForPreIncrement() diff --git a/src/Features/Core/Portable/ChangeSignature/AbstractChangeSignatureService.cs b/src/Features/Core/Portable/ChangeSignature/AbstractChangeSignatureService.cs index 0c6c0f5ce1817..b27637de58c64 100644 --- a/src/Features/Core/Portable/ChangeSignature/AbstractChangeSignatureService.cs +++ b/src/Features/Core/Portable/ChangeSignature/AbstractChangeSignatureService.cs @@ -950,7 +950,6 @@ protected ImmutableArray GetPermutedDocCommentTrivia(Document docu { var updatedLeadingTrivia = ImmutableArray.CreateBuilder(); var index = 0; - SyntaxTrivia lastWhiteSpaceTrivia = default; var syntaxFacts = document.GetRequiredLanguageService(); @@ -958,11 +957,6 @@ protected ImmutableArray GetPermutedDocCommentTrivia(Document docu { if (!trivia.HasStructure) { - if (syntaxFacts.IsWhitespaceTrivia(trivia)) - { - lastWhiteSpaceTrivia = trivia; - } - updatedLeadingTrivia.Add(trivia); continue; } @@ -1015,7 +1009,6 @@ protected ImmutableArray GetPermutedDocCommentTrivia(Document docu var extraDocComments = Generator.DocumentationCommentTrivia( extraNodeList, node.GetTrailingTrivia(), - lastWhiteSpaceTrivia, document.Project.Solution.Options.GetOption(FormattingOptions.NewLine, document.Project.Language)); var newTrivia = Generator.Trivia(extraDocComments); diff --git a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs index beb2041b9e5cf..8468718b34530 100644 --- a/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs +++ b/src/Workspaces/CSharp/Portable/CodeGeneration/CSharpSyntaxGenerator.cs @@ -80,7 +80,7 @@ internal override SyntaxTrivia Trivia(SyntaxNode node) throw ExceptionUtilities.UnexpectedValue(node.Kind()); } - internal override SyntaxNode DocumentationCommentTrivia(IEnumerable nodes, SyntaxTriviaList trailingTrivia, SyntaxTrivia lastWhitespaceTrivia, string endOfLineString) + internal override SyntaxNode DocumentationCommentTrivia(IEnumerable nodes, SyntaxTriviaList trailingTrivia, string endOfLineString) { var docTrivia = SyntaxFactory.DocumentationCommentTrivia( SyntaxKind.MultiLineDocumentationCommentTrivia, @@ -90,12 +90,7 @@ internal override SyntaxNode DocumentationCommentTrivia(IEnumerable docTrivia = docTrivia.WithLeadingTrivia(SyntaxFactory.DocumentationCommentExterior("/// ")) .WithTrailingTrivia(trailingTrivia); - if (lastWhitespaceTrivia == default) - return docTrivia.WithTrailingTrivia(SyntaxFactory.EndOfLine(endOfLineString)); - - return docTrivia.WithTrailingTrivia( - SyntaxFactory.EndOfLine(endOfLineString), - lastWhitespaceTrivia); + return docTrivia.WithTrailingTrivia(SyntaxFactory.EndOfLine(endOfLineString)); } internal override SyntaxNode DocumentationCommentTriviaWithUpdatedContent(SyntaxTrivia trivia, IEnumerable content) diff --git a/src/Workspaces/Core/Portable/Editing/SyntaxGenerator.cs b/src/Workspaces/Core/Portable/Editing/SyntaxGenerator.cs index 55b0c1a5d72c1..a1f5de87e5a8b 100644 --- a/src/Workspaces/Core/Portable/Editing/SyntaxGenerator.cs +++ b/src/Workspaces/Core/Portable/Editing/SyntaxGenerator.cs @@ -2194,7 +2194,7 @@ internal SyntaxNode AddParentheses(SyntaxNode expression, bool includeElasticTri internal abstract SyntaxTrivia Trivia(SyntaxNode node); - internal abstract SyntaxNode DocumentationCommentTrivia(IEnumerable nodes, SyntaxTriviaList trailingTrivia, SyntaxTrivia lastWhitespaceTrivia, string endOfLineString); + internal abstract SyntaxNode DocumentationCommentTrivia(IEnumerable nodes, SyntaxTriviaList trailingTrivia, string endOfLineString); internal abstract SyntaxNode DocumentationCommentTriviaWithUpdatedContent(SyntaxTrivia trivia, IEnumerable content); diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Formatting/Engine/Trivia/TriviaDataFactory.CodeShapeAnalyzer.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Formatting/Engine/Trivia/TriviaDataFactory.CodeShapeAnalyzer.cs index 5f468dba68861..536bd3bab7a43 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Formatting/Engine/Trivia/TriviaDataFactory.CodeShapeAnalyzer.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Formatting/Engine/Trivia/TriviaDataFactory.CodeShapeAnalyzer.cs @@ -106,6 +106,18 @@ private bool UseIndentation private static bool OnElastic(SyntaxTrivia trivia) { + // if this is structured trivia then we need to check for elastic trivia in any descendant + if (trivia.GetStructure() is { ContainsAnnotations: true } structure) + { + foreach (var t in structure.DescendantTrivia()) + { + if (t.IsElastic()) + { + return true; + } + } + } + // if it contains elastic trivia, always format return trivia.IsElastic(); } diff --git a/src/Workspaces/VisualBasic/Portable/CodeGeneration/VisualBasicSyntaxGenerator.vb b/src/Workspaces/VisualBasic/Portable/CodeGeneration/VisualBasicSyntaxGenerator.vb index 7778dff6bc081..27fc75c4dd436 100644 --- a/src/Workspaces/VisualBasic/Portable/CodeGeneration/VisualBasicSyntaxGenerator.vb +++ b/src/Workspaces/VisualBasic/Portable/CodeGeneration/VisualBasicSyntaxGenerator.vb @@ -65,16 +65,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CodeGeneration Throw ExceptionUtilities.UnexpectedValue(node.Kind()) End Function - Friend Overrides Function DocumentationCommentTrivia(nodes As IEnumerable(Of SyntaxNode), trailingTrivia As SyntaxTriviaList, lastWhitespaceTrivia As SyntaxTrivia, endOfLineString As String) As SyntaxNode + Friend Overrides Function DocumentationCommentTrivia(nodes As IEnumerable(Of SyntaxNode), trailingTrivia As SyntaxTriviaList, endOfLineString As String) As SyntaxNode Dim node = SyntaxFactory.DocumentationCommentTrivia(SyntaxFactory.List(nodes)) node = node.WithLeadingTrivia(SyntaxFactory.DocumentationCommentExteriorTrivia("''' ")). WithTrailingTrivia(node.GetTrailingTrivia()) - If lastWhitespaceTrivia = Nothing Then - Return node.WithTrailingTrivia(SyntaxFactory.EndOfLine(endOfLineString)) - End If - - Return node.WithTrailingTrivia(SyntaxFactory.EndOfLine(endOfLineString), lastWhitespaceTrivia) + Return node.WithTrailingTrivia(SyntaxFactory.EndOfLine(endOfLineString)) End Function Friend Overrides Function DocumentationCommentTriviaWithUpdatedContent(trivia As SyntaxTrivia, content As IEnumerable(Of SyntaxNode)) As SyntaxNode From e9d15a767c971d7ec248dbedfad452f4f26bbffd Mon Sep 17 00:00:00 2001 From: Jonathon Marolf Date: Tue, 21 Dec 2021 14:58:50 -0800 Subject: [PATCH 28/41] set compilers.sln as the default solution when opening with VS Code --- .vscode/settings.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 85140b2d44e86..399accea7023f 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -1,5 +1,6 @@ { "files.associations": { "**/eng/pipelines/*.yml": "azure-pipelines" - } + }, + "omnisharp.defaultLaunchSolution": "Compilers.sln", } \ No newline at end of file From 79c8b92af799856919bb3844b6635e8606970366 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Tue, 21 Dec 2021 15:35:14 -0800 Subject: [PATCH 29/41] Support TestWorkspace with multiple source generated files --- .../Core/SourceGeneration/TestGenerators.cs | 24 ++++++++++---- .../CSharpTest/NavigateTo/NavigateToTests.cs | 32 +++++++++++++++++++ .../TestWorkspace_XmlConsumption.cs | 11 +++++-- 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/src/Compilers/Test/Core/SourceGeneration/TestGenerators.cs b/src/Compilers/Test/Core/SourceGeneration/TestGenerators.cs index 57217b7bdad0a..f5da455501648 100644 --- a/src/Compilers/Test/Core/SourceGeneration/TestGenerators.cs +++ b/src/Compilers/Test/Core/SourceGeneration/TestGenerators.cs @@ -3,7 +3,8 @@ // See the LICENSE file in the project root for more information. using System; -using System.IO; +using System.Collections.Generic; +using System.Linq; using System.Text; using System.Threading; using Microsoft.CodeAnalysis; @@ -13,18 +14,27 @@ namespace Roslyn.Test.Utilities.TestGenerators { internal class SingleFileTestGenerator : ISourceGenerator { - private readonly string _content; - private readonly string _hintName; + private readonly List<(string content, string hintName)> _sources = new(); - public SingleFileTestGenerator(string content, string hintName = "generatedFile") + public SingleFileTestGenerator() { - _content = content; - _hintName = hintName; + } + + public SingleFileTestGenerator(string content, string? hintName = null) + { + AddSource(content, hintName); + } + + public void AddSource(string content, string? hintName = null) + { + hintName ??= "generatedFile" + (_sources.Any() ? (_sources.Count + 1).ToString() : ""); + _sources.Add((content, hintName)); } public void Execute(GeneratorExecutionContext context) { - context.AddSource(this._hintName, SourceText.From(_content, Encoding.UTF8)); + foreach (var (content, hintName) in _sources) + context.AddSource(hintName, SourceText.From(content, Encoding.UTF8)); } public void Initialize(GeneratorInitializationContext context) diff --git a/src/EditorFeatures/CSharpTest/NavigateTo/NavigateToTests.cs b/src/EditorFeatures/CSharpTest/NavigateTo/NavigateToTests.cs index 8156a2a56398e..00b8e03512eb6 100644 --- a/src/EditorFeatures/CSharpTest/NavigateTo/NavigateToTests.cs +++ b/src/EditorFeatures/CSharpTest/NavigateTo/NavigateToTests.cs @@ -1473,6 +1473,38 @@ public class C }, await _aggregator.GetItemsAsync("C")); } + + [Fact] + public async Task DoIncludeSymbolsFromMultipleSourceGeneratedFiles() + { + using var workspace = TestWorkspace.Create(@" + + + + public partial class C + { + } + + + public partial class C + { + } + + + +", composition: EditorTestCompositions.EditorFeatures); + + _provider = new NavigateToItemProvider(workspace, AsynchronousOperationListenerProvider.NullListener, workspace.GetService()); + _aggregator = new NavigateToTestAggregator(_provider); + + VerifyNavigateToResultItems( + new() + { + new NavigateToItem("C", NavigateToItemKind.Class, "csharp", null, null, s_emptyExactPatternMatch, null), + new NavigateToItem("C", NavigateToItemKind.Class, "csharp", null, null, s_emptyExactPatternMatch, null), + }, + await _aggregator.GetItemsAsync("C")); + } } } #pragma warning restore CS0618 // MatchKind is obsolete diff --git a/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_XmlConsumption.cs b/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_XmlConsumption.cs index 6dc371b3c01a4..09326b5f40882 100644 --- a/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_XmlConsumption.cs +++ b/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_XmlConsumption.cs @@ -14,7 +14,6 @@ using System.IO; using System.Linq; using System.Runtime.CompilerServices; -using System.ServiceModel.Description; using System.Threading; using System.Xml.Linq; using Microsoft.CodeAnalysis.CSharp; @@ -29,7 +28,6 @@ using Microsoft.CodeAnalysis.UnitTests; using Microsoft.CodeAnalysis.VisualBasic; using Microsoft.VisualStudio.Composition; -using Microsoft.VisualStudio.Text; using Roslyn.Test.Utilities; using Roslyn.Test.Utilities.TestGenerators; using Roslyn.Utilities; @@ -338,8 +336,15 @@ private static TestHostProject CreateProject( documents.Add(document); } + SingleFileTestGenerator testGenerator = null; foreach (var sourceGeneratedDocumentElement in projectElement.Elements(DocumentFromSourceGeneratorElementName)) { + if (testGenerator is null) + { + testGenerator = new SingleFileTestGenerator(); + analyzers.Add(new TestGeneratorReference(testGenerator)); + } + var name = GetFileName(workspace, sourceGeneratedDocumentElement, ref documentId); var markupCode = sourceGeneratedDocumentElement.NormalizedValue(); @@ -350,7 +355,7 @@ private static TestHostProject CreateProject( var document = new TestHostDocument(exportProvider, languageServices, code, name, documentFilePath, cursorPosition, spans, isSourceGenerated: true); documents.Add(document); - analyzers.Add(new TestGeneratorReference(new SingleFileTestGenerator(code, name))); + testGenerator.AddSource(code, name); } var additionalDocuments = new List(); From 7812951bd074f434af813135f87bf2bfd52cc82c Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Tue, 21 Dec 2021 15:41:40 -0800 Subject: [PATCH 30/41] Support TestWorkspace with source generated files without full XML syntax --- .../CrefCompletionProviderTests.cs | 2 +- .../OverrideCompletionProviderTests.cs | 2 +- .../CSharpTest/NavigateTo/NavigateToTests.cs | 31 +++++++++---------- ...osticsClassificationTaggerProviderTests.cs | 2 +- .../DiagnosticsSquiggleTaggerProviderTests.cs | 10 +++--- .../Workspaces/TestWorkspace_Create.cs | 14 ++++++--- .../TestWorkspace_XmlConsumption.cs | 1 + .../Workspaces/TestWorkspace_XmlCreation.cs | 24 +++++++++++--- .../CrefCompletionProviderTests.vb | 2 +- 9 files changed, 53 insertions(+), 35 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/CrefCompletionProviderTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/CrefCompletionProviderTests.cs index b9e38257411fe..7bf6a90253c83 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/CrefCompletionProviderTests.cs +++ b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/CrefCompletionProviderTests.cs @@ -440,7 +440,7 @@ public async Task CrefCompletionSpeculatesOutsideTrivia() class C { }"; - using var workspace = TestWorkspace.Create(LanguageNames.CSharp, new CSharpCompilationOptions(OutputKind.ConsoleApplication), new CSharpParseOptions(), new[] { text }, ExportProvider); + using var workspace = TestWorkspace.Create(LanguageNames.CSharp, new CSharpCompilationOptions(OutputKind.ConsoleApplication), new CSharpParseOptions(), new[] { text }, exportProvider: ExportProvider); var called = false; var hostDocument = workspace.DocumentWithCursor; diff --git a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/OverrideCompletionProviderTests.cs b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/OverrideCompletionProviderTests.cs index 7beee24d167d8..043ff896b0a6a 100644 --- a/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/OverrideCompletionProviderTests.cs +++ b/src/EditorFeatures/CSharpTest/Completion/CompletionProviders/OverrideCompletionProviderTests.cs @@ -2777,7 +2777,7 @@ static void Main(string[] args) override $$ } }"; - using var workspace = TestWorkspace.Create(LanguageNames.CSharp, new CSharpCompilationOptions(OutputKind.ConsoleApplication), new CSharpParseOptions(), new[] { text }, ExportProvider); + using var workspace = TestWorkspace.Create(LanguageNames.CSharp, new CSharpCompilationOptions(OutputKind.ConsoleApplication), new CSharpParseOptions(), new[] { text }, exportProvider: ExportProvider); var provider = new OverrideCompletionProvider(); var testDocument = workspace.Documents.Single(); var document = workspace.CurrentSolution.GetRequiredDocument(testDocument.Id); diff --git a/src/EditorFeatures/CSharpTest/NavigateTo/NavigateToTests.cs b/src/EditorFeatures/CSharpTest/NavigateTo/NavigateToTests.cs index 00b8e03512eb6..0aa8e949a25e4 100644 --- a/src/EditorFeatures/CSharpTest/NavigateTo/NavigateToTests.cs +++ b/src/EditorFeatures/CSharpTest/NavigateTo/NavigateToTests.cs @@ -2,6 +2,7 @@ // 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.Generic; using System.Linq; using System.Threading.Tasks; @@ -1477,22 +1478,20 @@ public class C [Fact] public async Task DoIncludeSymbolsFromMultipleSourceGeneratedFiles() { - using var workspace = TestWorkspace.Create(@" - - - - public partial class C - { - } - - - public partial class C - { - } - - - -", composition: EditorTestCompositions.EditorFeatures); + using var workspace = TestWorkspace.CreateCSharp( + files: Array.Empty(), + sourceGeneratedFiles: new[] + { + @" +public partial class C +{ +}", + @" +public partial class C +{ +}", + }, + composition: EditorTestCompositions.EditorFeatures); _provider = new NavigateToItemProvider(workspace, AsynchronousOperationListenerProvider.NullListener, workspace.GetService()); _aggregator = new NavigateToTestAggregator(_provider); diff --git a/src/EditorFeatures/Test/Diagnostics/DiagnosticsClassificationTaggerProviderTests.cs b/src/EditorFeatures/Test/Diagnostics/DiagnosticsClassificationTaggerProviderTests.cs index 87d71ba66948d..bd36ed6d69eb6 100644 --- a/src/EditorFeatures/Test/Diagnostics/DiagnosticsClassificationTaggerProviderTests.cs +++ b/src/EditorFeatures/Test/Diagnostics/DiagnosticsClassificationTaggerProviderTests.cs @@ -37,7 +37,7 @@ public async Task Test_FadingSpans() { LanguageNames.CSharp, ImmutableArray.Create(analyzer) } }; - using var workspace = TestWorkspace.CreateCSharp(new string[] { "class A { }", "class E { }" }, CSharpParseOptions.Default, composition: SquiggleUtilities.CompositionWithSolutionCrawler); + using var workspace = TestWorkspace.CreateCSharp(new string[] { "class A { }", "class E { }" }, parseOptions: CSharpParseOptions.Default, composition: SquiggleUtilities.CompositionWithSolutionCrawler); using var wrapper = new DiagnosticTaggerWrapper(workspace, analyzerMap); var tagger = wrapper.TaggerProvider.CreateTagger(workspace.Documents.First().GetTextBuffer()); using var disposable = tagger as IDisposable; diff --git a/src/EditorFeatures/Test/Diagnostics/DiagnosticsSquiggleTaggerProviderTests.cs b/src/EditorFeatures/Test/Diagnostics/DiagnosticsSquiggleTaggerProviderTests.cs index f84913a7e50a7..6f7aa429bfae8 100644 --- a/src/EditorFeatures/Test/Diagnostics/DiagnosticsSquiggleTaggerProviderTests.cs +++ b/src/EditorFeatures/Test/Diagnostics/DiagnosticsSquiggleTaggerProviderTests.cs @@ -41,7 +41,7 @@ public async Task Test_TagSourceDiffer() { LanguageNames.CSharp, ImmutableArray.Create(analyzer) } }; - using var workspace = TestWorkspace.CreateCSharp(new string[] { "class A { }", "class E { }" }, CSharpParseOptions.Default); + using var workspace = TestWorkspace.CreateCSharp(new string[] { "class A { }", "class E { }" }, parseOptions: CSharpParseOptions.Default); using var wrapper = new DiagnosticTaggerWrapper(workspace, analyzerMap); var tagger = wrapper.TaggerProvider.CreateTagger(workspace.Documents.First().GetTextBuffer()); using var disposable = tagger as IDisposable; @@ -69,7 +69,7 @@ public async Task Test_TagSourceDiffer() [WpfFact, Trait(Traits.Feature, Traits.Features.Diagnostics)] public async Task MultipleTaggersAndDispose() { - using var workspace = TestWorkspace.CreateCSharp(new string[] { "class A {" }, CSharpParseOptions.Default); + using var workspace = TestWorkspace.CreateCSharp(new string[] { "class A {" }, parseOptions: CSharpParseOptions.Default); using var wrapper = new DiagnosticTaggerWrapper(workspace); // Make two taggers. var tagger1 = wrapper.TaggerProvider.CreateTagger(workspace.Documents.First().GetTextBuffer()); @@ -89,7 +89,7 @@ public async Task MultipleTaggersAndDispose() [WpfFact, Trait(Traits.Feature, Traits.Features.Diagnostics)] public async Task TaggerProviderCreatedAfterInitialDiagnosticsReported() { - using var workspace = TestWorkspace.CreateCSharp(new string[] { "class C {" }, CSharpParseOptions.Default); + using var workspace = TestWorkspace.CreateCSharp(new string[] { "class C {" }, parseOptions: CSharpParseOptions.Default); using var wrapper = new DiagnosticTaggerWrapper(workspace, analyzerMap: null, createTaggerProvider: false); // First, make sure all diagnostics have been reported. await wrapper.WaitForTags(); @@ -119,7 +119,7 @@ public async Task TestWithMockDiagnosticService_TaggerProviderCreatedBeforeIniti using var workspace = TestWorkspace.CreateCSharp( new string[] { "class A { }" }, - CSharpParseOptions.Default, + parseOptions: CSharpParseOptions.Default, composition: s_compositionWithMockDiagnosticService); var listenerProvider = workspace.ExportProvider.GetExportedValue(); @@ -156,7 +156,7 @@ public async Task TestWithMockDiagnosticService_TaggerProviderCreatedAfterInitia using var workspace = TestWorkspace.CreateCSharp( new string[] { "class A { }" }, - CSharpParseOptions.Default, + parseOptions: CSharpParseOptions.Default, composition: s_compositionWithMockDiagnosticService); var listenerProvider = workspace.ExportProvider.GetExportedValue(); diff --git a/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_Create.cs b/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_Create.cs index 4fe217f35f5d9..5da952c24d78a 100644 --- a/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_Create.cs +++ b/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_Create.cs @@ -4,6 +4,7 @@ #nullable disable +using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq; @@ -128,6 +129,7 @@ internal static TestWorkspace Create( CompilationOptions compilationOptions, ParseOptions parseOptions, string[] files, + string[] sourceGeneratedFiles = null, ExportProvider exportProvider = null, TestComposition composition = null, string[] metadataReferences = null, @@ -137,7 +139,7 @@ internal static TestWorkspace Create( bool openDocuments = false, IDocumentServiceProvider documentServiceProvider = null) { - var workspaceElement = CreateWorkspaceElement(language, compilationOptions, parseOptions, files, metadataReferences, extension, commonReferences); + var workspaceElement = CreateWorkspaceElement(language, compilationOptions, parseOptions, files, sourceGeneratedFiles, metadataReferences, extension, commonReferences); return Create(workspaceElement, openDocuments, exportProvider, composition, workspaceKind, documentServiceProvider); } @@ -193,11 +195,12 @@ public static TestWorkspace CreateCSharp( string[] metadataReferences = null, bool openDocuments = false) { - return CreateCSharp(new[] { file }, parseOptions, compilationOptions, exportProvider, composition, metadataReferences, openDocuments); + return CreateCSharp(new[] { file }, Array.Empty(), parseOptions, compilationOptions, exportProvider, composition, metadataReferences, openDocuments); } public static TestWorkspace CreateCSharp( string[] files, + string[] sourceGeneratedFiles = null, ParseOptions parseOptions = null, CompilationOptions compilationOptions = null, ExportProvider exportProvider = null, @@ -205,7 +208,7 @@ public static TestWorkspace CreateCSharp( string[] metadataReferences = null, bool openDocuments = false) { - return Create(LanguageNames.CSharp, compilationOptions, parseOptions, files, exportProvider, composition, metadataReferences, openDocuments: openDocuments); + return Create(LanguageNames.CSharp, compilationOptions, parseOptions, files, sourceGeneratedFiles, exportProvider, composition, metadataReferences, openDocuments: openDocuments); } public static TestWorkspace CreateCSharp2( @@ -230,11 +233,12 @@ public static TestWorkspace CreateVisualBasic( string[] metadataReferences = null, bool openDocuments = false) { - return CreateVisualBasic(new[] { file }, parseOptions, compilationOptions, exportProvider, composition, metadataReferences, openDocuments); + return CreateVisualBasic(new[] { file }, Array.Empty(), parseOptions, compilationOptions, exportProvider, composition, metadataReferences, openDocuments); } public static TestWorkspace CreateVisualBasic( string[] files, + string[] sourceGeneratedFiles = null, ParseOptions parseOptions = null, CompilationOptions compilationOptions = null, ExportProvider exportProvider = null, @@ -242,7 +246,7 @@ public static TestWorkspace CreateVisualBasic( string[] metadataReferences = null, bool openDocuments = false) { - return Create(LanguageNames.VisualBasic, compilationOptions, parseOptions, files, exportProvider, composition, metadataReferences, openDocuments: openDocuments); + return Create(LanguageNames.VisualBasic, compilationOptions, parseOptions, files, sourceGeneratedFiles, exportProvider, composition, metadataReferences, openDocuments: openDocuments); } /// Can pass in multiple file contents with individual source kind: files will be named test1.vb, test2.vbx, etc. diff --git a/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_XmlConsumption.cs b/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_XmlConsumption.cs index 09326b5f40882..bd2b15e80b91e 100644 --- a/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_XmlConsumption.cs +++ b/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_XmlConsumption.cs @@ -101,6 +101,7 @@ internal void InitializeDocuments( compilationOptions, parseOptions, files, + sourceGeneratedFiles: Array.Empty(), metadataReferences, extension, commonReferences); diff --git a/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_XmlCreation.cs b/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_XmlCreation.cs index 0b7c578644977..6a10834bdce79 100644 --- a/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_XmlCreation.cs +++ b/src/EditorFeatures/TestUtilities/Workspaces/TestWorkspace_XmlCreation.cs @@ -4,12 +4,10 @@ #nullable disable -using System; using System.Collections.Generic; using System.Linq; using System.Xml.Linq; using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.Text; using Microsoft.CodeAnalysis.VisualBasic; using Roslyn.Utilities; @@ -22,23 +20,31 @@ internal static XElement CreateWorkspaceElement( CompilationOptions compilationOptions = null, ParseOptions parseOptions = null, string[] files = null, + string[] sourceGeneratedFiles = null, string[] metadataReferences = null, string extension = null, bool commonReferences = true) { var documentElements = new List(); + var index = 0; + extension ??= (language == LanguageNames.CSharp) ? CSharpExtension : VisualBasicExtension; if (files != null) { - var index = 0; - extension ??= (language == LanguageNames.CSharp) ? CSharpExtension : VisualBasicExtension; - foreach (var file in files) { documentElements.Add(CreateDocumentElement(file, GetDefaultTestSourceDocumentName(index++, extension), parseOptions)); } } + if (sourceGeneratedFiles != null) + { + foreach (var file in sourceGeneratedFiles) + { + documentElements.Add(CreateDocumentFromSourceGeneratorElement(file, GetDefaultTestSourceDocumentName(index++, extension), parseOptions)); + } + } + if (metadataReferences != null) { foreach (var reference in metadataReferences) @@ -171,6 +177,14 @@ protected static XElement CreateDocumentElement(string code, string filePath, Pa code.Replace("\r\n", "\n")); } + protected static XElement CreateDocumentFromSourceGeneratorElement(string code, string hintName, ParseOptions parseOptions = null) + { + return new XElement(DocumentFromSourceGeneratorElementName, + new XAttribute(FilePathAttributeName, hintName), + CreateParseOptionsElement(parseOptions), + code.Replace("\r\n", "\n")); + } + private static XElement CreateParseOptionsElement(ParseOptions parseOptions) { return parseOptions == null diff --git a/src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/CrefCompletionProviderTests.vb b/src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/CrefCompletionProviderTests.vb index 3c809d3e366e1..6bc616e38ba64 100644 --- a/src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/CrefCompletionProviderTests.vb +++ b/src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/CrefCompletionProviderTests.vb @@ -414,7 +414,7 @@ Class C End Sub End Class]]>.Value.NormalizeLineEndings() - Using workspace = TestWorkspace.Create(LanguageNames.VisualBasic, New VisualBasicCompilationOptions(OutputKind.ConsoleApplication), New VisualBasicParseOptions(), {text}, ExportProvider) + Using workspace = TestWorkspace.Create(LanguageNames.VisualBasic, New VisualBasicCompilationOptions(OutputKind.ConsoleApplication), New VisualBasicParseOptions(), {text}, exportProvider:=ExportProvider) Dim called = False Dim hostDocument = workspace.DocumentWithCursor From be874ec9fc872c37de1f81590ab5c2a415428bf2 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Tue, 21 Dec 2021 21:24:41 -0800 Subject: [PATCH 31/41] Clear prior build errors for additional and analyzer config docs at start of build Fixes [AB#1424584](https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1424584) The primary issue is that the lingering compiler warning CS0169 from prior solution snapshot is reported in `Index.razor`, which is an Additional document, not a regular source document. Existing code in clearing build diagnostics at start of a build was only clearing diagnostics with location in source documents. Doing the same for additional documents fixes this issue. --- .../TaskList/ExternalErrorDiagnosticUpdateSource.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/VisualStudio/Core/Def/Implementation/TaskList/ExternalErrorDiagnosticUpdateSource.cs b/src/VisualStudio/Core/Def/Implementation/TaskList/ExternalErrorDiagnosticUpdateSource.cs index 4488bddd0e8c8..b9facb2cb6f0d 100644 --- a/src/VisualStudio/Core/Def/Implementation/TaskList/ExternalErrorDiagnosticUpdateSource.cs +++ b/src/VisualStudio/Core/Def/Implementation/TaskList/ExternalErrorDiagnosticUpdateSource.cs @@ -383,7 +383,7 @@ private void ClearBuildOnlyProjectErrors(Solution solution, ProjectId? projectId } // Remove all document errors - foreach (var documentId in project.DocumentIds) + foreach (var documentId in project.DocumentIds.Concat(project.AdditionalDocumentIds).Concat(project.AnalyzerConfigDocumentIds)) { ClearBuildOnlyDocumentErrors(solution, projectId, documentId); } From e343cb049b9c9050cd521412560c413737f0255d Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Tue, 21 Dec 2021 22:24:02 -0800 Subject: [PATCH 32/41] Fix MakeFieldReadonly false positive for ref assignments Fixes #46785 Fixes #57983 --- .../MakeFieldReadonlyTests.cs | 24 +++++++++++++++++-- .../Core/Extensions/OperationExtensions.cs | 6 +++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs b/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs index 811f51493f746..9a7d768db6754 100644 --- a/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs +++ b/src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs @@ -1717,13 +1717,13 @@ await TestInRegularAndScript1Async( } [WorkItem(46785, "https://github.com/dotnet/roslyn/issues/46785")] - [Fact(Skip = "https://github.com/dotnet/roslyn/issues/46785"), Trait(Traits.Feature, Traits.Features.CodeActionsMakeFieldReadonly)] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsMakeFieldReadonly)] public async Task UsedAsRef_NoDiagnostic() { await TestMissingInRegularAndScriptAsync( @"public class C { - private string [|_x|] = string.Empty; + private string [|x|] = string.Empty; public bool M() { @@ -1733,6 +1733,26 @@ public bool M() }"); } + [WorkItem(57983, "https://github.com/dotnet/roslyn/issues/57983")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsMakeFieldReadonly)] + public async Task UsedAsRef_NoDiagnostic_02() + { + await TestMissingInRegularAndScriptAsync( +@"using System.Runtime.CompilerServices; + +public class Test +{ + private ulong [|nextD3D12ComputeFenceValue|]; + + internal void Repro() + { + ref ulong d3D12FenceValue = ref Unsafe.NullRef(); + d3D12FenceValue = ref nextD3D12ComputeFenceValue; + d3D12FenceValue++; + } +}"); + } + [WorkItem(42760, "https://github.com/dotnet/roslyn/issues/42760")] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsMakeFieldReadonly)] public async Task WithThreadStaticAttribute_NoDiagnostic() diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs index 800e930574599..3bde34e3df8b5 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs @@ -126,6 +126,12 @@ INegatedPatternOperation or ? ValueUsageInfo.ReadWrite : ValueUsageInfo.Write; } + else if (operation.Parent is ISimpleAssignmentOperation simpleAssignmentOperation && + simpleAssignmentOperation.Value == operation && + simpleAssignmentOperation.IsRef) + { + return ValueUsageInfo.ReadableWritableReference; + } else if (operation.Parent is IIncrementOrDecrementOperation) { return ValueUsageInfo.ReadWrite; From 5a6a566c7542553eb3f126d618b8f84530f78f14 Mon Sep 17 00:00:00 2001 From: "dotnet-maestro[bot]" <42748379+dotnet-maestro[bot]@users.noreply.github.com> Date: Wed, 22 Dec 2021 14:58:12 +0000 Subject: [PATCH 33/41] [main] Update dependencies from dotnet/arcade (#58408) [main] Update dependencies from dotnet/arcade --- eng/Version.Details.xml | 8 ++++---- eng/common/dotnet-install.sh | 3 +++ eng/common/native/CommonLibrary.psm1 | 3 ++- global.json | 4 ++-- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml index 85896ac039e85..fd98a9b586b16 100644 --- a/eng/Version.Details.xml +++ b/eng/Version.Details.xml @@ -13,18 +13,18 @@ - + https://github.com/dotnet/arcade - 943d03f62955c771825dfa1f1bdeb8f853a2d7dd + 0cd94b1d02c03377d99f3739beb191591f6abee5 https://github.com/dotnet/roslyn 818313426323d979747781a17c78860c833776da - + https://github.com/dotnet/arcade - 943d03f62955c771825dfa1f1bdeb8f853a2d7dd + 0cd94b1d02c03377d99f3739beb191591f6abee5 diff --git a/eng/common/dotnet-install.sh b/eng/common/dotnet-install.sh index fdfeea66e7d43..5c94e98632a0a 100755 --- a/eng/common/dotnet-install.sh +++ b/eng/common/dotnet-install.sh @@ -55,6 +55,9 @@ case $cpuname in aarch64) buildarch=arm64 ;; + loongarch64) + buildarch=loongarch64 + ;; amd64|x86_64) buildarch=x64 ;; diff --git a/eng/common/native/CommonLibrary.psm1 b/eng/common/native/CommonLibrary.psm1 index adf707c8fe700..ca38268c44d83 100644 --- a/eng/common/native/CommonLibrary.psm1 +++ b/eng/common/native/CommonLibrary.psm1 @@ -276,7 +276,8 @@ function Get-MachineArchitecture { } if (($ProcessorArchitecture -Eq "AMD64") -Or ($ProcessorArchitecture -Eq "IA64") -Or - ($ProcessorArchitecture -Eq "ARM64")) { + ($ProcessorArchitecture -Eq "ARM64") -Or + ($ProcessorArchitecture -Eq "LOONGARCH64")) { return "x64" } return "x86" diff --git a/global.json b/global.json index fd86f0793bb3e..02b28209e68ee 100644 --- a/global.json +++ b/global.json @@ -12,7 +12,7 @@ "xcopy-msbuild": "16.10.0-preview2" }, "msbuild-sdks": { - "Microsoft.DotNet.Arcade.Sdk": "7.0.0-beta.21615.1", - "Microsoft.DotNet.Helix.Sdk": "7.0.0-beta.21615.1" + "Microsoft.DotNet.Arcade.Sdk": "7.0.0-beta.21621.3", + "Microsoft.DotNet.Helix.Sdk": "7.0.0-beta.21621.3" } } From d589d90acb9165bd3834e6e6d213df629d807c16 Mon Sep 17 00:00:00 2001 From: "dotnet-maestro[bot]" <42748379+dotnet-maestro[bot]@users.noreply.github.com> Date: Thu, 23 Dec 2021 14:59:32 +0000 Subject: [PATCH 34/41] Update dependencies from https://github.com/dotnet/arcade build 20211223.1 (#58471) [main] Update dependencies from dotnet/arcade --- eng/Version.Details.xml | 8 ++++---- global.json | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml index fd98a9b586b16..61ea8dfc0871d 100644 --- a/eng/Version.Details.xml +++ b/eng/Version.Details.xml @@ -13,18 +13,18 @@ - + https://github.com/dotnet/arcade - 0cd94b1d02c03377d99f3739beb191591f6abee5 + 4abaab2bf44d06638abeb23fc96c4f6eef58a2f0 https://github.com/dotnet/roslyn 818313426323d979747781a17c78860c833776da - + https://github.com/dotnet/arcade - 0cd94b1d02c03377d99f3739beb191591f6abee5 + 4abaab2bf44d06638abeb23fc96c4f6eef58a2f0 diff --git a/global.json b/global.json index 02b28209e68ee..59c01b0b4680d 100644 --- a/global.json +++ b/global.json @@ -12,7 +12,7 @@ "xcopy-msbuild": "16.10.0-preview2" }, "msbuild-sdks": { - "Microsoft.DotNet.Arcade.Sdk": "7.0.0-beta.21621.3", - "Microsoft.DotNet.Helix.Sdk": "7.0.0-beta.21621.3" + "Microsoft.DotNet.Arcade.Sdk": "7.0.0-beta.21623.1", + "Microsoft.DotNet.Helix.Sdk": "7.0.0-beta.21623.1" } } From 678b269e0e31a1b31ed1dfc74b464929218b7039 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Fri, 24 Dec 2021 12:46:12 +1100 Subject: [PATCH 35/41] Support top level statements in event hookup (#58475) --- ...EventHookupCommandHandler_TabKeyCommand.cs | 18 +++++++++---- .../EventHookupCommandHandlerTests.cs | 25 +++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/EditorFeatures/CSharp/EventHookup/EventHookupCommandHandler_TabKeyCommand.cs b/src/EditorFeatures/CSharp/EventHookup/EventHookupCommandHandler_TabKeyCommand.cs index 76dc07e72e453..9217c543dad2b 100644 --- a/src/EditorFeatures/CSharp/EventHookup/EventHookupCommandHandler_TabKeyCommand.cs +++ b/src/EditorFeatures/CSharp/EventHookup/EventHookupCommandHandler_TabKeyCommand.cs @@ -186,9 +186,10 @@ private static Document AddMethodNameAndAnnotationsToSolution( var root = document.GetSyntaxRootSynchronously(cancellationToken); var plusEqualsToken = root.FindTokenOnLeftOfPosition(position); var eventHookupExpression = plusEqualsToken.GetAncestor(); + var typeDecl = eventHookupExpression.GetAncestor(); var textToInsert = eventHandlerMethodName + ";"; - if (!eventHookupExpression.IsInStaticContext()) + if (!eventHookupExpression.IsInStaticContext() && typeDecl is not null) { // This will be simplified later if it's not needed. textToInsert = "this." + textToInsert; @@ -222,24 +223,30 @@ private static SyntaxNode AddGeneratedHandlerMethodToSolution( var root = document.Root; var eventHookupExpression = root.GetAnnotatedNodesAndTokens(plusEqualsTokenAnnotation).Single().AsToken().GetAncestor(); - var generatedMethodSymbol = GetMethodSymbol(document, eventHandlerMethodName, eventHookupExpression, cancellationToken); + var typeDecl = eventHookupExpression.GetAncestor(); + var methodKind = typeDecl is null + ? MethodKind.LocalFunction + : MethodKind.Ordinary; + + var generatedMethodSymbol = GetMethodSymbol(document, eventHandlerMethodName, eventHookupExpression, methodKind, cancellationToken); if (generatedMethodSymbol == null) { return null; } - var typeDecl = eventHookupExpression.GetAncestor(); + var container = (SyntaxNode)typeDecl ?? eventHookupExpression.GetAncestor(); - var typeDeclWithMethodAdded = CodeGenerator.AddMethodDeclaration(typeDecl, generatedMethodSymbol, document.Project.Solution.Workspace, new CodeGenerationOptions(afterThisLocation: eventHookupExpression.GetLocation())); + var newContainer = CodeGenerator.AddMethodDeclaration(container, generatedMethodSymbol, document.Project.Solution.Workspace, new CodeGenerationOptions(afterThisLocation: eventHookupExpression.GetLocation())); - return root.ReplaceNode(typeDecl, typeDeclWithMethodAdded); + return root.ReplaceNode(container, newContainer); } private static IMethodSymbol GetMethodSymbol( SemanticDocument semanticDocument, string eventHandlerMethodName, AssignmentExpressionSyntax eventHookupExpression, + MethodKind methodKind, CancellationToken cancellationToken) { var semanticModel = semanticDocument.SemanticModel; @@ -271,6 +278,7 @@ private static IMethodSymbol GetMethodSymbol( name: eventHandlerMethodName, typeParameters: default, parameters: delegateInvokeMethod.Parameters, + methodKind: methodKind, statements: ImmutableArray.Create( CodeGenerationHelpers.GenerateThrowStatement(syntaxFactory, semanticDocument, "System.NotImplementedException"))); } diff --git a/src/EditorFeatures/CSharpTest/EventHookup/EventHookupCommandHandlerTests.cs b/src/EditorFeatures/CSharpTest/EventHookup/EventHookupCommandHandlerTests.cs index 6fbb7e3cbe8fb..2cf1fe41ca81a 100644 --- a/src/EditorFeatures/CSharpTest/EventHookup/EventHookupCommandHandlerTests.cs +++ b/src/EditorFeatures/CSharpTest/EventHookup/EventHookupCommandHandlerTests.cs @@ -1024,6 +1024,31 @@ private void C_MyEvent() testState.AssertCodeIs(expectedCode); } + [WpfFact, Trait(Traits.Feature, Traits.Features.EventHookup)] + [WorkItem(58474, "https://github.com/dotnet/roslyn/issues/58474")] + public async Task EventHookupInTopLevelCode() + { + var markup = @" + +System.AppDomain.CurrentDomain.UnhandledException +$$ + +"; + using var testState = EventHookupTestState.CreateTestState(markup); + testState.SendTypeChar('='); + testState.SendTab(); + await testState.WaitForAsynchronousOperationsAsync(); + + var expectedCode = @" + +System.AppDomain.CurrentDomain.UnhandledException += CurrentDomain_UnhandledException; + +void CurrentDomain_UnhandledException(object sender, System.UnhandledExceptionEventArgs e) +{ + throw new System.NotImplementedException(); +}"; + testState.AssertCodeIs(expectedCode); + } + private static OptionsCollection QualifyMethodAccessWithNotification(NotificationOption2 notification) => new OptionsCollection(LanguageNames.CSharp) { { CodeStyleOptions2.QualifyMethodAccess, true, notification } }; } From 8fb8592fcf5637eb4bc04c1fced5e585b821d445 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Fri, 24 Dec 2021 16:14:27 +1100 Subject: [PATCH 36/41] Fix event hookup at the end of a document (#58477) --- .../EventHookup/EventHookupSessionManager.cs | 24 +++++++++++++- ...HookupSessionManager_EventHookupSession.cs | 5 ++- .../EventHookupCommandHandlerTests.cs | 31 +++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/src/EditorFeatures/CSharp/EventHookup/EventHookupSessionManager.cs b/src/EditorFeatures/CSharp/EventHookup/EventHookupSessionManager.cs index 570b58457add4..de8b8614f8726 100644 --- a/src/EditorFeatures/CSharp/EventHookup/EventHookupSessionManager.cs +++ b/src/EditorFeatures/CSharp/EventHookup/EventHookupSessionManager.cs @@ -50,7 +50,7 @@ internal void EventHookupFoundInSession(EventHookupSession analyzedSession) if (_toolTipPresenter == null && CurrentSession == analyzedSession && caretPoint.HasValue && - analyzedSession.TrackingSpan.GetSpan(CurrentSession.TextView.TextSnapshot).Contains(caretPoint.Value)) + IsCaretWithinSpanOrAtEnd(analyzedSession.TrackingSpan, analyzedSession.TextView.TextSnapshot, caretPoint.Value)) { // Create a tooltip presenter that stays alive, even when the user types, without tracking the mouse. _toolTipPresenter = _toolTipService.CreatePresenter(analyzedSession.TextView, @@ -80,6 +80,28 @@ internal void EventHookupFoundInSession(EventHookupSession analyzedSession) } } + private static bool IsCaretWithinSpanOrAtEnd(ITrackingSpan trackingSpan, ITextSnapshot textSnapshot, SnapshotPoint caretPoint) + { + var snapshotSpan = trackingSpan.GetSpan(textSnapshot); + + // If the caret is within the span, then we want to show the tooltip + if (snapshotSpan.Contains(caretPoint)) + { + return true; + } + + // Otherwise if the span is empty, and at the end of the file, and the caret + // is also at the end of the file, then show the tooltip. + if (snapshotSpan.IsEmpty && + snapshotSpan.Start.Position == caretPoint.Position && + caretPoint.Position == textSnapshot.Length) + { + return true; + } + + return false; + } + internal void BeginSession( EventHookupCommandHandler eventHookupCommandHandler, ITextView textView, diff --git a/src/EditorFeatures/CSharp/EventHookup/EventHookupSessionManager_EventHookupSession.cs b/src/EditorFeatures/CSharp/EventHookup/EventHookupSessionManager_EventHookupSession.cs index e7f4f2bcbcfff..12dbd69c81d9e 100644 --- a/src/EditorFeatures/CSharp/EventHookup/EventHookupSessionManager_EventHookupSession.cs +++ b/src/EditorFeatures/CSharp/EventHookup/EventHookupSessionManager_EventHookupSession.cs @@ -114,7 +114,10 @@ public EventHookupSession( { var position = textView.GetCaretPoint(subjectBuffer).Value.Position; _trackingPoint = textView.TextSnapshot.CreateTrackingPoint(position, PointTrackingMode.Negative); - _trackingSpan = textView.TextSnapshot.CreateTrackingSpan(new Span(position, 1), SpanTrackingMode.EdgeInclusive); + + // If the caret is at the end of the document we just create an empty span + var length = textView.TextSnapshot.Length > position + 1 ? 1 : 0; + _trackingSpan = textView.TextSnapshot.CreateTrackingSpan(new Span(position, length), SpanTrackingMode.EdgeInclusive); var asyncToken = asyncListener.BeginAsyncOperation(GetType().Name + ".Start"); diff --git a/src/EditorFeatures/CSharpTest/EventHookup/EventHookupCommandHandlerTests.cs b/src/EditorFeatures/CSharpTest/EventHookup/EventHookupCommandHandlerTests.cs index 2cf1fe41ca81a..0d57db3ad040d 100644 --- a/src/EditorFeatures/CSharpTest/EventHookup/EventHookupCommandHandlerTests.cs +++ b/src/EditorFeatures/CSharpTest/EventHookup/EventHookupCommandHandlerTests.cs @@ -1042,6 +1042,37 @@ public async Task EventHookupInTopLevelCode() System.AppDomain.CurrentDomain.UnhandledException += CurrentDomain_UnhandledException; +void CurrentDomain_UnhandledException(object sender, System.UnhandledExceptionEventArgs e) +{ + throw new System.NotImplementedException(); +}"; + testState.AssertCodeIs(expectedCode); + } + + [WpfFact, Trait(Traits.Feature, Traits.Features.EventHookup)] + public async Task EventHookupAtEndOfDocument() + { + var markup = @" + +System.AppDomain.CurrentDomain.UnhandledException +$$"; + using var testState = EventHookupTestState.CreateTestState(markup); + testState.SendTypeChar('='); + + await testState.WaitForAsynchronousOperationsAsync(); + testState.AssertShowing("CurrentDomain_UnhandledException"); + + var expectedCode = @" + +System.AppDomain.CurrentDomain.UnhandledException +="; + testState.AssertCodeIs(expectedCode); + + testState.SendTab(); + await testState.WaitForAsynchronousOperationsAsync(); + + expectedCode = @" + +System.AppDomain.CurrentDomain.UnhandledException += CurrentDomain_UnhandledException; + void CurrentDomain_UnhandledException(object sender, System.UnhandledExceptionEventArgs e) { throw new System.NotImplementedException(); From 6e9947a83bc6eeb4b4bedca44d4b91523f12a3f1 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Thu, 23 Dec 2021 21:35:17 -0800 Subject: [PATCH 37/41] Enqueue additional and analyzer config documents in WorkCoordinator Fixes #53192 Currently, work coordinator only enqueues source document IDs for project analysis work items. This means we don't analyze the additional documents, which can now report standalone diagnostics through RegisterAdditionalFileAction. This PR fixes this by enqueuing project's additional document IDs and analyzer config document IDs. --- .../SolutionCrawler/WorkCoordinatorTests.cs | 25 ++++++++++++++++++- .../SolutionCrawler/WorkCoordinator.cs | 5 ++-- 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/EditorFeatures/Test/SolutionCrawler/WorkCoordinatorTests.cs b/src/EditorFeatures/Test/SolutionCrawler/WorkCoordinatorTests.cs index 2bb8424b5110d..955e649141d88 100644 --- a/src/EditorFeatures/Test/SolutionCrawler/WorkCoordinatorTests.cs +++ b/src/EditorFeatures/Test/SolutionCrawler/WorkCoordinatorTests.cs @@ -335,6 +335,8 @@ internal async Task Project_AnalyzerOptions_Change(BackgroundAnalysisScope analy Assert.Equal(expectedDocumentEvents, worker.SyntaxDocumentIds.Count); Assert.Equal(expectedDocumentEvents, worker.DocumentIds.Count); + + Assert.Equal(1, worker.NonSourceDocumentIds.Count); } [InlineData(BackgroundAnalysisScope.ActiveFile, false)] @@ -701,22 +703,26 @@ internal async Task Document_AdditionalFileChange(BackgroundAnalysisScope analys var expectedDocumentSyntaxEvents = 5; var expectedDocumentSemanticEvents = 5; + var expectedNonSourceDocumentEvents = 1; var ncfile = DocumentInfo.Create(DocumentId.CreateNewId(project.Id), "D6"); var worker = await ExecuteOperation(workspace, w => w.OnAdditionalDocumentAdded(ncfile)); Assert.Equal(expectedDocumentSyntaxEvents, worker.SyntaxDocumentIds.Count); Assert.Equal(expectedDocumentSemanticEvents, worker.DocumentIds.Count); + Assert.Equal(expectedNonSourceDocumentEvents, worker.NonSourceDocumentIds.Count); worker = await ExecuteOperation(workspace, w => w.ChangeAdditionalDocument(ncfile.Id, SourceText.From("//"))); Assert.Equal(expectedDocumentSyntaxEvents, worker.SyntaxDocumentIds.Count); Assert.Equal(expectedDocumentSemanticEvents, worker.DocumentIds.Count); + Assert.Equal(expectedNonSourceDocumentEvents, worker.NonSourceDocumentIds.Count); worker = await ExecuteOperation(workspace, w => w.OnAdditionalDocumentRemoved(ncfile.Id)); Assert.Equal(expectedDocumentSyntaxEvents, worker.SyntaxDocumentIds.Count); Assert.Equal(expectedDocumentSemanticEvents, worker.DocumentIds.Count); + Assert.Empty(worker.NonSourceDocumentIds); } [InlineData(BackgroundAnalysisScope.ActiveFile, false)] @@ -1619,7 +1625,7 @@ internal static class Metadata public static readonly IncrementalAnalyzerProviderMetadata Crawler = new IncrementalAnalyzerProviderMetadata(new Dictionary { { "WorkspaceKinds", new[] { SolutionCrawlerWorkspaceKind } }, { "HighPriorityForActiveFile", false }, { "Name", "TestAnalyzer" } }); } - private class Analyzer : IIncrementalAnalyzer + private class Analyzer : IIncrementalAnalyzer2 { public static readonly Option TestOption = new Option("TestOptions", "TestOption", defaultValue: true); @@ -1628,6 +1634,7 @@ private class Analyzer : IIncrementalAnalyzer public readonly HashSet SyntaxDocumentIds = new HashSet(); public readonly HashSet DocumentIds = new HashSet(); + public readonly HashSet NonSourceDocumentIds = new HashSet(); public readonly HashSet ProjectIds = new HashSet(); public readonly HashSet InvalidateDocumentIds = new HashSet(); @@ -1653,6 +1660,7 @@ public void Reset() SyntaxDocumentIds.Clear(); DocumentIds.Clear(); + NonSourceDocumentIds.Clear(); ProjectIds.Clear(); InvalidateDocumentIds.Clear(); @@ -1682,6 +1690,12 @@ public Task AnalyzeSyntaxAsync(Document document, InvocationReasons reasons, Can return Task.CompletedTask; } + public Task AnalyzeNonSourceDocumentAsync(TextDocument textDocument, InvocationReasons reasons, CancellationToken cancellationToken) + { + this.NonSourceDocumentIds.Add(textDocument.Id); + return Task.CompletedTask; + } + public Task RemoveDocumentAsync(DocumentId documentId, CancellationToken cancellationToken) { InvalidateDocumentIds.Add(documentId); @@ -1732,6 +1746,15 @@ public Task DocumentCloseAsync(Document document, CancellationToken cancellation public Task DocumentResetAsync(Document document, CancellationToken cancellationToken) => Task.CompletedTask; + + public Task NonSourceDocumentOpenAsync(TextDocument textDocument, CancellationToken cancellationToken) + => Task.CompletedTask; + + public Task NonSourceDocumentCloseAsync(TextDocument textDocument, CancellationToken cancellationToken) + => Task.CompletedTask; + + public Task NonSourceDocumentResetAsync(TextDocument textDocument, CancellationToken cancellationToken) + => Task.CompletedTask; #endregion } diff --git a/src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.cs b/src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.cs index 0a3a83b8c8615..81daf1f3cb999 100644 --- a/src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.cs +++ b/src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.cs @@ -439,7 +439,8 @@ private async Task EnqueueWorkItemAsync(Project project, DocumentId documentId, _shutdownToken.ThrowIfCancellationRequested(); var priorityService = project.GetLanguageService(); - var isLowPriority = priorityService != null && await priorityService.IsLowPriorityAsync(GetRequiredDocument(project, documentId, document), _shutdownToken).ConfigureAwait(false); + document ??= project.GetDocument(documentId); + var isLowPriority = priorityService != null && document != null && await priorityService.IsLowPriorityAsync(document, _shutdownToken).ConfigureAwait(false); var currentMember = GetSyntaxPath(changedMember); @@ -473,7 +474,7 @@ private static Document GetRequiredDocument(Project project, DocumentId document private async Task EnqueueWorkItemAsync(Project project, InvocationReasons invocationReasons) { - foreach (var documentId in project.DocumentIds) + foreach (var documentId in project.DocumentIds.Concat(project.AdditionalDocumentIds).Concat(project.AnalyzerConfigDocumentIds)) await EnqueueWorkItemAsync(project, documentId, document: null, invocationReasons).ConfigureAwait(false); } From ce24c9023757bf243b9b32b00abfadb0ddb8b43b Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Thu, 23 Dec 2021 22:20:10 -0800 Subject: [PATCH 38/41] Fix unused param (IDE0060) false positive Fixes #56317 #42408 handled bailing out reporting unused params for methods that only have a throw operation in its body. It missed a case where the throw might be wrapped within a conversion wrapped within a return operation, which happens when the method returns a non-void type. --- .../RemoveUnusedParametersTests.cs | 30 +++++++++++++++++++ ...lyzer.SymbolStartAnalyzer.BlockAnalyzer.cs | 13 ++++++-- .../Core/Extensions/OperationExtensions.cs | 15 ++++++++++ 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/Analyzers/CSharp/Tests/RemoveUnusedParametersAndValues/RemoveUnusedParametersTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnusedParametersAndValues/RemoveUnusedParametersTests.cs index 2ba5d60c76a3e..d65a510d172c2 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnusedParametersAndValues/RemoveUnusedParametersTests.cs +++ b/src/Analyzers/CSharp/Tests/RemoveUnusedParametersAndValues/RemoveUnusedParametersTests.cs @@ -1503,6 +1503,36 @@ public C(int [|i|]) }"); } + [WorkItem(56317, "https://github.com/dotnet/roslyn/issues/56317")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedParameters)] + public async Task NotImplementedException_NoDiagnostic4() + { + await TestDiagnosticMissingAsync( +@"using System; + +class C +{ + private int Goo(int [|i|]) + => throw new NotImplementedException(); +}"); + } + + [WorkItem(56317, "https://github.com/dotnet/roslyn/issues/56317")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedParameters)] + public async Task NotImplementedException_NoDiagnostic5() + { + await TestDiagnosticMissingAsync( +@"using System; + +class C +{ + private int Goo(int [|i|]) + { + throw new NotImplementedException(); + } +}"); + } + [WorkItem(41236, "https://github.com/dotnet/roslyn/issues/41236")] [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedParameters)] public async Task NotImplementedException_MultipleStatements1() diff --git a/src/Analyzers/Core/Analyzers/RemoveUnusedParametersAndValues/AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.SymbolStartAnalyzer.BlockAnalyzer.cs b/src/Analyzers/Core/Analyzers/RemoveUnusedParametersAndValues/AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.SymbolStartAnalyzer.BlockAnalyzer.cs index f110f9e90fdb9..54e1f0deb591e 100644 --- a/src/Analyzers/Core/Analyzers/RemoveUnusedParametersAndValues/AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.SymbolStartAnalyzer.BlockAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/RemoveUnusedParametersAndValues/AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.SymbolStartAnalyzer.BlockAnalyzer.cs @@ -152,9 +152,18 @@ static bool IsSingleThrowNotImplementedOperation(IOperation firstBlock) if (firstOp == null) return false; - // unwrap: { throw new NYI(); } if (firstOp is IExpressionStatementOperation expressionStatement) + { + // unwrap: { throw new NYI(); } firstOp = expressionStatement.Operation; + } + else if (firstOp is IReturnOperation returnOperation) + { + // unwrap: 'int M(int p) => throw new NYI();' + // For this case, the throw operation is wrapped within a conversion operation to 'int', + // which in turn is wrapped within a return operation. + firstOp = returnOperation.ReturnedValue.WalkDownConversion(); + } // => throw new NotImplementedOperation(...) return IsThrowNotImplementedOperation(notImplementedExceptionType, firstOp); @@ -177,7 +186,7 @@ static bool IsSingleThrowNotImplementedOperation(IOperation firstBlock) return firstOp; } - static bool IsThrowNotImplementedOperation(INamedTypeSymbol notImplementedExceptionType, IOperation operation) + static bool IsThrowNotImplementedOperation(INamedTypeSymbol notImplementedExceptionType, IOperation? operation) => operation is IThrowOperation throwOperation && UnwrapImplicitConversion(throwOperation.Exception) is IObjectCreationOperation objectCreation && notImplementedExceptionType.Equals(objectCreation.Type); diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs index 3bde34e3df8b5..5829096471015 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs @@ -370,5 +370,20 @@ public static bool IsNumericLiteral(this IOperation operation) public static bool IsNullLiteral(this IOperation operand) => operand is ILiteralOperation { ConstantValue: { HasValue: true, Value: null } }; + + /// + /// Walks down consecutive conversion operations until an operand is reached that isn't a conversion operation. + /// + /// The starting operation. + /// The inner non conversion operation or the starting operation if it wasn't a conversion operation. + public static IOperation WalkDownConversion(this IOperation? operation) + { + while (operation is IConversionOperation conversionOperation) + { + operation = conversionOperation.Operand; + } + + return operation; + } } } From bbb2c26f15179e95560b061541e835364b4de51b Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Thu, 23 Dec 2021 22:29:52 -0800 Subject: [PATCH 39/41] Address feedback --- .../Core/Portable/SolutionCrawler/WorkCoordinator.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.cs b/src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.cs index 81daf1f3cb999..5be133bc20d64 100644 --- a/src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.cs +++ b/src/Features/Core/Portable/SolutionCrawler/WorkCoordinator.cs @@ -474,7 +474,13 @@ private static Document GetRequiredDocument(Project project, DocumentId document private async Task EnqueueWorkItemAsync(Project project, InvocationReasons invocationReasons) { - foreach (var documentId in project.DocumentIds.Concat(project.AdditionalDocumentIds).Concat(project.AnalyzerConfigDocumentIds)) + foreach (var documentId in project.DocumentIds) + await EnqueueWorkItemAsync(project, documentId, document: null, invocationReasons).ConfigureAwait(false); + + foreach (var documentId in project.AdditionalDocumentIds) + await EnqueueWorkItemAsync(project, documentId, document: null, invocationReasons).ConfigureAwait(false); + + foreach (var documentId in project.AnalyzerConfigDocumentIds) await EnqueueWorkItemAsync(project, documentId, document: null, invocationReasons).ConfigureAwait(false); } From e7b9d146544a3f71ffdb008cbf7e3856f5d0e29a Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Thu, 23 Dec 2021 22:39:15 -0800 Subject: [PATCH 40/41] Fix build --- .../Compiler/Core/Extensions/OperationExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs index 5829096471015..b9027c0239b53 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs @@ -376,7 +376,7 @@ public static bool IsNullLiteral(this IOperation operand) /// /// The starting operation. /// The inner non conversion operation or the starting operation if it wasn't a conversion operation. - public static IOperation WalkDownConversion(this IOperation? operation) + public static IOperation? WalkDownConversion(this IOperation? operation) { while (operation is IConversionOperation conversionOperation) { From 9f3de42345b7ae1f6db28ce3822b7a3ae3136475 Mon Sep 17 00:00:00 2001 From: Youssef Victor Date: Fri, 24 Dec 2021 13:21:12 +0200 Subject: [PATCH 41/41] draft (#58481) --- .../EventHookup/EventHookupCommandHandler_TabKeyCommand.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/EditorFeatures/CSharp/EventHookup/EventHookupCommandHandler_TabKeyCommand.cs b/src/EditorFeatures/CSharp/EventHookup/EventHookupCommandHandler_TabKeyCommand.cs index 9217c543dad2b..a66120dc6f69e 100644 --- a/src/EditorFeatures/CSharp/EventHookup/EventHookupCommandHandler_TabKeyCommand.cs +++ b/src/EditorFeatures/CSharp/EventHookup/EventHookupCommandHandler_TabKeyCommand.cs @@ -224,11 +224,8 @@ private static SyntaxNode AddGeneratedHandlerMethodToSolution( var eventHookupExpression = root.GetAnnotatedNodesAndTokens(plusEqualsTokenAnnotation).Single().AsToken().GetAncestor(); var typeDecl = eventHookupExpression.GetAncestor(); - var methodKind = typeDecl is null - ? MethodKind.LocalFunction - : MethodKind.Ordinary; - var generatedMethodSymbol = GetMethodSymbol(document, eventHandlerMethodName, eventHookupExpression, methodKind, cancellationToken); + var generatedMethodSymbol = GetMethodSymbol(document, eventHandlerMethodName, eventHookupExpression, cancellationToken); if (generatedMethodSymbol == null) { @@ -246,7 +243,6 @@ private static IMethodSymbol GetMethodSymbol( SemanticDocument semanticDocument, string eventHandlerMethodName, AssignmentExpressionSyntax eventHookupExpression, - MethodKind methodKind, CancellationToken cancellationToken) { var semanticModel = semanticDocument.SemanticModel; @@ -278,7 +274,6 @@ private static IMethodSymbol GetMethodSymbol( name: eventHandlerMethodName, typeParameters: default, parameters: delegateInvokeMethod.Parameters, - methodKind: methodKind, statements: ImmutableArray.Create( CodeGenerationHelpers.GenerateThrowStatement(syntaxFactory, semanticDocument, "System.NotImplementedException"))); }