diff --git a/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs b/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs index 08ce2ae91c6..b6b13608cb1 100644 --- a/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs +++ b/vsintegration/src/FSharp.Editor/CodeFixes/AddOpenCodeFixProvider.fs @@ -2,40 +2,102 @@ namespace Microsoft.VisualStudio.FSharp.Editor +open System open System.Composition -open System.Threading.Tasks -open System.Threading open System.Collections.Immutable open Microsoft.CodeAnalysis.Text open Microsoft.CodeAnalysis.CodeFixes open FSharp.Compiler.EditorServices +open FSharp.Compiler.Syntax open FSharp.Compiler.Text + open CancellableTasks [] type internal AddOpenCodeFixProvider [] (assemblyContentProvider: AssemblyContentProvider) = inherit CodeFixProvider() + static let br = Environment.NewLine + let fixUnderscoresInMenuText (text: string) = text.Replace("_", "__") let qualifySymbolFix (context: CodeFixContext) (fullName, qualifier) = - context.RegisterFsharpFix(CodeFix.AddOpen, fixUnderscoresInMenuText fullName, [| TextChange(context.Span, qualifier) |]) - - let openNamespaceFix (context: CodeFixContext) ctx name ns multipleNames sourceText = - let displayText = "open " + ns + (if multipleNames then " (" + name + ")" else "") - let newText, _ = OpenDeclarationHelper.insertOpenDeclaration sourceText ctx ns - let changes = newText.GetTextChanges(sourceText) + { + Name = CodeFix.AddOpen + Message = fixUnderscoresInMenuText fullName + Changes = [ TextChange(context.Span, qualifier) ] + } - context.RegisterFsharpFix(CodeFix.AddOpen, fixUnderscoresInMenuText displayText, changes) + // Hey, I know what you're thinking: this is a horrible hack. + // Indeed it is, this is a better (but still bad) version of the OpenDeclarionHelper. + // The things should be actually fixed in the InsertionContext, it's bugged. + // But currently CompletionProvider also depends on InsertionContext and it's not tested enough. + // So fixing InsertionContext or OpenDeclarationHelper might break completion which would be bad. + // The hack below is at least heavily tested. + // And at least it shows what should be fixed down the line. + let getOpenDeclaration (sourceText: SourceText) (ctx: InsertionContext) (ns: string) = + // insertion context counts from 2, make the world sane + let insertionLineNumber = ctx.Pos.Line - 2 + let margin = String(' ', ctx.Pos.Column) + + let startLineNumber, openDeclaration = + match ctx.ScopeKind with + | ScopeKind.TopModule -> + match sourceText.Lines[ insertionLineNumber ].ToString().Trim() with + + // explicit top level module + | line when line.StartsWith "module" && not (line.EndsWith "=") -> insertionLineNumber + 2, $"{margin}open {ns}{br}{br}" + + // nested module, shouldn't be here + | line when line.StartsWith "module" -> insertionLineNumber, $"{margin}open {ns}{br}{br}" + + // attribute, shouldn't be here + | line when line.StartsWith "[<" && line.EndsWith ">]" -> + let moduleDeclLineNumber = + sourceText.Lines + |> Seq.skip insertionLineNumber + |> Seq.findIndex (fun line -> line.ToString().Contains "module") + // add back the skipped lines + |> fun i -> insertionLineNumber + i + + let moduleDeclLineText = sourceText.Lines[ moduleDeclLineNumber ].ToString().Trim() + + if moduleDeclLineText.EndsWith "=" then + insertionLineNumber, $"{margin}open {ns}{br}{br}" + else + moduleDeclLineNumber + 2, $"{margin}open {ns}{br}{br}" + + // something else, shot in the dark + | _ -> insertionLineNumber, $"{margin}open {ns}{br}{br}" + + | ScopeKind.Namespace -> insertionLineNumber + 3, $"{margin}open {ns}{br}{br}" + | ScopeKind.NestedModule -> insertionLineNumber + 2, $"{margin}open {ns}{br}{br}" + | ScopeKind.OpenDeclaration -> insertionLineNumber + 1, $"{margin}open {ns}{br}" + + // So far I don't know how to get here + | ScopeKind.HashDirective -> insertionLineNumber + 1, $"open {ns}{br}{br}" + + let start = sourceText.Lines[startLineNumber].Start + TextChange(TextSpan(start, 0), openDeclaration) + + let openNamespaceFix ctx name ns multipleNames sourceText = + let displayText = $"open {ns}" + (if multipleNames then " (" + name + ")" else "") + let change = getOpenDeclaration sourceText ctx ns + + { + Name = CodeFix.AddOpen + Message = fixUnderscoresInMenuText displayText + Changes = [ change ] + } - let addSuggestionsAsCodeFixes + let getSuggestionsAsCodeFixes (context: CodeFixContext) (sourceText: SourceText) (candidates: (InsertionContextEntity * InsertionContext) list) = - do + seq { candidates |> Seq.choose (fun (entity, ctx) -> entity.Namespace |> Option.map (fun ns -> ns, entity.FullDisplayName, ctx)) |> Seq.groupBy (fun (ns, _, _) -> ns) @@ -50,128 +112,125 @@ type internal AddOpenCodeFixProvider [] (assemblyContentPr let multipleNames = names |> Array.length > 1 names |> Seq.map (fun (name, ctx) -> ns, name, ctx, multipleNames)) |> Seq.concat - |> Seq.iter (fun (ns, name, ctx, multipleNames) -> openNamespaceFix context ctx name ns multipleNames sourceText) + |> Seq.map (fun (ns, name, ctx, multipleNames) -> openNamespaceFix ctx name ns multipleNames sourceText) - do candidates |> Seq.filter (fun (entity, _) -> not (entity.LastIdent.StartsWith "op_")) // Don't include qualified operator names. The resultant codefix won't compile because it won't be an infix operator anymore. |> Seq.map (fun (entity, _) -> entity.FullRelativeName, entity.Qualifier) |> Seq.distinct |> Seq.sort - |> Seq.iter (qualifySymbolFix context) + |> Seq.map (qualifySymbolFix context) + + } + |> Seq.concat override _.FixableDiagnosticIds = ImmutableArray.Create("FS0039", "FS0043") - override _.RegisterCodeFixesAsync context : Task = - asyncMaybe { - let document = context.Document - - let! sourceText = document.GetTextAsync(context.CancellationToken) - - let! parseResults, checkResults = - document.GetFSharpParseAndCheckResultsAsync(nameof (AddOpenCodeFixProvider)) - |> CancellableTask.start context.CancellationToken - |> Async.AwaitTask - |> liftAsync - - let line = sourceText.Lines.GetLineFromPosition(context.Span.End) - let linePos = sourceText.Lines.GetLinePosition(context.Span.End) - - let! defines, langVersion, strictIndentation = - document.GetFsharpParsingOptionsAsync(nameof (AddOpenCodeFixProvider)) - |> liftAsync - - let! symbol = - maybe { - let! lexerSymbol = - Tokenizer.getSymbolAtPosition ( - document.Id, - sourceText, - context.Span.End, - document.FilePath, - defines, - SymbolLookupKind.Greedy, - false, - false, - Some langVersion, - strictIndentation, - context.CancellationToken - ) - - return - checkResults.GetSymbolUseAtLocation( - Line.fromZ linePos.Line, - lexerSymbol.Ident.idRange.EndColumn, - line.ToString(), - lexerSymbol.FullIsland - ) - } - - do! Option.guard symbol.IsNone - - let unresolvedIdentRange = - let startLinePos = sourceText.Lines.GetLinePosition context.Span.Start - let startPos = Position.fromZ startLinePos.Line startLinePos.Character - let endLinePos = sourceText.Lines.GetLinePosition context.Span.End - let endPos = Position.fromZ endLinePos.Line endLinePos.Character - Range.mkRange context.Document.FilePath startPos endPos - - let isAttribute = - ParsedInput.GetEntityKind(unresolvedIdentRange.Start, parseResults.ParseTree) = Some EntityKind.Attribute - - let entities = - assemblyContentProvider.GetAllEntitiesInProjectAndReferencedAssemblies checkResults - |> List.collect (fun s -> - [ - yield s.TopRequireQualifiedAccessParent, s.AutoOpenParent, s.Namespace, s.CleanedIdents - if isAttribute then - let lastIdent = s.CleanedIdents.[s.CleanedIdents.Length - 1] - - if - lastIdent.EndsWith "Attribute" - && s.Kind LookupType.Precise = EntityKind.Attribute - then - yield - s.TopRequireQualifiedAccessParent, - s.AutoOpenParent, - s.Namespace, - s.CleanedIdents - |> Array.replace (s.CleanedIdents.Length - 1) (lastIdent.Substring(0, lastIdent.Length - 9)) - ]) - - let longIdent = - ParsedInput.GetLongIdentAt parseResults.ParseTree unresolvedIdentRange.End - - let! maybeUnresolvedIdents = - longIdent - |> Option.map (fun longIdent -> - longIdent - |> List.map (fun ident -> - { - Ident = ident.idText - Resolved = not (ident.idRange = unresolvedIdentRange) - }) - |> List.toArray) - - let insertionPoint = - if document.Project.IsFSharpCodeFixesAlwaysPlaceOpensAtTopLevelEnabled then - OpenStatementInsertionPoint.TopLevel - else - OpenStatementInsertionPoint.Nearest - - let createEntity = - ParsedInput.TryFindInsertionContext - unresolvedIdentRange.StartLine - parseResults.ParseTree - maybeUnresolvedIdents - insertionPoint - - return - entities - |> Seq.map createEntity - |> Seq.concat - |> Seq.toList - |> addSuggestionsAsCodeFixes context sourceText - } - |> Async.Ignore - |> RoslynHelpers.StartAsyncUnitAsTask(context.CancellationToken) + override this.RegisterCodeFixesAsync context = context.RegisterFsharpFix this + + interface IFSharpCodeFixProvider with + member _.GetCodeFixIfAppliesAsync context = + cancellableTask { + let document = context.Document + + let! sourceText = context.GetSourceTextAsync() + + let! parseResults, checkResults = document.GetFSharpParseAndCheckResultsAsync(nameof AddOpenCodeFixProvider) + + let line = sourceText.Lines.GetLineFromPosition(context.Span.End) + let linePos = sourceText.Lines.GetLinePosition(context.Span.End) + + let! defines, langVersion, strictIndentation = document.GetFsharpParsingOptionsAsync(nameof AddOpenCodeFixProvider) + + return + Tokenizer.getSymbolAtPosition ( + document.Id, + sourceText, + context.Span.End, + document.FilePath, + defines, + SymbolLookupKind.Greedy, + false, + false, + Some langVersion, + strictIndentation, + context.CancellationToken + ) + |> Option.filter (fun lexerSymbol -> + let symbolOpt = + checkResults.GetSymbolUseAtLocation( + Line.fromZ linePos.Line, + lexerSymbol.Ident.idRange.EndColumn, + line.ToString(), + lexerSymbol.FullIsland + ) + + match symbolOpt with + | None -> true + // this is for operators for FS0043 + | Some symbol when PrettyNaming.IsLogicalOpName symbol.Symbol.DisplayName -> true + | _ -> false) + |> Option.bind (fun _ -> + let unresolvedIdentRange = + let startLinePos = sourceText.Lines.GetLinePosition context.Span.Start + let startPos = Position.fromZ startLinePos.Line startLinePos.Character + let endLinePos = sourceText.Lines.GetLinePosition context.Span.End + let endPos = Position.fromZ endLinePos.Line endLinePos.Character + Range.mkRange context.Document.FilePath startPos endPos + + let isAttribute = + ParsedInput.GetEntityKind(unresolvedIdentRange.Start, parseResults.ParseTree) = Some EntityKind.Attribute + + let entities = + assemblyContentProvider.GetAllEntitiesInProjectAndReferencedAssemblies checkResults + |> List.collect (fun s -> + [ + yield s.TopRequireQualifiedAccessParent, s.AutoOpenParent, s.Namespace, s.CleanedIdents + if isAttribute then + let lastIdent = s.CleanedIdents.[s.CleanedIdents.Length - 1] + + if + lastIdent.EndsWith "Attribute" + && s.Kind LookupType.Precise = EntityKind.Attribute + then + yield + s.TopRequireQualifiedAccessParent, + s.AutoOpenParent, + s.Namespace, + s.CleanedIdents + |> Array.replace (s.CleanedIdents.Length - 1) (lastIdent.Substring(0, lastIdent.Length - 9)) + ]) + + ParsedInput.GetLongIdentAt parseResults.ParseTree unresolvedIdentRange.End + |> Option.bind (fun longIdent -> + let maybeUnresolvedIdents = + longIdent + |> List.map (fun ident -> + { + Ident = ident.idText + Resolved = not (ident.idRange = unresolvedIdentRange) + }) + |> List.toArray + + let insertionPoint = + if document.Project.IsFSharpCodeFixesAlwaysPlaceOpensAtTopLevelEnabled then + OpenStatementInsertionPoint.TopLevel + else + OpenStatementInsertionPoint.Nearest + + let createEntity = + ParsedInput.TryFindInsertionContext + unresolvedIdentRange.StartLine + parseResults.ParseTree + maybeUnresolvedIdents + insertionPoint + + entities + |> Seq.map createEntity + |> Seq.concat + |> Seq.toList + |> getSuggestionsAsCodeFixes context sourceText + |> Seq.tryHead)) + + |> ValueOption.ofOption + } diff --git a/vsintegration/src/FSharp.Editor/Options/EditorOptions.fs b/vsintegration/src/FSharp.Editor/Options/EditorOptions.fs index 90d4a1e47bd..fae67f36e32 100644 --- a/vsintegration/src/FSharp.Editor/Options/EditorOptions.fs +++ b/vsintegration/src/FSharp.Editor/Options/EditorOptions.fs @@ -162,6 +162,8 @@ type EditorOptions() = [)>] member private _.SettingsStore = store + member _.With value = store.Register value + interface Microsoft.CodeAnalysis.Host.IWorkspaceService module internal OptionsUI = diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs new file mode 100644 index 00000000000..297554f3784 --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOffTests.fs @@ -0,0 +1,407 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.AddOpenOnTopOffTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = AddOpenCodeFixProvider(AssemblyContentProvider()) + +let mode = + WithSettings + { CodeFixesOptions.Default with + AlwaysPlaceOpensAtTopLevel = false + } + +[] +let ``Fixes FS0039 for missing opens - basic`` () = + let code = + """Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - first line is empty`` () = + let code = + """ +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - multiple first lines are empty`` () = + let code = + """ + +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ + +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - there is already an open directive`` () = + let code = + """open System.IO + +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """open System.IO +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - top level module is explicit`` () = + let code = + """module Module1 + +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """module Module1 + +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - nested module`` () = + let code = + """module Module1 = + + Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """module Module1 = + + open System + + Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - module has attributes`` () = + let code = + """ +[] +module Module1 + +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +[] +module Module1 + +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) + +// TODO: the open statement should actually be within the module +[] +let ``Fixes FS0039 for missing opens - nested module has attributes`` () = + let code = + """ +[] +module Module1 = + + Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +open System + +[] +module Module1 = + + Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - module has multiple attributes`` () = + let code = + """ +[] +[] +module Module1 + +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +[] +[] +module Module1 + +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - attributes are mixed with empty lines`` () = + let code = + """ +[] + +[] +module Module1 + +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +[] + +[] +module Module1 + +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - multiple modules in one file`` () = + let code = + """ +module Module1 = + + let x = 42 + +module Module2 = + + Console.WriteLine(42) +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +module Module1 = + + let x = 42 + +module Module2 = + + open System + + Console.WriteLine(42) +""" + } + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - explicit namespace`` () = + let code = + """ +namespace N1 + +module M1 = + + Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +namespace N1 + +module M1 = + + open System + + Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) + +[] +let ``Doesn't fix FS0039 for random undefined symbols`` () = + let code = + """ +let f = g +""" + + let expected = None + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0043 for missing opens`` () = + let code = + """ +module M = + let (++) x y = 10 * x + y + +module N = + let theAnswer = 4 ++ 2 +""" + + let expected = + Some + { + Message = "open M" + FixedCode = + """ +module M = + let (++) x y = 10 * x + y + +open M + +module N = + let theAnswer = 4 ++ 2 +""" + } + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) + +[] +let ``Doesn't fix FS0043 for random unsupported values`` () = + let code = + """ +type RecordType = { X : int } + +let x : RecordType = null +""" + + let expected = None + + let actual = codeFix |> tryFix code mode + + Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs new file mode 100644 index 00000000000..ac95de440d4 --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/AddOpenOnTopOnTests.fs @@ -0,0 +1,400 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Editor.Tests.CodeFixes.AddOpenOnTopOnTests + +open Microsoft.VisualStudio.FSharp.Editor +open Xunit + +open CodeFixTestFramework + +let private codeFix = AddOpenCodeFixProvider(AssemblyContentProvider()) + +[] +let ``Fixes FS0039 for missing opens - basic`` () = + let code = + """Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - first line is empty`` () = + let code = + """ +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - multiple first lines are empty`` () = + let code = + """ + +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ + +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - there is already an open directive`` () = + let code = + """open System.IO + +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """open System.IO +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - top level module is explicit`` () = + let code = + """module Module1 + +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """module Module1 + +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - nested module`` () = + let code = + """module Module1 = + + Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """open System + +module Module1 = + + Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - module has attributes`` () = + let code = + """ +[] +module Module1 + +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +[] +module Module1 + +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - nested module has attributes`` () = + let code = + """ +[] +module Module1 = + + Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +open System + +[] +module Module1 = + + Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - module has multiple attributes`` () = + let code = + """ +[] +[] +module Module1 + +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +[] +[] +module Module1 + +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - attributes are mixed with empty lines`` () = + let code = + """ +[] + +[] +module Module1 + +Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +[] + +[] +module Module1 + +open System + +Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - multiple modules in one file`` () = + let code = + """ +module Module1 = + + let x = 42 + +module Module2 = + + Console.WriteLine(42) +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +open System + +module Module1 = + + let x = 42 + +module Module2 = + + Console.WriteLine(42) +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0039 for missing opens - explicit namespace`` () = + let code = + """ +namespace N1 + +module M1 = + + Console.WriteLine 42 +""" + + let expected = + Some + { + Message = "open System" + FixedCode = + """ +namespace N1 + +open System + +module M1 = + + Console.WriteLine 42 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Doesn't fix FS0039 for random undefined symbols`` () = + let code = + """ +let f = g +""" + + let expected = None + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Fixes FS0043 for missing opens`` () = + let code = + """ +module M = + let (++) x y = 10 * x + y + +module N = + let theAnswer = 4 ++ 2 +""" + + let expected = + Some + { + Message = "open M" + FixedCode = + """ +module M = + let (++) x y = 10 * x + y + +open M + +module N = + let theAnswer = 4 ++ 2 +""" + } + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) + +[] +let ``Doesn't fix FS0043 for random unsupported values`` () = + let code = + """ +type RecordType = { X : int } + +let x : RecordType = null +""" + + let expected = None + + let actual = codeFix |> tryFix code Auto + + Assert.Equal(expected, actual) diff --git a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs index 2098dc76958..d1986e6ded3 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/CodeFixes/CodeFixTestFramework.fs @@ -22,6 +22,7 @@ type Mode = | WithOption of CustomProjectOption: string | WithSignature of FsiCode: string | Manual of Squiggly: string * Diagnostic: string + | WithSettings of CodeFixesOptions let inline toOption o = match o with @@ -44,6 +45,7 @@ let getDocument code mode = | WithOption option -> RoslynTestHelpers.GetFsDocument(code, option) | WithSignature fsiCode -> RoslynTestHelpers.GetFsiAndFsDocuments fsiCode code |> Seq.last | Manual _ -> RoslynTestHelpers.GetFsDocument code + | WithSettings settings -> RoslynTestHelpers.GetFsDocument(code, customEditorOptions = settings) let getRelevantDiagnostics (document: Document) = cancellableTask { @@ -67,6 +69,7 @@ let createTestCodeFixContext (code: string) document (mode: Mode) diagnosticIds |> Array.filter (fun d -> diagnosticIds |> Seq.contains d.ErrorNumberText) | WithOption _ -> getRelevantDiagnostics document | WithSignature _ -> getRelevantDiagnostics document + | WithSettings _ -> getRelevantDiagnostics document | Manual (squiggly, diagnostic) -> let spanStart = code.IndexOf squiggly let span = TextSpan(spanStart, squiggly.Length) diff --git a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj index 27233890a44..415567694a6 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -58,6 +58,8 @@ + + diff --git a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs index ee069431c98..5b00301821b 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Helpers/RoslynHelpers.fs @@ -175,15 +175,6 @@ type TestHostWorkspaceServices(hostServices: HostServices, workspace: Workspace) let langServices = TestHostLanguageServices(this, LanguageNames.FSharp, exportProvider) - member this.SetEditorEptions(value) = - exportProvider - .GetExportedValue() - .SaveSettings(value) - - member this.WithEditorOptions(value) = - this.SetEditorEptions(value) - this - override _.Workspace = workspace override this.GetService<'T when 'T :> IWorkspaceService>() : 'T = @@ -281,7 +272,10 @@ type RoslynTestHelpers private () = options.OtherOptions |> ImmutableArray.CreateRange ) - static member CreateSolution(source, ?options: FSharpProjectOptions) = + static member SetEditorOptions (solution: Solution) options = + solution.Workspace.Services.GetService().With(options) + + static member CreateSolution(source, ?options: FSharpProjectOptions, ?editorOptions) = let projId = ProjectId.CreateNewId() let docInfo = RoslynTestHelpers.CreateDocumentInfo projId "C:\\test.fs" source @@ -294,6 +288,9 @@ type RoslynTestHelpers private () = |> Option.defaultValue RoslynTestHelpers.DefaultProjectOptions |> RoslynTestHelpers.SetProjectOptions projId solution + if editorOptions.IsSome then + RoslynTestHelpers.SetEditorOptions solution editorOptions.Value + solution static member GetSingleDocument(solution: Solution) = @@ -338,7 +335,7 @@ type RoslynTestHelpers private () = solution, checker - static member GetFsDocument(code, ?customProjectOption: string) = + static member GetFsDocument(code, ?customProjectOption: string, ?customEditorOptions) = let customProjectOptions = customProjectOption |> Option.map (fun o -> [| o |]) @@ -354,8 +351,12 @@ type RoslynTestHelpers private () = |> Array.append customProjectOptions } - RoslynTestHelpers.CreateSolution(code, options = options) - |> RoslynTestHelpers.GetSingleDocument + let solution = + match customEditorOptions with + | Some o -> RoslynTestHelpers.CreateSolution(code, options, o) + | None -> RoslynTestHelpers.CreateSolution(code, options) + + solution |> RoslynTestHelpers.GetSingleDocument static member GetFsiAndFsDocuments (fsiCode: string) (fsCode: string) = let projInfo =