From 9588eb39ad2c7b4253c224d77247aa349c28ebdf Mon Sep 17 00:00:00 2001 From: Saul Rennison Date: Fri, 7 Jul 2017 20:42:56 +0100 Subject: [PATCH 1/9] Add editor formatting service for auto-deindent --- .../src/FSharp.Editor/FSharp.Editor.fsproj | 1 + .../Formatting/BraceMatchingService.fs | 12 ++- .../Formatting/EditorFormattingService.fs | 101 ++++++++++++++++++ 3 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs diff --git a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj index a2b541fa40e..c5a7906c02f 100644 --- a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj +++ b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj @@ -55,6 +55,7 @@ + diff --git a/vsintegration/src/FSharp.Editor/Formatting/BraceMatchingService.fs b/vsintegration/src/FSharp.Editor/Formatting/BraceMatchingService.fs index 1dcd1fe8aab..d9abaf7aebd 100644 --- a/vsintegration/src/FSharp.Editor/Formatting/BraceMatchingService.fs +++ b/vsintegration/src/FSharp.Editor/Formatting/BraceMatchingService.fs @@ -15,14 +15,16 @@ type internal FSharpBraceMatchingService projectInfoManager: ProjectInfoManager ) = - 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 -> position - range.Start <= range.Length return matchedBraces |> Array.tryFind(fun (left, right) -> isPositionInRange left || isPositionInRange right) } @@ -31,7 +33,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) diff --git a/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs b/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs new file mode 100644 index 00000000000..5ff0d1f7231 --- /dev/null +++ b/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs @@ -0,0 +1,101 @@ +// 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.Host.Mef +open Microsoft.CodeAnalysis.Text + +open Microsoft.FSharp.Compiler.SourceCodeServices +open System.Threading + +[] +[, FSharpConstants.FSharpLanguageName)>] +type internal FSharpEditorFormattingService + [] + ( + checkerProvider: FSharpCheckerProvider, + projectInfoManager: ProjectInfoManager + ) = + + static member GetFormattingChanges(document: Document, sourceText: SourceText, checker: FSharpChecker, optionsOpt: FSharpProjectOptions option, position: int) = + // Logic should be: + // If first token on the current line is a closing brace, + // match the indent with the indent on the line that opened it + + asyncMaybe { + let! options = optionsOpt + + let line = sourceText.Lines.[sourceText.Lines.IndexOf position] + + let defines = CompilerEnvironment.GetCompilationDefinesForEditing(document.FilePath, options.OtherOptions |> List.ofArray) + + let tokens = Tokenizer.tokenizeLine(document.Id, sourceText, line.Start, document.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, document.FilePath, options, 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 optionsOpt = projectInfoManager.TryGetOptionsForEditingDocumentOrProject document + let! textChange = FSharpEditorFormattingService.GetFormattingChanges(document, sourceText, checkerProvider.Checker, optionsOpt, position) + + return + match textChange with + | Some change -> + List([change]) :> IList<_> + | None -> + List() :> IList<_> + } + + interface IEditorFormattingService with + member val SupportsFormatDocument = false + member val SupportsFormatSelection = false + member val SupportsFormatOnPaste = false + member val SupportsFormatOnReturn = true + + override __.SupportsFormattingOnTypedCharacter (_document, ch) = + match ch with + | ')' | ']' | '}' -> true + | _ -> false + + override __.GetFormattingChangesAsync (_document, _span, _cancellationToken) = null + + override __.GetFormattingChangesOnPasteAsync (_document, _span, _cancellationToken) = null + + 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 From bbf8eae162600db3aa1d6decc4cd314335118c6b Mon Sep 17 00:00:00 2001 From: Saul Rennison Date: Fri, 7 Jul 2017 22:34:57 +0100 Subject: [PATCH 2/9] Minor refactor of the indentation service - do not indent after 'function' --- .../Formatting/IndentationService.fs | 56 +++++++++---------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Formatting/IndentationService.fs b/vsintegration/src/FSharp.Editor/Formatting/IndentationService.fs index c26879c2dbd..de44728ed56 100644 --- a/vsintegration/src/FSharp.Editor/Formatting/IndentationService.fs +++ b/vsintegration/src/FSharp.Editor/Formatting/IndentationService.fs @@ -31,7 +31,7 @@ type internal FSharpIndentationService else tryFindPreviousNonEmptyLine (l - 1) - let rec tryFindLastNoneWhitespaceOrCommentToken (line: TextLine) = maybe { + let rec tryFindLastNonWhitespaceOrCommentToken (line: TextLine) = maybe { let! options = optionsOpt let defines = CompilerEnvironment.GetCompilationDefinesForEditing(filePath, options.OtherOptions |> Seq.toList) let tokens = Tokenizer.tokenizeLine(documentId, sourceText, line.Start, filePath, defines) @@ -40,7 +40,9 @@ type internal FSharpIndentationService 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 = @@ -49,39 +51,33 @@ 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 - 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 - - let lastToken = tryFindLastNoneWhitespaceOrCommentToken previousLine + let lastToken = tryFindLastNonWhitespaceOrCommentToken previousLine return match lastToken with | Some(NeedIndent) -> (lastIndent/tabSize + 1) * tabSize From a7784be5952e08c77552e6ce2a0019deb5b01d6a Mon Sep 17 00:00:00 2001 From: Saul Rennison Date: Sat, 8 Jul 2017 11:30:43 +0100 Subject: [PATCH 3/9] Only use smart indentation if indent style is set to 'Smart' --- .../Formatting/EditorFormattingService.fs | 19 +++++++++---- .../Formatting/IndentationService.fs | 23 +++++++++++---- .../unittests/IndentationServiceTests.fs | 28 +++++++++++++++---- 3 files changed, 53 insertions(+), 17 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs b/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs index 5ff0d1f7231..d0c82a9bac5 100644 --- a/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs +++ b/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs @@ -12,6 +12,7 @@ open Microsoft.CodeAnalysis.Text open Microsoft.FSharp.Compiler.SourceCodeServices open System.Threading +open Microsoft.CodeAnalysis.Formatting [] [, FSharpConstants.FSharpLanguageName)>] @@ -23,11 +24,16 @@ type internal FSharpEditorFormattingService ) = static member GetFormattingChanges(document: Document, sourceText: SourceText, checker: FSharpChecker, optionsOpt: FSharpProjectOptions option, position: int) = - // Logic should be: + // 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 (FSharpIndentationService.IsSmartIndentEnabled document.Project.Solution.Workspace) + let! options = optionsOpt let line = sourceText.Lines.[sourceText.Lines.IndexOf position] @@ -83,10 +89,13 @@ type internal FSharpEditorFormattingService member val SupportsFormatOnPaste = false member val SupportsFormatOnReturn = true - override __.SupportsFormattingOnTypedCharacter (_document, ch) = - match ch with - | ')' | ']' | '}' -> true - | _ -> false + override __.SupportsFormattingOnTypedCharacter (document, ch) = + if FSharpIndentationService.IsSmartIndentEnabled document.Project.Solution.Workspace then + match ch with + | ')' | ']' | '}' -> true + | _ -> false + else + false override __.GetFormattingChangesAsync (_document, _span, _cancellationToken) = null diff --git a/vsintegration/src/FSharp.Editor/Formatting/IndentationService.fs b/vsintegration/src/FSharp.Editor/Formatting/IndentationService.fs index de44728ed56..684fed75137 100644 --- a/vsintegration/src/FSharp.Editor/Formatting/IndentationService.fs +++ b/vsintegration/src/FSharp.Editor/Formatting/IndentationService.fs @@ -20,7 +20,11 @@ type internal FSharpIndentationService [] (projectInfoManager: ProjectInfoManager) = - static member GetDesiredIndentation(documentId: DocumentId, sourceText: SourceText, filePath: string, lineNumber: int, tabSize: int, optionsOpt: FSharpProjectOptions option): Option = + static member IsSmartIndentEnabled (workspace: Workspace) = + let options = workspace.Options + options.GetOption(FormattingOptions.SmartIndent, FSharpConstants.FSharpLanguageName) = FormattingOptions.IndentStyle.Smart + + static member GetDesiredIndentation(document: Document, sourceText: SourceText, lineNumber: int, tabSize: int, optionsOpt: FSharpProjectOptions option): Option = // Match indentation with previous line let rec tryFindPreviousNonEmptyLine l = if l <= 0 then None @@ -33,8 +37,8 @@ type internal FSharpIndentationService let rec tryFindLastNonWhitespaceOrCommentToken (line: TextLine) = maybe { let! options = optionsOpt - let defines = CompilerEnvironment.GetCompilationDefinesForEditing(filePath, options.OtherOptions |> Seq.toList) - let tokens = Tokenizer.tokenizeLine(documentId, sourceText, line.Start, filePath, defines) + let defines = CompilerEnvironment.GetCompilationDefinesForEditing(document.FilePath, options.OtherOptions |> Seq.toList) + let tokens = Tokenizer.tokenizeLine(document.Id, sourceText, line.Start, document.FilePath, defines) return! tokens @@ -77,10 +81,17 @@ type internal FSharpIndentationService |> Seq.takeWhile ((=) ' ') |> Seq.length - let lastToken = tryFindLastNonWhitespaceOrCommentToken previousLine + // Only use smart indentation after tokens that need indentation + // if the option is enabled + let lastToken = + if FSharpIndentationService.IsSmartIndentEnabled document.Project.Solution.Workspace then + tryFindLastNonWhitespaceOrCommentToken previousLine + else + None + return match lastToken with - | Some(NeedIndent) -> (lastIndent/tabSize + 1) * tabSize + | Some NeedIndent -> (lastIndent/tabSize + 1) * tabSize | _ -> lastIndent } @@ -92,7 +103,7 @@ type internal FSharpIndentationService let! options = document.GetOptionsAsync(cancellationToken) |> Async.AwaitTask let tabSize = options.GetOption(FormattingOptions.TabSize, FSharpConstants.FSharpLanguageName) let projectOptionsOpt = projectInfoManager.TryGetOptionsForEditingDocumentOrProject document - let indent = FSharpIndentationService.GetDesiredIndentation(document.Id, sourceText, document.FilePath, lineNumber, tabSize, projectOptionsOpt) + let indent = FSharpIndentationService.GetDesiredIndentation(document, sourceText, lineNumber, tabSize, projectOptionsOpt) return match indent with | None -> Nullable() diff --git a/vsintegration/tests/unittests/IndentationServiceTests.fs b/vsintegration/tests/unittests/IndentationServiceTests.fs index e833a4c5c18..674f27343b6 100644 --- a/vsintegration/tests/unittests/IndentationServiceTests.fs +++ b/vsintegration/tests/unittests/IndentationServiceTests.fs @@ -12,6 +12,7 @@ open Microsoft.CodeAnalysis.Editor open Microsoft.CodeAnalysis.Text open Microsoft.VisualStudio.FSharp.Editor open Microsoft.FSharp.Compiler.SourceCodeServices +open Microsoft.CodeAnalysis.Formatting [][] type IndentationServiceTests() = @@ -29,8 +30,12 @@ type IndentationServiceTests() = ExtraProjectInfo = None Stamp = None } - static let documentId = DocumentId.CreateNewId(ProjectId.CreateNewId()) + static let makeAdhocProject () = + let workspace = new AdhocWorkspace() + let projectInfo = ProjectInfo.Create(ProjectId.CreateNewId(), VersionStamp.Create(), "NewProject", "NewProject", FSharpConstants.FSharpLanguageName) + workspace, workspace.AddProject(projectInfo) + static let indentComment = System.Text.RegularExpressions.Regex(@"\$\s*Indent:\s*(\d+)\s*\$") static let tabSize = 4 @@ -104,7 +109,8 @@ let t = seq { // $Indent: 0$ yield 1 // $Indent: 4$ } -let g = function +let g = + function | None -> 1 // $Indent: 4$ | Some _ -> 0 @@ -166,14 +172,24 @@ while true do [] member this.TestIndentation(expectedIndentation: Option, lineNumber: int, template: string) = - let actualIndentation = FSharpIndentationService.GetDesiredIndentation(documentId, SourceText.From(template), filePath, lineNumber, tabSize, (Some options)) + let workspace, project = makeAdhocProject() + let sourceText = SourceText.From(template) + let document = workspace.AddDocument(project.Id, filePath, sourceText) + + let actualIndentation = FSharpIndentationService.GetDesiredIndentation(document, sourceText, lineNumber, tabSize, Some options) match expectedIndentation with | None -> Assert.IsTrue(actualIndentation.IsNone, "No indentation was expected at line {0}", lineNumber) - | Some(indentation) -> Assert.AreEqual(expectedIndentation.Value, actualIndentation.Value, "Indentation on line {0} doesn't match", lineNumber) + | Some indentation -> Assert.AreEqual(expectedIndentation.Value, actualIndentation.Value, "Indentation on line {0} doesn't match", lineNumber) [] member this.TestAutoIndentation(expectedIndentation: Option, lineNumber: int, template: string) = - let actualIndentation = FSharpIndentationService.GetDesiredIndentation(documentId, SourceText.From(template), filePath, lineNumber, tabSize, (Some options)) + let workspace, project = makeAdhocProject() + workspace.Options <- + workspace.Options.WithChangedOption(FormattingOptions.SmartIndent, FSharpConstants.FSharpLanguageName, FormattingOptions.IndentStyle.Smart) + let sourceText = SourceText.From(template) + let document = workspace.AddDocument(project.Id, filePath, sourceText) + + let actualIndentation = FSharpIndentationService.GetDesiredIndentation(document, sourceText, lineNumber, tabSize, Some options) match expectedIndentation with | None -> Assert.IsTrue(actualIndentation.IsNone, "No indentation was expected at line {0}", lineNumber) - | Some(indentation) -> Assert.AreEqual(expectedIndentation.Value, actualIndentation.Value, "Indentation on line {0} doesn't match", lineNumber) + | Some indentation -> Assert.AreEqual(expectedIndentation.Value, actualIndentation.Value, "Indentation on line {0} doesn't match", lineNumber) From 76614906b7af364cb2dccb76fcd3716f572415d6 Mon Sep 17 00:00:00 2001 From: Saul Rennison Date: Tue, 11 Jul 2017 19:04:33 +0100 Subject: [PATCH 4/9] Fix broken unit test build --- vsintegration/tests/unittests/BraceMatchingServiceTests.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vsintegration/tests/unittests/BraceMatchingServiceTests.fs b/vsintegration/tests/unittests/BraceMatchingServiceTests.fs index 180224985db..00b801a8028 100644 --- a/vsintegration/tests/unittests/BraceMatchingServiceTests.fs +++ b/vsintegration/tests/unittests/BraceMatchingServiceTests.fs @@ -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) @@ -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) = From 6fb0534175beedbb0d65138dc54b4c5b38712d76 Mon Sep 17 00:00:00 2001 From: Saul Rennison Date: Wed, 23 Aug 2017 23:19:15 +0100 Subject: [PATCH 5/9] Implement review comments, fix build --- .../Formatting/EditorFormattingService.fs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs b/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs index d0c82a9bac5..d9f616023ae 100644 --- a/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs +++ b/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs @@ -12,7 +12,6 @@ open Microsoft.CodeAnalysis.Text open Microsoft.FSharp.Compiler.SourceCodeServices open System.Threading -open Microsoft.CodeAnalysis.Formatting [] [, FSharpConstants.FSharpLanguageName)>] @@ -20,7 +19,7 @@ type internal FSharpEditorFormattingService [] ( checkerProvider: FSharpCheckerProvider, - projectInfoManager: ProjectInfoManager + projectInfoManager: FSharpProjectOptionsManager ) = static member GetFormattingChanges(document: Document, sourceText: SourceText, checker: FSharpChecker, optionsOpt: FSharpProjectOptions option, position: int) = @@ -78,9 +77,10 @@ type internal FSharpEditorFormattingService return match textChange with | Some change -> - List([change]) :> IList<_> + ResizeArray([change]) :> IList<_> + | None -> - List() :> IList<_> + ResizeArray() :> IList<_> } interface IEditorFormattingService with @@ -97,9 +97,13 @@ type internal FSharpEditorFormattingService else false - override __.GetFormattingChangesAsync (_document, _span, _cancellationToken) = null + override __.GetFormattingChangesAsync (_document, _span, cancellationToken) = + async { return ResizeArray() :> IList<_> } + |> RoslynHelpers.StartAsyncAsTask cancellationToken - override __.GetFormattingChangesOnPasteAsync (_document, _span, _cancellationToken) = null + override __.GetFormattingChangesOnPasteAsync (_document, _span, cancellationToken) = + async { return ResizeArray() :> IList<_> } + |> RoslynHelpers.StartAsyncAsTask cancellationToken override this.GetFormattingChangesAsync (document, _typedChar, position, cancellationToken) = this.GetFormattingChangesAsync (document, position, cancellationToken) From 6e5debceec2b04f18ac831e8895b729318d30e9e Mon Sep 17 00:00:00 2001 From: Saul Rennison Date: Fri, 1 Sep 2017 22:48:39 +0100 Subject: [PATCH 6/9] Fix some broken brace matching tests Still WIP, other tests still broken --- .../src/FSharp.Editor/Formatting/BraceMatchingService.fs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/vsintegration/src/FSharp.Editor/Formatting/BraceMatchingService.fs b/vsintegration/src/FSharp.Editor/Formatting/BraceMatchingService.fs index 36f36357a8b..041fb73a4c5 100644 --- a/vsintegration/src/FSharp.Editor/Formatting/BraceMatchingService.fs +++ b/vsintegration/src/FSharp.Editor/Formatting/BraceMatchingService.fs @@ -24,7 +24,9 @@ type internal FSharpBraceMatchingService let isPositionInRange range = match RoslynHelpers.TryFSharpRangeToTextSpan(sourceText, range) with | None -> false - | Some range -> position - range.Start <= range.Length + | Some range -> + let length = position - range.Start + length >= 0 && length <= range.Length return matchedBraces |> Array.tryFind(fun (left, right) -> isPositionInRange left || isPositionInRange right) } From 4259c7cfff9595a5066d5b13e0de30fec0cbde8c Mon Sep 17 00:00:00 2001 From: Saul Rennison Date: Sat, 2 Sep 2017 10:51:44 +0100 Subject: [PATCH 7/9] Fix failing indentation tests --- .../Formatting/EditorFormattingService.fs | 21 +++++++++++-------- .../Formatting/IndentationService.fs | 19 +++++++++-------- .../unittests/IndentationServiceTests.fs | 19 +++++------------ 3 files changed, 27 insertions(+), 32 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs b/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs index d9f616023ae..0abbd0abddf 100644 --- a/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs +++ b/vsintegration/src/FSharp.Editor/Formatting/EditorFormattingService.fs @@ -7,6 +7,7 @@ open System.Collections.Generic open Microsoft.CodeAnalysis open Microsoft.CodeAnalysis.Editor +open Microsoft.CodeAnalysis.Formatting open Microsoft.CodeAnalysis.Host.Mef open Microsoft.CodeAnalysis.Text @@ -22,7 +23,7 @@ type internal FSharpEditorFormattingService projectInfoManager: FSharpProjectOptionsManager ) = - static member GetFormattingChanges(document: Document, sourceText: SourceText, checker: FSharpChecker, optionsOpt: FSharpProjectOptions option, position: int) = + 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 @@ -31,15 +32,15 @@ type internal FSharpEditorFormattingService // Gate formatting on whether smart indentation is enabled // (this is what C# does) - do! Option.guard (FSharpIndentationService.IsSmartIndentEnabled document.Project.Solution.Workspace) + do! Option.guard (indentStyle = FormattingOptions.IndentStyle.Smart) - let! options = optionsOpt + let! projectOptions = projectOptions let line = sourceText.Lines.[sourceText.Lines.IndexOf position] - let defines = CompilerEnvironment.GetCompilationDefinesForEditing(document.FilePath, options.OtherOptions |> List.ofArray) + let defines = CompilerEnvironment.GetCompilationDefinesForEditing(filePath, projectOptions.OtherOptions |> List.ofArray) - let tokens = Tokenizer.tokenizeLine(document.Id, sourceText, line.Start, document.FilePath, defines) + let tokens = Tokenizer.tokenizeLine(documentId, sourceText, line.Start, filePath, defines) let! firstMeaningfulToken = tokens @@ -49,7 +50,7 @@ type internal FSharpEditorFormattingService x.Tag <> FSharpTokenTag.LINE_COMMENT) let! (left, right) = - FSharpBraceMatchingService.GetBraceMatchingResult(checker, sourceText, document.FilePath, options, position, "FormattingService") + 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 @@ -71,8 +72,10 @@ type internal FSharpEditorFormattingService member __.GetFormattingChangesAsync (document: Document, position: int, cancellationToken: CancellationToken) = async { let! sourceText = document.GetTextAsync(cancellationToken) |> Async.AwaitTask - let optionsOpt = projectInfoManager.TryGetOptionsForEditingDocumentOrProject document - let! textChange = FSharpEditorFormattingService.GetFormattingChanges(document, sourceText, checkerProvider.Checker, optionsOpt, position) + 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 @@ -90,7 +93,7 @@ type internal FSharpEditorFormattingService member val SupportsFormatOnReturn = true override __.SupportsFormattingOnTypedCharacter (document, ch) = - if FSharpIndentationService.IsSmartIndentEnabled document.Project.Solution.Workspace then + if FSharpIndentationService.IsSmartIndentEnabled document.Project.Solution.Workspace.Options then match ch with | ')' | ']' | '}' -> true | _ -> false diff --git a/vsintegration/src/FSharp.Editor/Formatting/IndentationService.fs b/vsintegration/src/FSharp.Editor/Formatting/IndentationService.fs index 6b20ee835d4..9a2199ca893 100644 --- a/vsintegration/src/FSharp.Editor/Formatting/IndentationService.fs +++ b/vsintegration/src/FSharp.Editor/Formatting/IndentationService.fs @@ -20,11 +20,11 @@ type internal FSharpIndentationService [] (projectInfoManager: FSharpProjectOptionsManager) = - static member IsSmartIndentEnabled (workspace: Workspace) = - let options = workspace.Options + static member IsSmartIndentEnabled (options: Microsoft.CodeAnalysis.Options.OptionSet) = options.GetOption(FormattingOptions.SmartIndent, FSharpConstants.FSharpLanguageName) = FormattingOptions.IndentStyle.Smart - static member GetDesiredIndentation(document: Document, sourceText: SourceText, lineNumber: int, tabSize: int, optionsOpt: FSharpProjectOptions option): Option = + static member GetDesiredIndentation(documentId: DocumentId, sourceText: SourceText, filePath: string, lineNumber: int, tabSize: int, indentStyle: FormattingOptions.IndentStyle, projectOptions: FSharpProjectOptions option): Option = + // Match indentation with previous line let rec tryFindPreviousNonEmptyLine l = if l <= 0 then None @@ -36,9 +36,9 @@ type internal FSharpIndentationService tryFindPreviousNonEmptyLine (l - 1) let rec tryFindLastNonWhitespaceOrCommentToken (line: TextLine) = maybe { - let! options = optionsOpt - let defines = CompilerEnvironment.GetCompilationDefinesForEditing(document.FilePath, options.OtherOptions |> Seq.toList) - let tokens = Tokenizer.tokenizeLine(document.Id, sourceText, line.Start, document.FilePath, defines) + let! projectOptions = projectOptions + let defines = CompilerEnvironment.GetCompilationDefinesForEditing(filePath, projectOptions.OtherOptions |> Seq.toList) + let tokens = Tokenizer.tokenizeLine(documentId, sourceText, line.Start, filePath, defines) return! tokens @@ -84,7 +84,7 @@ type internal FSharpIndentationService // Only use smart indentation after tokens that need indentation // if the option is enabled let lastToken = - if FSharpIndentationService.IsSmartIndentEnabled document.Project.Solution.Workspace then + if indentStyle = FormattingOptions.IndentStyle.Smart then tryFindLastNonWhitespaceOrCommentToken previousLine else None @@ -101,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(FormattingOptions.TabSize, FSharpConstants.FSharpLanguageName) + let indentStyle = options.GetOption(FormattingOptions.SmartIndent, FSharpConstants.FSharpLanguageName) let projectOptionsOpt = projectInfoManager.TryGetOptionsForEditingDocumentOrProject document - let indent = FSharpIndentationService.GetDesiredIndentation(document, sourceText, lineNumber, tabSize, projectOptionsOpt) + let indent = FSharpIndentationService.GetDesiredIndentation(document.Id, sourceText, document.FilePath, lineNumber, tabSize, indentStyle, projectOptionsOpt) return match indent with | None -> Nullable() diff --git a/vsintegration/tests/unittests/IndentationServiceTests.fs b/vsintegration/tests/unittests/IndentationServiceTests.fs index 674f27343b6..06d53d38c32 100644 --- a/vsintegration/tests/unittests/IndentationServiceTests.fs +++ b/vsintegration/tests/unittests/IndentationServiceTests.fs @@ -31,15 +31,12 @@ type IndentationServiceTests() = Stamp = None } - static let makeAdhocProject () = - let workspace = new AdhocWorkspace() - let projectInfo = ProjectInfo.Create(ProjectId.CreateNewId(), VersionStamp.Create(), "NewProject", "NewProject", FSharpConstants.FSharpLanguageName) - workspace, workspace.AddProject(projectInfo) + static let documentId = DocumentId.CreateNewId(ProjectId.CreateNewId()) + static let tabSize = 4 + static let indentStyle = FormattingOptions.IndentStyle.Smart static let indentComment = System.Text.RegularExpressions.Regex(@"\$\s*Indent:\s*(\d+)\s*\$") - static let tabSize = 4 - static let consoleProjectTemplate = " // Learn more about F# at http://fsharp.org // See the 'F# Tutorial' project for more help. @@ -172,24 +169,18 @@ while true do [] member this.TestIndentation(expectedIndentation: Option, lineNumber: int, template: string) = - let workspace, project = makeAdhocProject() let sourceText = SourceText.From(template) - let document = workspace.AddDocument(project.Id, filePath, sourceText) - let actualIndentation = FSharpIndentationService.GetDesiredIndentation(document, sourceText, lineNumber, tabSize, Some options) + let actualIndentation = FSharpIndentationService.GetDesiredIndentation(documentId, sourceText, filePath, lineNumber, tabSize, indentStyle, Some options) match expectedIndentation with | None -> Assert.IsTrue(actualIndentation.IsNone, "No indentation was expected at line {0}", lineNumber) | Some indentation -> Assert.AreEqual(expectedIndentation.Value, actualIndentation.Value, "Indentation on line {0} doesn't match", lineNumber) [] member this.TestAutoIndentation(expectedIndentation: Option, lineNumber: int, template: string) = - let workspace, project = makeAdhocProject() - workspace.Options <- - workspace.Options.WithChangedOption(FormattingOptions.SmartIndent, FSharpConstants.FSharpLanguageName, FormattingOptions.IndentStyle.Smart) let sourceText = SourceText.From(template) - let document = workspace.AddDocument(project.Id, filePath, sourceText) - let actualIndentation = FSharpIndentationService.GetDesiredIndentation(document, sourceText, lineNumber, tabSize, Some options) + let actualIndentation = FSharpIndentationService.GetDesiredIndentation(documentId, sourceText, filePath, lineNumber, tabSize, indentStyle, Some options) match expectedIndentation with | None -> Assert.IsTrue(actualIndentation.IsNone, "No indentation was expected at line {0}", lineNumber) | Some indentation -> Assert.AreEqual(expectedIndentation.Value, actualIndentation.Value, "Indentation on line {0} doesn't match", lineNumber) From 4cd00d501afba8cea1b70b9cabb9ea9c970bbd0a Mon Sep 17 00:00:00 2001 From: Saul Rennison Date: Sat, 2 Sep 2017 11:22:00 +0100 Subject: [PATCH 8/9] Add formatting service tests --- .../unittests/EditorFormattingServiceTests.fs | 76 +++++++++++++++++++ .../unittests/VisualFSharp.Unittests.fsproj | 10 +-- 2 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 vsintegration/tests/unittests/EditorFormattingServiceTests.fs diff --git a/vsintegration/tests/unittests/EditorFormattingServiceTests.fs b/vsintegration/tests/unittests/EditorFormattingServiceTests.fs new file mode 100644 index 00000000000..83cd4c381fd --- /dev/null +++ b/vsintegration/tests/unittests/EditorFormattingServiceTests.fs @@ -0,0 +1,76 @@ +// 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.Tests.Roslyn + +open System +open System.Threading + +open NUnit.Framework + +open Microsoft.CodeAnalysis +open Microsoft.CodeAnalysis.Classification +open Microsoft.CodeAnalysis.Editor +open Microsoft.CodeAnalysis.Text +open Microsoft.VisualStudio.FSharp.Editor +open Microsoft.FSharp.Compiler.SourceCodeServices +open Microsoft.CodeAnalysis.Formatting + +[] +[] +type EditorFormattingServiceTests() = + static let filePath = "C:\\test.fs" + static let options: FSharpProjectOptions = { + ProjectFileName = "C:\\test.fsproj" + SourceFiles = [| filePath |] + ReferencedProjects = [| |] + OtherOptions = [| |] + IsIncompleteTypeCheckEnvironment = true + UseScriptResolutionRules = false + LoadTime = DateTime.MaxValue + OriginalLoadReferences = [] + UnresolvedReferences = None + ExtraProjectInfo = None + Stamp = None + } + + static let documentId = DocumentId.CreateNewId(ProjectId.CreateNewId()) + static let indentStyle = FormattingOptions.IndentStyle.Smart + + static let template = """ +let foo = [ + 15 + ]marker1 + +async { + return 10 + }marker2 + +let abc = + [| + 10 + |]marker3 + +let def = + ( + "hi" + )marker4 +""" + + [] + [] + [] + [] + member this.TestIndentation(marker: string, expectedLine: string) = + let checker = FSharpChecker.Create() + let position = template.IndexOf(marker) + Assert.IsTrue(position >= 0, "Precondition failed: unable to find marker in template") + + let sourceText = SourceText.From(template) + let lineNumber = sourceText.Lines |> Seq.findIndex (fun line -> line.Span.Contains position) + + let changesOpt = FSharpEditorFormattingService.GetFormattingChanges(documentId, sourceText, filePath, checker, indentStyle, Some options, position) |> Async.RunSynchronously + match changesOpt with + | None -> Assert.Fail("Expected a text change, but got None") + | Some change -> + let changedText = sourceText.WithChanges(change) + let lineText = changedText.Lines.[lineNumber].ToString() + Assert.IsTrue(lineText.StartsWith(expectedLine), "Changed line does not start with expected text") diff --git a/vsintegration/tests/unittests/VisualFSharp.Unittests.fsproj b/vsintegration/tests/unittests/VisualFSharp.Unittests.fsproj index e1fb16c8469..862baa9755c 100644 --- a/vsintegration/tests/unittests/VisualFSharp.Unittests.fsproj +++ b/vsintegration/tests/unittests/VisualFSharp.Unittests.fsproj @@ -101,6 +101,9 @@ Roslyn\HelpContextServiceTests.fs + + Roslyn\EditorFormattingServiceTests.fs + Roslyn\IndentationServiceTests.fs @@ -138,9 +141,8 @@ {FinalDir} $([System.IO.Path]::GetFullPath('$(OutputPath)'))\ - + - @@ -156,7 +158,6 @@ - $(FSharpSourcesRoot)\..\packages\Microsoft.VisualFSharp.Msbuild.15.0.1.0.1\lib\net45\Microsoft.Build.Framework.dll @@ -177,7 +178,7 @@ $(FSharpSourcesRoot)\..\packages\EnvDTE80.8.0.1\lib\net10\EnvDTE80.dll True - + $(FSharpSourcesRoot)\..\packages\VSSDK.VSLangProj.7.0.4\lib\net20\VSLangProj.dll True @@ -272,7 +273,6 @@ $(FSharpSourcesRoot)\..\packages\Microsoft.VisualStudio.TextManager.Interop.8.0.8.0.50727\lib\Microsoft.VisualStudio.TextManager.Interop.8.0.dll True - $(FSharpSourcesRoot)\..\packages\Microsoft.VisualStudio.Shell.Immutable.10.0.10.0.30319\lib\net40\Microsoft.VisualStudio.Shell.Immutable.10.0.dll From b4781b6f937388c2ca0d05a6b6b0e2264c444770 Mon Sep 17 00:00:00 2001 From: Saul Rennison Date: Sat, 2 Sep 2017 13:45:32 +0100 Subject: [PATCH 9/9] Add more brace matching tests Fixes #2092 --- .../unittests/BraceMatchingServiceTests.fs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/vsintegration/tests/unittests/BraceMatchingServiceTests.fs b/vsintegration/tests/unittests/BraceMatchingServiceTests.fs index 00b801a8028..465011721ad 100644 --- a/vsintegration/tests/unittests/BraceMatchingServiceTests.fs +++ b/vsintegration/tests/unittests/BraceMatchingServiceTests.fs @@ -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") + + [] + [] + [", [|9;10;11;14;15;16|])>] + [", [|9;10;11;12;15;15;16;17|])>] + [] + []\nlet a7 = 70", [|0;1;2;21;22;23|])>] + [] + 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 + ) \ No newline at end of file