From 816241169c89491d62527648cdece582df6a11b0 Mon Sep 17 00:00:00 2001 From: Florian Verdonck Date: Thu, 14 Mar 2024 16:21:00 +0100 Subject: [PATCH] Detect type alias in implementation. (#1243) * Detect type alias in implementation. * Check if is file pair. * Initial insert in signature file. * Add first unit test * Format code * Clean up --- .editorconfig | 2 +- .../ParseAndCheckResults.fs | 4 + .../ParseAndCheckResults.fsi | 2 + .../CodeFixes/AddTypeAliasToSignatureFile.fs | 200 ++++++++++++++++++ .../CodeFixes/AddTypeAliasToSignatureFile.fsi | 6 + .../LspServers/AdaptiveServerState.fs | 3 +- .../AddTypeAliasToSignatureFileTests.fs | 186 ++++++++++++++++ .../CodeFixTests/Tests.fs | 3 +- .../UpdateValueInSignatureFileTests.fs | 43 +--- .../Utils/CursorbasedTests.fs | 32 ++- .../Utils/CursorbasedTests.fsi | 10 + 11 files changed, 449 insertions(+), 42 deletions(-) create mode 100644 src/FsAutoComplete/CodeFixes/AddTypeAliasToSignatureFile.fs create mode 100644 src/FsAutoComplete/CodeFixes/AddTypeAliasToSignatureFile.fsi create mode 100644 test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddTypeAliasToSignatureFileTests.fs diff --git a/.editorconfig b/.editorconfig index 319a5a0c3..8d1e1bb03 100644 --- a/.editorconfig +++ b/.editorconfig @@ -22,4 +22,4 @@ indent_size = 2 fsharp_max_array_or_list_width=80 fsharp_max_dot_get_expression_width=80 fsharp_max_function_binding_width=80 -fsharp_max_value_binding_width=80 \ No newline at end of file +fsharp_max_value_binding_width=80 diff --git a/src/FsAutoComplete.Core/ParseAndCheckResults.fs b/src/FsAutoComplete.Core/ParseAndCheckResults.fs index d912e4e19..49149a55d 100644 --- a/src/FsAutoComplete.Core/ParseAndCheckResults.fs +++ b/src/FsAutoComplete.Core/ParseAndCheckResults.fs @@ -519,6 +519,10 @@ type ParseAndCheckResults let identIsland = Array.toList identIsland checkResults.GetSymbolUseAtLocation(pos.Line, colu, lineStr, identIsland) + member x.TryGetSymbolUseFromIdent (sourceText: ISourceText) (ident: Ident) : FSharpSymbolUse option = + let line = sourceText.GetLineString(ident.idRange.EndLine - 1) + x.GetCheckResults.GetSymbolUseAtLocation(ident.idRange.EndLine, ident.idRange.EndColumn, line, [ ident.idText ]) + member __.TryGetSymbolUses (pos: Position) (lineStr: LineStr) : FSharpSymbolUse list = match Lexer.findLongIdents (pos.Column, lineStr) with | None -> [] diff --git a/src/FsAutoComplete.Core/ParseAndCheckResults.fsi b/src/FsAutoComplete.Core/ParseAndCheckResults.fsi index f3099f45a..524615055 100644 --- a/src/FsAutoComplete.Core/ParseAndCheckResults.fsi +++ b/src/FsAutoComplete.Core/ParseAndCheckResults.fsi @@ -67,6 +67,8 @@ type ParseAndCheckResults = member TryGetSymbolUse: pos: Position -> lineStr: LineStr -> FSharpSymbolUse option + member TryGetSymbolUseFromIdent: ISourceText -> Ident -> FSharpSymbolUse option + member TryGetSymbolUses: pos: Position -> lineStr: LineStr -> FSharpSymbolUse list member TryGetSymbolUseAndUsages: diff --git a/src/FsAutoComplete/CodeFixes/AddTypeAliasToSignatureFile.fs b/src/FsAutoComplete/CodeFixes/AddTypeAliasToSignatureFile.fs new file mode 100644 index 000000000..539f6a68d --- /dev/null +++ b/src/FsAutoComplete/CodeFixes/AddTypeAliasToSignatureFile.fs @@ -0,0 +1,200 @@ +module FsAutoComplete.CodeFix.AddTypeAliasToSignatureFile + +open System +open FSharp.Compiler.Symbols +open FSharp.Compiler.Syntax +open FSharp.Compiler.Text +open FSharp.Compiler.CodeAnalysis +open FsToolkit.ErrorHandling +open Ionide.LanguageServerProtocol.Types +open FsAutoComplete.CodeFix.Types +open FsAutoComplete +open FsAutoComplete.LspHelpers + +let mkLongIdRange (lid: LongIdent) = lid |> List.map (fun ident -> ident.idRange) |> List.reduce Range.unionRanges + +let (|AllOpenOrHashDirective|_|) (decls: SynModuleSigDecl list) : range option = + match decls with + | [] -> None + | decls -> + + let allOpenOrHashDirective = + decls + |> List.forall (function + | SynModuleSigDecl.Open _ + | SynModuleSigDecl.HashDirective _ -> true + | _ -> false) + + if not allOpenOrHashDirective then + None + else + Some (List.last decls).Range.EndRange + +type SynTypeDefn with + + member x.FullRange = + match x with + | SynTypeDefn(range = m; trivia = { LeadingKeyword = lk }) -> Range.unionRanges lk.Range m + +let title = "Add type alias to signature file" + +let codeFixForImplementationFileWithSignature + (getProjectOptionsForFile: GetProjectOptionsForFile) + (codeFix: CodeFix) + (codeActionParams: CodeActionParams) + : Async> = + async { + let fileName = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath + let! project = getProjectOptionsForFile fileName + + match project with + | Error _ -> return Ok [] + | Ok projectOptions -> + + let signatureFile = String.Concat(fileName, "i") + let hasSig = projectOptions.SourceFiles |> Array.contains signatureFile + + if not hasSig then + return Ok [] + else + return! codeFix codeActionParams + } + +let fix + (getProjectOptionsForFile: GetProjectOptionsForFile) + (getParseResultsForFile: GetParseResultsForFile) + : CodeFix = + codeFixForImplementationFileWithSignature getProjectOptionsForFile (fun (codeActionParams: CodeActionParams) -> + asyncResult { + let fileName = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath + // The converted LSP start position to an FCS start position. + let fcsPos = protocolPosToPos codeActionParams.Range.Start + // The syntax tree and typed tree, current line and sourceText of the current file. + let! (parseAndCheckResults: ParseAndCheckResults, _line: string, sourceText: IFSACSourceText) = + getParseResultsForFile fileName fcsPos + + let typeDefnInfo = + (fcsPos, parseAndCheckResults.GetParseResults.ParseTree) + ||> ParsedInput.tryPick (fun _path node -> + match node with + | SyntaxNode.SynTypeDefn(SynTypeDefn( + typeInfo = SynComponentInfo(longId = [ typeIdent ]) + typeRepr = SynTypeDefnRepr.Simple(simpleRepr = SynTypeDefnSimpleRepr.TypeAbbrev _)) as tdn) when + (Range.rangeContainsPos tdn.FullRange fcsPos) + -> + Some(typeIdent, tdn.FullRange) + | _ -> None) + + match typeDefnInfo with + | None -> return [] + | Some(typeName, mTypeDefn) -> + + match parseAndCheckResults.TryGetSymbolUseFromIdent sourceText typeName with + | None -> return [] + | Some typeSymbolUse -> + + match typeSymbolUse.Symbol with + | :? FSharpEntity as entity -> + let isPartOfSignature = + match entity.SignatureLocation with + | None -> false + | Some sigLocation -> Utils.isSignatureFile sigLocation.FileName + + if isPartOfSignature then + return [] + else + + let implFilePath = codeActionParams.TextDocument.GetFilePath() + let sigFilePath = $"%s{implFilePath}i" + let sigFileName = Utils.normalizePath sigFilePath + + let sigTextDocumentIdentifier: TextDocumentIdentifier = + { Uri = $"%s{codeActionParams.TextDocument.Uri}i" } + + let! (sigParseAndCheckResults: ParseAndCheckResults, _sigLine: string, sigSourceText: IFSACSourceText) = + getParseResultsForFile sigFileName (Position.mkPos 1 0) + + let parentSigLocation = + entity.DeclaringEntity + |> Option.bind (fun parentEntity -> + match parentEntity.SignatureLocation with + | Some sigLocation when Utils.isSignatureFile sigLocation.FileName -> Some sigLocation + | _ -> None) + + match parentSigLocation with + | None -> return [] + | Some parentSigLocation -> + + // Find a good location to insert the type alias + let insertText = + (parentSigLocation.Start, sigParseAndCheckResults.GetParseResults.ParseTree) + ||> ParsedInput.tryPick (fun _path node -> + match node with + | SyntaxNode.SynModuleOrNamespaceSig(SynModuleOrNamespaceSig(longId = longId; decls = decls)) + | SyntaxNode.SynModuleSigDecl(SynModuleSigDecl.NestedModule( + moduleInfo = SynComponentInfo(longId = longId); moduleDecls = decls)) -> + let mSigName = mkLongIdRange longId + + // `parentSigLocation` will only contain the single identifier in case a module is prefixed with a namespace. + if not (Range.rangeContainsRange mSigName parentSigLocation) then + None + else + + let aliasText = + let text = sourceText.GetSubTextFromRange mTypeDefn + + if not (text.StartsWith("and", StringComparison.Ordinal)) then + text + else + String.Concat("type", text.Substring 3) + + match decls with + | [] -> + match node with + | SyntaxNode.SynModuleOrNamespaceSig nm -> + Some(nm.Range.EndRange, String.Concat("\n\n", aliasText)) + + | SyntaxNode.SynModuleSigDecl(SynModuleSigDecl.NestedModule( + range = mNested + trivia = { ModuleKeyword = Some mModule + EqualsRange = Some mEquals })) -> + let moduleEqualsText = + sigSourceText.GetSubTextFromRange(Range.unionRanges mModule mEquals) + // Can this grabbed from configuration? + let indent = " " + + Some(mNested, String.Concat(moduleEqualsText, "\n", indent, aliasText)) + | _ -> None + | AllOpenOrHashDirective mLastDecl -> Some(mLastDecl, String.Concat("\n\n", aliasText)) + | decls -> + + decls + // Skip open statements + |> List.tryFind (function + | SynModuleSigDecl.Open _ + | SynModuleSigDecl.HashDirective _ -> false + | _ -> true) + |> Option.map (fun mdl -> + let offset = + if mdl.Range.StartColumn = 0 then + String.Empty + else + String.replicate mdl.Range.StartColumn " " + + mdl.Range.StartRange, String.Concat(aliasText, "\n\n", offset)) + | _ -> None) + + match insertText with + | None -> return [] + | Some(mInsert, newText) -> + + return + [ { SourceDiagnostic = None + Title = title + File = sigTextDocumentIdentifier + Edits = + [| { Range = fcsRangeToLsp mInsert + NewText = newText } |] + Kind = FixKind.Fix } ] + | _ -> return [] + }) diff --git a/src/FsAutoComplete/CodeFixes/AddTypeAliasToSignatureFile.fsi b/src/FsAutoComplete/CodeFixes/AddTypeAliasToSignatureFile.fsi new file mode 100644 index 000000000..8cd97449e --- /dev/null +++ b/src/FsAutoComplete/CodeFixes/AddTypeAliasToSignatureFile.fsi @@ -0,0 +1,6 @@ +module FsAutoComplete.CodeFix.AddTypeAliasToSignatureFile + +open FsAutoComplete.CodeFix.Types + +val title: string +val fix: getProjectOptionsForFile: GetProjectOptionsForFile -> getParseResultsForFile: GetParseResultsForFile -> CodeFix diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs index 938caa1ce..84591cf26 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs @@ -1902,7 +1902,8 @@ type AdaptiveState(lspClient: FSharpLspClient, sourceTextFactory: ISourceTextFac ToInterpolatedString.fix tryGetParseAndCheckResultsForFile getLanguageVersion AdjustConstant.fix tryGetParseAndCheckResultsForFile UpdateValueInSignatureFile.fix tryGetParseAndCheckResultsForFile - RemoveUnnecessaryParentheses.fix forceFindSourceText |]) + RemoveUnnecessaryParentheses.fix forceFindSourceText + AddTypeAliasToSignatureFile.fix forceGetFSharpProjectOptions tryGetParseAndCheckResultsForFile |]) let forgetDocument (uri: DocumentUri) = async { diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddTypeAliasToSignatureFileTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddTypeAliasToSignatureFileTests.fs new file mode 100644 index 000000000..78ac766d2 --- /dev/null +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/AddTypeAliasToSignatureFileTests.fs @@ -0,0 +1,186 @@ +module private FsAutoComplete.Tests.CodeFixTests.AddTypeAliasToSignatureFileTests + +open System.IO +open Expecto +open Helpers +open Utils.ServerTests +open Utils.CursorbasedTests +open FsAutoComplete.CodeFix + +let path = + Path.Combine(__SOURCE_DIRECTORY__, @"../TestCases/CodeFixTests/RenameParamToMatchSignature/") + +let tests state = + serverTestList (nameof AddTypeAliasToSignatureFile) state defaultConfigDto (Some path) (fun server -> + let selectCodeFix = CodeFix.withTitle AddTypeAliasToSignatureFile.title + + let test name sigBefore impl sigAfter = + testCaseAsync + name + (CodeFix.checkCodeFixInImplementationAndVerifySignature + server + sigBefore + impl + Diagnostics.acceptAll + selectCodeFix + sigAfter) + + [ test + "Sample use-case for AddTypeAliasToSignatureFile" + """ +module Foo + +open System +""" + """ +module Foo + +open System + +type Bar = $0int +""" + """ +module Foo + +open System + +type Bar = int +""" + + test + "place type alias above existing val" + """ +module Foo + +open System + +val a: int +""" + """ +module Foo + +open System + +let a = 8 + +type Bar$0 = string +""" + """ +module Foo + +open System + +type Bar = string + +val a: int +""" + + test + "place type alias above existing type" + """ +namespace Foo + +[] +type A = + new: unit -> A +""" + """ +namespace Foo + +type A() = class end + +type $0P = int -> int +""" + """ +namespace Foo + +type P = int -> int + +[] +type A = + new: unit -> A +""" + + test + "replace and with type keyword" + """ +namespace Foo + +open System +""" + """ +namespace Foo + +open System + +type Foo = class end +and Bar$0 = string +""" + """ +namespace Foo + +open System + +type Bar = string +""" + + test + "empty module" + """ +namespace Foo +""" + """ +namespace Foo + +type $0Foo = int +""" + """ +namespace Foo + +type Foo = int +""" + + test + "empty nested module" + """ +namespace Foo + +module Bar = + begin end +""" + """ +namespace Foo + +module Bar = + type $0Foo = int +""" + """ +namespace Foo + +module Bar = + type Foo = int +""" + + test + "non empty nested module" + """ +namespace Foo + +module Bar = + val a: int +""" + """ +namespace Foo + +module Bar = + let a = -9 + type $0Foo = int +""" + """ +namespace Foo + +module Bar = + type Foo = int + + val a: int +""" ]) diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs index df903fe4c..301ff1ed1 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs @@ -3432,4 +3432,5 @@ let tests textFactory state = removeRedundantAttributeSuffixTests state removePatternArgumentTests state UpdateValueInSignatureFileTests.tests state - removeUnnecessaryParenthesesTests state ] + removeUnnecessaryParenthesesTests state + AddTypeAliasToSignatureFileTests.tests state ] diff --git a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/UpdateValueInSignatureFileTests.fs b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/UpdateValueInSignatureFileTests.fs index 4b3a19273..82ffb6979 100644 --- a/test/FsAutoComplete.Tests.Lsp/CodeFixTests/UpdateValueInSignatureFileTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/CodeFixTests/UpdateValueInSignatureFileTests.fs @@ -6,63 +6,32 @@ open Helpers open Utils.ServerTests open Utils.CursorbasedTests open FsAutoComplete.CodeFix -open Utils.Utils -open Utils.TextEdit -open Utils.Server open Utils.CursorbasedTests.CodeFix -let path = Path.Combine(__SOURCE_DIRECTORY__, @"../TestCases/CodeFixTests/RenameParamToMatchSignature/") -let fsiFile, fsFile = ("Code.fsi", "Code.fs") - -let checkWithFsi - server - fsiSource - fsSourceWithCursor - selectCodeFix - fsiSourceExpected - = async { - let fsiSource = fsiSource |> Text.trimTripleQuotation - let cursor, fsSource = - fsSourceWithCursor - |> Text.trimTripleQuotation - |> Cursor.assertExtractRange - let! fsiDoc, diags = server |> Server.openDocumentWithText fsiFile fsiSource - use fsiDoc = fsiDoc - Expect.isEmpty diags "There should be no diagnostics in fsi doc" - let! fsDoc, diags = server |> Server.openDocumentWithText fsFile fsSource - use fsDoc = fsDoc - - do! - checkFixAt - (fsDoc, diags) - fsiDoc.VersionedTextDocumentIdentifier - (fsiSource, cursor) - (Diagnostics.expectCode "34") - selectCodeFix - (After (fsiSourceExpected |> Text.trimTripleQuotation)) - } +let path = + Path.Combine(__SOURCE_DIRECTORY__, @"../TestCases/CodeFixTests/RenameParamToMatchSignature/") let tests state = serverTestList (nameof UpdateValueInSignatureFile) state defaultConfigDto (Some path) (fun server -> [ let selectCodeFix = CodeFix.withTitle UpdateValueInSignatureFile.title testCaseAsync "first unit test for UpdateValueInSignatureFile" - <| checkWithFsi + <| checkCodeFixInImplementationAndVerifySignature server """ module A val a: b:int -> int """ -""" + """ module A let a$0 (b:int) (c: string) = 0 """ + (Diagnostics.expectCode "34") selectCodeFix """ module A val a: b: int -> c: string -> int -""" - ]) +""" ]) diff --git a/test/FsAutoComplete.Tests.Lsp/Utils/CursorbasedTests.fs b/test/FsAutoComplete.Tests.Lsp/Utils/CursorbasedTests.fs index 00fa8725f..f67bc61fc 100644 --- a/test/FsAutoComplete.Tests.Lsp/Utils/CursorbasedTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/Utils/CursorbasedTests.fs @@ -56,8 +56,6 @@ module CodeFix = | Some(Helpers.CodeActions actions), _ -> actions | Some _, _ -> failwith "Expected some code actions from the server" - - // select code action to use let codeActions = chooseFix allCodeActions @@ -180,6 +178,36 @@ module CodeFix = let withTitle title = matching (fun f -> f.Title = title) let ofKind kind = matching (fun f -> f.Kind = Some kind) + let checkCodeFixInImplementationAndVerifySignature + (server: CachedServer) + (fsiSource: string) + (fsSourceWithCursor: string) + (validateDiagnostics: Diagnostic[] -> unit) + (selectCodeFix: ChooseFix) + (fsiSourceExpected: string) + : Async = + async { + let fsiFile, fsFile = ("Code.fsi", "Code.fs") + let fsiSource = fsiSource |> Text.trimTripleQuotation + + let cursor, fsSource = + fsSourceWithCursor |> Text.trimTripleQuotation |> Cursor.assertExtractRange + + let! fsiDoc, _diags = server |> Server.openDocumentWithText fsiFile fsiSource + use fsiDoc = fsiDoc + let! fsDoc, diags = server |> Server.openDocumentWithText fsFile fsSource + use fsDoc = fsDoc + + do! + checkFixAt + (fsDoc, diags) + fsiDoc.VersionedTextDocumentIdentifier + (fsiSource, cursor) + validateDiagnostics + selectCodeFix + (After(fsiSourceExpected |> Text.trimTripleQuotation)) + } + /// Bundled tests in Expecto test module private Test = /// One `testCaseAsync` for each cursorRange. diff --git a/test/FsAutoComplete.Tests.Lsp/Utils/CursorbasedTests.fsi b/test/FsAutoComplete.Tests.Lsp/Utils/CursorbasedTests.fsi index 0812f801e..7eeb78398 100644 --- a/test/FsAutoComplete.Tests.Lsp/Utils/CursorbasedTests.fsi +++ b/test/FsAutoComplete.Tests.Lsp/Utils/CursorbasedTests.fsi @@ -89,6 +89,16 @@ module CodeFix = val withTitle: title: string -> (CodeAction array -> CodeAction array) val ofKind: kind: string -> (CodeAction array -> CodeAction array) + /// Execute a codefix in an implementation file and assert the resulting change in a signature file. + val checkCodeFixInImplementationAndVerifySignature: + server: CachedServer -> + fsiSource: string -> + fsSourceWithCursor: string -> + validateDiagnostics: (Diagnostic array -> unit) -> + selectCodeFix: ChooseFix -> + fsiSourceExpected: string -> + Async + /// Bundled tests in Expecto test module private Test = /// One `testCaseAsync` for each cursorRange.