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.

Sign up for GitHub

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

Port: Don't assume rename text parses as IdentiferNameSyntax #3126

Merged
merged 2 commits into from
May 27, 2015
Merged

Port: Don't assume rename text parses as IdentiferNameSyntax #3126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f94813
Don't assume rename text parses as IdentiferNameSyntax
May 24, 2015
d2c164d
Improve handling of invalid rename text in Inline Rename
May 24, 2015
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
12 changes: 6 additions & 6 deletions src/EditorFeatures/Core/Implementation/InlineRename/Dashboard/DashboardViewModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ private void OnReplacementsComputed(object sender, IInlineRenameReplacementInfo
{
_errorText = string.IsNullOrEmpty(session.ReplacementText)
? null
: string.Format(EditorFeaturesResources.IsNotAValidIdentifier, session.ReplacementText);
: string.Format(EditorFeaturesResources.IsNotAValidIdentifier, GetTruncatedName(session.ReplacementText));
}

UpdateSeverity();
Expand Down Expand Up @@ -156,15 +156,15 @@ public string Description
{
get
{
return string.Format(EditorFeaturesResources.Rename1, GetTruncatedSymbolName());
return string.Format(EditorFeaturesResources.Rename1, GetTruncatedName(Session.OriginalSymbolName));
}
}

private string GetTruncatedSymbolName()
private static string GetTruncatedName(string fullName)
{
return this.Session.OriginalSymbolName.Length < SymbolDescriptionTextLength
? this.Session.OriginalSymbolName
: this.Session.OriginalSymbolName.Substring(0, SymbolDescriptionTextLength) + "...";
return fullName.Length < SymbolDescriptionTextLength
? fullName
: fullName.Substring(0, SymbolDescriptionTextLength) + "...";
}

public string SearchText
Expand Down
24 changes: 12 additions & 12 deletions ...torFeatures/Core/Implementation/InlineRename/InlineRenameSession.OpenTextBufferManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.ErrorReporting;
using Microsoft.CodeAnalysis.Internal.Log;
using Microsoft.CodeAnalysis.Rename.ConflictEngine;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Text;
using Microsoft.CodeAnalysis.Text.Shared.Extensions;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.Utilities;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Editor.Implementation.InlineRename
Expand Down Expand Up @@ -227,24 +226,25 @@ private void OnTextBufferChanged(object sender, TextContentChangedEventArgs args
var trackingSpansAfterEdit = new NormalizedSpanCollection(GetEditableSpansForSnapshot(args.After).Select(ss => (Span)ss));
var spansTouchedInEdit = new NormalizedSpanCollection(args.Changes.Select(c => c.NewSpan));

var intersection = NormalizedSpanCollection.Intersection(trackingSpansAfterEdit, spansTouchedInEdit);

if (intersection.Count == 0)
var intersectionSpans = NormalizedSpanCollection.Intersection(trackingSpansAfterEdit, spansTouchedInEdit);
if (intersectionSpans.Count == 0)
{
// In Razor we sometimes get formatting changes during inline rename that
// do not intersect with any of our spans. Ideally this shouldn't happen at
// all, but if it does happen we can just ignore it.
return;
}
else if (intersection.Count > 1)
{
Contract.Fail("we can't allow edits to touch multiple spans");
}

var intersectionSpan = intersection.Single();
var singleTrackingSpanTouched = GetEditableSpansForSnapshot(args.After).Single(ss => ss.IntersectsWith(intersectionSpan));
_activeSpan = _referenceSpanToLinkedRenameSpanMap.Where(kvp => kvp.Value.TrackingSpan.GetSpan(args.After).Contains(intersectionSpan)).Single().Key;
// Cases with invalid identifiers may cause there to be multiple intersection
// spans, but they should still all map to a single tracked rename span (e.g.
// renaming "two" to "one two three" may be interpreted as two distinct
// additions of "one" and "three").
var boundingIntersectionSpan = Span.FromBounds(intersectionSpans.First().Start, intersectionSpans.Last().End);
var trackingSpansTouched = GetEditableSpansForSnapshot(args.After).Where(ss => ss.IntersectsWith(boundingIntersectionSpan));
Contract.Assert(trackingSpansTouched.Count() == 1);

var singleTrackingSpanTouched = trackingSpansTouched.Single();
_activeSpan = _referenceSpanToLinkedRenameSpanMap.Where(kvp => kvp.Value.TrackingSpan.GetSpan(args.After).Contains(boundingIntersectionSpan)).Single().Key;
_session.UndoManager.OnTextChanged(this.ActiveTextview.Selection, singleTrackingSpanTouched);
}
}
Expand Down
38 changes: 38 additions & 0 deletions src/EditorFeatures/Test2/Rename/RenameEngineTests.CSharpConflicts.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3306,5 +3306,43 @@ class Program
result.AssertLabeledSpansAre("conflict", "Console", RelatedLocationType.UnresolvedConflict)
End Using
End Sub

<WorkItem(1031, "https://github.com/dotnet/roslyn/issues/1031")>
<Fact>
<Trait(Traits.Feature, Traits.Features.Rename)>
Public Sub InvalidNamesDoNotCauseCrash_IntroduceQualifiedName()
Using result = RenameEngineResult.Create(
<Workspace>
<Project Language="C#" CommonReferences="true">
<Document>
class {|conflict:C$$|} { }
</Document>
</Project>
</Workspace>, renameTo:="C.D")

result.AssertReplacementTextInvalid()
result.AssertLabeledSpansAre("conflict", "C.D", RelatedLocationType.UnresolvedConflict)
End Using
End Sub

<WorkItem(1031, "https://github.com/dotnet/roslyn/issues/1031")>
<Fact>
<Trait(Traits.Feature, Traits.Features.Rename)>
Public Sub InvalidNamesDoNotCauseCrash_AccidentallyPasteLotsOfCode()
Dim renameTo = "class C { public void M() { for (int i = 0; i < 10; i++) { System.Console.Writeline(""This is a test""); } } }"

Using result = RenameEngineResult.Create(
<Workspace>
<Project Language="C#" CommonReferences="true">
<Document>
class {|conflict:C$$|} { }
</Document>
</Project>
</Workspace>, renameTo)

result.AssertReplacementTextInvalid()
result.AssertLabeledSpansAre("conflict", renameTo, RelatedLocationType.UnresolvedConflict)
End Using
End Sub
End Class
End Namespace
46 changes: 46 additions & 0 deletions src/EditorFeatures/Test2/Rename/RenameEngineTests.VisualBasicConflicts.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3013,6 +3013,52 @@ End Class
result.AssertLabeledSpansAre("conflict", "Test", RelatedLocationType.UnresolvedConflict)
End Using
End Sub

<WorkItem(1031, "https://github.com/dotnet/roslyn/issues/1031")>
<Fact>
<Trait(Traits.Feature, Traits.Features.Rename)>
Public Sub InvalidNamesDoNotCauseCrash_IntroduceQualifiedName()
Using result = RenameEngineResult.Create(
<Workspace>
<Project Language="Visual Basic" CommonReferences="true">
<Document FilePath="Test.cs"><![CDATA[
Class {|conflict:C$$|}
End Class
]]>
</Document>
</Project>
</Workspace>, renameTo:="C.D")

result.AssertReplacementTextInvalid()
result.AssertLabeledSpansAre("conflict", "C.D", RelatedLocationType.UnresolvedConflict)
End Using
End Sub

<WorkItem(1031, "https://github.com/dotnet/roslyn/issues/1031")>
<Fact>
<Trait(Traits.Feature, Traits.Features.Rename)>
Public Sub InvalidNamesDoNotCauseCrash_AccidentallyPasteLotsOfCode()
Dim renameTo = "
Class C
Sub M()
System.Console.WriteLine(""Hello, Test!"")
End Sub
End Class"
Using result = RenameEngineResult.Create(
<Workspace>
<Project Language="Visual Basic" CommonReferences="true">
<Document FilePath="Test.cs"><![CDATA[
Class {|conflict:C$$|}
End Class
]]>
</Document>
</Project>
</Workspace>, renameTo)

result.AssertReplacementTextInvalid()
result.AssertLabeledSpansAre("conflict", renameTo, RelatedLocationType.UnresolvedConflict)
End Using
End Sub
End Class
End Class
End Namespace
10 changes: 6 additions & 4 deletions src/Workspaces/CSharp/Portable/Rename/CSharpRenameRewriterLanguageService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,17 +570,19 @@ private SyntaxToken RenameToken(SyntaxToken oldToken, SyntaxToken newToken, stri
}

// determine the canonical identifier name (unescaped, no unicode escaping, ...)
string valueText;
string valueText = currentNewIdentifier;
var kind = SyntaxFacts.GetKeywordKind(currentNewIdentifier);
if (kind != SyntaxKind.None)
{
valueText = SyntaxFacts.GetText(kind);
}
else
{
var parsedIdentifier = (IdentifierNameSyntax)SyntaxFactory.ParseName(currentNewIdentifier);
Debug.Assert(parsedIdentifier.Kind() == SyntaxKind.IdentifierName);
valueText = parsedIdentifier.Identifier.ValueText;
var parsedIdentifier = SyntaxFactory.ParseName(currentNewIdentifier);
if (parsedIdentifier.IsKind(SyntaxKind.IdentifierName))
{
valueText = ((IdentifierNameSyntax)parsedIdentifier).Identifier.ValueText;
}
}

// TODO: we can't use escaped unicode characters in xml doc comments, so we need to pass the valuetext as text as well.
Expand Down
3 changes: 1 addition & 2 deletions src/Workspaces/VisualBasic/Portable/Rename/VisualBasicRenameRewriterLanguageService.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,8 +511,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Rename

If name.IsKind(SyntaxKind.GlobalName) Then
valueText = currentNewIdentifier
Else
Debug.Assert(name.IsKind(SyntaxKind.IdentifierName))
ElseIf name.IsKind(SyntaxKind.IdentifierName) Then
valueText = DirectCast(name, IdentifierNameSyntax).Identifier.ValueText
End If

Expand Down