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

Latest fcs #1122

Merged
merged 13 commits into from
Jul 1, 2023
171 changes: 149 additions & 22 deletions src/FsAutoComplete.Core/Commands.fs
Original file line number Diff line number Diff line change
Expand Up @@ -741,25 +741,7 @@ module Commands =
Position = pos
Scope = ic.ScopeKind }))

/// * `includeDeclarations`:
/// if `false` only returns usage locations and excludes declarations
/// * Note: if `true` you can still separate usages and declarations from each other
/// with `Symbol.partitionInfoDeclarationsAndUsages`
/// * `includeBackticks`:
/// if `true` returns ranges including existing backticks, otherwise without:
/// `let _ = ``my value`` + 42`
/// * `true`: ` ``my value`` `
/// * `false`: `my value`
/// * `errorOnFailureToFixRange`:
/// Ranges returned by FCS don't just span the actual identifier, but include Namespace, Module, Type: `System.String.IsNullOrEmpty`
/// These ranges gets adjusted to include just the concrete identifier (`IsNullOrEmpty`)
/// * If `false` and range cannot be adjust, the original range gets used.
/// * When results are more important than always exact range
/// -> for "Find All References"
/// * If `true`: Instead of using the source range, this function instead returns an Error
/// * When exact ranges are required
/// -> for "Rename"
let symbolUseWorkspace
let symbolUseWorkspaceAux
(getDeclarationLocation: FSharpSymbolUse * NamedText -> Async<SymbolDeclarationLocation option>)
(findReferencesForSymbolInFile: (string<LocalPath> * FSharpProjectOptions * FSharpSymbol) -> Async<Range seq>)
(tryGetFileSource: string<LocalPath> -> Async<ResultOrString<NamedText>>)
Expand All @@ -768,13 +750,11 @@ module Commands =
(includeDeclarations: bool)
(includeBackticks: bool)
(errorOnFailureToFixRange: bool)
pos
lineStr
(text: NamedText)
(tyRes: ParseAndCheckResults)
(symbolUse: FSharpSymbolUse)
: Async<Result<(FSharpSymbol * IDictionary<string<LocalPath>, Range[]>), string>> =
asyncResult {
let! symbolUse = tyRes.TryGetSymbolUse pos lineStr |> Result.ofOption (fun _ -> "No symbol")
let symbol = symbolUse.Symbol

let symbolNameCore = symbol.DisplayNameCore
Expand Down Expand Up @@ -944,6 +924,98 @@ module Commands =
}


/// * `includeDeclarations`:
/// if `false` only returns usage locations and excludes declarations
/// * Note: if `true` you can still separate usages and declarations from each other
/// with `Symbol.partitionInfoDeclarationsAndUsages`
/// * `includeBackticks`:
/// if `true` returns ranges including existing backticks, otherwise without:
/// `let _ = ``my value`` + 42`
/// * `true`: ` ``my value`` `
/// * `false`: `my value`
/// * `errorOnFailureToFixRange`:
/// Ranges returned by FCS don't just span the actual identifier, but include Namespace, Module, Type: `System.String.IsNullOrEmpty`
/// These ranges gets adjusted to include just the concrete identifier (`IsNullOrEmpty`)
/// * If `false` and range cannot be adjust, the original range gets used.
/// * When results are more important than always exact range
/// -> for "Find All References"
/// * If `true`: Instead of using the source range, this function instead returns an Error
/// * When exact ranges are required
/// -> for "Rename"
let symbolUseWorkspace
(getDeclarationLocation: FSharpSymbolUse * NamedText -> Async<SymbolDeclarationLocation option>)
(findReferencesForSymbolInFile: (string<LocalPath> * FSharpProjectOptions * FSharpSymbol) -> Async<Range seq>)
(tryGetFileSource: string<LocalPath> -> Async<ResultOrString<NamedText>>)
(tryGetProjectOptionsForFsproj: string<LocalPath> -> Async<FSharpProjectOptions option>)
(getAllProjectOptions: unit -> Async<FSharpProjectOptions seq>)
(includeDeclarations: bool)
(includeBackticks: bool)
(errorOnFailureToFixRange: bool)
pos
lineStr
(text: NamedText)
(tyRes: ParseAndCheckResults)
: Async<Result<(FSharpSymbol * IDictionary<string<LocalPath>, Range[]>), string>> =
asyncResult {
let! symbolUse = tyRes.TryGetSymbolUse pos lineStr |> Result.ofOption (fun _ -> "No symbol")

return!
symbolUseWorkspaceAux
getDeclarationLocation
findReferencesForSymbolInFile
tryGetFileSource
tryGetProjectOptionsForFsproj
getAllProjectOptions
includeDeclarations
includeBackticks
errorOnFailureToFixRange
text
tyRes
symbolUse
}

let symbolUseWorkspace2
Copy link
Member

Choose a reason for hiding this comment

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

Should we keep a separate implementation or could you leave a comment for why there's a second implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, we still mean to clean that up. Not sure if we should rename or consider moving everything to the v2 implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

I made a version that uses v2 for everything and takes the simplest approach 😏
nojaf#5
@TheAngryByrd Let me know, if you prefer a version that keeps the symbol(s) in it's return type.

(getDeclarationLocation: FSharpSymbolUse * NamedText -> Async<SymbolDeclarationLocation option>)
(findReferencesForSymbolInFile: (string<LocalPath> * FSharpProjectOptions * FSharpSymbol) -> Async<Range seq>)
(tryGetFileSource: string<LocalPath> -> Async<ResultOrString<NamedText>>)
(tryGetProjectOptionsForFsproj: string<LocalPath> -> Async<FSharpProjectOptions option>)
(getAllProjectOptions: unit -> Async<FSharpProjectOptions seq>)
(includeDeclarations: bool)
(includeBackticks: bool)
(errorOnFailureToFixRange: bool)
pos
lineStr
(text: NamedText)
(tyRes: ParseAndCheckResults)
: Async<Result<(IDictionary<string<LocalPath>, Range[]>), string>> =
asyncResult {
let multipleSymbols = tyRes.TryGetSymbolUses pos lineStr
let result = Dictionary<string<LocalPath>, Range[]>()

for symbolUse in multipleSymbols do
let! symbolResult =
symbolUseWorkspaceAux
getDeclarationLocation
findReferencesForSymbolInFile
tryGetFileSource
tryGetProjectOptionsForFsproj
getAllProjectOptions
includeDeclarations
includeBackticks
errorOnFailureToFixRange
text
tyRes
symbolUse

for KeyValue(k, v) in snd symbolResult do
if result.ContainsKey k then
result.[k] <- [| yield! result.[k]; yield! v |]
else
result.Add(k, v)

return result
}

/// Puts `newName` into backticks if necessary.
///
///
Expand Down Expand Up @@ -2219,6 +2291,61 @@ type Commands(checker: FSharpCompilerServiceChecker, state: State, hasAnalyzers:
tyRes
}

member x.SymbolUseWorkspace2
(
pos,
lineStr,
text: NamedText,
tyRes: ParseAndCheckResults,
includeDeclarations: bool,
includeBackticks: bool,
errorOnFailureToFixRange: bool
) =
asyncResult {
let findReferencesForSymbolInFile (file: string<LocalPath>, project, symbol) =
if File.Exists(UMX.untag file) then
// `FSharpChecker.FindBackgroundReferencesInFile` only works with existing files
checker.FindReferencesForSymbolInFile(UMX.untag file, project, symbol)
else
// untitled script files
async {
match state.TryGetFileCheckerOptionsWithLines(file) with
| Error _ -> return [||]
| Ok(opts, source) ->
match checker.TryGetRecentCheckResultsForFile(file, opts, source) with
| None -> return [||]
| Some tyRes ->
let! ct = Async.CancellationToken
let usages = tyRes.GetCheckResults.GetUsesOfSymbolInFile(symbol, ct)
return usages |> Seq.map (fun u -> u.Range)
}

let tryGetFileSource symbolFile =
state.TryGetFileSource symbolFile |> Async.singleton

let tryGetProjectOptionsForFsproj (fsprojPath: string<LocalPath>) =
state.ProjectController.GetProjectOptionsForFsproj(UMX.untag fsprojPath)
|> Async.singleton

let getAllProjectOptions () =
state.ProjectController.ProjectOptions |> Seq.map snd |> Async.singleton

return!
Commands.symbolUseWorkspace2
x.GetDeclarationLocation
findReferencesForSymbolInFile
tryGetFileSource
tryGetProjectOptionsForFsproj
getAllProjectOptions
includeDeclarations
includeBackticks
errorOnFailureToFixRange
pos
lineStr
text
tyRes
}

member x.RenameSymbolRange(pos: Position, tyRes: ParseAndCheckResults, lineStr: LineStr, text: NamedText) =
Commands.renameSymbolRange x.GetDeclarationLocation false pos lineStr text tyRes

Expand Down
7 changes: 7 additions & 0 deletions src/FsAutoComplete.Core/ParseAndCheckResults.fs
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,13 @@ type ParseAndCheckResults
let identIsland = Array.toList identIsland
checkResults.GetSymbolUseAtLocation(pos.Line, colu, lineStr, identIsland)

member __.TryGetSymbolUses (pos: Position) (lineStr: LineStr) : FSharpSymbolUse list =
match Lexer.findLongIdents (pos.Column, lineStr) with
| None -> []
| Some(colu, identIsland) ->
let identIsland = Array.toList identIsland
checkResults.GetSymbolUsesAtLocation(pos.Line, colu, lineStr, identIsland)

member x.TryGetSymbolUseAndUsages (pos: Position) (lineStr: LineStr) =
let symboluse = x.TryGetSymbolUse pos lineStr

Expand Down
5 changes: 2 additions & 3 deletions src/FsAutoComplete/CodeFixes/AddPrivateAccessModifier.fs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ type SymbolUseWorkspace =
-> LineStr
-> NamedText
-> ParseAndCheckResults
-> Async<Result<FSharp.Compiler.Symbols.FSharpSymbol *
System.Collections.Generic.IDictionary<FSharp.UMX.string<LocalPath>, FSharp.Compiler.Text.range array>, string>>
-> Async<Result<System.Collections.Generic.IDictionary<FSharp.UMX.string<LocalPath>, FSharp.Compiler.Text.range array>, string>>

type private Placement =
| Before
Expand Down Expand Up @@ -204,7 +203,7 @@ let fix (getParseResultsForFile: GetParseResultsForFile) (symbolUseWorkspace: Sy
match rangesAndPlacement with
| Some(editRange, declRange, placement) ->

let! (_, uses) = symbolUseWorkspace false true true fcsPos lineStr sourceText parseAndCheck
let! uses = symbolUseWorkspace false true true fcsPos lineStr sourceText parseAndCheck
let useRanges = uses.Values |> Array.concat

let usedOutsideOfDecl =
Expand Down
44 changes: 43 additions & 1 deletion src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1542,6 +1542,48 @@ type AdaptiveFSharpLspServer(workspaceLoader: IWorkspaceLoader, lspClient: FShar
text
tyRes

let symbolUseWorkspace2
(includeDeclarations: bool)
(includeBackticks: bool)
(errorOnFailureToFixRange: bool)
pos
lineStr
text
tyRes
=
let findReferencesForSymbolInFile (file: string<LocalPath>, project, symbol) =
async {
let checker = checker |> AVal.force

if File.Exists(UMX.untag file) then
// `FSharpChecker.FindBackgroundReferencesInFile` only works with existing files
return! checker.FindReferencesForSymbolInFile(UMX.untag file, project, symbol)
else
// untitled script files
match! forceGetTypeCheckResultsStale file with
| Error _ -> return [||]
| Ok tyRes ->
let! ct = Async.CancellationToken
let usages = tyRes.GetCheckResults.GetUsesOfSymbolInFile(symbol, ct)
return usages |> Seq.map (fun u -> u.Range)
}

let tryGetProjectOptionsForFsproj (file: string<LocalPath>) =
forceGetProjectOptions file |> Async.map Option.ofResult

Commands.symbolUseWorkspace2
getDeclarationLocation
findReferencesForSymbolInFile
forceFindSourceText
tryGetProjectOptionsForFsproj
(getAllProjectOptions >> Async.map Array.toSeq)
includeDeclarations
includeBackticks
errorOnFailureToFixRange
pos
lineStr
text
tyRes

let codefixes =
let getFileLines = forceFindSourceText
Expand Down Expand Up @@ -1667,7 +1709,7 @@ type AdaptiveFSharpLspServer(workspaceLoader: IWorkspaceLoader, lspClient: FShar
GenerateXmlDocumentation.fix tryGetParseResultsForFile
Run.ifEnabled
(fun _ -> config.AddPrivateAccessModifier)
(AddPrivateAccessModifier.fix tryGetParseResultsForFile symbolUseWorkspace)
(AddPrivateAccessModifier.fix tryGetParseResultsForFile symbolUseWorkspace2)
UseTripleQuotedInterpolation.fix tryGetParseResultsForFile getRangeText
RenameParamToMatchSignature.fix tryGetParseResultsForFile |])

Expand Down
21 changes: 20 additions & 1 deletion src/FsAutoComplete/LspServers/FsAutoComplete.Lsp.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1164,6 +1164,25 @@ type FSharpLspServer(state: State, lspClient: FSharpLspClient) =
errorOnFailureToFixRange
)

let symbolUseWorkspace2
(includeDeclarations: bool)
(includeBackticks: bool)
(errorOnFailureToFixRange: bool)
pos
lineStr
text
tyRes
=
commands.SymbolUseWorkspace2(
pos,
lineStr,
text,
tyRes,
includeDeclarations,
includeBackticks,
errorOnFailureToFixRange
)

codefixes <-
[| Run.ifEnabled (fun _ -> config.UnusedOpensAnalyzer) (RemoveUnusedOpens.fix getFileLines)
Run.ifEnabled
Expand Down Expand Up @@ -1224,7 +1243,7 @@ type FSharpLspServer(state: State, lspClient: FSharpLspClient) =
GenerateXmlDocumentation.fix tryGetParseResultsForFile
Run.ifEnabled
(fun _ -> config.AddPrivateAccessModifier)
(AddPrivateAccessModifier.fix tryGetParseResultsForFile symbolUseWorkspace)
(AddPrivateAccessModifier.fix tryGetParseResultsForFile symbolUseWorkspace2)
UseTripleQuotedInterpolation.fix tryGetParseResultsForFile getRangeText
RenameParamToMatchSignature.fix tryGetParseResultsForFile |]

Expand Down