From 30ae321ce657a54e67990ca59a473660ff9ad078 Mon Sep 17 00:00:00 2001 From: Sam Harwell Date: Fri, 1 Mar 2019 12:15:41 -0600 Subject: [PATCH 1/3] Use the correct iteration count in IterationDataAttribute --- .../IntegrationTest/TestUtilities/IterationDataAttribute.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/VisualStudio/IntegrationTest/TestUtilities/IterationDataAttribute.cs b/src/VisualStudio/IntegrationTest/TestUtilities/IterationDataAttribute.cs index c0b43c37e0d57..34f5766b8a8d6 100644 --- a/src/VisualStudio/IntegrationTest/TestUtilities/IterationDataAttribute.cs +++ b/src/VisualStudio/IntegrationTest/TestUtilities/IterationDataAttribute.cs @@ -19,7 +19,7 @@ public sealed class IterationDataAttribute : DataAttribute { public IterationDataAttribute(int iterations = 100) { - Iterations = 100; + Iterations = iterations; } public int Iterations { get; } From c9183bdd5ac2493ad95c6eb6b6f49b7ada913893 Mon Sep 17 00:00:00 2001 From: Manish Vasani Date: Tue, 26 Feb 2019 12:47:20 -0800 Subject: [PATCH 2/3] Merge pull request #33631 from CyrusNajmabadi/wrapPriority Code wrapping refactorings should be lower pri than other refactorings (cherry picked from commit 8d530655b59a98ac8f0546cadb3671f0a94f10fc) --- .../Suggestions/SuggestedActionsSource.cs | 73 ++++++++++++------- .../Wrapping/AbstractCodeActionComputer.cs | 7 +- .../Core/Wrapping/WrapItemsAction.cs | 8 ++ .../Core/Portable/CodeActions/CodeAction.cs | 6 +- 4 files changed, 65 insertions(+), 29 deletions(-) diff --git a/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs b/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs index 87a4a424c5270..09c8aeb24302d 100644 --- a/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs +++ b/src/EditorFeatures/Core.Wpf/Suggestions/SuggestedActionsSource.cs @@ -176,18 +176,16 @@ public IEnumerable GetSuggestedActions( var fixes = GetCodeFixes(supportsFeatureService, requestedActionCategories, workspace, document, range, cancellationToken); var refactorings = GetRefactorings(supportsFeatureService, requestedActionCategories, workspace, document, selectionOpt, cancellationToken); - // If there's a selection, it's likely the user is trying to perform some operation - // directly on that operation (like 'extract method'). Prioritize refactorings over - // fixes in that case. Otherwise, it's likely that the user is just on some error - // and wants to fix it (in which case, prioritize fixes). - var result = selectionOpt?.Length > 0 - ? refactorings.Concat(fixes) - : fixes.Concat(refactorings); + // Get the initial set of action sets, with refactorings and fixes appropriately + // ordered against each other. + var result = GetInitiallyOrderedActionSets(selectionOpt, fixes, refactorings); if (result.IsEmpty) { return null; } + // Now that we have the entire set of action sets, inline, sort and filter + // them appropriately against each other. var allActionSets = InlineActionSetsIfDesirable(result); var orderedActionSets = OrderActionSets(allActionSets); var filteredSets = FilterActionSetsByTitle(orderedActionSets); @@ -196,6 +194,37 @@ public IEnumerable GetSuggestedActions( } } + private ImmutableArray GetInitiallyOrderedActionSets( + TextSpan? selectionOpt, ImmutableArray fixes, ImmutableArray refactorings) + { + // First, order refactorings based on the order the providers actually gave for their actions. + // This way, a low pri refactoring always shows after a medium pri refactoring, no matter what + // we do below. + refactorings = OrderActionSets(refactorings); + + // If there's a selection, it's likely the user is trying to perform some operation + // directly on that operation (like 'extract method'). Prioritize refactorings over + // fixes in that case. Otherwise, it's likely that the user is just on some error + // and wants to fix it (in which case, prioritize fixes). + + if (selectionOpt?.Length > 0) + { + // There was a selection. Treat refactorings as more important than + // fixes. Note: we still will sort after this. So any high pri fixes + // will come to the front. Any low-pri refactorings will go to the end. + return refactorings.Concat(fixes); + } + else + { + // No selection. Treat all refactorings as low priority, and place + // after fixes. Even a low pri fixes will be above what was *originally* + // a medium pri refactoring. + refactorings = refactorings.SelectAsArray(r => new SuggestedActionSet( + r.CategoryName, r.Actions, r.Title, SuggestedActionSetPriority.Low, r.ApplicableToSpan)); + return fixes.Concat(refactorings); + } + } + private ImmutableArray OrderActionSets( ImmutableArray actionSets) { @@ -452,7 +481,7 @@ private void AddCodeActions( nestedAction, getFixAllSuggestedActionSet(nestedAction))); var set = new SuggestedActionSet(categoryName: null, - actions: nestedActions, priority: SuggestedActionSetPriority.Medium, + actions: nestedActions, priority: GetSuggestedActionSetPriority(fix.Action.Priority), applicableToSpan: fix.PrimaryDiagnostic.Location.SourceSpan.ToSpan()); suggestedAction = new SuggestedActionWithNestedActions( @@ -626,21 +655,8 @@ private ImmutableArray GetRefactorings( var filteredRefactorings = FilterOnUIThread(refactorings, workspace); - // Refactorings are given the span the user currently has selected. That - // way they can be accurately sorted against other refactorings/fixes that - // are of the same priority. i.e. refactorings are LowPriority by default. - // But we still want them to come first over a low-pri code fix that is - // further away. A good example of this is "Add null parameter check" which - // should be higher in the list when the caret is on a parameter, vs the - // code-fix for "use expression body" which is given the entire span of a - // method. - - var priority = selection.Length > 0 - ? SuggestedActionSetPriority.Medium - : SuggestedActionSetPriority.Low; - return filteredRefactorings.SelectAsArray( - r => OrganizeRefactorings(workspace, r, priority, selection.ToSpan())); + r => OrganizeRefactorings(workspace, r, selection.ToSpan())); } return ImmutableArray.Empty; @@ -655,8 +671,7 @@ private ImmutableArray GetRefactorings( /// and should show up after fixes but before suppression fixes in the light bulb menu. /// private SuggestedActionSet OrganizeRefactorings( - Workspace workspace, CodeRefactoring refactoring, - SuggestedActionSetPriority priority, Span applicableSpan) + Workspace workspace, CodeRefactoring refactoring, Span applicableSpan) { var refactoringSuggestedActions = ArrayBuilder.GetInstance(); @@ -670,7 +685,7 @@ private SuggestedActionSet OrganizeRefactorings( _owner, workspace, _subjectBuffer, refactoring.Provider, na)); var set = new SuggestedActionSet(categoryName: null, - actions: nestedActions, priority: SuggestedActionSetPriority.Medium, applicableToSpan: applicableSpan); + actions: nestedActions, priority: GetSuggestedActionSetPriority(action.Priority), applicableToSpan: applicableSpan); refactoringSuggestedActions.Add(new SuggestedActionWithNestedActions( ThreadingContext, @@ -685,10 +700,14 @@ private SuggestedActionSet OrganizeRefactorings( } } + var actions = refactoringSuggestedActions.ToImmutableAndFree(); + + // An action set gets the the same priority as the highest priority + // action within in. return new SuggestedActionSet( PredefinedSuggestedActionCategoryNames.Refactoring, - refactoringSuggestedActions.ToImmutableAndFree(), - priority: priority, + actions: actions, + priority: GetSuggestedActionSetPriority(actions.Max(a => a.Priority)), applicableToSpan: applicableSpan); } diff --git a/src/EditorFeatures/Core/Wrapping/AbstractCodeActionComputer.cs b/src/EditorFeatures/Core/Wrapping/AbstractCodeActionComputer.cs index 6092e6a690124..7e76d3935a9fd 100644 --- a/src/EditorFeatures/Core/Wrapping/AbstractCodeActionComputer.cs +++ b/src/EditorFeatures/Core/Wrapping/AbstractCodeActionComputer.cs @@ -273,8 +273,13 @@ public async Task> GetTopLevelCodeActionsAsync() // Otherwise, sort items and add to the resultant list var sorted = WrapItemsAction.SortActionsByMostRecentlyUsed(ImmutableArray.CastUp(wrappingActions)); + + // Make our code action low priority. This option will be offered *a lot*, and + // much of the time will not be something the user particularly wants to do. + // It should be offered after all other normal refactorings. result.Add(new CodeActionWithNestedActions( - wrappingActions[0].ParentTitle, sorted, group.IsInlinable)); + wrappingActions[0].ParentTitle, sorted, + group.IsInlinable, CodeActionPriority.Low)); } // Finally, sort the topmost list we're building and return that. This ensures that diff --git a/src/EditorFeatures/Core/Wrapping/WrapItemsAction.cs b/src/EditorFeatures/Core/Wrapping/WrapItemsAction.cs index 0d7ccbb5220fb..d9a58ac13e11c 100644 --- a/src/EditorFeatures/Core/Wrapping/WrapItemsAction.cs +++ b/src/EditorFeatures/Core/Wrapping/WrapItemsAction.cs @@ -29,6 +29,14 @@ internal class WrapItemsAction : DocumentChangeAction public string SortTitle { get; } + // Make our code action low priority. This option will be offered *a lot*, and + // much of the time will not be something the user particularly wants to do. + // It should be offered after all other normal refactorings. + // + // This value is only relevant if this code action is the only one in its group, + // and it ends up getting inlined as a top-level-action that is offered. + internal override CodeActionPriority Priority => CodeActionPriority.Low; + public WrapItemsAction(string title, string parentTitle, Func> createChangedDocument) : base(title, createChangedDocument) { diff --git a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs index 5294517d44ea3..3200ab4066cca 100644 --- a/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs +++ b/src/Workspaces/Core/Portable/CodeActions/CodeAction.cs @@ -359,14 +359,18 @@ protected override Task GetChangedDocumentAsync(CancellationToken canc internal class CodeActionWithNestedActions : SimpleCodeAction { public CodeActionWithNestedActions( - string title, ImmutableArray nestedActions, bool isInlinable) + string title, ImmutableArray nestedActions, + bool isInlinable, CodeActionPriority priority = CodeActionPriority.Medium) : base(title, ComputeEquivalenceKey(nestedActions)) { Debug.Assert(nestedActions.Length > 0); NestedCodeActions = nestedActions; IsInlinable = isInlinable; + Priority = priority; } + internal override CodeActionPriority Priority { get; } + internal sealed override bool IsInlinable { get; } internal sealed override ImmutableArray NestedCodeActions { get; } From a15ea1c2ca775c7f7ed1aa711e79f25f9d6814e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Matou=C5=A1ek?= Date: Fri, 1 Mar 2019 15:49:19 -0800 Subject: [PATCH 3/3] EnC: Remove dependency on solution from AnalyzeDocumentAsync (#33796) --- .../CSharpEditAndContinueAnalyzerTests.cs | 61 +++++++++++-------- ...VisualBasicEditAndContinueAnalyzerTests.vb | 39 +++++++----- .../AbstractEditAndContinueAnalyzer.cs | 60 +++++++----------- .../Portable/EditAndContinue/EditSession.cs | 6 +- .../IEditAndContinueAnalyzer.cs | 2 +- 5 files changed, 85 insertions(+), 83 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/EditAndContinue/CSharpEditAndContinueAnalyzerTests.cs b/src/EditorFeatures/CSharpTest/EditAndContinue/CSharpEditAndContinueAnalyzerTests.cs index be1d3f400186f..3e61602b1f2d0 100644 --- a/src/EditorFeatures/CSharpTest/EditAndContinue/CSharpEditAndContinueAnalyzerTests.cs +++ b/src/EditorFeatures/CSharpTest/EditAndContinue/CSharpEditAndContinueAnalyzerTests.cs @@ -259,7 +259,8 @@ public static void Main() using (var workspace = TestWorkspace.CreateCSharp(source1)) { - var documentId = workspace.CurrentSolution.Projects.First().Documents.First().Id; + var oldProject = workspace.CurrentSolution.Projects.First(); + var documentId = oldProject.Documents.First().Id; var oldSolution = workspace.CurrentSolution; var newSolution = workspace.CurrentSolution.WithDocumentText(documentId, SourceText.From(source2)); var oldDocument = oldSolution.GetDocument(documentId); @@ -276,7 +277,7 @@ public static void Main() var oldStatementSyntax = oldSyntaxRoot.FindNode(oldStatementTextSpan); var baseActiveStatements = ImmutableArray.Create(ActiveStatementsDescription.CreateActiveStatement(ActiveStatementFlags.IsLeafFrame, oldStatementSpan, DocumentId.CreateNewId(ProjectId.CreateNewId()))); - var result = await analyzer.AnalyzeDocumentAsync(oldSolution, baseActiveStatements, newDocument, CancellationToken.None); + var result = await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, newDocument, trackingServiceOpt: null, CancellationToken.None); Assert.True(result.HasChanges); Assert.True(result.SemanticEdits[0].PreserveLocalVariables); @@ -316,12 +317,13 @@ public static void Main() using (var workspace = TestWorkspace.CreateCSharp(source1)) { - var documentId = workspace.CurrentSolution.Projects.First().Documents.First().Id; + var oldProject = workspace.CurrentSolution.Projects.First(); + var documentId = oldProject.Documents.First().Id; var oldSolution = workspace.CurrentSolution; var newSolution = workspace.CurrentSolution.WithDocumentText(documentId, SourceText.From(source2)); var baseActiveStatements = ImmutableArray.Create(); - var result = await analyzer.AnalyzeDocumentAsync(oldSolution, baseActiveStatements, newSolution.GetDocument(documentId), default(CancellationToken)); + var result = await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, newSolution.GetDocument(documentId), trackingServiceOpt: null, CancellationToken.None); Assert.True(result.HasChanges); Assert.True(result.HasChangesAndErrors); @@ -345,9 +347,10 @@ public static void Main() using (var workspace = TestWorkspace.CreateCSharp(source)) { - var document = workspace.CurrentSolution.Projects.First().Documents.First(); + var oldProject = workspace.CurrentSolution.Projects.First(); + var document = oldProject.Documents.First(); var baseActiveStatements = ImmutableArray.Create(); - var result = await analyzer.AnalyzeDocumentAsync(workspace.CurrentSolution, baseActiveStatements, document, default(CancellationToken)); + var result = await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, document, trackingServiceOpt: null, CancellationToken.None); Assert.False(result.HasChanges); Assert.False(result.HasChangesAndErrors); @@ -380,12 +383,13 @@ public static void Main() using (var workspace = TestWorkspace.CreateCSharp(source1)) { - var documentId = workspace.CurrentSolution.Projects.First().Documents.First().Id; + var oldProject = workspace.CurrentSolution.Projects.First(); + var documentId = oldProject.Documents.First().Id; var oldSolution = workspace.CurrentSolution; var newSolution = workspace.CurrentSolution.WithDocumentText(documentId, SourceText.From(source2)); var baseActiveStatements = ImmutableArray.Create(); - var result = await analyzer.AnalyzeDocumentAsync(oldSolution, baseActiveStatements, newSolution.GetDocument(documentId), default(CancellationToken)); + var result = await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, newSolution.GetDocument(documentId), trackingServiceOpt: null, CancellationToken.None); Assert.False(result.HasChanges); Assert.False(result.HasChangesAndErrors); @@ -412,9 +416,10 @@ public static void Main() using (var workspace = TestWorkspace.CreateCSharp( source, parseOptions: experimental, compilationOptions: null, exportProvider: null)) { - var document = workspace.CurrentSolution.Projects.First().Documents.First(); + var oldProject = workspace.CurrentSolution.Projects.First(); + var document = oldProject.Documents.First(); var baseActiveStatements = ImmutableArray.Create(); - var result = await analyzer.AnalyzeDocumentAsync(workspace.CurrentSolution, baseActiveStatements, document, default(CancellationToken)); + var result = await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, document, trackingServiceOpt: null, CancellationToken.None); Assert.False(result.HasChanges); Assert.False(result.HasChangesAndErrors); @@ -457,12 +462,13 @@ public static void Main() using (var workspace = TestWorkspace.CreateCSharp( source1, parseOptions: experimental, compilationOptions: null, exportProvider: null)) { - var documentId = workspace.CurrentSolution.Projects.First().Documents.First().Id; + var oldProject = workspace.CurrentSolution.Projects.First(); + var documentId = oldProject.Documents.First().Id; var oldSolution = workspace.CurrentSolution; var newSolution = workspace.CurrentSolution.WithDocumentText(documentId, SourceText.From(source2)); var baseActiveStatements = ImmutableArray.Create(); - var result = await analyzer.AnalyzeDocumentAsync(oldSolution, baseActiveStatements, newSolution.GetDocument(documentId), default(CancellationToken)); + var result = await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, newSolution.GetDocument(documentId), trackingServiceOpt: null, CancellationToken.None); Assert.True(result.HasChanges); Assert.True(result.HasChangesAndErrors); @@ -489,9 +495,10 @@ public static void Main() using (var workspace = TestWorkspace.CreateCSharp(source)) { - var document = workspace.CurrentSolution.Projects.First().Documents.First(); + var oldProject = workspace.CurrentSolution.Projects.First(); + var document = oldProject.Documents.First(); var baseActiveStatements = ImmutableArray.Create(); - var result = await analyzer.AnalyzeDocumentAsync(workspace.CurrentSolution, baseActiveStatements, document, default(CancellationToken)); + var result = await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, document, trackingServiceOpt: null, CancellationToken.None); Assert.False(result.HasChanges); Assert.False(result.HasChangesAndErrors); @@ -526,12 +533,13 @@ public static void Main() using (var workspace = TestWorkspace.CreateCSharp(source1)) { - var documentId = workspace.CurrentSolution.Projects.First().Documents.First().Id; + var oldProject = workspace.CurrentSolution.Projects.First(); + var documentId = oldProject.Documents.First().Id; var oldSolution = workspace.CurrentSolution; var newSolution = workspace.CurrentSolution.WithDocumentText(documentId, SourceText.From(source2)); var baseActiveStatements = ImmutableArray.Create(); - var result = await analyzer.AnalyzeDocumentAsync(oldSolution, baseActiveStatements, newSolution.GetDocument(documentId), default(CancellationToken)); + var result = await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, newSolution.GetDocument(documentId), trackingServiceOpt: null, CancellationToken.None); Assert.True(result.HasChanges); @@ -566,12 +574,13 @@ public static void Main(Bar x) using (var workspace = TestWorkspace.CreateCSharp(source1)) { - var documentId = workspace.CurrentSolution.Projects.First().Documents.First().Id; + var oldProject = workspace.CurrentSolution.Projects.First(); + var documentId = oldProject.Documents.First().Id; var oldSolution = workspace.CurrentSolution; var newSolution = workspace.CurrentSolution.WithDocumentText(documentId, SourceText.From(source2)); var baseActiveStatements = ImmutableArray.Create(); - var result = await analyzer.AnalyzeDocumentAsync(oldSolution, baseActiveStatements, newSolution.GetDocument(documentId), default(CancellationToken)); + var result = await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, newSolution.GetDocument(documentId), trackingServiceOpt: null, CancellationToken.None); Assert.True(result.HasChanges); Assert.True(result.HasChangesAndErrors); @@ -606,15 +615,15 @@ public class D using (var workspace = TestWorkspace.CreateCSharp(source1)) { // fork the solution to introduce a change - var project = workspace.CurrentSolution.Projects.Single(); - var newDocId = DocumentId.CreateNewId(project.Id); + var oldProject = workspace.CurrentSolution.Projects.Single(); + var newDocId = DocumentId.CreateNewId(oldProject.Id); var oldSolution = workspace.CurrentSolution; var newSolution = oldSolution.AddDocument(newDocId, "goo.cs", SourceText.From(source2)); workspace.TryApplyChanges(newSolution); var newProject = newSolution.Projects.Single(); - var changes = newProject.GetChanges(project); + var changes = newProject.GetChanges(oldProject); Assert.Equal(2, newProject.Documents.Count()); Assert.Equal(0, changes.GetChangedDocuments().Count()); @@ -626,7 +635,7 @@ public class D var baseActiveStatements = ImmutableArray.Create(); foreach (var changedDocumentId in changedDocuments) { - result.Add(await analyzer.AnalyzeDocumentAsync(oldSolution, baseActiveStatements, newProject.GetDocument(changedDocumentId), default(CancellationToken))); + result.Add(await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, newProject.GetDocument(changedDocumentId), trackingServiceOpt: null, CancellationToken.None)); } Assert.True(result.IsSingle()); @@ -656,15 +665,15 @@ public static void Main() using (var workspace = TestWorkspace.CreateCSharp(source1)) { // fork the solution to introduce a change - var project = workspace.CurrentSolution.Projects.Single(); - var newDocId = DocumentId.CreateNewId(project.Id); + var oldProject = workspace.CurrentSolution.Projects.Single(); + var newDocId = DocumentId.CreateNewId(oldProject.Id); var oldSolution = workspace.CurrentSolution; var newSolution = oldSolution.AddDocument(newDocId, "goo.cs", SourceText.From(source2)); workspace.TryApplyChanges(newSolution); var newProject = newSolution.Projects.Single(); - var changes = newProject.GetChanges(project); + var changes = newProject.GetChanges(oldProject); Assert.Equal(2, newProject.Documents.Count()); Assert.Equal(0, changes.GetChangedDocuments().Count()); @@ -676,7 +685,7 @@ public static void Main() var baseActiveStatements = ImmutableArray.Create(); foreach (var changedDocumentId in changedDocuments) { - result.Add(await analyzer.AnalyzeDocumentAsync(oldSolution, baseActiveStatements, newProject.GetDocument(changedDocumentId), default(CancellationToken))); + result.Add(await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, newProject.GetDocument(changedDocumentId), trackingServiceOpt: null, CancellationToken.None)); } Assert.True(result.IsSingle()); diff --git a/src/EditorFeatures/VisualBasicTest/EditAndContinue/VisualBasicEditAndContinueAnalyzerTests.vb b/src/EditorFeatures/VisualBasicTest/EditAndContinue/VisualBasicEditAndContinueAnalyzerTests.vb index bc32b3af802e1..0e2de5cdb9960 100644 --- a/src/EditorFeatures/VisualBasicTest/EditAndContinue/VisualBasicEditAndContinueAnalyzerTests.vb +++ b/src/EditorFeatures/VisualBasicTest/EditAndContinue/VisualBasicEditAndContinueAnalyzerTests.vb @@ -1,6 +1,7 @@ ' Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. Imports System.Collections.Immutable +Imports System.Threading Imports Microsoft.CodeAnalysis.Differencing Imports Microsoft.CodeAnalysis.EditAndContinue Imports Microsoft.CodeAnalysis.EditAndContinue.UnitTests @@ -436,7 +437,8 @@ End Class Dim analyzer = New VisualBasicEditAndContinueAnalyzer() Using workspace = TestWorkspace.CreateVisualBasic(source1) - Dim documentId = workspace.CurrentSolution.Projects.First().Documents.First().Id + Dim oldProject = workspace.CurrentSolution.Projects.First() + Dim documentId = oldProject.Documents.First().Id Dim oldSolution = workspace.CurrentSolution Dim newSolution = workspace.CurrentSolution.WithDocumentText(documentId, SourceText.From(source2)) Dim oldDocument = oldSolution.GetDocument(documentId) @@ -453,7 +455,7 @@ End Class Dim oldStatementSyntax = oldSyntaxRoot.FindNode(oldStatementTextSpan) Dim baseActiveStatements = ImmutableArray.Create(ActiveStatementsDescription.CreateActiveStatement(ActiveStatementFlags.IsLeafFrame, oldStatementSpan, DocumentId.CreateNewId(ProjectId.CreateNewId()))) - Dim result = Await analyzer.AnalyzeDocumentAsync(oldSolution, baseActiveStatements, newDocument, Nothing) + Dim result = Await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, newDocument, trackingServiceOpt:=Nothing, CancellationToken.None) Assert.True(result.HasChanges) Assert.True(result.SemanticEdits(0).PreserveLocalVariables) @@ -480,9 +482,10 @@ End Class Dim analyzer = New VisualBasicEditAndContinueAnalyzer() Using workspace = TestWorkspace.CreateVisualBasic(source) - Dim document = workspace.CurrentSolution.Projects.First().Documents.First() + Dim oldProject = workspace.CurrentSolution.Projects.First() + Dim document = oldProject.Documents.First() Dim baseActiveStatements = ImmutableArray.Create(Of ActiveStatement)() - Dim result = Await analyzer.AnalyzeDocumentAsync(workspace.CurrentSolution, baseActiveStatements, document, Nothing) + Dim result = Await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, document, trackingServiceOpt:=Nothing, CancellationToken.None) Assert.False(result.HasChanges) Assert.False(result.HasChangesAndErrors) @@ -509,12 +512,13 @@ End Class Dim analyzer = New VisualBasicEditAndContinueAnalyzer() Using workspace = TestWorkspace.CreateVisualBasic(source1) - Dim documentId = workspace.CurrentSolution.Projects.First().Documents.First().Id + Dim oldProject = workspace.CurrentSolution.Projects.First() + Dim documentId = oldProject.Documents.First().Id Dim oldSolution = workspace.CurrentSolution Dim newSolution = workspace.CurrentSolution.WithDocumentText(documentId, SourceText.From(source2)) Dim baseActiveStatements = ImmutableArray.Create(Of ActiveStatement)() - Dim result = Await analyzer.AnalyzeDocumentAsync(oldSolution, baseActiveStatements, newSolution.GetDocument(documentId), Nothing) + Dim result = Await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, newSolution.GetDocument(documentId), trackingServiceOpt:=Nothing, CancellationToken.None) Assert.False(result.HasChanges) Assert.False(result.HasChangesAndErrors) @@ -535,9 +539,10 @@ End Class Dim analyzer = New VisualBasicEditAndContinueAnalyzer() Using workspace = TestWorkspace.CreateVisualBasic(source) - Dim document = workspace.CurrentSolution.Projects.First().Documents.First() + Dim oldProject = workspace.CurrentSolution.Projects.First() + Dim document = oldProject.Documents.First() Dim baseActiveStatements = ImmutableArray.Create(Of ActiveStatement)() - Dim result = Await analyzer.AnalyzeDocumentAsync(workspace.CurrentSolution, baseActiveStatements, document, Nothing) + Dim result = Await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, document, trackingServiceOpt:=Nothing, CancellationToken.None) Assert.False(result.HasChanges) Assert.False(result.HasChangesAndErrors) @@ -566,12 +571,13 @@ End Class Dim analyzer = New VisualBasicEditAndContinueAnalyzer() Using workspace = TestWorkspace.CreateVisualBasic(source1) - Dim documentId = workspace.CurrentSolution.Projects.First().Documents.First().Id + Dim oldProject = workspace.CurrentSolution.Projects.First() + Dim documentId = oldProject.Documents.First().Id Dim oldSolution = workspace.CurrentSolution Dim newSolution = workspace.CurrentSolution.WithDocumentText(documentId, SourceText.From(source2)) Dim baseActiveStatements = ImmutableArray.Create(Of ActiveStatement)() - Dim result = Await analyzer.AnalyzeDocumentAsync(oldSolution, baseActiveStatements, newSolution.GetDocument(documentId), Nothing) + Dim result = Await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, newSolution.GetDocument(documentId), trackingServiceOpt:=Nothing, CancellationToken.None) ' no declaration errors (error in method body is only reported when emitting) Assert.False(result.HasChangesAndErrors) @@ -598,12 +604,13 @@ End Class Dim analyzer = New VisualBasicEditAndContinueAnalyzer() Using workspace = TestWorkspace.CreateVisualBasic(source1) - Dim documentId = workspace.CurrentSolution.Projects.First().Documents.First().Id + Dim oldProject = workspace.CurrentSolution.Projects.First() + Dim documentId = oldProject.Documents.First().Id Dim oldSolution = workspace.CurrentSolution Dim newSolution = workspace.CurrentSolution.WithDocumentText(documentId, SourceText.From(source2)) Dim baseActiveStatements = ImmutableArray.Create(Of ActiveStatement)() - Dim result = Await analyzer.AnalyzeDocumentAsync(oldSolution, baseActiveStatements, newSolution.GetDocument(documentId), Nothing) + Dim result = Await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, newSolution.GetDocument(documentId), trackingServiceOpt:=Nothing, CancellationToken.None) Assert.True(result.HasChanges) Assert.True(result.HasChangesAndErrors) @@ -650,15 +657,15 @@ End Class Using workspace = TestWorkspace.CreateVisualBasic(source1) ' fork the solution to introduce a change - Dim project = workspace.CurrentSolution.Projects.Single() - Dim newDocId = Microsoft.CodeAnalysis.DocumentId.CreateNewId(project.Id) + Dim oldProject = workspace.CurrentSolution.Projects.Single() + Dim newDocId = DocumentId.CreateNewId(oldProject.Id) Dim oldSolution = workspace.CurrentSolution Dim newSolution = oldSolution.AddDocument(newDocId, "goo.vb", SourceText.From(source2)) workspace.TryApplyChanges(newSolution) Dim newProject = newSolution.Projects.Single() - Dim changes = newProject.GetChanges(project) + Dim changes = newProject.GetChanges(oldProject) Assert.Equal(2, newProject.Documents.Count()) Assert.Equal(0, changes.GetChangedDocuments().Count()) @@ -669,7 +676,7 @@ End Class Dim result = New List(Of DocumentAnalysisResults)() Dim baseActiveStatements = ImmutableArray.Create(Of ActiveStatement)() For Each changedDocumentId In changedDocuments - result.Add(Await analyzer.AnalyzeDocumentAsync(oldSolution, baseActiveStatements, newProject.GetDocument(changedDocumentId), Nothing)) + result.Add(Await analyzer.AnalyzeDocumentAsync(oldProject, baseActiveStatements, newProject.GetDocument(changedDocumentId), trackingServiceOpt:=Nothing, CancellationToken.None)) Next Assert.True(result.IsSingle()) diff --git a/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs b/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs index b5a4a4fc5b7b0..8ef337f3501f4 100644 --- a/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs +++ b/src/Features/Core/Portable/EditAndContinue/AbstractEditAndContinueAnalyzer.cs @@ -8,7 +8,6 @@ using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.Collections; using Microsoft.CodeAnalysis.Differencing; using Microsoft.CodeAnalysis.Emit; using Microsoft.CodeAnalysis.ErrorReporting; @@ -129,9 +128,7 @@ protected SyntaxNode GetEncompassingAncestor(SyntaxNode bodyOrMatchRootOpt) protected abstract SyntaxNode FindStatementAndPartner(SyntaxNode declarationBody, int position, SyntaxNode partnerDeclarationBodyOpt, out SyntaxNode partnerOpt, out int statementPart); private SyntaxNode FindStatement(SyntaxNode declarationBody, int position, out int statementPart) - { - return FindStatementAndPartner(declarationBody, position, null, out var partner, out statementPart); - } + => FindStatementAndPartner(declarationBody, position, null, out _, out statementPart); /// /// Maps descendant of to corresponding descendant node @@ -226,7 +223,7 @@ protected abstract bool TryMatchActiveStatement( // diagnostic spans: protected abstract TextSpan GetDiagnosticSpan(SyntaxNode node, EditKind editKind); internal abstract TextSpan GetLambdaParameterDiagnosticSpan(SyntaxNode lambda, int ordinal); - private TextSpan GetBodyDiagnosticSpan(SyntaxNode body, EditKind editKind) => GetDiagnosticSpan(IsMethod(body) ? body : body.Parent, EditKind.Update); + private TextSpan GetBodyDiagnosticSpan(SyntaxNode body) => GetDiagnosticSpan(IsMethod(body) ? body : body.Parent, EditKind.Update); protected abstract string GetTopLevelDisplayName(SyntaxNode node, EditKind editKind); protected abstract string GetStatementDisplayName(SyntaxNode node, EditKind editKind); @@ -294,9 +291,10 @@ protected abstract bool TryMatchActiveStatement( #region Document Analysis public async Task AnalyzeDocumentAsync( - Solution baseSolution, + Project baseProjectOpt, ImmutableArray baseActiveStatements, Document document, + IActiveStatementTrackingService trackingServiceOpt, CancellationToken cancellationToken) { DocumentAnalysisResults.Log.Write("Analyzing document {0}", document.Name); @@ -309,9 +307,7 @@ public async Task AnalyzeDocumentAsync( SyntaxNode oldRoot; SourceText oldText; - var oldProject = baseSolution.GetProject(document.Project.Id); - var oldDocumentOpt = oldProject?.GetDocument(document.Id); - + var oldDocumentOpt = baseProjectOpt?.GetDocument(document.Id); if (oldDocumentOpt != null) { oldTreeOpt = await oldDocumentOpt.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false); @@ -333,7 +329,6 @@ public async Task AnalyzeDocumentAsync( var newRoot = await newTree.GetRootAsync(cancellationToken).ConfigureAwait(false); var newText = await document.GetTextAsync(cancellationToken).ConfigureAwait(false); - var trackingService = baseSolution.Workspace.Services.GetService(); cancellationToken.ThrowIfCancellationRequested(); @@ -356,7 +351,7 @@ public async Task AnalyzeDocumentAsync( newText, newRoot, document.Id, - trackingService, + trackingServiceOpt, newActiveStatements, newExceptionRegions); @@ -412,7 +407,7 @@ public async Task AnalyzeDocumentAsync( oldText, newText, document.Id, - trackingService, + trackingServiceOpt, baseActiveStatements, newActiveStatements, newExceptionRegions, @@ -560,7 +555,7 @@ internal void AnalyzeSyntax( SourceText oldText, SourceText newText, DocumentId documentId, - IActiveStatementTrackingService trackingService, + IActiveStatementTrackingService trackingServiceOpt, ImmutableArray oldActiveStatements, [Out]ActiveStatement[] newActiveStatements, [Out]ImmutableArray[] newExceptionRegions, @@ -578,17 +573,17 @@ internal void AnalyzeSyntax( { var edit = script.Edits[i]; - AnalyzeUpdatedActiveMethodBodies(script, i, editMap, oldText, newText, documentId, trackingService, oldActiveStatements, newActiveStatements, newExceptionRegions, updatedMethods, updatedTrackingSpans, diagnostics); + AnalyzeUpdatedActiveMethodBodies(script, i, editMap, oldText, newText, documentId, trackingServiceOpt, oldActiveStatements, newActiveStatements, newExceptionRegions, updatedMethods, updatedTrackingSpans, diagnostics); ReportSyntacticRudeEdits(diagnostics, script.Match, edit, editMap); } - UpdateUneditedSpans(diagnostics, script.Match, oldText, newText, documentId, trackingService, oldActiveStatements, newActiveStatements, newExceptionRegions, updatedTrackingSpans); + UpdateUneditedSpans(diagnostics, script.Match, oldText, newText, documentId, trackingServiceOpt, oldActiveStatements, newActiveStatements, newExceptionRegions, updatedTrackingSpans); Debug.Assert(newActiveStatements.All(a => a != null)); if (updatedTrackingSpans.Count > 0) { - trackingService.UpdateActiveStatementSpans(newText, updatedTrackingSpans); + trackingServiceOpt.UpdateActiveStatementSpans(newText, updatedTrackingSpans); } updatedTrackingSpans.Free(); @@ -600,7 +595,7 @@ private void UpdateUneditedSpans( SourceText oldText, SourceText newText, DocumentId documentId, - IActiveStatementTrackingService trackingService, + IActiveStatementTrackingService trackingServiceOpt, ImmutableArray oldActiveStatements, [In, Out]ActiveStatement[] newActiveStatements, [In, Out]ImmutableArray[] newExceptionRegions, @@ -618,8 +613,8 @@ private void UpdateUneditedSpans( { Contract.ThrowIfFalse(newExceptionRegions[i].IsDefault); TextSpan trackedSpan = default; - bool isTracked = trackingService != null && - trackingService.TryGetSpan(new ActiveStatementId(documentId, i), newText, out trackedSpan); + bool isTracked = trackingServiceOpt != null && + trackingServiceOpt.TryGetSpan(new ActiveStatementId(documentId, i), newText, out trackedSpan); if (!TryGetTextSpan(oldText.Lines, oldActiveStatements[i].Span, out var oldStatementSpan)) { DocumentAnalysisResults.Log.Write("Invalid active statement span: [{0}..{1})", oldStatementSpan.Start, oldStatementSpan.End); @@ -715,7 +710,7 @@ internal void AnalyzeUnchangedDocument( SourceText newText, SyntaxNode newRoot, DocumentId documentId, - IActiveStatementTrackingService trackingService, + IActiveStatementTrackingService trackingServiceOpt, [In, Out]ActiveStatement[] newActiveStatements, [In, Out]ImmutableArray[] newExceptionRegionsOpt) { @@ -750,8 +745,8 @@ internal void AnalyzeUnchangedDocument( // Update tracking span if we found a matching active statement whose span is different. TextSpan trackedSpan = default; - bool isTracked = trackingService != null && - trackingService.TryGetSpan(new ActiveStatementId(documentId, i), newText, out trackedSpan); + bool isTracked = trackingServiceOpt != null && + trackingServiceOpt.TryGetSpan(new ActiveStatementId(documentId, i), newText, out trackedSpan); if (isTracked && newStatementSpan != trackedSpan) { @@ -761,7 +756,7 @@ internal void AnalyzeUnchangedDocument( if (updatedTrackingSpans.Count > 0) { - trackingService.UpdateActiveStatementSpans(newText, updatedTrackingSpans); + trackingServiceOpt.UpdateActiveStatementSpans(newText, updatedTrackingSpans); } updatedTrackingSpans.Free(); @@ -874,7 +869,7 @@ private void AnalyzeUpdatedActiveMethodBodies( SourceText oldText, SourceText newText, DocumentId documentId, - IActiveStatementTrackingService trackingService, + IActiveStatementTrackingService trackingServiceOpt, ImmutableArray oldActiveStatements, [Out]ActiveStatement[] newActiveStatements, [Out]ImmutableArray[] newExceptionRegions, @@ -982,7 +977,7 @@ private void AnalyzeUpdatedActiveMethodBodies( // We seed the method body matching algorithm with tracking spans (unless they were deleted) // to get precise matching. TextSpan trackedSpan = default; - bool isTracked = trackingService?.TryGetSpan(new ActiveStatementId(documentId, ordinal), newText, out trackedSpan) ?? false; + bool isTracked = trackingServiceOpt?.TryGetSpan(new ActiveStatementId(documentId, ordinal), newText, out trackedSpan) ?? false; if (isTracked) { @@ -1380,7 +1375,7 @@ internal Match ComputeBodyMatch( { diagnostics.Add(new RudeEditDiagnostic( RudeEditKind.UpdatingStateMachineMethodAroundActiveStatement, - GetBodyDiagnosticSpan(newBody, EditKind.Update))); + GetBodyDiagnosticSpan(newBody))); } } @@ -2353,7 +2348,6 @@ internal void AnalyzeSemantics( ReportLambdaAndClosureRudeEdits( oldModel, updatedMember.OldBody, - oldSymbol, newModel, updatedMember.NewBody, newSymbol, @@ -2488,7 +2482,6 @@ internal void AnalyzeSemantics( AddConstructorEdits( instanceConstructorEdits, editScript.Match, - oldText, oldModel, newSymbolsWithEdit, isStatic: false, @@ -2502,7 +2495,6 @@ internal void AnalyzeSemantics( AddConstructorEdits( staticConstructorEdits, editScript.Match, - oldText, oldModel, newSymbolsWithEdit, isStatic: true, @@ -2815,7 +2807,6 @@ private bool DeferConstructorEdit( private void AddConstructorEdits( Dictionary updatedTypes, Match topMatch, - SourceText oldText, SemanticModel oldModel, HashSet newSymbolsWithEdit, bool isStatic, @@ -2974,7 +2965,6 @@ private static ISymbol TryGetParameterlessConstructor(INamedTypeSymbol type, boo private void ReportLambdaAndClosureRudeEdits( SemanticModel oldModel, SyntaxNode oldMemberBody, - ISymbol oldMember, SemanticModel newModel, SyntaxNode newMemberBody, ISymbol newMember, @@ -3048,11 +3038,8 @@ select clausesByQuery.First()) var oldCapturesToClosureScopes = ArrayBuilder.GetInstance(oldCaptures.Length, null); CalculateCapturedVariablesMaps( - oldModel, oldCaptures, - oldMember, oldMemberBody, - newModel, newCaptures, newMember, newMemberBody, @@ -3361,11 +3348,8 @@ private bool TryMapParameter(ValueTuple parameterKey, IReadOnly } private void CalculateCapturedVariablesMaps( - SemanticModel oldModel, ImmutableArray oldCaptures, - ISymbol oldMember, SyntaxNode oldMemberBody, - SemanticModel newModel, ImmutableArray newCaptures, ISymbol newMember, SyntaxNode newMemberBody, @@ -3772,7 +3756,7 @@ private void ReportStateMachineRudeEdits( { diagnostics.Add(new RudeEditDiagnostic( RudeEditKind.UpdatingStateMachineMethodMissingAttribute, - GetBodyDiagnosticSpan(updatedInfo.NewBody, EditKind.Update), + GetBodyDiagnosticSpan(updatedInfo.NewBody), updatedInfo.NewBody, new[] { stateMachineAttributeQualifiedName })); } diff --git a/src/Features/Core/Portable/EditAndContinue/EditSession.cs b/src/Features/Core/Portable/EditAndContinue/EditSession.cs index 4480bfbcc8248..11bf911a58efa 100644 --- a/src/Features/Core/Portable/EditAndContinue/EditSession.cs +++ b/src/Features/Core/Portable/EditAndContinue/EditSession.cs @@ -262,7 +262,7 @@ private async Task> GetBaseActiv internal bool HasProject(ProjectId id) { - return Projects.TryGetValue(id, out var reason); + return Projects.TryGetValue(id, out _); } private List<(DocumentId, AsyncLazy)> GetChangedDocumentsAnalyses(Project baseProject, Project project) @@ -353,7 +353,9 @@ private AsyncLazy GetDocumentAnalysisNoLock(Document do documentBaseActiveStatements = ImmutableArray.Empty; } - var result = await analyzer.AnalyzeDocumentAsync(_baseSolution, documentBaseActiveStatements, document, cancellationToken).ConfigureAwait(false); + var trackingService = _baseSolution.Workspace.Services.GetService(); + var baseProject = _baseSolution.GetProject(document.Project.Id); + var result = await analyzer.AnalyzeDocumentAsync(baseProject, documentBaseActiveStatements, document, trackingService, cancellationToken).ConfigureAwait(false); if (!result.RudeEditErrors.IsDefault) { diff --git a/src/Features/Core/Portable/EditAndContinue/IEditAndContinueAnalyzer.cs b/src/Features/Core/Portable/EditAndContinue/IEditAndContinueAnalyzer.cs index e0a9b3bb03504..5b7517677fd7e 100644 --- a/src/Features/Core/Portable/EditAndContinue/IEditAndContinueAnalyzer.cs +++ b/src/Features/Core/Portable/EditAndContinue/IEditAndContinueAnalyzer.cs @@ -10,7 +10,7 @@ namespace Microsoft.CodeAnalysis.EditAndContinue { internal interface IEditAndContinueAnalyzer : ILanguageService { - Task AnalyzeDocumentAsync(Solution baseSolution, ImmutableArray activeStatements, Document document, CancellationToken cancellationToken); + Task AnalyzeDocumentAsync(Project baseProjectOpt, ImmutableArray activeStatements, Document document, IActiveStatementTrackingService trackingService, CancellationToken cancellationToken); ImmutableArray GetExceptionRegions(SourceText text, SyntaxNode syntaxRoot, LinePositionSpan activeStatementSpan, bool isLeaf, out bool isCovered); } }