Skip to content

Conversation

@ThobiWanKenobi
Copy link
Contributor

@ThobiWanKenobi ThobiWanKenobi commented Jul 30, 2025

Description

This PR enables displaying external metadata for enum fields in the decompiled view.
Previously, enum fields (e.g. System.DateTimeKind.Utc) would not include their associated XML documentation in metadata view.
This change ensures that the documentation is correctly surfaced.

The PR also adds a test to cover a simple enum from the System namespace.

Fixes # (issue, if applicable)

Skaermoptagelse.2025-07-30.211243.mp4

Checklist

  • Test cases added

  • Performance benchmarks added in case of performance changes

  • Release notes entry updated:

    Please make sure to add an entry with short succinct description of the change as well as link to this pull request to the respective release notes file, if applicable.

    Release notes files:

    • If anything under src/Compiler has been changed, please make sure to make an entry in docs/release-notes/.FSharp.Compiler.Service/<version>.md, where <version> is usually "highest" one, e.g. 42.8.200
    • If language feature was added (i.e. LanguageFeatures.fsi was changed), please add it to docs/release-notes/.Language/preview.md
    • If a change to FSharp.Core was made, please make sure to edit docs/release-notes/.FSharp.Core/<version>.md where version is "highest" one, e.g. 8.0.200.

    Information about the release notes entries format can be found in the documentation.
    Example:

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

@github-actions
Copy link
Contributor

github-actions bot commented Jul 30, 2025

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/Compiler docs/release-notes/.FSharp.Compiler.Service/10.0.100.md

@ThobiWanKenobi
Copy link
Contributor Author

@dotnet-policy-service agree

@edgarfgp
Copy link
Contributor

edgarfgp commented Aug 2, 2025

/run ilverify

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2025

🔧 CLI Command Report

  • Command: /run ilverify
  • Outcome: success

✅ Command succeeded, no changes needed.

@github-project-automation github-project-automation bot moved this from New to In Progress in F# Compiler and Tooling Aug 4, 2025
@ThobiWanKenobi
Copy link
Contributor Author

So, I have been attempting to fix the ILVerify fail, but following the steps in the DEVGUIDE. However, I keep getting an error saying the following each time I run the ilverify.ps1 script from the /tests/ILVerify folder:

& : The module 'fsharp' could not be loaded. For more information, run 'Import-Module fsharp'.
At C:\Users\Stati\Projects\fsharp\tests\ILVerify\ilverify.ps1:44 char:11
+         & $script -c $configuration
+           ~~~~~~~
    + CategoryInfo          : ObjectNotFound: (fsharp\build.sh:String) [], CommandNotFoundException
    + FullyQualifiedErrorId : CouldNotAutoLoadModule

Does anyone know how I can resolve this issue, I cannot seem to figure out exactly what the problem seems to be

@T-Gro
Copy link
Member

T-Gro commented Aug 4, 2025

So, I have been attempting to fix the ILVerify fail, but following the steps in the DEVGUIDE. However, I keep getting an error saying the following each time I run the ilverify.ps1 script from the /tests/ILVerify folder:

& : The module 'fsharp' could not be loaded. For more information, run 'Import-Module fsharp'.
At C:\Users\Stati\Projects\fsharp\tests\ILVerify\ilverify.ps1:44 char:11
+         & $script -c $configuration
+           ~~~~~~~
    + CategoryInfo          : ObjectNotFound: (fsharp\build.sh:String) [], CommandNotFoundException
    + FullyQualifiedErrorId : CouldNotAutoLoadModule

Does anyone know how I can resolve this issue, I cannot seem to figure out exactly what the problem seems to be

What is your OS and working directory when invoking the script?
Maybe the script is written in a way the expects you cd into the directory first and then launch it? (this can be changed if this is the case)

@ThobiWanKenobi
Copy link
Contributor Author

So, I have been attempting to fix the ILVerify fail, but following the steps in the DEVGUIDE. However, I keep getting an error saying the following each time I run the ilverify.ps1 script from the /tests/ILVerify folder:

& : The module 'fsharp' could not be loaded. For more information, run 'Import-Module fsharp'.
At C:\Users\Stati\Projects\fsharp\tests\ILVerify\ilverify.ps1:44 char:11
+         & $script -c $configuration
+           ~~~~~~~
    + CategoryInfo          : ObjectNotFound: (fsharp\build.sh:String) [], CommandNotFoundException
    + FullyQualifiedErrorId : CouldNotAutoLoadModule

Does anyone know how I can resolve this issue, I cannot seem to figure out exactly what the problem seems to be

What is your OS and working directory when invoking the script? Maybe the script is written in a way the expects you cd into the directory first and then launch it? (this can be changed if this is the case)

I am working on Windows 11 Pro 24H2. I tried running the script directly from the /tests/ILVerify folder. I also attempted to run it from the git root "fsharp", but it still returns the same kind of error.

@majocha
Copy link
Contributor

majocha commented Aug 7, 2025

Ah, you need to use new powershell, v 7.*

@ThobiWanKenobi
Copy link
Contributor Author

Wait what... It turns out the default "Terminal" which I consider Powershell.... Is version 5.1.* 🤔🤔🤔I thought that it had the new version. I will give it a try with the updated version!

@vzarytovskii
Copy link
Member

Yeah, I should've specified pwsh version, or just make it an fsx.

@majocha
Copy link
Contributor

majocha commented Aug 7, 2025

CI Failure unrelated.

@ThobiWanKenobi
Copy link
Contributor Author

aw well, how nice, if its not the Macos one, then its just a Windows one... I don't understand, why does this pipeline seem so... unstable? Unpredictable? What is going on with this

@majocha
Copy link
Contributor

majocha commented Aug 9, 2025

Yeah, it's an ongoing effort to deflake it. The test suite has some layers of legacy stuff.

@ThobiWanKenobi
Copy link
Contributor Author

Ah yeah that makes sense. So we are actively trying to improve it, that is nice to hear! :D

@T-Gro T-Gro enabled auto-merge (squash) August 11, 2025 09:32
@ThobiWanKenobi
Copy link
Contributor Author

Thanks for the review! I will address the comments and see if I can improve the code later today :D

auto-merge was automatically disabled August 11, 2025 16:46

Head branch was pushed to by a user without write access

@ThobiWanKenobi
Copy link
Contributor Author

I addressed the comments from the PR, and attempted in general to improve the code quality of my PR. Does this need release notes, or should I just add the label to push it without?

@T-Gro
Copy link
Member

T-Gro commented Aug 12, 2025

Please add a one-liner to docs/release-notes/.FSharp.Compiler.Service/10.0.100.md , that will do it 👍

@T-Gro T-Gro merged commit 22815df into dotnet:main Aug 12, 2025
38 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in F# Compiler and Tooling Aug 12, 2025
@Martin521
Copy link
Contributor

@ThobiWanKenobi: Thank you and congrats to your first F# compiler PR!

@ThobiWanKenobi
Copy link
Contributor Author

Thank you! :D This is also my first open source contribution in general! I learned so much from this experience! I will probably be back in not too soon!

@T-Gro
Copy link
Member

T-Gro commented Aug 12, 2025

Lovely to read that. Eagerly waiting to seeing you soon 👍 :))

@edgarfgp
Copy link
Contributor

Thank you! :D This is also my first open source contribution in general! I learned so much from this experience! I will probably be back in not too soon!

@ThobiWanKenobi Awesome job.

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.

6 participants