Fix semantic classification errors for generics, slices, CEs, and open type

docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@@ -85,6 +85,12 @@
 * Emit debug points at a stack-empty position ([PR #19877](https://github.com/dotnet/fsharp/pull/19877))
 * Fix spurious XmlDoc warnings (unknown parameter / no documentation for parameter) under `--warnon:3390` when a get/set property documents the full parameter set across both accessors. ([Issue #13684](https://github.com/dotnet/fsharp/issues/13684), [PR #19884](https://github.com/dotnet/fsharp/pull/19884))
 * Quotations of `match s with "" -> _` no longer leak the `s <> null && s.Length = 0` lowering; the empty-string optimization moved from pattern-match compilation to the optimizer so quoted expressions keep `op_Equality(s, "")`. ([Issue #19873](https://github.com/dotnet/fsharp/issues/19873))
+* Editor: `open type T` is no longer reported as unused by `UnusedOpens.getUnusedOpens` in cases where the imported member is matched against the target type entity rather than the namespace it lives in. ([Issue #19905 item 7](https://github.com/dotnet/fsharp/issues/19905), [PR #19939](https://github.com/dotnet/fsharp/pull/19939))
+* Fix `<` / `>` and type-argument text in generic static method calls (e.g. `Type.Method<int>()`) being classified as `Method` over the whole `Type.Method<int>` span in the F# editor; the classification is now narrowed to the method identifier. ([Issue #19905 item 3](https://github.com/dotnet/fsharp/issues/19905), [PR #19939](https://github.com/dotnet/fsharp/pull/19939))
+* Fix `<` / `>` and type-argument text in generic constructor calls (e.g. `new MailboxProcessor<int * int>(...)`) being classified as `DisposableType` over the whole `MailboxProcessor<int * int>` span in the F# editor; the classification is now narrowed to the type identifier. ([Issue #19905 item 4](https://github.com/dotnet/fsharp/issues/19905), [PR #19939](https://github.com/dotnet/fsharp/pull/19939))
+* Fix the trailing `]` of an open-ended slice (e.g. `list[0..]`) being included in the `Method` classification of the indexer in the F# editor. ([Issue #19905 item 5](https://github.com/dotnet/fsharp/issues/19905), [PR #19939](https://github.com/dotnet/fsharp/pull/19939))
+* Fix `<` / `>` and type-argument text in `Type<args>.Method(...)` expressions (e.g. `MailboxProcessor<int>.Start(...)`) being mis-classified by widening `Method` and `DisposableType` spans over `<args>` in the F# editor. ([Issue #19905 item 6](https://github.com/dotnet/fsharp/issues/19905), [PR #19939](https://github.com/dotnet/fsharp/pull/19939))
+* Fix computation-expression builder names inside list/array/sequence comprehensions (e.g. `[ for i in xs do optional { ... } ]`) being classified as a local `Value` instead of `ComputationExpression`. ([Issue #19905 item 2](https://github.com/dotnet/fsharp/issues/19905), [PR #19939](https://github.com/dotnet/fsharp/pull/19939))
 
 ### Added
 

docs/release-notes/.VisualStudio/18.vNext.md

@@ -10,6 +10,7 @@
 * Improve static compilation of state machines. ([PR #19297](https://github.com/dotnet/fsharp/pull/19297))
 * Make Alt+F1 (momentary toggle) work for inlay hints. ([PR #19421](https://github.com/dotnet/fsharp/pull/19421))
 * Fix doubled F# diagnostics in tooltips. ([Issue #16360](https://github.com/dotnet/fsharp/issues/16360))
+* Fix several semantic classification errors: `<`/`>` in generic calls no longer classified as Function/Type, trailing `]` in open-ended slices no longer classified as Function, CE builder names in comprehensions classified as ComputationExpression, and `open type` no longer always reported as unused. ([Issue #19905](https://github.com/dotnet/fsharp/issues/19905), [PR #19939](https://github.com/dotnet/fsharp/pull/19939))
 
 ### Changed
 

src/Compiler/Checking/Expressions/CheckExpressions.fs

@@ -6048,9 +6048,14 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE
         // knows this is provably non-null (even for AllowNullLiteral types).
         let objTy = if g.checkNullness then replaceNullnessOfTy Nullness.KnownFromConstructor objTy else objTy
 
+        // Use the narrow leading-identifier range for the constructor item so the classifier paints
+        // only the type name as ConstructorForReferenceType/DisposableType, not the surrounding
+        // `<...>` type-arg punctuation (#19905 item 4).
+        let mObjTy = synTypeLeadingIdentRange synObjTy
+
         TcNonControlFlowExpr env <| fun env ->
         TcPropagatingExprLeafThenConvert cenv overallTy objTy env (* true *) mNewExpr (fun () ->
-          TcNewExpr cenv env tpenv objTy (Some synObjTy.Range) superInit arg mNewExpr
+          TcNewExpr cenv env tpenv objTy (Some mObjTy) superInit arg mNewExpr
         )
 
     | SynExpr.ObjExpr (synObjTy, argopt, _mWith, binds, members, extraImpls, mNewExpr, m) ->
@@ -6772,7 +6777,7 @@ and ExpandIndexArgs (cenv: cenv) (synLeftExprOpt: SynExpr option) indexArgs =
                    | Some (a2, isFromEnd2) ->
                        yield mkSynSomeExpr range2 (if isFromEnd2 then rewriteReverseExpr pos a2 range2 else a2)
                    | None ->
-                       yield mkSynNoneExpr range1
+                       yield mkSynNoneExpr range2
                 ]
         )
         |> List.collect id
@@ -6926,11 +6931,18 @@ and TcIndexingThen cenv env overallTy mWholeExpr mDot tpenv setInfo synLeftExprO
             match propName with
             | None -> "Item"
             | Some nm -> nm
+        // For slice expressions (not regular indexing), the synthesized GetSlice/SetSlice
+        // identifier has no user-visible source range. Use a synthetic range so the name
+        // resolution sink (NotifyNameResolution) drops the CNR, otherwise the semantic
+        // classifier paints the entire `list[...]` span as Method (#19905 item 5).
+        // Regular indexing (`x.[n]`/`x[n]`) keeps the visible range so find-all-references
+        // continues to surface implicit Item indexer usages.
+        let mNm = if isIndex then mWholeExpr else mWholeExpr.MakeSynthetic()
         let delayed =
             match setInfo with
             // expr1.[expr2]
             | None  ->
-                [ DelayedDotLookup([ ident(nm, mWholeExpr)], mWholeExpr)
+                [ DelayedDotLookup([ ident(nm, mNm)], mNm)
                   DelayedApp(ExprAtomicFlag.Atomic, true, synLeftExprOpt, MakeIndexParam None, mWholeExpr)
                   yield! delayed ]
 
@@ -6942,7 +6954,8 @@ and TcIndexingThen cenv env overallTy mWholeExpr mDot tpenv setInfo synLeftExprO
                       MakeDelayedSet(expr3, mWholeExpr)
                       yield! delayed ]
                 else
-                    [ DelayedDotLookup([ident("SetSlice", mOfLeftOfSet)], mOfLeftOfSet)
+                    let mSynNm = mOfLeftOfSet.MakeSynthetic()
+                    [ DelayedDotLookup([ident("SetSlice", mSynNm)], mSynNm)
                       DelayedApp(ExprAtomicFlag.Atomic, true, synLeftExprOpt, MakeIndexParam (Some expr3), mWholeExpr)
                       yield! delayed ]
 
@@ -9154,24 +9167,31 @@ and TcTypeItemThen (cenv: cenv) overallTy env nm ty tpenv mItem tinstEnclosing d
     let g = cenv.g
     let ad = env.eAccessRights
     match delayed with
-    | DelayedTypeApp(tyargs, _mTypeArgs, mExprAndTypeArgs) :: DelayedDotLookup (longId, mLongId) :: otherDelayed ->
+    | DelayedTypeApp(tyargs, _mTypeArgs, mExprAndTypeArgs) :: DelayedDotLookup (longId, _mLongId) :: otherDelayed ->
         // If Item.Types is returned then the ty will be of the form TType_app(tcref, genericTyargs) where tyargs
         // is a fresh instantiation for tcref. TcNestedTypeApplication will chop off precisely #genericTyargs args
         // and replace them by 'tyargs'
         let ty, tpenv = TcNestedTypeApplication cenv NewTyparsOK CheckCxs ItemOccurrence.UseInType WarnOnIWSAM.Yes env tpenv mExprAndTypeArgs ty tinstEnclosing tyargs
 
-        // Report information about the whole expression including type arguments to VS
+        // Report information about the type to VS. Use the narrow mItem (the type identifier only)
+        // so the classifier does not paint the surrounding `<...>` type-arg punctuation as part of the
+        // type classification (#19905 item 6).
         let item = Item.Types(nm, [ty])
-        CallNameResolutionSink cenv.tcSink (mExprAndTypeArgs, env.NameEnv, item, emptyTyparInst, ItemOccurrence.Use, env.eAccessRights)
+        CallNameResolutionSink cenv.tcSink (mItem, env.NameEnv, item, emptyTyparInst, ItemOccurrence.Use, env.eAccessRights)
         let typeNameResInfo = GetLongIdentTypeNameInfo otherDelayed
-        let item, mItem, mItemIdent, rest, afterResolution = ResolveExprDotLongIdentAndComputeRange cenv.tcSink cenv.nameResolver (unionRanges mExprAndTypeArgs mLongId) ad env.eNameResEnv ty longId typeNameResInfo IgnoreOverrides true None
+        // Resolve `.longId` using only the longId range as wholem, so the resulting mItem / Method CNR
+        // does not extend back across the `<...>` type-arg span (#19905 item 6).
+        let item, mItem, mItemIdent, rest, afterResolution =
+            ResolveExprDotLongIdentAndComputeRange cenv.tcSink cenv.nameResolver (rangeOfLid longId) ad env.eNameResEnv ty longId typeNameResInfo IgnoreOverrides true None
         TcItemThen cenv overallTy env tpenv ((argsOfAppTy g ty), item, mItem, mItemIdent, rest, afterResolution) None otherDelayed
 
     | DelayedTypeApp(tyargs, _mTypeArgs, mExprAndTypeArgs) :: _delayed' ->
         // A case where we have an incomplete name e.g. 'Foo<int>.' - we still want to report it to VS!
         let ty, _ = TcNestedTypeApplication cenv NewTyparsOK CheckCxs ItemOccurrence.UseInType WarnOnIWSAM.Yes env tpenv mExprAndTypeArgs ty tinstEnclosing tyargs
         let item = Item.Types(nm, [ty])
-        CallNameResolutionSink cenv.tcSink (mExprAndTypeArgs, env.NameEnv, item, emptyTyparInst, ItemOccurrence.Use, env.eAccessRights)
+        // Use the narrow mItem range so the type classification does not paint the surrounding
+        // `<...>` type-arg punctuation (#19905 items 4/6).
+        CallNameResolutionSink cenv.tcSink (mItem, env.NameEnv, item, emptyTyparInst, ItemOccurrence.Use, env.eAccessRights)
 
         // Same error as in the following case
         error(Error(FSComp.SR.tcInvalidUseOfTypeName(), mItem))
@@ -9217,7 +9237,10 @@ and TcMethodItemThen (cenv: cenv) overallTy env item methodName minfos tpenv mIt
         // FUTURE: can we do better than emptyTyparInst here, in order to display instantiations
         // of type variables in the quick info provided in the IDE? But note we haven't yet even checked if the
         // number of type arguments is correct...
-        CallNameResolutionSink cenv.tcSink (mExprAndTypeArgs, env.NameEnv, item, emptyTyparInst, ItemOccurrence.Use, env.eAccessRights)
+        // Note: we deliberately use the narrow mItem range here (the identifier-only range), not
+        // mExprAndTypeArgs, so the semantic classifier paints only the method identifier as a Method
+        // and not the surrounding `<...>` type-arg punctuation (#19905 item 3).
+        CallNameResolutionSink cenv.tcSink (mItem, env.NameEnv, item, emptyTyparInst, ItemOccurrence.Use, env.eAccessRights)
 
         match otherDelayed with
         | DelayedApp(atomicFlag, _, _, arg, mExprAndArg) :: otherDelayed ->
@@ -9251,6 +9274,11 @@ and TcCtorItemThen (cenv: cenv) overallTy env item nm minfos tinstEnclosing tpen
     | DelayedTypeApp(tyargs, _mTypeArgs, mExprAndTypeArgs) :: DelayedApp(_, _, _, arg, mExprAndArg) :: otherDelayed ->
 
         let objTyAfterTyArgs, tpenv = TcNestedTypeApplication cenv NewTyparsOK CheckCxs ItemOccurrence.UseInType WarnOnIWSAM.Yes env tpenv mExprAndTypeArgs objTy tinstEnclosing tyargs
+        // Mirror the non-generic ctor path above: use mExprAndArg so the expression-typing entry
+        // used for Quick Info / dot-completion (GetExprTypingForPosition) reflects the full
+        // `Foo<...>(arg)` expression, not just the identifier. The narrow semantic-classification
+        // CNR for #19905 item 4 is emitted via CallNameResolutionSink in the DelayedTypeApp-only
+        // branch below.
         CallExprHasTypeSink cenv.tcSink (mExprAndArg, env.NameEnv, objTyAfterTyArgs, env.eAccessRights)
         let itemAfterTyArgs, minfosAfterTyArgs =
 #if !NO_TYPEPROVIDERS
@@ -9274,8 +9302,10 @@ and TcCtorItemThen (cenv: cenv) overallTy env item nm minfos tinstEnclosing tpen
         let objTy, tpenv = TcNestedTypeApplication cenv NewTyparsOK CheckCxs ItemOccurrence.UseInType WarnOnIWSAM.Yes env tpenv mExprAndTypeArgs objTy tinstEnclosing tyargs
 
         // A case where we have an incomplete name e.g. 'Foo<int>.' - we still want to report it to VS!
+        // Use the narrow mItem range (the ctor identifier only) so the classifier does not paint the
+        // surrounding `<...>` type-arg punctuation as part of the type/ctor classification (#19905 item 4/6).
         let resolvedItem = Item.Types(nm, [objTy])
-        CallNameResolutionSink cenv.tcSink (mExprAndTypeArgs, env.NameEnv, resolvedItem, emptyTyparInst, ItemOccurrence.Use, env.eAccessRights)
+        CallNameResolutionSink cenv.tcSink (mItem, env.NameEnv, resolvedItem, emptyTyparInst, ItemOccurrence.Use, env.eAccessRights)
 
         minfos |> List.iter (fun minfo -> UnifyTypes cenv env mExprAndTypeArgs minfo.ApparentEnclosingType objTy)
         TcCtorCall true cenv env tpenv overallTy objTy (Some mItemIdent) item false [] mExprAndTypeArgs otherDelayed (Some afterResolution)

src/Compiler/Service/SemanticClassification.fs

@@ -178,29 +178,36 @@ module TcResolutionsExtensions =
                             | _ -> None
                         | _ -> None
 
-                    // Custom builders like 'async { }' are both Item.Value and Item.CustomBuilder.
-                    // We should prefer the latter, otherwise they would not get classified as CEs.
+                    // Custom builders like 'async { }' are both Item.Value and Item.CustomBuilder
+                    // (and sometimes additional Item.CustomOperation entries inside a list/array
+                    // comprehension's CE-machinery). We should prefer any CustomBuilder/CustomOperation
+                    // hit over the Value one so the builder name gets a ComputationExpression
+                    // classification instead of being painted as a local value.
                     let takeCustomBuilder (cnrs: CapturedNameResolution[]) =
                         assert (cnrs.Length > 0)
 
-                        if cnrs.Length = 1 then
-                            cnrs
-                        elif cnrs.Length = 2 then
-                            match cnrs[0].Item, cnrs[1].Item with
-                            | Item.Value _, Item.CustomBuilder _ -> [| cnrs[1] |]
-                            | Item.CustomBuilder _, Item.Value _ -> [| cnrs[0] |]
-                            | _ -> cnrs
-                        else
+                        let preferred =
                             cnrs
+                            |> Array.filter (fun cnr ->
+                                match cnr.Item with
+                                | Item.CustomBuilder _
+                                | Item.CustomOperation _ -> true
+                                | _ -> false)
+
+                        if preferred.Length > 0 then preferred else cnrs
 
                     let resolutions =
+                        let groupAndPickBuilder (cnrs: CapturedNameResolution[]) =
+                            cnrs
+                            |> Array.groupBy (fun cnr -> cnr.Range)
+                            |> Array.collect (fun (_, cnrs) -> takeCustomBuilder cnrs)
+
                         match range with
                         | Some range ->
                             sResolutions.CapturedNameResolutions.ToArray()
                             |> Array.filter (fun cnr -> rangeContainsPos range cnr.Range.Start || rangeContainsPos range cnr.Range.End)
-                            |> Array.groupBy (fun cnr -> cnr.Range)
-                            |> Array.collect (fun (_, cnrs) -> takeCustomBuilder cnrs)
-                        | None -> sResolutions.CapturedNameResolutions.ToArray()
+                            |> groupAndPickBuilder
+                        | None -> sResolutions.CapturedNameResolutions.ToArray() |> groupAndPickBuilder
 
                     let duplicates = HashSet<range>(comparer)
 

src/Compiler/Service/ServiceAnalysis.fs

@@ -273,20 +273,27 @@ module UnusedOpens =
     /// Async to allow cancellation.
     let filterOpenStatements (symbolUses1: FSharpSymbolUse[], symbolUses2: FSharpSymbolUse[]) openStatements =
         async {
-            // the key is a namespace or module, the value is a list of FSharpSymbolUse range of symbols defined in the
-            // namespace or module. So, it's just symbol uses ranges grouped by namespace or module where they are _defined_.
+            // the key is a namespace, module, or type entity, the value is a list of FSharpSymbolUse range of symbols
+            // defined in that entity. So, it's just symbol uses ranges grouped by the entity where they are _defined_.
+            // Type entities are kept here so that `open type T` is recognised as used when its imported members are.
+            // `symbolUses2` still flows through the RevealedSymbols path below, so this is purely additive.
             let symbolUsesRangesByDeclaringEntity =
                 Dictionary<FSharpEntity, range list>(entityHash)
 
-            for symbolUse in symbolUses1 do
+            let recordByDeclaringEntity (symbolUse: FSharpSymbolUse) =
                 match symbolUse.Symbol with
                 | :? FSharpMemberOrFunctionOrValue as f ->
                     match f.DeclaringEntity with
-                    | Some entity when entity.IsNamespace || entity.IsFSharpModule ->
-                        symbolUsesRangesByDeclaringEntity.BagAdd(entity, symbolUse.Range)
-                    | _ -> ()
+                    | Some entity -> symbolUsesRangesByDeclaringEntity.BagAdd(entity, symbolUse.Range)
+                    | None -> ()
                 | _ -> ()
 
+            for symbolUse in symbolUses1 do
+                recordByDeclaringEntity symbolUse
+
+            for symbolUse in symbolUses2 do
+                recordByDeclaringEntity symbolUse
+
             let! results =
                 filterOpenStatementsIncremental
                     symbolUses2

src/Compiler/SyntaxTree/SyntaxTreeOps.fs

@@ -586,6 +586,22 @@ let rec stripParenTypes synType =
 
 let (|StripParenTypes|) synType = stripParenTypes synType
 
+/// Return the range of the principal type identifier of a SynType, falling back to the type's
+/// full range. Used to emit narrow semantic-classification CNRs that don't paint surrounding
+/// `<...>` type-argument punctuation (#19905 items 4/6).
+///
+/// For `SynType.LongIdentApp(typeName, longDotId, ...)` representing e.g. `A.B.C<X>`, the
+/// trailing `longDotId` (`.C`) is the identifier being instantiated and is therefore the
+/// "principal" type identifier returned here. The qualifier `A.B` (`typeName`) has its own
+/// name-resolution entries reported separately.
+let rec synTypeLeadingIdentRange (synType: SynType) =
+    match synType with
+    | SynType.LongIdent synLongId -> synLongId.Range
+    | SynType.App(typeName = StripParenTypes (SynType.LongIdent synLongId)) -> synLongId.Range
+    | SynType.LongIdentApp(longDotId = synLongId) -> synLongId.Range
+    | SynType.Paren(innerType, _) -> synTypeLeadingIdentRange innerType
+    | _ -> synType.Range
+
 /// Operations related to the syntactic analysis of arguments of value, function and member definitions and signatures.
 module SynInfo =
 

src/Compiler/SyntaxTree/SyntaxTreeOps.fsi

@@ -200,6 +200,15 @@ val stripParenTypes: synType: SynType -> SynType
 
 val (|StripParenTypes|): synType: SynType -> SynType
 
+/// Return the range of the principal type identifier of a SynType, falling back to the type's
+/// full range. Used to emit narrow semantic-classification CNRs that don't paint surrounding
+/// `<...>` type-argument punctuation.
+///
+/// For `SynType.LongIdentApp(typeName, longDotId, ...)` (e.g. `A.B.C<X>`), the trailing
+/// `longDotId` (`.C`) is the identifier being instantiated and is therefore the principal
+/// type identifier returned here.
+val synTypeLeadingIdentRange: synType: SynType -> range
+
 /// Operations related to the syntactic analysis of arguments of value, function and member definitions and signatures.
 module SynInfo =
     /// The argument information for an argument without a name