Prefer extension methods to intrinsic properties when arguments are p…

…rovided (#16032)

* Initial look at issue meant to give priority to extension method if a property of same name exists and arguments are passed to it (assuming property is not a function value).

Lots of things to sort out, this just takes care of the first layer of interaction between CheckExpression and NameResolution.

Given impact on service / tooling, it is not clear that adding an entry in NameResolution.Item is the best choice, may reconsider into changing the return type of ResolveLongIdentInTypePrim instead.

* bring it back to the simplest thing that could possibly work, and it seems to work on toy example.

* Do not break all indexed properties in the world (that is, when there are no extension methods in scope).

* Implementation: singling out indexer properties and properties with FSharpFunc as type.

Tests: covering a bunch of allowed and disallowed shadowing cases.

* Update LanguageFeatures.fs

* minimize diff

* do not break chained calls when there is an extension method with same name defined.

* fantomas

* actual chained calls should remain in the sample...

* minimize diff

* adding test for support of BCL System.Linq.Enumerable.Count()
adding tests for before the feature on all the previous positive tests

* (minor) formatting.

* fixing partially applied function piped to ignore (fsharp/fslang-suggestions#1327) making .net framework tests fail

* adjust baselines that differ between .net framework and .netcore

* checking event handlers aren't affected by the feature

* separate out Property and Method in DeclarationListInfo.Create, not clear how many things will be broken, but it does what is expected in VS

* handling of static property

* adjust DeclarationListInfo grouping logic to be specific to singling out extension method only, rather than the kind of all items (type and ctor method were being split, causing a test to fail and more entries in the completion list not related to accessing properties or extension methods).

* add test checking the latest opened type extension takes precedence

* minimize diff

* minimize diff

* Fix test (new error message)

* fix bad merge on baseline file

* better comments explaining the properties that are singled out / can't be overtaken by extension methods.

* revisit DeclarationListInfo.Create to avoid regression where we'd show both event and extension method (only event can be called).

The new logic 100% preserves the previous behaviour in absence of the particular condition we are interested for in RFC-1137.

In case the condition for RFC-1137 hits, we partition the items for those to be split apart (property and extension methods), taking care of discarding items as we yield the expected tuples that correspond to each item and the rest is processed as it was originally.

There are still probably loopholes with items showing up separated, but this would only impact if there are actually extension method and property with same name.

* minimize diff

* try to make fantomas happy, despite I don't see the difference

* minimize diff

* formatting & add a comment explaining usage of `createSublists`

---------

Co-authored-by: Vlad Zarytovskii <vzaritovsky@hotmail.com>

src/Compiler/Checking/CheckComputationExpressions.fs

@@ -58,7 +58,7 @@ let (|JoinRelation|_|) cenv env (expr: SynExpr) =
 
     let isOpName opName vref s =
         (s = opName) &&
-        match ResolveExprLongIdent cenv.tcSink cenv.nameResolver m ad env.eNameResEnv TypeNameResolutionInfo.Default [ident(opName, m)] with
+        match ResolveExprLongIdent cenv.tcSink cenv.nameResolver m ad env.eNameResEnv TypeNameResolutionInfo.Default [ident(opName, m)] None with
         | Result (_, Item.Value vref2, []) -> valRefEq cenv.g vref vref2
         | _ -> false
 

src/Compiler/Checking/CheckDeclarations.fs

@@ -2387,7 +2387,7 @@ module TcExceptionDeclarations =
           match reprIdOpt with 
           | Some longId ->
               let resolution =
-                  ResolveExprLongIdent cenv.tcSink cenv.nameResolver m ad env.NameEnv TypeNameResolutionInfo.Default longId 
+                  ResolveExprLongIdent cenv.tcSink cenv.nameResolver m ad env.NameEnv TypeNameResolutionInfo.Default longId None
                   |> ForceRaise
               match resolution with
               | _, Item.ExnCase exnc, [] -> 

src/Compiler/Checking/CheckExpressions.fs

@@ -3811,6 +3811,13 @@ type DelayedItem =
   /// Represents the valueExpr in "item <- valueExpr", also "item.[indexerArgs] <- valueExpr" etc.
   | DelayedSet of SynExpr * range
 
+module DelayedItem =
+    let maybeAppliedArgForPreferExtensionOverProperty delayed = 
+        match delayed with
+        | [] -> None
+        | DelayedItem.DelayedApp(argExpr=argExpr) :: _ -> Some argExpr
+        | _ -> None
+
 let MakeDelayedSet(e: SynExpr, m) =
     // We have longId <- e. Wrap 'e' in another pair of parentheses to ensure it's never interpreted as
     // a named argument, e.g. for "el.Checked <- (el = el2)"
@@ -8119,7 +8126,7 @@ and TcNameOfExpr (cenv: cenv) env tpenv (synArg: SynExpr) =
             // However we don't commit for a type names - nameof allows 'naked' type names and thus all type name
             // resolutions are checked separately in the next step.
             let typeNameResInfo = GetLongIdentTypeNameInfo delayed
-            let nameResolutionResult = ResolveLongIdentAsExprAndComputeRange cenv.tcSink cenv.nameResolver (rangeOfLid longId) ad env.eNameResEnv typeNameResInfo longId
+            let nameResolutionResult = ResolveLongIdentAsExprAndComputeRange cenv.tcSink cenv.nameResolver (rangeOfLid longId) ad env.eNameResEnv typeNameResInfo longId None
             let resolvesAsExpr =
                 match nameResolutionResult with
                 | Result (_, item, _, _, _ as res)
@@ -8330,7 +8337,8 @@ and TcLongIdentThen (cenv: cenv) (overallTy: OverallTy) env tpenv (SynLongIdent(
     let ad = env.eAccessRights
     let typeNameResInfo = GetLongIdentTypeNameInfo delayed
     let nameResolutionResult =
-        ResolveLongIdentAsExprAndComputeRange cenv.tcSink cenv.nameResolver (rangeOfLid longId) ad env.eNameResEnv typeNameResInfo longId
+        let maybeAppliedArgExpr = DelayedItem.maybeAppliedArgForPreferExtensionOverProperty delayed
+        ResolveLongIdentAsExprAndComputeRange cenv.tcSink cenv.nameResolver (rangeOfLid longId) ad env.eNameResEnv typeNameResInfo longId maybeAppliedArgExpr
         |> ForceRaise
     TcItemThen cenv overallTy env tpenv nameResolutionResult None delayed
 
@@ -8582,7 +8590,7 @@ and TcTypeItemThen (cenv: cenv) overallTy env nm ty tpenv mItem tinstEnclosing d
         let item = Item.Types(nm, [ty])
         CallNameResolutionSink cenv.tcSink (mExprAndTypeArgs, env.NameEnv, item, emptyTyparInst, ItemOccurence.Use, env.eAccessRights)
         let typeNameResInfo = GetLongIdentTypeNameInfo otherDelayed
-        let item, mItem, rest, afterResolution = ResolveExprDotLongIdentAndComputeRange cenv.tcSink cenv.nameResolver (unionRanges mExprAndTypeArgs mLongId) ad env.eNameResEnv ty longId typeNameResInfo IgnoreOverrides true
+        let item, mItem, rest, afterResolution = ResolveExprDotLongIdentAndComputeRange cenv.tcSink cenv.nameResolver (unionRanges mExprAndTypeArgs mLongId) ad env.eNameResEnv ty longId typeNameResInfo IgnoreOverrides true None
         TcItemThen cenv overallTy env tpenv ((argsOfAppTy g ty), item, mItem, rest, afterResolution) None otherDelayed
 
     | DelayedTypeApp(tyargs, _mTypeArgs, mExprAndTypeArgs) :: _delayed' ->
@@ -9146,8 +9154,9 @@ and TcLookupThen cenv overallTy env tpenv mObjExpr objExpr objExprTy longId dela
     // Canonicalize inference problem prior to '.' lookup on variable types
     if isTyparTy g objExprTy then
         CanonicalizePartialInferenceProblem cenv.css env.DisplayEnv mExprAndLongId (freeInTypeLeftToRight g false objExprTy)
-
-    let item, mItem, rest, afterResolution = ResolveExprDotLongIdentAndComputeRange cenv.tcSink cenv.nameResolver mExprAndLongId ad env.NameEnv objExprTy longId TypeNameResolutionInfo.Default findFlag false
+
+    let maybeAppliedArgExpr = DelayedItem.maybeAppliedArgForPreferExtensionOverProperty delayed
+    let item, mItem, rest, afterResolution = ResolveExprDotLongIdentAndComputeRange cenv.tcSink cenv.nameResolver mExprAndLongId ad env.NameEnv objExprTy longId TypeNameResolutionInfo.Default findFlag false maybeAppliedArgExpr
     TcLookupItemThen cenv overallTy env tpenv mObjExpr objExpr objExprTy delayed item mItem rest afterResolution
 
 and TcLookupItemThen cenv overallTy env tpenv mObjExpr objExpr objExprTy delayed item mItem rest afterResolution =

src/Compiler/Checking/CheckPatterns.fs

@@ -486,7 +486,7 @@ and IsNameOf (cenv: cenv) (env: TcEnv) ad m (id: Ident) =
     let g = cenv.g
     id.idText = "nameof" &&
     try
-        match ResolveExprLongIdent cenv.tcSink cenv.nameResolver m ad env.NameEnv TypeNameResolutionInfo.Default [id] with
+        match ResolveExprLongIdent cenv.tcSink cenv.nameResolver m ad env.NameEnv TypeNameResolutionInfo.Default [id] None with
         | Result (_, Item.Value vref, _) -> valRefEq g vref g.nameof_vref
         | _ -> false
     with _ -> false