Skip to content

Commit

Permalink
Merge pull request #31096 from CyrusNajmabadi/indentationCleanup
Browse files Browse the repository at this point in the history 
Pull up check in indentation code, and remove the need for nullable.
  • Loading branch information
@sharwell
sharwell committed Nov 12, 2018
2 parents 704b724 + 71efab2 commit 3925ac2
Show file tree
Hide file tree
Showing 18 changed files with 347 additions and 238 deletions.
20 changes: 12 additions & 8 deletions src/EditorFeatures/CSharp/Formatting/Indentation/CSharpIndentationService.Indenter.cs
View file
Expand Up @@ -35,7 +35,11 @@ internal class Indenter : AbstractIndenter
{
}

protected override IndentationResult? GetDesiredIndentationWorker(
public override bool ShouldUseFormatterIfAvailable()
=> ShouldUseSmartTokenFormatterInsteadOfIndenter(
Rules, Root, LineToBeIndented, OptionSet, CancellationToken);

protected override IndentationResult GetDesiredIndentationWorker(
SyntaxToken token, TextLine previousLine, int lastNonWhitespacePosition)
{
// okay, now check whether the text we found is trivia or actual token.
Expand All @@ -56,7 +60,7 @@ internal class Indenter : AbstractIndenter
return IndentFromStartOfLine(0);
}

var trivia = Tree.GetRoot(CancellationToken).FindTrivia(firstNonWhitespacePosition.Value, findInsideTrivia: true);
var trivia = Root.FindTrivia(firstNonWhitespacePosition.Value, findInsideTrivia: true);
if (trivia.Kind() == SyntaxKind.None || this.LineToBeIndented.LineNumber > previousLine.LineNumber + 1)
{
// If the token belongs to the next statement and is also the first token of the statement, then it means the user wants
Expand Down Expand Up @@ -106,7 +110,7 @@ internal class Indenter : AbstractIndenter
}
}

private IndentationResult? GetIndentationBasedOnToken(SyntaxToken token)
private IndentationResult GetIndentationBasedOnToken(SyntaxToken token)
{
Contract.ThrowIfNull(Tree);
Contract.ThrowIfTrue(token.Kind() == SyntaxKind.None);
Expand Down Expand Up @@ -254,7 +258,7 @@ internal class Indenter : AbstractIndenter
}
}

private IndentationResult? GetIndentationFromCommaSeparatedList(SyntaxToken token)
private IndentationResult GetIndentationFromCommaSeparatedList(SyntaxToken token)
{
var node = token.Parent;
switch (node)
Expand All @@ -276,7 +280,7 @@ internal class Indenter : AbstractIndenter
return GetDefaultIndentationFromToken(token);
}

private IndentationResult? GetIndentationFromCommaSeparatedList<T>(SeparatedSyntaxList<T> list, SyntaxToken token) where T : SyntaxNode
private IndentationResult GetIndentationFromCommaSeparatedList<T>(SeparatedSyntaxList<T> list, SyntaxToken token) where T : SyntaxNode
{
var index = list.GetWithSeparators().IndexOf(token);
if (index < 0)
Expand All @@ -302,7 +306,7 @@ internal class Indenter : AbstractIndenter
return GetDefaultIndentationFromTokenLine(token, additionalSpace: 0);
}

private IndentationResult? GetDefaultIndentationFromToken(SyntaxToken token)
private IndentationResult GetDefaultIndentationFromToken(SyntaxToken token)
{
if (IsPartOfQueryExpression(token))
{
Expand All @@ -312,7 +316,7 @@ internal class Indenter : AbstractIndenter
return GetDefaultIndentationFromTokenLine(token);
}

private IndentationResult? GetIndentationForQueryExpression(SyntaxToken token)
private IndentationResult GetIndentationForQueryExpression(SyntaxToken token)
{
// find containing non terminal node
var queryExpressionClause = GetQueryExpressionClause(token);
Expand Down Expand Up @@ -402,7 +406,7 @@ private bool IsPartOfQueryExpression(SyntaxToken token)
return queryExpression != null;
}

private IndentationResult? GetDefaultIndentationFromTokenLine(SyntaxToken token, int? additionalSpace = null)
private IndentationResult GetDefaultIndentationFromTokenLine(SyntaxToken token, int? additionalSpace = null)
{
var spaceToAdd = additionalSpace ?? this.OptionSet.GetOption(FormattingOptions.IndentationSize, token.Language);

Expand Down
13 changes: 1 addition & 12 deletions src/EditorFeatures/CSharp/Formatting/Indentation/CSharpIndentationService.cs
View file
Expand Up @@ -23,7 +23,7 @@
namespace Microsoft.CodeAnalysis.Editor.CSharp.Formatting.Indentation
{
[ExportLanguageService(typeof(ISynchronousIndentationService), LanguageNames.CSharp), Shared]
internal partial class CSharpIndentationService : AbstractIndentationService
internal partial class CSharpIndentationService : AbstractIndentationService<CompilationUnitSyntax>
{
private static readonly IFormattingRule s_instance = new FormattingRule();

Expand All @@ -40,17 +40,6 @@ protected override IFormattingRule GetSpecializedIndentationFormattingRule()
optionSet, lineToBeIndented, cancellationToken);
}

protected override bool ShouldUseSmartTokenFormatterInsteadOfIndenter(
IEnumerable<IFormattingRule> formattingRules,
SyntaxNode root,
TextLine line,
OptionSet optionSet,
CancellationToken cancellationToken)
{
return ShouldUseSmartTokenFormatterInsteadOfIndenter(
formattingRules, (CompilationUnitSyntax)root, line, optionSet, cancellationToken);
}

public static bool ShouldUseSmartTokenFormatterInsteadOfIndenter(
IEnumerable<IFormattingRule> formattingRules,
CompilationUnitSyntax root,
Expand Down
77 changes: 11 additions & 66 deletions ...matting/Indentation/FormatterTestsBase.cs → ...g/Indentation/CSharpFormatterTestsBase.cs
View file
Expand Up @@ -7,7 +7,10 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Editor.CSharp.Formatting.Indentation;
using Microsoft.CodeAnalysis.Editor.Implementation.Formatting.Indentation;
using Microsoft.CodeAnalysis.Editor.Implementation.SmartIndent;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Editor.UnitTests.Formatting;
using Microsoft.CodeAnalysis.Editor.UnitTests.Utilities;
using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces;
using Microsoft.CodeAnalysis.Formatting;
Expand All @@ -23,11 +26,12 @@
using Microsoft.VisualStudio.Text.Projection;
using Moq;
using Xunit;
using static Microsoft.CodeAnalysis.Formatting.FormattingOptions;

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Formatting.Indentation
{
[UseExportProvider]
public class FormatterTestsBase
public class CSharpFormatterTestsBase : CoreFormatterTestsBase
{
protected const string HtmlMarkup = @"<html>
<body>
Expand All @@ -36,6 +40,12 @@ public class FormatterTestsBase
</html>";
protected const int BaseIndentationOfNugget = 8;

internal override string GetLanguageName()
=> LanguageNames.CSharp;

internal override AbstractSmartTokenFormatterCommandHandler CreateSmartTokenFormatterCommandHandler(ITextUndoHistoryRegistry registry, IEditorOperationsFactoryService operations)
=> new SmartTokenFormatterCommandHandler(registry, operations);

protected static async Task<int> GetSmartTokenFormatterIndentationWorkerAsync(
TestWorkspace workspace,
ITextBuffer buffer,
Expand Down Expand Up @@ -119,70 +129,5 @@ private static void ApplyChanges(ITextBuffer buffer, IList<TextChange> changes)
return await GetSmartTokenFormatterIndentationWorkerAsync(workspace, buffer, indentationLine, ch);
}
}

internal static void TestIndentation(int point, int? expectedIndentation, ITextView textView, TestHostDocument subjectDocument)
{
var textUndoHistory = new Mock<ITextUndoHistoryRegistry>();
var editorOperationsFactory = new Mock<IEditorOperationsFactoryService>();
var editorOperations = new Mock<IEditorOperations>();
editorOperationsFactory.Setup(x => x.GetEditorOperations(textView)).Returns(editorOperations.Object);

var snapshot = subjectDocument.TextBuffer.CurrentSnapshot;
var indentationLineFromBuffer = snapshot.GetLineFromPosition(point);

var commandHandler = new SmartTokenFormatterCommandHandler(textUndoHistory.Object, editorOperationsFactory.Object);
commandHandler.ExecuteCommand(new ReturnKeyCommandArgs(textView, subjectDocument.TextBuffer), () => { }, TestCommandExecutionContext.Create());
var newSnapshot = subjectDocument.TextBuffer.CurrentSnapshot;

int? actualIndentation;
if (newSnapshot.Version.VersionNumber > snapshot.Version.VersionNumber)
{
actualIndentation = newSnapshot.GetLineFromLineNumber(indentationLineFromBuffer.LineNumber).GetFirstNonWhitespaceOffset();
}
else
{
var provider = new SmartIndent(textView);
actualIndentation = provider.GetDesiredIndentation(indentationLineFromBuffer);
}

Assert.Equal(expectedIndentation, actualIndentation.Value);
}

public static void TestIndentation(int indentationLine, int? expectedIndentation, TestWorkspace workspace)
{
var snapshot = workspace.Documents.First().TextBuffer.CurrentSnapshot;
var bufferGraph = new Mock<IBufferGraph>(MockBehavior.Strict);
bufferGraph.Setup(x => x.MapUpToSnapshot(It.IsAny<SnapshotPoint>(),
It.IsAny<PointTrackingMode>(),
It.IsAny<PositionAffinity>(),
It.IsAny<ITextSnapshot>()))
.Returns<SnapshotPoint, PointTrackingMode, PositionAffinity, ITextSnapshot>((p, m, a, s) =>
{
if (workspace.Services.GetService<IHostDependentFormattingRuleFactoryService>() is TestFormattingRuleFactoryServiceFactory.Factory factory && factory.BaseIndentation != 0 && factory.TextSpan.Contains(p.Position))
{
var line = p.GetContainingLine();
var projectedOffset = line.GetFirstNonWhitespaceOffset().Value - factory.BaseIndentation;
return new SnapshotPoint(p.Snapshot, p.Position - projectedOffset);
}
return p;
});

var projectionBuffer = new Mock<ITextBuffer>(MockBehavior.Strict);
projectionBuffer.Setup(x => x.ContentType.DisplayName).Returns("None");

var textView = new Mock<ITextView>(MockBehavior.Strict);
textView.Setup(x => x.Options).Returns(TestEditorOptions.Instance);
textView.Setup(x => x.BufferGraph).Returns(bufferGraph.Object);
textView.SetupGet(x => x.TextSnapshot.TextBuffer).Returns(projectionBuffer.Object);

var provider = new SmartIndent(textView.Object);

var indentationLineFromBuffer = snapshot.GetLineFromLineNumber(indentationLine);
var actualIndentation = provider.GetDesiredIndentation(indentationLineFromBuffer);

Assert.Equal(expectedIndentation, actualIndentation);
}
}
}
12 changes: 9 additions & 3 deletions src/EditorFeatures/CSharpTest/Formatting/Indentation/SmartIndenterEnterOnTokenTests.cs
View file
Expand Up @@ -11,10 +11,11 @@
using Microsoft.CodeAnalysis.Text;
using Roslyn.Test.Utilities;
using Xunit;
using static Microsoft.CodeAnalysis.Formatting.FormattingOptions;

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Formatting.Indentation
{
public class SmartIndenterEnterOnTokenTests : FormatterTestsBase
public class SmartIndenterEnterOnTokenTests : CSharpFormatterTestsBase
{
[WorkItem(537808, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/537808")]
[WpfFact]
Expand Down Expand Up @@ -1376,11 +1377,14 @@ void Main(object o)
private async Task AssertIndentNotUsingSmartTokenFormatterButUsingIndenterAsync(
string code,
int indentationLine,
int? expectedIndentation)
int? expectedIndentation,
int? expectedBlankLineIndentation = null,
IndentStyle indentStyle = IndentStyle.Smart)
{
// create tree service
using (var workspace = TestWorkspace.CreateCSharp(code))
{
workspace.Options = workspace.Options.WithChangedOption(SmartIndent, LanguageNames.CSharp, indentStyle);
var hostdoc = workspace.Documents.First();
var buffer = hostdoc.GetTextBuffer();
var snapshot = buffer.CurrentSnapshot;
Expand All @@ -1395,7 +1399,9 @@ void Main(object o)
Formatter.GetDefaultFormattingRules(workspace, root.Language),
root, line.AsTextLine(), await document.GetOptionsAsync(), CancellationToken.None));

TestIndentation(indentationLine, expectedIndentation, workspace);
TestIndentation(
workspace, indentationLine,
expectedIndentation, expectedBlankLineIndentation);
}
}
}
Expand Down
65 changes: 55 additions & 10 deletions src/EditorFeatures/CSharpTest/Formatting/Indentation/SmartIndenterTests.cs
View file
Expand Up @@ -8,10 +8,11 @@
using Microsoft.VisualStudio.Text;
using Roslyn.Test.Utilities;
using Xunit;
using static Microsoft.CodeAnalysis.Formatting.FormattingOptions;

namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.Formatting.Indentation
{
public partial class SmartIndenterTests : FormatterTestsBase
public partial class SmartIndenterTests : CSharpFormatterTestsBase
{
[WpfFact]
[Trait(Traits.Feature, Traits.Features.SmartIndent)]
Expand Down Expand Up @@ -99,6 +100,26 @@ class Class
expectedIndentation: 4);
}

[WpfFact]
[Trait(Traits.Feature, Traits.Features.SmartIndent)]
public void TestExplicitNoneIndentStyle()
{
var code = @"using System;
class Class
{
// Comments
/// Xml Comments
";
AssertSmartIndent(
code,
indentationLine: 6,
expectedIndentation: null,
expectedBlankLineIndentation: 0,
indentStyle: IndentStyle.None);
}

[WpfFact]
[Trait(Traits.Feature, Traits.Features.SmartIndent)]
public void UsingDirective()
Expand Down Expand Up @@ -389,10 +410,16 @@ void Method()
code,
indentationLine: 6,
expectedIndentation: 12);

// This is the line after the method call. ISynchronousIndentationService will bail in
// this case as it thinks this is a case for "smart formatting". However,
// IBlankLineIndentationService appropriately picks 8 columns as the location to indent
// to.
AssertSmartIndent(
code,
indentationLine: 7,
expectedIndentation: null);
expectedIndentation: null,
expectedBlankLineIndentation: 8);
}

[WpfFact]
Expand Down Expand Up @@ -2684,7 +2711,11 @@ void M(object o)
expectedIndentation: 16);
}

private static void AssertSmartIndentInProjection(string markup, int expectedIndentation, CSharpParseOptions options = null)
private void AssertSmartIndentInProjection(
string markup, int expectedIndentation,
int? expectedBlankLineIndentation = null,
CSharpParseOptions options = null,
IndentStyle indentStyle = IndentStyle.Smart)
{
var optionsSet = options != null
? new[] { options }
Expand All @@ -2694,6 +2725,7 @@ private static void AssertSmartIndentInProjection(string markup, int expectedInd
{
using (var workspace = TestWorkspace.CreateCSharp(markup, parseOptions: option))
{
workspace.Options = workspace.Options.WithChangedOption(SmartIndent, LanguageNames.CSharp, indentStyle);
var subjectDocument = workspace.Documents.Single();

var projectedDocument =
Expand All @@ -2708,16 +2740,21 @@ private static void AssertSmartIndentInProjection(string markup, int expectedInd
var indentationLine = projectedDocument.TextBuffer.CurrentSnapshot.GetLineFromPosition(projectedDocument.CursorPosition.Value);
var point = projectedDocument.GetTextView().BufferGraph.MapDownToBuffer(indentationLine.Start, PointTrackingMode.Negative, subjectDocument.TextBuffer, PositionAffinity.Predecessor);

TestIndentation(point.Value, expectedIndentation, projectedDocument.GetTextView(), subjectDocument);
TestIndentation(
workspace, point.Value,
expectedIndentation, expectedBlankLineIndentation,
projectedDocument.GetTextView(), subjectDocument);
}
}
}

private static void AssertSmartIndent(
private void AssertSmartIndent(
string code,
int indentationLine,
int? expectedIndentation,
CSharpParseOptions options = null)
int? expectedBlankLineIndentation = null,
CSharpParseOptions options = null,
IndentStyle indentStyle = IndentStyle.Smart)
{
var optionsSet = options != null
? new[] { options }
Expand All @@ -2727,15 +2764,20 @@ private static void AssertSmartIndentInProjection(string markup, int expectedInd
{
using (var workspace = TestWorkspace.CreateCSharp(code, parseOptions: option))
{
TestIndentation(indentationLine, expectedIndentation, workspace);
workspace.Options = workspace.Options.WithChangedOption(SmartIndent, LanguageNames.CSharp, indentStyle);
TestIndentation(
workspace, indentationLine,
expectedIndentation, expectedBlankLineIndentation);
}
}
}

private static void AssertSmartIndent(
private void AssertSmartIndent(
string code,
int? expectedIndentation,
CSharpParseOptions options = null)
int? expectedBlankLineIndentation = null,
CSharpParseOptions options = null,
IndentStyle indentStyle = IndentStyle.Smart)
{
var optionsSet = options != null
? new[] { options }
Expand All @@ -2745,9 +2787,12 @@ private static void AssertSmartIndentInProjection(string markup, int expectedInd
{
using (var workspace = TestWorkspace.CreateCSharp(code, parseOptions: option))
{
workspace.Options = workspace.Options.WithChangedOption(SmartIndent, LanguageNames.CSharp, indentStyle);
var wpfTextView = workspace.Documents.First().GetTextView();
var line = wpfTextView.TextBuffer.CurrentSnapshot.GetLineFromPosition(wpfTextView.Caret.Position.BufferPosition).LineNumber;
TestIndentation(line, expectedIndentation, workspace);
TestIndentation(
workspace, line,
expectedIndentation, expectedBlankLineIndentation);
}
}
}
Expand Down

0 comments on commit 3925ac2

Please sign in to comment.