Skip to content

Commit

Permalink
Add editor formatting service to auto-deindent closing brackets (#3313)
Browse files Browse the repository at this point in the history
* Add editor formatting service for auto-deindent

* Minor refactor of the indentation service - do not indent after 'function'

* Only use smart indentation if indent style is set to 'Smart'

* Fix broken unit test build

* Implement review comments, fix build

* Fix some broken brace matching tests

Still WIP, other tests still broken

* Fix failing indentation tests

* Add formatting service tests

* Add more brace matching tests

Fixes #2092
  • Loading branch information
saul authored and cartermp committed Sep 6, 2017
1 parent 6794923 commit a6cfc81
Show file tree
Hide file tree
Showing 8 changed files with 290 additions and 54 deletions.
1 change: 1 addition & 0 deletions vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
<Compile Include="Classification\ColorizationService.fs" />
<Compile Include="Formatting\BraceMatchingService.fs" />
<Compile Include="Formatting\IndentationService.fs" />
<Compile Include="Formatting\EditorFormattingService.fs" />
<Compile Include="Debugging\BreakpointResolutionService.fs" />
<Compile Include="Debugging\LanguageDebugInfoService.fs" />
<Compile Include="Diagnostics\DocumentDiagnosticAnalyzer.fs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,18 @@ type internal FSharpBraceMatchingService
projectInfoManager: FSharpProjectOptionsManager
) =

static let userOpName = "BraceMatching"
static member GetBraceMatchingResult(checker: FSharpChecker, sourceText, fileName, options, position: int) =

static let defaultUserOpName = "BraceMatching"

static member GetBraceMatchingResult(checker: FSharpChecker, sourceText, fileName, options, position: int, userOpName: string) =
async {
let! matchedBraces = checker.MatchBraces(fileName, sourceText.ToString(), options, userOpName = userOpName)
let! matchedBraces = checker.MatchBraces(fileName, sourceText.ToString(), options, userOpName)
let isPositionInRange range =
match RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, range) with
| None -> false
| Some range -> range.Contains(position)
| Some range ->
let length = position - range.Start
length >= 0 && length <= range.Length
return matchedBraces |> Array.tryFind(fun (left, right) -> isPositionInRange left || isPositionInRange right)
}

Expand All @@ -31,7 +35,7 @@ type internal FSharpBraceMatchingService
asyncMaybe {
let! options = projectInfoManager.TryGetOptionsForEditingDocumentOrProject(document)
let! sourceText = document.GetTextAsync(cancellationToken)
let! (left, right) = FSharpBraceMatchingService.GetBraceMatchingResult(checkerProvider.Checker, sourceText, document.Name, options, position)
let! (left, right) = FSharpBraceMatchingService.GetBraceMatchingResult(checkerProvider.Checker, sourceText, document.Name, options, position, defaultUserOpName)
let! leftSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, left)
let! rightSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, right)
return BraceMatchingResult(leftSpan, rightSpan)
Expand Down
117 changes: 117 additions & 0 deletions vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
// Copyright (c) Microsoft Corporation. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.VisualStudio.FSharp.Editor

open System.Composition
open System.Collections.Generic

open Microsoft.CodeAnalysis
open Microsoft.CodeAnalysis.Editor
open Microsoft.CodeAnalysis.Formatting
open Microsoft.CodeAnalysis.Host.Mef
open Microsoft.CodeAnalysis.Text

open Microsoft.FSharp.Compiler.SourceCodeServices
open System.Threading

[<Shared>]
[<ExportLanguageService(typeof<IEditorFormattingService>, FSharpConstants.FSharpLanguageName)>]
type internal FSharpEditorFormattingService
[<ImportingConstructor>]
(
checkerProvider: FSharpCheckerProvider,
projectInfoManager: FSharpProjectOptionsManager
) =

static member GetFormattingChanges(documentId: DocumentId, sourceText: SourceText, filePath: string, checker: FSharpChecker, indentStyle: FormattingOptions.IndentStyle, projectOptions: FSharpProjectOptions option, position: int) =
// Logic for determining formatting changes:
// If first token on the current line is a closing brace,
// match the indent with the indent on the line that opened it

asyncMaybe {

// Gate formatting on whether smart indentation is enabled
// (this is what C# does)
do! Option.guard (indentStyle = FormattingOptions.IndentStyle.Smart)

let! projectOptions = projectOptions

let line = sourceText.Lines.[sourceText.Lines.IndexOf position]

let defines = CompilerEnvironment.GetCompilationDefinesForEditing(filePath, projectOptions.OtherOptions |> List.ofArray)

let tokens = Tokenizer.tokenizeLine(documentId, sourceText, line.Start, filePath, defines)

let! firstMeaningfulToken =
tokens
|> List.tryFind (fun x ->
x.Tag <> FSharpTokenTag.WHITESPACE &&
x.Tag <> FSharpTokenTag.COMMENT &&
x.Tag <> FSharpTokenTag.LINE_COMMENT)

let! (left, right) =
FSharpBraceMatchingService.GetBraceMatchingResult(checker, sourceText, filePath, projectOptions, position, "FormattingService")

if right.StartColumn = firstMeaningfulToken.LeftColumn then
// Replace the indentation on this line with the indentation of the left bracket
let! leftSpan = RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, left)

let indentChars (line : TextLine) =
line.ToString()
|> Seq.takeWhile ((=) ' ')
|> Seq.length

let startIndent = indentChars sourceText.Lines.[sourceText.Lines.IndexOf leftSpan.Start]
let currentIndent = indentChars line

return TextChange(TextSpan(line.Start, currentIndent), String.replicate startIndent " ")
else
return! None
}

member __.GetFormattingChangesAsync (document: Document, position: int, cancellationToken: CancellationToken) =
async {
let! sourceText = document.GetTextAsync(cancellationToken) |> Async.AwaitTask
let! options = document.GetOptionsAsync(cancellationToken) |> Async.AwaitTask
let indentStyle = options.GetOption(FormattingOptions.SmartIndent, FSharpConstants.FSharpLanguageName)
let projectOptionsOpt = projectInfoManager.TryGetOptionsForEditingDocumentOrProject document
let! textChange = FSharpEditorFormattingService.GetFormattingChanges(document.Id, sourceText, document.FilePath, checkerProvider.Checker, indentStyle, projectOptionsOpt, position)

return
match textChange with
| Some change ->
ResizeArray([change]) :> IList<_>

| None ->
ResizeArray() :> IList<_>
}

interface IEditorFormattingService with
member val SupportsFormatDocument = false
member val SupportsFormatSelection = false
member val SupportsFormatOnPaste = false
member val SupportsFormatOnReturn = true

override __.SupportsFormattingOnTypedCharacter (document, ch) =
if FSharpIndentationService.IsSmartIndentEnabled document.Project.Solution.Workspace.Options then
match ch with
| ')' | ']' | '}' -> true
| _ -> false
else
false

override __.GetFormattingChangesAsync (_document, _span, cancellationToken) =
async { return ResizeArray() :> IList<_> }
|> RoslynHelpers.StartAsyncAsTask cancellationToken

override __.GetFormattingChangesOnPasteAsync (_document, _span, cancellationToken) =
async { return ResizeArray() :> IList<_> }
|> RoslynHelpers.StartAsyncAsTask cancellationToken

override this.GetFormattingChangesAsync (document, _typedChar, position, cancellationToken) =
this.GetFormattingChangesAsync (document, position, cancellationToken)
|> RoslynHelpers.StartAsyncAsTask cancellationToken

override this.GetFormattingChangesOnReturnAsync (document, position, cancellationToken) =
this.GetFormattingChangesAsync (document, position, cancellationToken)
|> RoslynHelpers.StartAsyncAsTask cancellationToken
76 changes: 42 additions & 34 deletions vsintegration/src/FSharp.Editor/Formatting/IndentationService.fs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ type internal FSharpIndentationService
[<ImportingConstructor>]
(projectInfoManager: FSharpProjectOptionsManager) =

static member GetDesiredIndentation(documentId: DocumentId, sourceText: SourceText, filePath: string, lineNumber: int, tabSize: int, optionsOpt: FSharpProjectOptions option): Option<int> =
static member IsSmartIndentEnabled (options: Microsoft.CodeAnalysis.Options.OptionSet) =
options.GetOption(FormattingOptions.SmartIndent, FSharpConstants.FSharpLanguageName) = FormattingOptions.IndentStyle.Smart

static member GetDesiredIndentation(documentId: DocumentId, sourceText: SourceText, filePath: string, lineNumber: int, tabSize: int, indentStyle: FormattingOptions.IndentStyle, projectOptions: FSharpProjectOptions option): Option<int> =

// Match indentation with previous line
let rec tryFindPreviousNonEmptyLine l =
if l <= 0 then None
Expand All @@ -31,16 +35,18 @@ type internal FSharpIndentationService
else
tryFindPreviousNonEmptyLine (l - 1)

let rec tryFindLastNoneWhitespaceOrCommentToken (line: TextLine) = maybe {
let! options = optionsOpt
let defines = CompilerEnvironment.GetCompilationDefinesForEditing(filePath, options.OtherOptions |> Seq.toList)
let rec tryFindLastNonWhitespaceOrCommentToken (line: TextLine) = maybe {
let! projectOptions = projectOptions
let defines = CompilerEnvironment.GetCompilationDefinesForEditing(filePath, projectOptions.OtherOptions |> Seq.toList)
let tokens = Tokenizer.tokenizeLine(documentId, sourceText, line.Start, filePath, defines)

return!
tokens
|> List.rev
|> List.tryFind (fun x ->
x.Tag <> FSharpTokenTag.WHITESPACE && x.Tag <> FSharpTokenTag.COMMENT && x.Tag <> FSharpTokenTag.LINE_COMMENT)
x.Tag <> FSharpTokenTag.WHITESPACE &&
x.Tag <> FSharpTokenTag.COMMENT &&
x.Tag <> FSharpTokenTag.LINE_COMMENT)
}

let (|Eq|_|) y x =
Expand All @@ -49,42 +55,43 @@ type internal FSharpIndentationService

let (|NeedIndent|_|) (token: FSharpTokenInfo) =
match token.Tag with
| Eq FSharpTokenTag.EQUALS
| Eq FSharpTokenTag.LARROW
| Eq FSharpTokenTag.RARROW
| Eq FSharpTokenTag.LPAREN
| Eq FSharpTokenTag.LBRACK
| Eq FSharpTokenTag.LBRACK_BAR
| Eq FSharpTokenTag.LBRACK_LESS
| Eq FSharpTokenTag.LBRACE
| Eq FSharpTokenTag.BEGIN
| Eq FSharpTokenTag.DO
| Eq FSharpTokenTag.FUNCTION
| Eq FSharpTokenTag.THEN
| Eq FSharpTokenTag.ELSE
| Eq FSharpTokenTag.STRUCT
| Eq FSharpTokenTag.CLASS
| Eq FSharpTokenTag.TRY -> Some ()
| Eq FSharpTokenTag.EQUALS // =
| Eq FSharpTokenTag.LARROW // <-
| Eq FSharpTokenTag.RARROW // ->
| Eq FSharpTokenTag.LPAREN // (
| Eq FSharpTokenTag.LBRACK // [
| Eq FSharpTokenTag.LBRACK_BAR // [|
| Eq FSharpTokenTag.LBRACK_LESS // [<
| Eq FSharpTokenTag.LBRACE // {
| Eq FSharpTokenTag.BEGIN // begin
| Eq FSharpTokenTag.DO // do
| Eq FSharpTokenTag.THEN // then
| Eq FSharpTokenTag.ELSE // else
| Eq FSharpTokenTag.STRUCT // struct
| Eq FSharpTokenTag.CLASS // class
| Eq FSharpTokenTag.TRY -> // try
Some ()
| _ -> None

maybe {
let! previousLine = tryFindPreviousNonEmptyLine lineNumber

let lastIndent =
previousLine.ToString()
|> Seq.takeWhile ((=) ' ')
|> Seq.length

let rec loop column spaces =
if previousLine.Start + column >= previousLine.End then
spaces
// Only use smart indentation after tokens that need indentation
// if the option is enabled
let lastToken =
if indentStyle = FormattingOptions.IndentStyle.Smart then
tryFindLastNonWhitespaceOrCommentToken previousLine
else
match previousLine.Text.[previousLine.Start + column] with
| ' ' -> loop (column + 1) (spaces + 1)
| '\t' -> loop (column + 1) (((spaces / tabSize) + 1) * tabSize)
| _ -> spaces

let lastIndent = loop 0 0
None

let lastToken = tryFindLastNoneWhitespaceOrCommentToken previousLine
return
match lastToken with
| Some(NeedIndent) -> (lastIndent/tabSize + 1) * tabSize
| Some NeedIndent -> (lastIndent/tabSize + 1) * tabSize
| _ -> lastIndent
}

Expand All @@ -94,9 +101,10 @@ type internal FSharpIndentationService
let! cancellationToken = Async.CancellationToken
let! sourceText = document.GetTextAsync(cancellationToken) |> Async.AwaitTask
let! options = document.GetOptionsAsync(cancellationToken) |> Async.AwaitTask
let tabSize = options.GetOption(FormattingOptions.TabSize, FSharpConstants.FSharpLanguageName)
let tabSize = options.GetOption<int>(FormattingOptions.TabSize, FSharpConstants.FSharpLanguageName)
let indentStyle = options.GetOption(FormattingOptions.SmartIndent, FSharpConstants.FSharpLanguageName)
let projectOptionsOpt = projectInfoManager.TryGetOptionsForEditingDocumentOrProject document
let indent = FSharpIndentationService.GetDesiredIndentation(document.Id, sourceText, document.FilePath, lineNumber, tabSize, projectOptionsOpt)
let indent = FSharpIndentationService.GetDesiredIndentation(document.Id, sourceText, document.FilePath, lineNumber, tabSize, indentStyle, projectOptionsOpt)
return
match indent with
| None -> Nullable()
Expand Down
27 changes: 25 additions & 2 deletions vsintegration/tests/unittests/BraceMatchingServiceTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type BraceMatchingServiceTests() =
let position = fileContents.IndexOf(marker)
Assert.IsTrue(position >= 0, "Cannot find marker '{0}' in file contents", marker)

match FSharpBraceMatchingService.GetBraceMatchingResult(checker, sourceText, fileName, options, position) |> Async.RunSynchronously with
match FSharpBraceMatchingService.GetBraceMatchingResult(checker, sourceText, fileName, options, position, "UnitTest") |> Async.RunSynchronously with
| None -> ()
| Some(left, right) -> Assert.Fail("Found match for brace '{0}'", marker)

Expand All @@ -48,7 +48,7 @@ type BraceMatchingServiceTests() =
Assert.IsTrue(startMarkerPosition >= 0, "Cannot find start marker '{0}' in file contents", startMarkerPosition)
Assert.IsTrue(endMarkerPosition >= 0, "Cannot find end marker '{0}' in file contents", endMarkerPosition)

match FSharpBraceMatchingService.GetBraceMatchingResult(checker, sourceText, fileName, options, startMarkerPosition) |> Async.RunSynchronously with
match FSharpBraceMatchingService.GetBraceMatchingResult(checker, sourceText, fileName, options, startMarkerPosition, "UnitTest") |> Async.RunSynchronously with
| None -> Assert.Fail("Didn't find a match for start brace at position '{0}", startMarkerPosition)
| Some(left, right) ->
let endPositionInRange(range) =
Expand Down Expand Up @@ -157,3 +157,26 @@ let main argv =
(printfn "%A '%A' '%A'" (arg1) (arg2) (arg3))endBrace
0 // return an integer exit code"""
this.VerifyBraceMatch(code, "(printfn", ")endBrace")

[<TestCase ("let a1 = [ 0 .. 100 ]", [|9;10;20;21|])>]
[<TestCase ("let a2 = [| 0 .. 100 |]", [|9;10;11;21;22;23|])>]
[<TestCase ("let a3 = <@ 0 @>", [|9;10;11;14;15;16|])>]
[<TestCase ("let a4 = <@@ 0 @@>", [|9;10;11;12;15;15;16;17|])>]
[<TestCase ("let a6 = ( () )", [|9;10;16;17|])>]
[<TestCase ("[<ReflectedDefinition>]\nlet a7 = 70", [|0;1;2;21;22;23|])>]
[<TestCase ("let a8 = seq { yield() }", [|13;14;23;24|])>]
member this.BraceMatchingBothSides_Bug2092(fileContents: string, matchingPositions: int[]) =
// https://github.com/Microsoft/visualfsharp/issues/2092
let sourceText = SourceText.From(fileContents)

matchingPositions
|> Array.iter (fun position ->
match FSharpBraceMatchingService.GetBraceMatchingResult(checker, sourceText, fileName, options, position, "UnitTest") |> Async.RunSynchronously with
| Some _ -> ()
| None ->
match position with
| 0 -> ""
| _ -> fileContents.[position - 1] |> sprintf " (previous character '%c')"
|> sprintf "Didn't find a matching brace at position '%d', character '%c'%s" position fileContents.[position]
|> Assert.Fail
)
Loading

0 comments on commit a6cfc81

Please sign in to comment.