Skip to content

Commit 726845e

Browse files
authored
Fix AddReturnType refactoring throwing when file not in project (#19052)
1 parent f7e5f18 commit 726845e

File tree

2 files changed

+87
-38
lines changed

2 files changed

+87
-38
lines changed

vsintegration/src/FSharp.Editor/Refactor/AddReturnType.fs

Lines changed: 44 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -76,43 +76,49 @@ type internal AddReturnType [<ImportingConstructor>] () =
7676

7777
override _.ComputeRefactoringsAsync context =
7878
cancellableTask {
79-
let document = context.Document
80-
let position = context.Span.Start
81-
let! sourceText = document.GetTextAsync()
82-
let textLine = sourceText.Lines.GetLineFromPosition position
83-
let textLinePos = sourceText.Lines.GetLinePosition position
84-
let fcsTextLineNumber = Line.fromZ textLinePos.Line
85-
86-
let! lexerSymbol =
87-
document.TryFindFSharpLexerSymbolAsync(position, SymbolLookupKind.Greedy, false, false, nameof (AddReturnType))
88-
89-
let! (parseFileResults, checkFileResults) = document.GetFSharpParseAndCheckResultsAsync(nameof (AddReturnType))
90-
91-
let symbolUseOpt =
92-
lexerSymbol
93-
|> Option.bind (fun lexer ->
94-
checkFileResults.GetSymbolUseAtLocation(
95-
fcsTextLineNumber,
96-
lexer.Ident.idRange.EndColumn,
97-
textLine.ToString(),
98-
lexer.FullIsland
99-
))
100-
101-
let memberFuncOpt =
102-
symbolUseOpt
103-
|> Option.bind (fun sym -> sym.Symbol |> AddReturnType.ofFSharpMemberOrFunctionOrValue)
104-
105-
match (symbolUseOpt, memberFuncOpt) with
106-
| (Some symbolUse, Some memberFunc) ->
107-
let isValidMethod =
108-
memberFunc
109-
|> AddReturnType.isValidMethodWithoutTypeAnnotation symbolUse parseFileResults
110-
111-
match isValidMethod with
112-
| Some(memberFunc, typeRange) -> do AddReturnType.refactor context (memberFunc, typeRange, symbolUse)
113-
| None -> ()
114-
| _ -> ()
115-
116-
return ()
79+
try
80+
let document = context.Document
81+
let position = context.Span.Start
82+
let! sourceText = document.GetTextAsync()
83+
let textLine = sourceText.Lines.GetLineFromPosition position
84+
let textLinePos = sourceText.Lines.GetLinePosition position
85+
let fcsTextLineNumber = Line.fromZ textLinePos.Line
86+
87+
let! lexerSymbol =
88+
document.TryFindFSharpLexerSymbolAsync(position, SymbolLookupKind.Greedy, false, false, nameof (AddReturnType))
89+
90+
let! (parseFileResults, checkFileResults) = document.GetFSharpParseAndCheckResultsAsync(nameof (AddReturnType))
91+
92+
let symbolUseOpt =
93+
lexerSymbol
94+
|> Option.bind (fun lexer ->
95+
checkFileResults.GetSymbolUseAtLocation(
96+
fcsTextLineNumber,
97+
lexer.Ident.idRange.EndColumn,
98+
textLine.ToString(),
99+
lexer.FullIsland
100+
))
101+
102+
let memberFuncOpt =
103+
symbolUseOpt
104+
|> Option.bind (fun sym -> sym.Symbol |> AddReturnType.ofFSharpMemberOrFunctionOrValue)
105+
106+
match (symbolUseOpt, memberFuncOpt) with
107+
| (Some symbolUse, Some memberFunc) ->
108+
let isValidMethod =
109+
memberFunc
110+
|> AddReturnType.isValidMethodWithoutTypeAnnotation symbolUse parseFileResults
111+
112+
match isValidMethod with
113+
| Some(memberFunc, typeRange) -> do AddReturnType.refactor context (memberFunc, typeRange, symbolUse)
114+
| None -> ()
115+
| _ -> ()
116+
117+
return ()
118+
with _ ->
119+
// File is not part of the project yet, or project options are not ready.
120+
// This can happen when files are added/copied before the project system updates.
121+
// Just return without offering any refactorings.
122+
return ()
117123
}
118124
|> CancellableTask.startAsTask context.CancellationToken

vsintegration/tests/FSharp.Editor.Tests/Refactors/AddReturnTypeTests.fs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ open Xunit
55
open FSharp.Editor.Tests.Refactors.RefactorTestFramework
66
open FSharp.Test.ProjectGeneration
77
open FSharp.Editor.Tests.Helpers
8+
open Microsoft.CodeAnalysis
9+
open Microsoft.CodeAnalysis.Text
10+
open Microsoft.CodeAnalysis.CodeRefactorings
11+
open Microsoft.CodeAnalysis.CodeActions
812

913
[<Theory>]
1014
[<InlineData(":int")>]
@@ -479,3 +483,42 @@ let sum a b : MyType = {Value=a+b}
479483

480484
let resultText = newDoc.GetTextAsync() |> GetTaskResult
481485
Assert.Equal(expectedCode.Trim(' ', '\r', '\n'), resultText.ToString().Trim(' ', '\r', '\n'))
486+
487+
[<Fact>]
488+
let ``Should not throw when file is not properly configured`` () =
489+
let symbolName = "sum"
490+
491+
let code =
492+
"""
493+
let sum a b = a + b
494+
"""
495+
496+
use context = TestContext.CreateWithCode code
497+
498+
let spanStart = code.IndexOf symbolName
499+
500+
// Create a document that's not in the F# project options by adding it to solution
501+
// but not to the project's source files list. This simulates the race condition
502+
// where a file is copied but project options haven't been refreshed yet.
503+
let project = context.Solution.Projects |> Seq.head
504+
let newDocId = DocumentId.CreateNewId(project.Id)
505+
506+
let newDoc =
507+
context.Solution.AddDocument(newDocId, "NotInProject.fs", code, filePath = "C:\\NotInProject.fs")
508+
509+
let documentNotInProject = newDoc.GetDocument(newDocId)
510+
511+
let refactoringActions = new System.Collections.Generic.List<CodeAction>()
512+
513+
let refactoringContext =
514+
CodeRefactoringContext(documentNotInProject, TextSpan(spanStart, 1), (fun a -> refactoringActions.Add a), context.CancellationToken)
515+
516+
let refactorProvider = new AddReturnType()
517+
518+
// This should not throw even though the document is not in the project options
519+
let computeTask = refactorProvider.ComputeRefactoringsAsync refactoringContext
520+
521+
computeTask.Wait(context.CancellationToken)
522+
523+
// The test passes if no exception was thrown
524+
Assert.True(true)

0 commit comments

Comments
 (0)