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

Add editor formatting service to auto-deindent closing brackets #3313

Merged
merged 10 commits into from
Sep 6, 2017
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]
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, or perhaps check this before hand if a startIndent of 0 would cause trouble?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tested with startIndent 0 and everything's fine :)

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

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