Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Move the 'application/validation/resolution' portion of rename OOP. #43592

Merged
merged 25 commits into from
Apr 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
4e41fdc
Remote ConflictResolver calls to OOP
CyrusNajmabadi Apr 23, 2020
e46fb57
Merge branch 'renameOOP5' into renameOOP6
CyrusNajmabadi Apr 23, 2020
0e0c2a1
Marshal error messages across OOP as well
CyrusNajmabadi Apr 23, 2020
8bce494
Use simple arrays.
CyrusNajmabadi Apr 23, 2020
dedad29
Merge remote-tracking branch 'upstream/master' into renameOOP6
CyrusNajmabadi Apr 23, 2020
33b9574
Have callers deal with ErrorMessage
CyrusNajmabadi Apr 23, 2020
f15f28c
Rename file
CyrusNajmabadi Apr 23, 2020
d7ba995
Move code to constructor
CyrusNajmabadi Apr 23, 2020
45bb7f6
Merge remote-tracking branch 'upstream/master' into renameOOP6
CyrusNajmabadi Apr 24, 2020
e1f15cd
Use tuple
CyrusNajmabadi Apr 24, 2020
d237d7f
Add overload for rpc that can work on streams, not just object writers.
CyrusNajmabadi Apr 24, 2020
83f8433
Merge branch 'streamOverload' into renameOOP6
CyrusNajmabadi Apr 24, 2020
d2c4574
Enable OOP testing of encapsulte field
CyrusNajmabadi Apr 24, 2020
2d98434
Move type to file
CyrusNajmabadi Apr 24, 2020
af236d4
Make an option2
CyrusNajmabadi Apr 24, 2020
b3098d2
Cleanup encapsulate field prior to OOPing it
CyrusNajmabadi Apr 24, 2020
dd12bce
Cleanup encapsulate field prior to OOPing it
CyrusNajmabadi Apr 24, 2020
4fd0f9f
Cleanup encapsulate field prior to OOPing it
CyrusNajmabadi Apr 24, 2020
9d30c3e
Move encapsulte field OOP
CyrusNajmabadi Apr 25, 2020
218d1bf
Run VB tests OOP
CyrusNajmabadi Apr 25, 2020
98143ca
Update src/Features/Core/Portable/EncapsulateField/AbstractEncapsulat…
CyrusNajmabadi Apr 25, 2020
d29fb04
Rename
CyrusNajmabadi Apr 25, 2020
2c410b9
Add comment
CyrusNajmabadi Apr 25, 2020
39c154b
Add docs
CyrusNajmabadi Apr 25, 2020
e04b8e0
use arrays
CyrusNajmabadi Apr 25, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Rename;
using Microsoft.CodeAnalysis.Rename.ConflictEngine;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Editor.Implementation.InlineRename
{
Expand Down Expand Up @@ -43,8 +44,7 @@ public async Task<IInlineRenameReplacementInfo> GetReplacementsAsync(string repl
var conflicts = await _renameLocationSet.ResolveConflictsAsync(
_renameInfo.GetFinalSymbolName(replacementText), nonConflictSymbols: null, cancellationToken: cancellationToken).ConfigureAwait(false);

if (conflicts.ErrorMessage != null)
throw new ArgumentException(conflicts.ErrorMessage);
Contract.ThrowIfTrue(conflicts.ErrorMessage != null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add Contract.ThrowIfNotNull?

Copy link
Member Author

Choose a reason for hiding this comment

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

i thought about it.. but it didn't really seem important/common enough to warrant it :)


return new InlineRenameReplacementInfo(conflicts);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ private bool Execute(EncapsulateFieldCommandArgs args, IUIThreadOperationScope w

var service = document.GetLanguageService<AbstractEncapsulateFieldService>();

var result = service.EncapsulateFieldAsync(document, spans.First().Span.ToTextSpan(), true, cancellationToken).WaitAndGetResult(cancellationToken);
var result = service.EncapsulateFieldsInSpanAsync(document, spans.First().Span.ToTextSpan(), true, cancellationToken).WaitAndGetResult(cancellationToken);

// We are about to show a modal UI dialog so we should take over the command execution
// wait context. That means the command system won't attempt to show its own wait dialog
Expand All @@ -91,8 +91,8 @@ private bool Execute(EncapsulateFieldCommandArgs args, IUIThreadOperationScope w
string.Format(EditorFeaturesResources.Preview_Changes_0, EditorFeaturesResources.Encapsulate_Field),
"vs.csharp.refactoring.preview",
EditorFeaturesResources.Encapsulate_Field_colon,
result.GetNameAsync(cancellationToken).WaitAndGetResult(cancellationToken),
result.GetGlyphAsync(cancellationToken).WaitAndGetResult(cancellationToken),
result.Name,
result.Glyph,
finalSolution,
document.Project.Solution);
}
Expand Down
18 changes: 9 additions & 9 deletions src/EditorFeatures/Test2/Rename/RenameEngineResult.vb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename
workspaceXml As XElement,
renameTo As String,
host As TestHost,
Optional changedOptionSet As Dictionary(Of OptionKey, Object) = Nothing) As RenameEngineResult
Optional changedOptionSet As Dictionary(Of OptionKey, Object) = Nothing,
Optional expectFailure As Boolean = False) As RenameEngineResult
Dim workspace = TestWorkspace.CreateWorkspace(workspaceXml)
workspace.SetTestLogger(AddressOf helper.WriteLine)

Expand Down Expand Up @@ -84,8 +85,12 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename
workspace.CurrentSolution, symbol, optionSet, CancellationToken.None).Result

Dim result = locations.ResolveConflictsAsync(renameTo, nonConflictSymbols:=Nothing, cancellationToken:=CancellationToken.None).GetAwaiter().GetResult()
If result.ErrorMessage IsNot Nothing Then
Throw New ArgumentException(result.ErrorMessage)

If expectFailure Then
Assert.NotNull(result.ErrorMessage)
Return engineResult
Else
Assert.Null(result.ErrorMessage)
End If

engineResult = New RenameEngineResult(workspace, result, renameTo)
Expand Down Expand Up @@ -188,12 +193,7 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.Rename
ElseIf isRenameWithinStringOrComment AndAlso newToken.FullSpan.Contains(newLocation) Then
newText = newToken.ToFullString().Substring(newLocation.Start - newToken.FullSpan.Start, newLocation.Length)
Else
Dim newNode = newToken.Parent
While (newNode IsNot Nothing AndAlso newNode.Span <> newLocation)
newNode = newNode.Parent
End While

newText = newNode.ToString()
newText = newTree.GetText().ToString(newLocation)
End If

Assert.Equal(replacementText, newText)
Expand Down
28 changes: 12 additions & 16 deletions src/EditorFeatures/Test2/Rename/RenameEngineTests.vb
Original file line number Diff line number Diff line change
Expand Up @@ -3672,11 +3672,10 @@ Class C
<WorkItem(627297, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/627297")>
<Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.Rename)>
Public Sub Bug622086_CSRenameVarNotSupported(host As TestHost)
Assert.ThrowsAny(Of ArgumentException)(Sub()
Dim result = RenameEngineResult.Create(_outputHelper,
<Workspace>
<Project Language="C#" AssemblyName="Project1" CommonReferences="true">
<Document><![CDATA[
Dim result = RenameEngineResult.Create(_outputHelper,
<Workspace>
<Project Language="C#" AssemblyName="Project1" CommonReferences="true">
<Document><![CDATA[
using System;

namespace X
Expand All @@ -3691,9 +3690,8 @@ namespace X
}
]]>]
</Document>
</Project>
</Workspace>, host:=host, renameTo:="Int32")
End Sub)
</Project>
</Workspace>, host:=host, renameTo:="Int32", expectFailure:=True)
End Sub

<WorkItem(627297, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/627297")>
Expand Down Expand Up @@ -3731,11 +3729,10 @@ namespace X
<WorkItem(627297, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/627297")>
<Theory, CombinatorialData, Trait(Traits.Feature, Traits.Features.Rename)>
Public Sub Bug622086_CSRenameDynamicNotSupported(host As TestHost)
Assert.ThrowsAny(Of ArgumentException)(Sub()
Dim result = RenameEngineResult.Create(_outputHelper,
<Workspace>
<Project Language="C#" AssemblyName="Project1" CommonReferences="true">
<Document><![CDATA[
Dim result = RenameEngineResult.Create(_outputHelper,
<Workspace>
<Project Language="C#" AssemblyName="Project1" CommonReferences="true">
<Document><![CDATA[
using System;

namespace X
Expand All @@ -3750,9 +3747,8 @@ namespace X
}
]]>]
</Document>
</Project>
</Workspace>, host:=host, renameTo:="Int32")
End Sub)
</Project>
</Workspace>, host:=host, renameTo:="Int32", expectFailure:=True)
End Sub

<WorkItem(627297, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/627297")>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace Microsoft.CodeAnalysis.Test.Utilities.RemoteHost
{
internal static class RemoteHostOptions
{
public static readonly Option<bool> RemoteHostTest = new Option<bool>(
public static readonly Option2<bool> RemoteHostTest = new Option2<bool>(
nameof(RemoteHostOptions), nameof(RemoteHostTest), defaultValue: false);
}

Expand Down
Loading