From b8a6df851f01ec9328a4432bb225d0b228c08296 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 4 Nov 2025 12:31:14 +0000 Subject: [PATCH 1/4] Initial plan From bff0a0cf2b091b689da3d75f654063cd142904f1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 4 Nov 2025 13:11:30 +0000 Subject: [PATCH 2/4] Make AddReturnType refactoring robust to missing project files Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- .../FSharp.Editor/Refactor/AddReturnType.fs | 84 ++++++++++--------- .../Refactors/AddReturnTypeTests.fs | 21 +++++ 2 files changed, 67 insertions(+), 38 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Refactor/AddReturnType.fs b/vsintegration/src/FSharp.Editor/Refactor/AddReturnType.fs index b3c586b445e..b6c91e89d63 100644 --- a/vsintegration/src/FSharp.Editor/Refactor/AddReturnType.fs +++ b/vsintegration/src/FSharp.Editor/Refactor/AddReturnType.fs @@ -76,43 +76,51 @@ type internal AddReturnType [] () = override _.ComputeRefactoringsAsync context = cancellableTask { - let document = context.Document - let position = context.Span.Start - let! sourceText = document.GetTextAsync() - let textLine = sourceText.Lines.GetLineFromPosition position - let textLinePos = sourceText.Lines.GetLinePosition position - let fcsTextLineNumber = Line.fromZ textLinePos.Line - - let! lexerSymbol = - document.TryFindFSharpLexerSymbolAsync(position, SymbolLookupKind.Greedy, false, false, nameof (AddReturnType)) - - let! (parseFileResults, checkFileResults) = document.GetFSharpParseAndCheckResultsAsync(nameof (AddReturnType)) - - let symbolUseOpt = - lexerSymbol - |> Option.bind (fun lexer -> - checkFileResults.GetSymbolUseAtLocation( - fcsTextLineNumber, - lexer.Ident.idRange.EndColumn, - textLine.ToString(), - lexer.FullIsland - )) - - let memberFuncOpt = - symbolUseOpt - |> Option.bind (fun sym -> sym.Symbol |> AddReturnType.ofFSharpMemberOrFunctionOrValue) - - match (symbolUseOpt, memberFuncOpt) with - | (Some symbolUse, Some memberFunc) -> - let isValidMethod = - memberFunc - |> AddReturnType.isValidMethodWithoutTypeAnnotation symbolUse parseFileResults - - match isValidMethod with - | Some(memberFunc, typeRange) -> do AddReturnType.refactor context (memberFunc, typeRange, symbolUse) - | None -> () - | _ -> () - - return () + try + let document = context.Document + let position = context.Span.Start + let! sourceText = document.GetTextAsync() + let textLine = sourceText.Lines.GetLineFromPosition position + let textLinePos = sourceText.Lines.GetLinePosition position + let fcsTextLineNumber = Line.fromZ textLinePos.Line + + let! lexerSymbol = + document.TryFindFSharpLexerSymbolAsync(position, SymbolLookupKind.Greedy, false, false, nameof (AddReturnType)) + + let! (parseFileResults, checkFileResults) = document.GetFSharpParseAndCheckResultsAsync(nameof (AddReturnType)) + + let symbolUseOpt = + lexerSymbol + |> Option.bind (fun lexer -> + checkFileResults.GetSymbolUseAtLocation( + fcsTextLineNumber, + lexer.Ident.idRange.EndColumn, + textLine.ToString(), + lexer.FullIsland + )) + + let memberFuncOpt = + symbolUseOpt + |> Option.bind (fun sym -> sym.Symbol |> AddReturnType.ofFSharpMemberOrFunctionOrValue) + + match (symbolUseOpt, memberFuncOpt) with + | (Some symbolUse, Some memberFunc) -> + let isValidMethod = + memberFunc + |> AddReturnType.isValidMethodWithoutTypeAnnotation symbolUse parseFileResults + + match isValidMethod with + | Some(memberFunc, typeRange) -> do AddReturnType.refactor context (memberFunc, typeRange, symbolUse) + | None -> () + | _ -> () + + return () + with + | :? System.OperationCanceledException + | :? System.Exception -> + // File is not part of the project yet, or project options are not ready. + // This can happen when files are added/copied before the project system updates. + // Just return without offering any refactorings. + return () } |> CancellableTask.startAsTask context.CancellationToken diff --git a/vsintegration/tests/FSharp.Editor.Tests/Refactors/AddReturnTypeTests.fs b/vsintegration/tests/FSharp.Editor.Tests/Refactors/AddReturnTypeTests.fs index b5d6ef23e41..d3b3553ffff 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Refactors/AddReturnTypeTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Refactors/AddReturnTypeTests.fs @@ -479,3 +479,24 @@ let sum a b : MyType = {Value=a+b} let resultText = newDoc.GetTextAsync() |> GetTaskResult Assert.Equal(expectedCode.Trim(' ', '\r', '\n'), resultText.ToString().Trim(' ', '\r', '\n')) + +[] +let ``Should not throw when file is not properly configured`` () = + let symbolName = "sum" + + let code = + """ +let sum a b = a + b + """ + + use context = TestContext.CreateWithCode code + + let spanStart = code.IndexOf symbolName + + // This test verifies that even if internal operations fail (e.g., file not in project), + // the refactoring provider doesn't throw and just returns no actions + let actions = tryGetRefactoringActions code spanStart context (new AddReturnType()) + + // Should complete without throwing + // The number of actions may vary, but it should not crash + Assert.True(true) From e22b6104440ece93ba2e7a6409f54d73d68c41e1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 10:43:35 +0000 Subject: [PATCH 3/4] Simplify exception handling to use wildcard pattern Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- vsintegration/src/FSharp.Editor/Refactor/AddReturnType.fs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Refactor/AddReturnType.fs b/vsintegration/src/FSharp.Editor/Refactor/AddReturnType.fs index b6c91e89d63..e18eb2b33be 100644 --- a/vsintegration/src/FSharp.Editor/Refactor/AddReturnType.fs +++ b/vsintegration/src/FSharp.Editor/Refactor/AddReturnType.fs @@ -115,9 +115,7 @@ type internal AddReturnType [] () = | _ -> () return () - with - | :? System.OperationCanceledException - | :? System.Exception -> + with _ -> // File is not part of the project yet, or project options are not ready. // This can happen when files are added/copied before the project system updates. // Just return without offering any refactorings. From 6bf325d2cee978c231d5741c7c53878bcc04ef29 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Nov 2025 10:56:17 +0000 Subject: [PATCH 4/4] Improve test to actually simulate document outside project context Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com> --- .../Refactors/AddReturnTypeTests.fs | 32 ++++++++++++++++--- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Refactors/AddReturnTypeTests.fs b/vsintegration/tests/FSharp.Editor.Tests/Refactors/AddReturnTypeTests.fs index d3b3553ffff..9ce09af31aa 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Refactors/AddReturnTypeTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Refactors/AddReturnTypeTests.fs @@ -5,6 +5,10 @@ open Xunit open FSharp.Editor.Tests.Refactors.RefactorTestFramework open FSharp.Test.ProjectGeneration open FSharp.Editor.Tests.Helpers +open Microsoft.CodeAnalysis +open Microsoft.CodeAnalysis.Text +open Microsoft.CodeAnalysis.CodeRefactorings +open Microsoft.CodeAnalysis.CodeActions [] [] @@ -493,10 +497,28 @@ let sum a b = a + b let spanStart = code.IndexOf symbolName - // This test verifies that even if internal operations fail (e.g., file not in project), - // the refactoring provider doesn't throw and just returns no actions - let actions = tryGetRefactoringActions code spanStart context (new AddReturnType()) + // Create a document that's not in the F# project options by adding it to solution + // but not to the project's source files list. This simulates the race condition + // where a file is copied but project options haven't been refreshed yet. + let project = context.Solution.Projects |> Seq.head + let newDocId = DocumentId.CreateNewId(project.Id) + + let newDoc = + context.Solution.AddDocument(newDocId, "NotInProject.fs", code, filePath = "C:\\NotInProject.fs") + + let documentNotInProject = newDoc.GetDocument(newDocId) + + let refactoringActions = new System.Collections.Generic.List() + + let refactoringContext = + CodeRefactoringContext(documentNotInProject, TextSpan(spanStart, 1), (fun a -> refactoringActions.Add a), context.CancellationToken) + + let refactorProvider = new AddReturnType() + + // This should not throw even though the document is not in the project options + let computeTask = refactorProvider.ComputeRefactoringsAsync refactoringContext + + computeTask.Wait(context.CancellationToken) - // Should complete without throwing - // The number of actions may vary, but it should not crash + // The test passes if no exception was thrown Assert.True(true)