Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor GetCSharpStyleIndexedExtensionMembersForTyconRef: unify logic #16839

Merged
merged 2 commits into from
Mar 11, 2024

Conversation

DedSec256
Copy link
Contributor

Remove code duplicates, generalize check

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

@DedSec256 DedSec256 requested a review from a team as a code owner March 7, 2024 23:39
Copy link
Contributor

github-actions bot commented Mar 7, 2024

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

@vzarytovskii vzarytovskii added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Mar 8, 2024
Copy link
Member

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please point me out where this is tested? Just to be more confident about the refactoring :)

@edgarfgp
Copy link
Contributor

edgarfgp commented Mar 8, 2024

Can you please point me out where this is tested? Just to be more confident about the refactoring :)

We tests are here https://github.com/dotnet/fsharp/blob/main/tests/FSharp.Compiler.ComponentTests/Language/ExtensionMethodTests.fs

Edit: Implemented feature Allow extension methods without type attribute PR #13839

@DedSec256
Copy link
Contributor Author

@psfinaki, the most important test is that this repository is completely built (including VisualFSharp), because, as the practice of my other PR (#16168) shows, the dotnet/fsharp project contains more interesting cases in its code than there are in tests.

There are also tests for old C#-compatible extensions, for example, https://github.com/dotnet/fsharp/blob/main/tests/fsharp/Compiler/Language/StaticNameResolutionTests.fs

and for new F# 8 CSharpExtensionAttributeNotRequired feature

let ``Opened types do allow unqualified access to C#-style extension methods if type has no [<Extension>] attribute`` () =

@psfinaki
Copy link
Member

psfinaki commented Mar 8, 2024

Alright, yeah, thanks for the links :)

@T-Gro T-Gro merged commit f2fcad0 into dotnet:main Mar 11, 2024
32 of 33 checks passed
@DedSec256 DedSec256 deleted the ber.a/unify branch March 11, 2024 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants