Allow extension methods without type attribute.

[nojaf]

@edgarfgp and I are trying to address https://github.com/fsharp/fslang-suggestions/issues/835.

It currently works when the extension method is being consumed in the same F# project. It doesn't appear to be exposed when consumed in a C# project. I'm probably still missing something and I could use a pointer.

[vzarytovskii]

As discussed on Slack: C# compiler expects type to have ExtensionAttribute to "see" its methods and treat them as extensions.

Probably a different approach will work better here - if type has static method(s) with ExtensionAttribute, then add this attribute to the type itself if it is missing it.

[abelbraaksma]

As @vzarytovskii mentions, and same is true for assembly, and possibly module (the latter I'm not sure of, it wasn't clear from the suggestions thread, I don't know if C# also adds it to .NET modules). Also, assuming there's an upcoming RFC, can we specify there what kind of members will be "detected" and what rules specifically apply?

[vzarytovskii]

can we specify there what kind of members will be "detected" and what rules specifically apply?

Regarding of detecting those, it is already in place, and probably should be changed to not check enclosing type, may be used as reference:

https://github.com/dotnet/fsharp/blob/e06e079ebfecc48db530351a1cbf48b93dfb2979/src/Compiler/Checking/NameResolution.fs#L509-L515

[edgarfgp]

Humm. There seems to be some error regarding vsintegration that have not touched . Maybe we could rerun the CI to see if that helps

[nojaf]

@edgarfgp I should open the VisualFSharp.sln when I find some time. Another that struct me is that we didn't consider the impact of signature files in this PR.

What happens when you have:

module Foo

[<System.Runtime.CompilerServices.Extension>]
val PlusOne: a: int -> int

and

module Foo

[<System.Runtime.CompilerServices.Extension>]
let PlusOne (a:int) = a + 1

will that still be picked up?

[edgarfgp]

@edgarfgp I should open the VisualFSharp.sln when I find some time. Another that struct me is that we didn't consider the impact of signature files in this PR.

What happens when you have:

module Foo

[<System.Runtime.CompilerServices.Extension>]
val PlusOne: a: int -> int

and

module Foo

[<System.Runtime.CompilerServices.Extension>]
let PlusOne (a:int) = a + 1

will that still be picked up?

I think for the sake of completeness. We should look at it and see how doable is :)

[edgarfgp]

@vzarytovskii Should we put this under a preview flag ?

[smoothdeveloper]

@edgarfgp can there be a test that it works with VB.NET too?

[edgarfgp]

@smoothdeveloper As far I can see there is not way to do that in the compiler. If you know how please let me know :)

Update : We have tested with a local compiler version with a VB project and works Ok

[smoothdeveloper]

@edgarfp this test checks things among F#, C# & VB.NET:

https://github.com/dotnet/fsharp/blob/eaa723db6a80d4fca15cbc70a78f9662363cd007/tests/fsharp/tests.fs#L2069-L2074

Maybe there is a way which is similar with string literal with code in it in the unit tests, albeit I don't favour having those written like this, and generally prefer to deal with standalone code files.

Update : We have tested with a local compiler version with a VB project and works Ok

Can you confirm if this works without adding the Extension attribute to the assembly?

[vzarytovskii]

@vzarytovskii Should we put this under a preview flag ?

I would say so, yes. Better guard it by the preview version.

[nojaf]

Can you confirm if this works without adding the Extension attribute to the assembly?

This is outside the scope of our proposal. The fact that VB still needs the assembly attribute is no change in the current behaviour.

[edgarfgp]

@T-Gro @psfinaki @0101 @abonie Florian and I have been working on this language suggestion for the last couple months https://github.com/fsharp/fslang-suggestions/issues/835.

• We have added a complete site of test to make sure it is "bullet proof"
• It is under a Lang preview

It is now ready for a first review :)

Pending:

• Investigating CI test issues(test failing in F# 5)

Thanks in advance :)

[T-Gro]

LGTM , just the test failures need to get resolved

[vzarytovskii]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 2 pipeline(s).

[nojaf]

these tests still fail in net7.0, the errors all contain The module/namespace 'System' from compilation unit 'netstandard' did not contain the namespace, module or type 'IAsyncDisposable', which leads me to believe the system libraries are not longer imported correctly.

The test file for ref-ops-deprecation-langversion-preview looks like

let r = ref 3
r := 4   // generates an informational in preview
let rv = !r   // generates an informational in preview
incr r   // generates an informational in preview
decr r   // generates an informational in preview

type X() =
    member x.M(a:int) = a
    member x.M(b:int) = b

I do not see how our changes could have an influence here. At the same time, running these tests on the main branch does work for me locally. So there must be something, but I'm out of ideas at this point.

[T-Gro]

I am failing to see how the newly added code is connected with IAsyncDisposable, I do not see any connection at all.

@vzarytovskii ; @KevinRansom : Have you ever seen an error like this before?

The module/namespace 'System' from compilation unit 'netstandard' did not contain the namespace, module or type 'IAsyncDisposable'

[vzarytovskii]

I am failing to see how the newly added code is connected with IAsyncDisposable, I do not see any connection at all.

@vzarytovskii ; @KevinRansom : Have you ever seen an error like this before?

The module/namespace 'System' from compilation unit 'netstandard' did not contain the namespace, module or type 'IAsyncDisposable'

Hm, haven't seen it before. It's even weirder that it works in main.

[nojaf]

The module/namespace 'System' from compilation unit 'netstandard' did not contain the namespace, module or type 'IAsyncDisposable'

I actually see this on a weekly basis when working with the FSharp.Compiler.Service.sln in Rider. I can't put my finger on it when exactly it happens, so I didn't bother Eugene with it. But when I try do build the solution in the editor, it doesn't work, stops early and shows that exact message.

That is why I think it is a DLL resolution thing or MSBuild thing. But no real clue to be honest 🙈.

[edgarfgp]

The module/namespace 'System' from compilation unit 'netstandard' did not contain the namespace, module or type 'IAsyncDisposable'

I have also see this when using Rider FSharp.Compiler.Service.sln

Wishful thinking : Could this be related to the fact that we need to add some similar tests like here https://github.com/edgarfgp/fsharp/blob/78fa515eeb44eee1f702e3325dc42982eb77a8a9/tests/fsharp/tests.fs#L93

I run out of ideas to fix this :(

[edgarfgp]

We are still trying to find the reason of the CI failing 😔. We could use some help , I might try to create a new branch and apply the changes one by one to see where the CI complains

[abelbraaksma]

The module/namespace 'System' from compilation unit 'netstandard' did not contain the namespace, module or type 'IAsyncDisposable'

Not sure this will help, but if you’ve added code or tests that reference this, these should be excluded for builds that expect NetStandard 2.0 or below. It was only added in 2.1 iirc.

The test suits run with different config in CI, so the same test may succeed locally, but fail here when, for instance, it runs in .NET Framework mode, or conversely in NetStandard20 mode. I didn’t look in details at the logs, but this is the first I can think of with that error.

Also, the CI GitHub actions don’t split these versions as separate runs I believe, so it’s quite hard to interpret.

[T-Gro]

I will try to check with what change this failure was introduced, still does not make any sense to me :((

[vzarytovskii]

Okay, since the tests are failing in preview, it leads me to believe that something is funny in the name resolution for IAsyncDisposable (and possibly some other types). When the feature is turned off by default (the check in NameResultion is inverted for example), tests are passing. So, failures are 100% related to the feature itself.

[edgarfgp]

Okay, since the tests are failing in preview, it leads me to believe that something is funny in the name resolution for IAsyncDisposable. When the feature is turned off by default (the check in NameResultion is inverted for example), tests are passing. So, failures are 100% related to the feature itself.

I added a preview flag as normally I do for other contributions. Is there anything special I need to do for this case ?

[vzarytovskii]

Okay, since the tests are failing in preview, it leads me to believe that something is funny in the name resolution for IAsyncDisposable. When the feature is turned off by default (the check in NameResultion is inverted for example), tests are passing. So, failures are 100% related to the feature itself.

I added a preview flag as normally I do for other contributions. Is there anything special I need to do for this case ?

We load a hardcoded netstandard2.0 FSharp.Core which is missing asyncdisposable, it seems that in the new logic (this change), causes to load this type eagerly.

Additionally, @dsyme, could you please take a look at this change?

[T-Gro]

My latest suspect is the error handling done in this method, with the call to errorRecovery. This produces a new diagnostical output, even though this method should very likely swallow assembly loading exceptions (?).

[T-Gro]

This is what is falling trough:

FSharp.Compiler.DiagnosticsLogger+ReportedError: The exception has been reported. This internal exception should now be caught at an error recovery point on the stack. Original message: InternalUndefinedItemRef
  (<fun:get_Deref@3399-1>, "System", "netstandard", "IAsyncDisposable"))
   at FSharp.Compiler.DiagnosticsLogger.DiagnosticsLoggerExtensions.DiagnosticsLogger.Error[T](DiagnosticsLogger x, Exception exn) in C:\code\fsharp\src\Compiler\Facilities\DiagnosticsLogger.fs:line 448
   at FSharp.Compiler.TypedTree.EntityRef.get_Deref() in C:\code\fsharp\src\Compiler\TypedTree\TypedTree.fs:line 3399
   at FSharp.Compiler.TypedTreeOps.stripTyEqnsA(TcGlobals g, Boolean canShortcut, TType ty) in C:\code\fsharp\src\Compiler\TypedTree\TypedTreeOps.fs:line 746
   at FSharp.Compiler.TypedTreeOps.stripTyEqnsAndErase(Boolean eraseFuncAndTuple, TcGlobals g, TType ty) in C:\code\fsharp\src\Compiler\TypedTree\TypedTreeOps.fs:line 780
   at FSharp.Compiler.TypedTreeOps.stripTyEqnsWrtErasure(Erasure erasureFlag, TcGlobals g, TType ty) in C:\code\fsharp\src\Compiler\TypedTree\TypedTreeOps.fs:line 806
   at FSharp.Compiler.TypedTreeOps.typeAEquivAux(Erasure erasureFlag, TcGlobals g, TypeEquivEnv aenv, TType ty1, TType ty2) in C:\code\fsharp\src\Compiler\TypedTree\TypedTreeOps.fs:line 1042
   at FSharp.Compiler.TypedTreeOps.typarConstraintSetsAEquivAux@1030.Invoke(TyparConstraint arg30@, TyparConstraint arg40@)
   at Microsoft.FSharp.Primitives.Basics.List.exists[T](FSharpFunc`2 predicate, FSharpList`1 xs1) in C:\code\fsharp\src\FSharp.Core\local.fs:line 403
   at Microsoft.FSharp.Collections.ListModule.Exists[T](FSharpFunc`2 predicate, FSharpList`1 list) in C:\code\fsharp\src\FSharp.Core\list.fs:line 452
   at Microsoft.FSharp.Primitives.Basics.List.forall[T](FSharpFunc`2 predicate, FSharpList`1 xs1) in C:\code\fsharp\src\FSharp.Core\local.fs:line 398
   at Microsoft.FSharp.Collections.ListModule.ForAll[T](FSharpFunc`2 predicate, FSharpList`1 list) in C:\code\fsharp\src\FSharp.Core\list.fs:line 448
   at Internal.Utilities.Library.Extras.ListSet.equals[a,b](FSharpFunc`2 f, FSharpList`1 l1, FSharpList`1 l2) in C:\code\fsharp\src\Compiler\Utilities\lib.fs:line 188
   at FSharp.Compiler.TypedTreeOps.typarConstraintSetsAEquivAux(Erasure erasureFlag, TcGlobals g, TypeEquivEnv aenv, Typar tp1, Typar tp2) in C:\code\fsharp\src\Compiler\TypedTree\TypedTreeOps.fs:line 1030
   at [email protected](Typar tp1, Typar tp2)
   at Microsoft.FSharp.Collections.ListModule.forall2aux[a,b](FSharpFunc`3 f, FSharpList`1 list1, FSharpList`1 list2) in C:\code\fsharp\src\FSharp.Core\list.fs:line 434
   at Microsoft.FSharp.Collections.ListModule.ForAll2[T1,T2](FSharpFunc`2 predicate, FSharpList`1 list1, FSharpList`1 list2) in C:\code\fsharp\src\FSharp.Core\list.fs:line 444
   at FSharp.Compiler.TypedTreeOps.typeAEquivAux(Erasure erasureFlag, TcGlobals g, TypeEquivEnv aenv, TType ty1, TType ty2) in C:\code\fsharp\src\Compiler\TypedTree\TypedTreeOps.fs:line 1046
   at FSharp.Compiler.TypedTreeOps.typeEquivAux(Erasure erasureFlag, TcGlobals g, TType ty1, TType ty2) in C:\code\fsharp\src\Compiler\TypedTree\TypedTreeOps.fs:line 1106
   at Microsoft.FSharp.Collections.ListModule.TryFind[T](FSharpFunc`2 predicate, FSharpList`1 list) in C:\code\fsharp\src\FSharp.Core\list.fs:line 492
   at FSharp.Compiler.TypedTree.ModuleOrNamespaceType.TryLinkVal(CcuThunk ccu, ValLinkageFullKey key) in C:\code\fsharp\src\Compiler\TypedTree\TypedTree.fs:line 2025
   at FSharp.Compiler.TypedTree.ValRef.get_Deref() in C:\code\fsharp\src\Compiler\TypedTree\TypedTree.fs:line 3765
   at [email protected](ValRef vref) in C:\code\fsharp\src\Compiler\Checking\InfoReader.fs:line 30
   at Microsoft.FSharp.Primitives.Basics.List.choose[T,TResult](FSharpFunc`2 f, FSharpList`1 xs) in C:\code\fsharp\src\FSharp.Core\local.fs:line 188
   at Microsoft.FSharp.Collections.ListModule.Choose[T,TResult](FSharpFunc`2 chooser, FSharpList`1 list) in C:\code\fsharp\src\FSharp.Core\list.fs:line 206
   at [email protected](String _arg1, FSharpList`1 x, FSharpList`1 sofar) in C:\code\fsharp\src\Compiler\Utilities\illib.fs:line 1356
   at Microsoft.FSharp.Collections.MapTreeModule.foldBackOpt[TKey,TValue,a](FSharpFunc`4 f, MapTree`2 m, a x) in C:\code\fsharp\src\FSharp.Core\map.fs:line 444
   at Microsoft.FSharp.Collections.MapTreeModule.foldBackOpt[TKey,TValue,a](FSharpFunc`4 f, MapTree`2 m, a x) in C:\code\fsharp\src\FSharp.Core\map.fs:line 444
   at FSharp.Compiler.InfoReader.GetImmediateIntrinsicMethInfosOfTypeAux(FSharpOption`1 optFilter, AccessorDomain ad, TcGlobals g, ImportMap amap, Range m, TType origTy, TType metadataTy) in C:\code\fsharp\src\Compiler\Checking\InfoReader.fs:line 90
   at FSharp.Compiler.InfoReader.GetImmediateIntrinsicMethInfosOfType(FSharpOption`1 optFilter, AccessorDomain ad, TcGlobals g, ImportMap amap, Range m, TType ty) in C:\code\fsharp\src\Compiler\Checking\InfoReader.fs:line 97
   at FSharp.Compiler.NameResolution.GetCSharpStyleIndexedExtensionMembersForTyconRefOriginal(ImportMap amap, Range m, EntityRef tcrefOfStaticClass) in C:\code\fsharp\src\Compiler\Checking\NameResolution.fs:line 537
   at FSharp.Compiler.NameResolution.GetCSharpStyleIndexedExtensionMembersForTyconRef(ImportMap amap, Range m, EntityRef tcrefOfStaticClass) in C:\code\fsharp\src\Compiler\Checking\NameResolution.fs:line 649

[T-Gro]

Btw. after the tests are working, I also want to have a look at some of code duplication within GetCSharpStyleIndexedExtensionMembersForTyconRef , seems somewhat doubled in there.