Misc. unused opens analyzer fixes
Description
- Fix https://github.com/dotnet/fsharp/issues/17629.
- Fix https://github.com/dotnet/fsharp/issues/17929 (and probably fixes https://github.com/dotnet/fsharp/issues/18389?).
- Fix https://github.com/dotnet/fsharp/issues/16226.
Checklist
- [x] Test cases added.
- [x] Release notes entry updated.
:heavy_exclamation_mark: Release notes required
@brianrourkeboll,
[!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 |
|---|---|---|
src/Compiler |
docs/release-notes/.FSharp.Compiler.Service/10.0.100.md | No release notes found or release notes format is not correct |
(getting a fresh build to see what the test failure was about, the previous results have expired)
(getting a fresh build to see what the test failure was about, the previous results have expired)
Sorry, I should have left a comment. Those test failures were legit. The reverse-processing trick that was meant to fix https://github.com/dotnet/fsharp/issues/16226 breaks other scenarios.
I think the real solution would require keeping some kind of open → shadow → reopen tracking to detect potential intervening shadowings that make a subsequent reopen not actually redundant.
Some of the fixes in this PR do not require that, though, so I could keep those and leave the open → shadow → reopen tracking for another PR.
(getting a fresh build to see what the test failure was about, the previous results have expired)
Sorry, I should have left a comment. Those test failures were legit. The reverse-processing trick that was meant to fix #16226 breaks other scenarios.
I think the real solution would require keeping some kind of open → shadow → reopen tracking to detect potential intervening shadowings that make a subsequent reopen not actually redundant.
Some of the fixes in this PR do not require that, though, so I could keep those and leave the open → shadow → reopen tracking for another PR.
Got it, we would exchange a fixed bug for opening another one when it comes to shadowings. That's not good indeed.
The analyzer could maybe detect that after a suggested removal, the resolution from names to symbols has changed - but that would increase CPU intensity of the analysis.