Fix static name resolution in presence of instance extension members with matching names (#13973)

* fix unqualified name resolution for nested types in presence of instance extension members

* add tests

* add tests

* apply formatting
Co-authored-by: NTomas Grosup <tomasgrosup@microsoft.com>
Co-authored-by: NKevin Ransom (msft) <codecutter@hotmail.com>

src/Compiler/Checking/CheckExpressions.fs

@@ -1780,7 +1780,7 @@ let FreshenAbstractSlot g amap m synTyparDecls absMethInfo =
 //-------------------------------------------------------------------------
 
 /// Helper used to check record expressions and record patterns
-let BuildFieldMap (cenv: cenv) env isPartial ty flds m =
+let BuildFieldMap (cenv: cenv) env isPartial ty (flds: ((Ident list * Ident) * 'T) list) m =
     let g = cenv.g
     let ad = env.eAccessRights
 
@@ -1792,7 +1792,8 @@ let BuildFieldMap (cenv: cenv) env isPartial ty flds m =
         let allFields = flds |> List.map (fun ((_, ident), _) -> ident)
         flds
         |> List.map (fun (fld, fldExpr) ->
-            let frefSet = ResolveField cenv.tcSink cenv.nameResolver env.eNameResEnv ad ty fld allFields
+            let (fldPath, fldId) = fld
+            let frefSet = ResolveField cenv.tcSink cenv.nameResolver env.eNameResEnv ad ty fldPath fldId allFields
             fld, frefSet, fldExpr)
 
     let relevantTypeSets =
@@ -6167,7 +6168,8 @@ and TcTyparExprThen (cenv: cenv) overallTy env tpenv synTypar m delayed =
         let tp, tpenv = TcTypar cenv env NoNewTypars tpenv synTypar
         let mExprAndLongId = unionRanges synTypar.Range ident.idRange
         let ty = mkTyparTy tp
-        let item, _rest = ResolveLongIdentInType cenv.tcSink cenv.nameResolver env.NameEnv LookupKind.Expr ident.idRange ad ident IgnoreOverrides TypeNameResolutionInfo.Default ty
+        let lookupKind = LookupKind.Expr LookupIsInstance.Ambivalent
+        let item, _rest = ResolveLongIdentInType cenv.tcSink cenv.nameResolver env.NameEnv lookupKind ident.idRange ad ident IgnoreOverrides TypeNameResolutionInfo.Default ty
         let delayed3 =
             match rest with
             | [] -> delayed2
@@ -10618,7 +10620,8 @@ and TcAttributeEx canFail (cenv: cenv) (env: TcEnv) attrTgt attrEx (synAttr: Syn
                   attributeAssignedNamedItems |> List.map (fun (CallerNamedArg(id, CallerArg(callerArgTy, m, isOpt, callerArgExpr))) ->
                     if isOpt then error(Error(FSComp.SR.tcOptionalArgumentsCannotBeUsedInCustomAttribute(), m))
                     let m = callerArgExpr.Range
-                    let setterItem, _ = ResolveLongIdentInType cenv.tcSink cenv.nameResolver env.NameEnv LookupKind.Expr m ad id IgnoreOverrides TypeNameResolutionInfo.Default ty
+                    let lookupKind = LookupKind.Expr LookupIsInstance.Ambivalent
+                    let setterItem, _ = ResolveLongIdentInType cenv.tcSink cenv.nameResolver env.NameEnv lookupKind m ad id IgnoreOverrides TypeNameResolutionInfo.Default ty
                     let nm, isProp, argTy =
                       match setterItem with
                       | Item.Property (_, [pinfo]) ->

src/Compiler/Checking/MethodCalls.fs

@@ -634,7 +634,7 @@ type CalledMeth<'T>
                     let pinfos = GetIntrinsicPropInfoSetsOfType infoReader (Some nm) ad AllowMultiIntfInstantiations.Yes IgnoreOverrides id.idRange returnedObjTy
                     let pinfos = pinfos |> ExcludeHiddenOfPropInfos g infoReader.amap m 
                     match pinfos with 
-                    | [pinfo] when pinfo.HasSetter && not pinfo.IsIndexer -> 
+                    | [pinfo] when pinfo.HasSetter && not pinfo.IsStatic && not pinfo.IsIndexer -> 
                         let pminfo = pinfo.SetterMethod
                         let pminst = freshenMethInfo m pminfo
                         let propStaticTyOpt = if isTyparTy g returnedObjTy then Some returnedObjTy else None
@@ -642,10 +642,11 @@ type CalledMeth<'T>
                     | _ ->
                         let epinfos = 
                             match nameEnv with  
-                            | Some ne -> ExtensionPropInfosOfTypeInScope ResultCollectionSettings.AllResults infoReader ne (Some nm) ad m returnedObjTy
+                            | Some ne -> ExtensionPropInfosOfTypeInScope ResultCollectionSettings.AllResults infoReader ne (Some nm) LookupIsInstance.Ambivalent ad m returnedObjTy
                             | _ -> []
+
                         match epinfos with 
-                        | [pinfo] when pinfo.HasSetter && not pinfo.IsIndexer -> 
+                        | [pinfo] when pinfo.HasSetter && not pinfo.IsStatic && not pinfo.IsIndexer -> 
                             let pminfo = pinfo.SetterMethod
                             let pminst =
                                 match minfo with
@@ -661,11 +662,11 @@ type CalledMeth<'T>
                             Choice1Of2(AssignedItemSetter(id, AssignedPropSetter(propStaticTyOpt, pinfo, pminfo, pminst), e))
                         |  _ ->    
                             match infoReader.GetILFieldInfosOfType(Some(nm), ad, m, returnedObjTy) with
-                            | finfo :: _ -> 
+                            | finfo :: _ when not finfo.IsStatic -> 
                                 Choice1Of2(AssignedItemSetter(id, AssignedILFieldSetter(finfo), e))
                             | _ ->              
                               match infoReader.TryFindRecdOrClassFieldInfoOfType(nm, m, returnedObjTy) with
-                              | ValueSome rfinfo -> 
+                              | ValueSome rfinfo when not rfinfo.IsStatic -> 
                                   Choice1Of2(AssignedItemSetter(id, AssignedRecdFieldSetter(rfinfo), e))
                               | _ -> 
                                   Choice2Of2(arg))

src/Compiler/Checking/NameResolution.fsi

@@ -525,6 +525,67 @@ type ResultCollectionSettings =
     | AllResults
     | AtMostOneResult
 
+/// Indicates if a lookup requires a match on the instance/static characteristic.
+///
+/// This is not supplied at all lookup sites - in theory it could be, but currently diagnostics on many paths
+/// rely on returning all the content and then filtering it later for instance/static characteristic.
+///
+/// When applied, this also currently doesn't filter all content - it is currently only applied to filter out extension methods
+/// that have a static/instance mismatch.
+[<RequireQualifiedAccess>]
+type LookupIsInstance =
+    | Ambivalent
+    | Yes
+    | No
+
+/// Indicates the kind of lookup being performed. Note, this type should be made private to nameres.fs.
+[<RequireQualifiedAccess>]
+type LookupKind =
+    | RecdField
+
+    | Pattern
+
+    /// Indicates resolution within an expression, either A.B.C (static) or expr.A.B.C (instance) and
+    /// whether we should filter content according to instance/static characteristic.
+    | Expr of isInstanceFilter: LookupIsInstance
+
+    | Type
+
+    | Ctor
+
+/// Indicates if a warning should be given for the use of upper-case identifiers in patterns
+type WarnOnUpperFlag =
+    | WarnOnUpperCase
+    | AllIdsOK
+
+/// Indicates whether we permit a direct reference to a type generator. Only set when resolving the
+/// right-hand-side of a [<Generate>] declaration.
+[<RequireQualifiedAccess>]
+type PermitDirectReferenceToGeneratedType =
+    | Yes
+    | No
+
+/// Specifies extra work to do after overload resolution
+[<RequireQualifiedAccess>]
+type AfterResolution =
+    /// Notification is not needed
+    | DoNothing
+
+    /// Notify the sink of the information needed to complete recording a use of a symbol
+    /// for the purposes of the language service.  One of the callbacks should be called by
+    /// the checker.
+    ///
+    /// The first callback represents a case where we have learned the type
+    /// instantiation of a generic method or value.
+    ///
+    /// The second represents the case where we have resolved overloading and/or
+    /// a specific override. The 'Item option' contains the candidate overrides.
+    | RecordResolution of
+        Item option *
+        (TyparInstantiation -> unit) *
+        (MethInfo * PropInfo option * TyparInstantiation -> unit) *
+        (unit -> unit)
+
 /// Temporarily redirect reporting of name resolution and type checking results
 val internal WithNewTypecheckResultsSink: ITypecheckResultsSink * TcResultsSink -> System.IDisposable
 
@@ -556,37 +617,38 @@ val internal CallOpenDeclarationSink: TcResultsSink -> OpenDeclaration -> unit
 
 /// Get all the available properties of a type (both intrinsic and extension)
 val internal AllPropInfosOfTypeInScope:
-    ResultCollectionSettings ->
-    InfoReader ->
-    NameResolutionEnv ->
-    string option ->
-    AccessorDomain ->
-    FindMemberFlag ->
-    range ->
-    TType ->
+    collectionSettings: ResultCollectionSettings ->
+    infoReader: InfoReader ->
+    nenv: NameResolutionEnv ->
+    optFilter: string option ->
+    ad: AccessorDomain ->
+    findFlag: FindMemberFlag ->
+    m: range ->
+    ty: TType ->
         PropInfo list
 
 /// Get all the available properties of a type (only extension)
 val internal ExtensionPropInfosOfTypeInScope:
-    ResultCollectionSettings ->
-    InfoReader ->
-    NameResolutionEnv ->
-    string option ->
-    AccessorDomain ->
-    range ->
-    TType ->
+    collectionSettings: ResultCollectionSettings ->
+    infoReader: InfoReader ->
+    nenv: NameResolutionEnv ->
+    optFilter: string option ->
+    isInstanceFilter: LookupIsInstance ->
+    ad: AccessorDomain ->
+    m: range ->
+    ty: TType ->
         PropInfo list
 
 /// Get the available methods of a type (both declared and inherited)
 val internal AllMethInfosOfTypeInScope:
-    ResultCollectionSettings ->
-    InfoReader ->
-    NameResolutionEnv ->
-    string option ->
-    AccessorDomain ->
-    FindMemberFlag ->
-    range ->
-    TType ->
+    collectionSettings: ResultCollectionSettings ->
+    infoReader: InfoReader ->
+    nenv: NameResolutionEnv ->
+    optFilter: string option ->
+    ad: AccessorDomain ->
+    findFlag: FindMemberFlag ->
+    m: range ->
+    ty: TType ->
         MethInfo list
 
 /// Used to report an error condition where name resolution failed due to an indeterminate type
@@ -598,118 +660,98 @@ exception internal UpperCaseIdentifierInPattern of range
 /// Generate a new reference to a record field with a fresh type instantiation
 val FreshenRecdFieldRef: NameResolver -> range -> RecdFieldRef -> RecdFieldInfo
 
-/// Indicates the kind of lookup being performed. Note, this type should be made private to nameres.fs.
-[<RequireQualifiedAccess>]
-type LookupKind =
-    | RecdField
-    | Pattern
-    | Expr
-    | Type
-    | Ctor
-
-/// Indicates if a warning should be given for the use of upper-case identifiers in patterns
-type WarnOnUpperFlag =
-    | WarnOnUpperCase
-    | AllIdsOK
-
-/// Indicates whether we permit a direct reference to a type generator. Only set when resolving the
-/// right-hand-side of a [<Generate>] declaration.
-[<RequireQualifiedAccess>]
-type PermitDirectReferenceToGeneratedType =
-    | Yes
-    | No
-
 /// Resolve a long identifier to a namespace, module.
 val internal ResolveLongIdentAsModuleOrNamespace:
-    TcResultsSink ->
-    ResultCollectionSettings ->
-    ImportMap ->
-    range ->
+    sink: TcResultsSink ->
+    atMostOne: ResultCollectionSettings ->
+    amap: ImportMap ->
+    m: range ->
     first: bool ->
-    FullyQualifiedFlag ->
-    NameResolutionEnv ->
-    AccessorDomain ->
-    Ident ->
-    Ident list ->
+    fullyQualified: FullyQualifiedFlag ->
+    nenv: NameResolutionEnv ->
+    ad: AccessorDomain ->
+    id: Ident ->
+    rest: Ident list ->
     isOpenDecl: bool ->
         ResultOrException<(int * ModuleOrNamespaceRef * ModuleOrNamespaceType) list>
 
 /// Resolve a long identifier to an object constructor.
 val internal ResolveObjectConstructor:
-    NameResolver -> DisplayEnv -> range -> AccessorDomain -> TType -> ResultOrException<Item>
+    ncenv: NameResolver -> denv: DisplayEnv -> m: range -> ad: AccessorDomain -> ty: TType -> ResultOrException<Item>
 
 /// Resolve a long identifier using type-qualified name resolution.
 val internal ResolveLongIdentInType:
-    TcResultsSink ->
-    NameResolver ->
-    NameResolutionEnv ->
-    LookupKind ->
-    range ->
-    AccessorDomain ->
-    Ident ->
-    FindMemberFlag ->
-    TypeNameResolutionInfo ->
-    TType ->
+    sink: TcResultsSink ->
+    ncenv: NameResolver ->
+    nenv: NameResolutionEnv ->
+    lookupKind: LookupKind ->
+    m: range ->
+    ad: AccessorDomain ->
+    id: Ident ->
+    findFlag: FindMemberFlag ->
+    typeNameResInfo: TypeNameResolutionInfo ->
+    ty: TType ->
         Item * Ident list
 
 /// Resolve a long identifier when used in a pattern.
 val internal ResolvePatternLongIdent:
-    TcResultsSink ->
-    NameResolver ->
-    WarnOnUpperFlag ->
-    bool ->
-    range ->
-    AccessorDomain ->
-    NameResolutionEnv ->
-    TypeNameResolutionInfo ->
-    Ident list ->
+    sink: TcResultsSink ->
+    ncenv: NameResolver ->
+    warnOnUpper: WarnOnUpperFlag ->
+    newDef: bool ->
+    m: range ->
+    ad: AccessorDomain ->
+    nenv: NameResolutionEnv ->
+    numTyArgsOpt: TypeNameResolutionInfo ->
+    lid: Ident list ->
         Item
 
 /// Resolve a long identifier representing a type name
 val internal ResolveTypeLongIdentInTyconRef:
-    TcResultsSink ->
-    NameResolver ->
-    NameResolutionEnv ->
-    TypeNameResolutionInfo ->
-    AccessorDomain ->
-    range ->
-    ModuleOrNamespaceRef ->
-    Ident list ->
+    sink: TcResultsSink ->
+    ncenv: NameResolver ->
+    nenv: NameResolutionEnv ->
+    typeNameResInfo: TypeNameResolutionInfo ->
+    ad: AccessorDomain ->
+    m: range ->
+    tcref: TyconRef ->
+    lid: Ident list ->
         TyconRef
 
 /// Resolve a long identifier to a type definition
 val internal ResolveTypeLongIdent:
-    TcResultsSink ->
-    NameResolver ->
-    ItemOccurence ->
-    FullyQualifiedFlag ->
-    NameResolutionEnv ->
-    AccessorDomain ->
-    Ident list ->
-    TypeNameResolutionStaticArgsInfo ->
-    PermitDirectReferenceToGeneratedType ->
+    sink: TcResultsSink ->
+    ncenv: NameResolver ->
+    occurence: ItemOccurence ->
+    fullyQualified: FullyQualifiedFlag ->
+    nenv: NameResolutionEnv ->
+    ad: AccessorDomain ->
+    lid: Ident list ->
+    staticResInfo: TypeNameResolutionStaticArgsInfo ->
+    genOk: PermitDirectReferenceToGeneratedType ->
         ResultOrException<EnclosingTypeInst * TyconRef>
 
 /// Resolve a long identifier to a field
 val internal ResolveField:
-    TcResultsSink ->
-    NameResolver ->
-    NameResolutionEnv ->
-    AccessorDomain ->
-    TType ->
-    Ident list * Ident ->
-        Ident list ->
-            FieldResolution list
+    sink: TcResultsSink ->
+    ncenv: NameResolver ->
+    nenv: NameResolutionEnv ->
+    ad: AccessorDomain ->
+    ty: TType ->
+    mp: Ident list ->
+    id: Ident ->
+    allFields: Ident list ->
+        FieldResolution list
 
 /// Resolve a long identifier occurring in an expression position
 val internal ResolveExprLongIdent:
-    TcResultsSink ->
-    NameResolver ->
-    range ->
-    AccessorDomain ->
-    NameResolutionEnv ->
-    TypeNameResolutionInfo ->
-    Ident list ->
+    sink: TcResultsSink ->
+    ncenv: NameResolver ->
+    m: range ->
+    ad: AccessorDomain ->
+    nenv: NameResolutionEnv ->
+    typeNameResInfo: TypeNameResolutionInfo ->
+    lid: Ident list ->
         ResultOrException<EnclosingTypeInst * Item * Ident list>
 
 val internal getRecordFieldsInScope: NameResolutionEnv -> Item list
@@ -721,50 +763,29 @@ val internal ResolvePartialLongIdentToClassOrRecdFields:
 /// Return the fields for the given class or record
 val internal ResolveRecordOrClassFieldsOfType: NameResolver -> range -> AccessorDomain -> TType -> bool -> Item list
 
-/// Specifies extra work to do after overload resolution
-[<RequireQualifiedAccess>]
-type AfterResolution =
-    /// Notification is not needed
-    | DoNothing
-
-    /// Notify the sink of the information needed to complete recording a use of a symbol
-    /// for the purposes of the language service.  One of the callbacks should be called by
-    /// the checker.
-    ///
-    /// The first callback represents a case where we have learned the type
-    /// instantiation of a generic method or value.
-    ///
-    /// The second represents the case where we have resolved overloading and/or
-    /// a specific override. The 'Item option' contains the candidate overrides.
-    | RecordResolution of
-        Item option *
-        (TyparInstantiation -> unit) *
-        (MethInfo * PropInfo option * TyparInstantiation -> unit) *
-        (unit -> unit)
-
 /// Resolve a long identifier occurring in an expression position.
 val internal ResolveLongIdentAsExprAndComputeRange:
-    TcResultsSink ->
-    NameResolver ->
-    range ->
-    AccessorDomain ->
-    NameResolutionEnv ->
-    TypeNameResolutionInfo ->
-    Ident list ->
+    sink: TcResultsSink ->
+    ncenv: NameResolver ->
+    wholem: range ->
+    ad: AccessorDomain ->
+    nenv: NameResolutionEnv ->
+    typeNameResInfo: TypeNameResolutionInfo ->
+    lid: Ident list ->
         ResultOrException<EnclosingTypeInst * Item * range * Ident list * AfterResolution>
 
 /// Resolve a long identifier occurring in an expression position, qualified by a type.
 val internal ResolveExprDotLongIdentAndComputeRange:
-    TcResultsSink ->
-    NameResolver ->
-    range ->
-    AccessorDomain ->
-    NameResolutionEnv ->
-    TType ->
-    Ident list ->
-    TypeNameResolutionInfo ->
-    FindMemberFlag ->
-    bool ->
+    sink: TcResultsSink ->
+    ncenv: NameResolver ->
+    wholem: range ->
+    ad: AccessorDomain ->
+    nenv: NameResolutionEnv ->
+    ty: TType ->
+    lid: Ident list ->
+    typeNameResInfo: TypeNameResolutionInfo ->
+    findFlag: FindMemberFlag ->
+    staticOnly: bool ->
         Item * range * Ident list * AfterResolution
 
 /// A generator of type instantiations used when no more specific type instantiation is known.
@@ -775,13 +796,13 @@ val TryToResolveLongIdentAsType: NameResolver -> NameResolutionEnv -> range -> s
 
 /// Resolve a (possibly incomplete) long identifier to a set of possible resolutions.
 val ResolvePartialLongIdent:
-    NameResolver ->
-    NameResolutionEnv ->
-    (MethInfo -> TType -> bool) ->
-    range ->
-    AccessorDomain ->
-    string list ->
-    bool ->
+    ncenv: NameResolver ->
+    nenv: NameResolutionEnv ->
+    isApplicableMeth: (MethInfo -> TType -> bool) ->
+    m: range ->
+    ad: AccessorDomain ->
+    plid: string list ->
+    allowObsolete: bool ->
         Item list
 
 [<RequireQualifiedAccess>]

tests/fsharp/Compiler/Language/StaticNameResolutionTests.fs

+// Copyright (c) Microsoft Corporation.  All Rights Reserved.  See License.txt in the project root for license information.
+
+namespace FSharp.Compiler.UnitTests
+
+open System.Collections.Immutable
+open NUnit.Framework
+open FSharp.Test
+open FSharp.Test.Utilities
+open Microsoft.CodeAnalysis
+
+[<TestFixture>]
+module StaticNameResolutionTests =
+
+    [<Test>]
+    let ``C# nested type is accessible even when extension method is present with same name``() =
+        let csSrc =
+            """
+namespace CSharpLib
+{
+    public static class Extensions
+    {
+        // Note instance extension member has same name as nested type
+        static public T cuda<T>(this T x) { return x; }
+    }
+
+    public class torch
+    {
+        // Note instance extension member has same name as nested type
+        public class cuda
+        {
+            public static bool isAvailable() { return true; }
+            public bool isAvailable2() { return true; }
+        }
+    }
+}
+           """
+
+        let fsSrc =
+            """
+open CSharpLib
+
+let res1 : bool = torch.cuda.isAvailable()
+let res2 : bool = torch.cuda().isAvailable2()
+let res3 : bool = torch.cuda().cuda().isAvailable2()
+            """
+
+        let fsharpCoreAssembly =
+            typeof<_ list>.Assembly.Location
+            |> MetadataReference.CreateFromFile
+
+        let cs =
+            CompilationUtil.CreateCSharpCompilation(csSrc, CSharpLanguageVersion.CSharp8, TargetFramework.NetCoreApp31, additionalReferences = ImmutableArray.CreateRange [fsharpCoreAssembly])
+            |> CompilationReference.Create
+
+        let fs = Compilation.Create(fsSrc, CompileOutput.Exe, options = [| |], cmplRefs = [cs])
+        CompilerAssert.Compile fs
+

tests/fsharp/FSharpSuite.Tests.fsproj

@@ -54,6 +54,7 @@
     <Compile Include="Compiler\Language\CompilerDirectiveTests.fs" />
     <Compile Include="Compiler\Language\DefaultInterfaceMemberTests.fs" />
     <Compile Include="Compiler\Language\OptionalInteropTests.fs" />
+    <Compile Include="Compiler\Language\StaticNameResolutionTests.fs" />
     <Compile Include="Compiler\Language\UIntTests.fs" />
     <Compile Include="Compiler\Language\FixedIndexSliceTests.fs" />
     <Compile Include="Compiler\Language\SlicingQuotationTests.fs" />