Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 4, 2025

Fix AddReturnType refactoring throwing on non-existent files

Problem: The AddReturnType refactoring throws when invoked on a file that's not yet known to the F# extension (e.g., file copied but project options not refreshed).

Solution: Wrapped the refactoring logic in a try-catch block to handle exceptions gracefully.

Changes Made:

  • Understand the problem: exception thrown from GetFSharpParseAndCheckResultsAsync when file not in project
  • Wrap the call to GetFSharpParseAndCheckResultsAsync in a try-catch in AddReturnType.fs
  • Return early (no refactorings) when exception occurs
  • Add test that actually simulates a document outside project context
  • Format code
  • Simplify exception handling (use wildcard pattern instead of redundant type tests)
  • Commit changes

Files Changed:

  1. vsintegration/src/FSharp.Editor/Refactor/AddReturnType.fs - Added try-catch block with wildcard pattern
  2. vsintegration/tests/FSharp.Editor.Tests/Refactors/AddReturnTypeTests.fs - Improved test to create a document not in F# project options

Ready for merge - The fix is minimal and surgical, addressing exactly the issue described. The test now properly simulates the race condition scenario.

Original prompt

This section details on the original issue you should resolve

<issue_title>AddReturnType refactoring occasionally throws</issue_title>
<issue_description>I cannot come up with a definitive repo but this sometimes happens when I just copy a file in the solution explorer.

Here is the stack trace:

System.Exception : The file 'C:\code\fsharp3\tests\benchmarks\FCSBenchmarks\CompilerServiceBenchmarks\GraphTypeCheckingBenchmarks - Copy.fs' was not part of the project. Did you call InvalidateConfiguration when the list of files in the project changed?
   at FSharp.Compiler.CodeAnalysis.IncrementalBuilder.GetSlotOfFileName(String fileName)
   at <StartupCode$FSharp-Compiler-Service>.$BackgroundCompiler.ParseAndCheckFileInProject@919-3.Invoke(FSharpOption`1 cachedResults)
   at FSharp.Compiler.BuildGraph.Bind@63-1.Invoke(T x)
   at Microsoft.FSharp.Control.AsyncPrimitives.CallThenInvokeNoHijackCheck[a,b](AsyncActivation`1 ctxt,b result1,FSharpFunc`2 userCode)
   at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at async Microsoft.VisualStudio.FSharp.Editor.WorkspaceExtensions.CheckerExtensions.FSharpChecker-ParseAndCheckDocumentWithPossibleStaleResults@388-3(<Unknown Parameters>)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at async Microsoft.VisualStudio.FSharp.Editor.WorkspaceExtensions.CheckerExtensions.FSharpChecker-ParseAndCheckDocumentWithPossibleStaleResults@378(<Unknown Parameters>)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at async Microsoft.VisualStudio.FSharp.Editor.WorkspaceExtensions.CheckerExtensions.FSharpChecker-ParseAndCheckDocument@449(<Unknown Parameters>)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at async Microsoft.VisualStudio.FSharp.Editor.WorkspaceExtensions.Document-GetFSharpParseAndCheckResultsAsync@539(<Unknown Parameters>)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at async StartupCode$FSharp-Editor(<Unknown Parameters>)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at async Microsoft.CodeAnalysis.CodeRefactorings.CodeRefactoringService.<>c__DisplayClass12_0.<GetRefactoringFromProviderAsync>b__0(<Unknown Parameters>)
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at async Microsoft.CodeAnalysis.Extensions.IExtensionManagerExtensions.PerformFunctionAsync[T](<Unknown Parameters>)
```</issue_description>

<agent_instructions>AddReturnType refactoring (from vsintegration/src/FSharp.Editor/Refactor/AddReturnType.fs) throws then it is requested too fast, before the project options are refreshed.

Since this is annoying, lets make it more robust.
If the exception mentioned in this issue happens inside processing of the refactoring, it should just return () and not crash.

Locate existing refactoring tests and try to request refactoring for a non-existent file to verify it (non existent as in - F# extension not being aware of it)</agent_instructions>

## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@vzarytovskii</author><body>
This likely happens because project system doesn't send us new command line options fast enough and we ask for tc results for file which is not in the project yet.</body></comment_new>
<comment_new><author>@T-Gro</author><body>
It can be if the VisualSVN modifies files on disk (e.g. a project file) faster then VS sends new project arguments to the F# extension.
When it comes to the refactoring service, a solution here can likely be making it more robust and delaying to a no-op in that situation (i.e. when refactoring provider asks for refactorings about a file that is not yet known to the extension, yield empty instead of throwing)</body></comment_new>
</comments>

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix AddReturnType refactoring throwing exceptions Fix AddReturnType refactoring throwing when file not in project Nov 4, 2025
Copilot AI requested a review from T-Gro November 4, 2025 13:25
Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
Co-authored-by: T-Gro <46543583+T-Gro@users.noreply.github.com>
@T-Gro T-Gro marked this pull request as ready for review November 5, 2025 14:50
@T-Gro T-Gro requested a review from a team as a code owner November 5, 2025 14:50
@T-Gro T-Gro enabled auto-merge (squash) November 5, 2025 14:50
@T-Gro T-Gro requested a review from abonie November 5, 2025 14:50
@github-project-automation github-project-automation bot moved this from New to In Progress in F# Compiler and Tooling Nov 5, 2025
@T-Gro T-Gro merged commit 726845e into main Nov 6, 2025
37 of 38 checks passed
@T-Gro T-Gro deleted the copilot/fix-addreturntype-refactoring branch November 6, 2025 09:24
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2025

❗ Release notes required

@copilot,

Caution

No release notes found for the changed paths (see table below).

Please make sure to add an entry with an informative description of the change as well as link to this pull request, issue and language suggestion if applicable. Release notes for this repository are based on Keep A Changelog format.

The following format is recommended for this repository:

* <Informative description>. ([PR #XXXXX](https://github.com/dotnet/fsharp/pull/XXXXX))

See examples in the files, listed in the table below or in th full documentation at https://fsharp.github.io/fsharp-compiler-docs/release-notes/About.html.

If you believe that release notes are not necessary for this PR, please add NO_RELEASE_NOTES label to the pull request.

You can open this PR in browser to add release notes: open in github.dev

Change path Release notes path Description
vsintegration/src docs/release-notes/.VisualStudio/18.0.md No release notes found or release notes format is not correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

AddReturnType refactoring occasionally throws

3 participants