diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index b89cf11dc7e..2718dfbdd40 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/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()`) being classified as `Method` over the whole `Type.Method` 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(...)`) being classified as `DisposableType` over the whole `MailboxProcessor` 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.Method(...)` expressions (e.g. `MailboxProcessor.Start(...)`) being mis-classified by widening `Method` and `DisposableType` spans over `` 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 diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 6205e8caef0..bbff4b603e5 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/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 diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index bfc90ecf1ce..c0d34e445ec 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/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.' - 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.' - 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) diff --git a/src/Compiler/Service/SemanticClassification.fs b/src/Compiler/Service/SemanticClassification.fs index da0193e4405..251e395ada3 100644 --- a/src/Compiler/Service/SemanticClassification.fs +++ b/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(comparer) diff --git a/src/Compiler/Service/ServiceAnalysis.fs b/src/Compiler/Service/ServiceAnalysis.fs index 6455d9f0ff3..e9498cb3728 100644 --- a/src/Compiler/Service/ServiceAnalysis.fs +++ b/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(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 diff --git a/src/Compiler/SyntaxTree/SyntaxTreeOps.fs b/src/Compiler/SyntaxTree/SyntaxTreeOps.fs index 1fb56dd697d..ab8fbb1f2d4 100644 --- a/src/Compiler/SyntaxTree/SyntaxTreeOps.fs +++ b/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`, 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 = diff --git a/src/Compiler/SyntaxTree/SyntaxTreeOps.fsi b/src/Compiler/SyntaxTree/SyntaxTreeOps.fsi index cb5fe46a1b6..26661534b7e 100644 --- a/src/Compiler/SyntaxTree/SyntaxTreeOps.fsi +++ b/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`), 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 diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionTypes.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionTypes.fs index 7d5db82abde..9498f53905f 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionTypes.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/UnionTypes/UnionTypes.fs @@ -124,7 +124,7 @@ module UnionTypes = |> shouldFail |> withDiagnostics [ (Warning 1125, Line 11, Col 29, Line 11, Col 41, "The instantiation of the generic type 'list1' is missing and can't be inferred from the arguments or return type of this member. Consider providing a type instantiation when accessing this type, e.g. 'list1<_>'.") - (Error 3068, Line 15, Col 10, Line 15, Col 25, "The function or member 'toList' is used in a way that requires further type annotations at its definition to ensure consistency of inferred types. The inferred signature is 'static member private list1.toList: ('a list1 -> 'a list)'.") + (Error 3068, Line 15, Col 19, Line 15, Col 25, "The function or member 'toList' is used in a way that requires further type annotations at its definition to ensure consistency of inferred types. The inferred signature is 'static member private list1.toList: ('a list1 -> 'a list)'.") ] //SOURCE=E_Interface_IComparable.fsx SCFLAGS="--test:ErrorRanges" # E_Interface_IComparable.fsx diff --git a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/SemanticClassificationRegressions.fs b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/SemanticClassificationRegressions.fs index f2a2503da1f..4d5eef68cc3 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharpChecker/SemanticClassificationRegressions.fs +++ b/tests/FSharp.Compiler.ComponentTests/FSharpChecker/SemanticClassificationRegressions.fs @@ -271,3 +271,431 @@ type MyDelegate = delegate of int -> string && (let text = substringOfRange source c.Range text = "BeginInvoke" || text = "EndInvoke")) Assert.Empty(asyncInvokeMethods) + +// ===================================================================== +// Issue #19905 - F# editor classification cluster (Phase 0 baseline pins) +// These tests describe the CURRENT GREEN behaviour on HEAD for items +// already fixed upstream (item 1 by #19813; item 7 simple-case observed +// passing). Later sprints add RED tests for items 2, 3+4+6, 5, and the +// remaining item-7 cases together with the production fixes. +// ===================================================================== + +/// (#19905 item 1) Delegate signature must not produce a synthesized Invoke +/// Method classification. Already fixed by #19813. Confirmation pin. +[] +let ``19905 item 1 - delegate sig not classified as method (pin)`` () = + let source = + """ +type SumDelegate = delegate of x: int * y: int -> int +""" + let items = getClassifications source + let invokeMethods = + items + |> Array.filter (fun c -> + c.Type = SemanticClassificationType.Method + && substringOfRange source c.Range = "Invoke") + Assert.Empty(invokeMethods) + +/// (#19905 item 7 simple-case) Minimal `open type` usage already classifies +/// the open as used on HEAD. Pin so later sprints don't regress this path. +[] +let ``19905 item 7 simple - open type used by static call is not flagged unused (pin)`` () = + let source = + """ +module Test +module Inner = + type Helper() = + static member Greet() = "hi" +open type Inner.Helper +let _ = Greet() +""" + let fileName, snapshot, checker = singleFileChecker source + let results = checker.ParseAndCheckFileInProject(fileName, snapshot) |> Async.RunSynchronously + let checkResults = getTypeCheckResult results + let lines = source.Replace("\r\n", "\n").Split('\n') + let getSourceLineStr n = + if n >= 1 && n <= lines.Length then lines[n - 1] else "" + let unused = + UnusedOpens.getUnusedOpens(checkResults, getSourceLineStr) + |> Async.RunSynchronously + // The `open type Inner.Helper` is on line 6. + let unusedOnOpenLine = + unused + |> List.filter (fun r -> r.StartLine = 6) + Assert.True( + unusedOnOpenLine.IsEmpty, + sprintf "'open type Inner.Helper' (line 6) must not be flagged unused; got ranges: %A" + (unusedOnOpenLine |> List.map (fun r -> r.StartLine, r.StartColumn, r.EndColumn))) + +/// (#19905 item 7 regression-guard) `open System` with nothing referenced from +/// it must still be flagged unused. Sprint 05 will widen entity tracking and +/// could over-reach; this pin prevents that. +[] +let ``19905 item 7 regression - unused open System is still flagged unused (pin)`` () = + let source = + """ +module Test +open System +let _ = 1 +""" + let fileName, snapshot, checker = singleFileChecker source + let results = checker.ParseAndCheckFileInProject(fileName, snapshot) |> Async.RunSynchronously + let checkResults = getTypeCheckResult results + let lines = source.Replace("\r\n", "\n").Split('\n') + let getSourceLineStr n = + if n >= 1 && n <= lines.Length then lines[n - 1] else "" + let unused = + UnusedOpens.getUnusedOpens(checkResults, getSourceLineStr) + |> Async.RunSynchronously + let unusedOnOpenLine = + unused + |> List.filter (fun r -> r.StartLine = 3) + Assert.True( + not unusedOnOpenLine.IsEmpty, + "'open System' with no usage must still be flagged unused") + +/// (#19905 item 7 unused-negative) `open type X.Y` with no usage of imported +/// members must still be flagged unused. Regression-guard for Sprint 05. +[] +let ``19905 item 7 negative - open type with no usage is still flagged unused (pin)`` () = + let source = + """ +module Test +module Inner = + type Helper() = + static member Greet() = "hi" +open type Inner.Helper +let _ = 1 +""" + let fileName, snapshot, checker = singleFileChecker source + let results = checker.ParseAndCheckFileInProject(fileName, snapshot) |> Async.RunSynchronously + let checkResults = getTypeCheckResult results + let lines = source.Replace("\r\n", "\n").Split('\n') + let getSourceLineStr n = + if n >= 1 && n <= lines.Length then lines[n - 1] else "" + let unused = + UnusedOpens.getUnusedOpens(checkResults, getSourceLineStr) + |> Async.RunSynchronously + let unusedOnOpenLine = + unused + |> List.filter (fun r -> r.StartLine = 6) + Assert.True( + not unusedOnOpenLine.IsEmpty, + "'open type Inner.Helper' with no usage of imported members must still be flagged unused") + +// ===================================================================== +// Issue #19905 - Items 3 + 4 + 6 (typeapp CNR-range overrun) +// ===================================================================== + +/// (#19905 item 3) Generic static method call `Type.Method()` must not +/// emit a Method classification covering the `` type-argument text. +[] +let ``19905 item 3 - generic static method does not classify type args as method`` () = + let source = + """ +module Test +type MyType() = + static member Method<'a>() = Unchecked.defaultof<'a> + static member Method2<'a, 'b>() = Unchecked.defaultof<'a>, Unchecked.defaultof<'b> +let x = MyType.Method() +let y, z = MyType.Method2() +""" + let items = getClassifications source + // Line 6: "let x = MyType.Method()" + // "MyType" ends at col 14, ".Method" ends at col 21, "" ends at col 26. + let badL6 = + items + |> Array.filter (fun c -> + c.Range.StartLine = 6 + && (c.Type = SemanticClassificationType.Method + || c.Type = SemanticClassificationType.Function) + && c.Range.EndColumn > 21) + Assert.True( + badL6.Length = 0, + sprintf "No Method/Function classification on line 6 should extend past column 21 (end of 'Method'). Found: %A" + (badL6 |> Array.map (fun c -> c.Range.StartColumn, c.Range.EndColumn, c.Type))) + // Line 7: "let y, z = MyType.Method2()" + // ".Method2" ends at col 25, "" ends at col 38. + let badL7 = + items + |> Array.filter (fun c -> + c.Range.StartLine = 7 + && (c.Type = SemanticClassificationType.Method + || c.Type = SemanticClassificationType.Function) + && c.Range.EndColumn > 25) + Assert.True( + badL7.Length = 0, + sprintf "No Method/Function classification on line 7 should extend past column 25 (end of 'Method2'). Found: %A" + (badL7 |> Array.map (fun c -> c.Range.StartColumn, c.Range.EndColumn, c.Type))) + +/// (#19905 item 4) Generic constructor `new MailboxProcessor(...)` +/// must not emit a type classification covering the `` text. +[] +let ``19905 item 4 - generic ctor does not classify type args as type`` () = + let source = + """ +module Test +let myMailbox = new MailboxProcessor(fun mbx -> async { return () }) +""" + let items = getClassifications source + // Line 3: "let myMailbox = new MailboxProcessor(...)" + // "MailboxProcessor" ends at col 36, "" ends at col 47. + let badTypeSpans = + items + |> Array.filter (fun c -> + c.Range.StartLine = 3 + && (c.Type = SemanticClassificationType.ReferenceType + || c.Type = SemanticClassificationType.DisposableType + || c.Type = SemanticClassificationType.ConstructorForReferenceType) + && c.Range.EndColumn > 36) + Assert.True( + badTypeSpans.Length = 0, + sprintf "No type-like classification on line 3 should extend past column 36 (end of 'MailboxProcessor'). Found: %A" + (badTypeSpans |> Array.map (fun c -> c.Range.StartColumn, c.Range.EndColumn, c.Type))) + let badFuncSpans = + items + |> Array.filter (fun c -> + c.Range.StartLine = 3 + && (c.Type = SemanticClassificationType.Method + || c.Type = SemanticClassificationType.Function) + && c.Range.StartColumn >= 36 + && c.Range.EndColumn <= 47) + Assert.True( + badFuncSpans.Length = 0, + sprintf "No Method/Function classification should cover the punctuation on line 3. Found: %A" + (badFuncSpans |> Array.map (fun c -> c.Range.StartColumn, c.Range.EndColumn, c.Type))) + +/// (#19905 item 6) `MailboxProcessor.Start(...)` must not emit a Method +/// classification whose range starts before the `.` (spans the type name +/// and angle brackets). +[] +let ``19905 item 6 - generic type static method classifies only the method name`` () = + let source = + """ +module Test +let mbx = MailboxProcessor.Start(fun mbx -> async { return () }) +""" + let items = getClassifications source + // Line 3: "let mbx = MailboxProcessor.Start(...)" + // "MailboxProcessor" ends at col 26, "" ends at col 31, ".Start" ends at col 37. + let badMethodSpans = + items + |> Array.filter (fun c -> + c.Range.StartLine = 3 + && c.Type = SemanticClassificationType.Method + && c.Range.StartColumn < 32 + && c.Range.EndColumn = 37) + Assert.True( + badMethodSpans.Length = 0, + sprintf "Method classification on line 3 should cover only 'Start' (32-37), not a wider span. Found: %A" + (badMethodSpans |> Array.map (fun c -> c.Range.StartColumn, c.Range.EndColumn, c.Type))) + // The type-name classification for MailboxProcessor must not extend past col 26 either. + let badTypeSpans = + items + |> Array.filter (fun c -> + c.Range.StartLine = 3 + && (c.Type = SemanticClassificationType.ReferenceType + || c.Type = SemanticClassificationType.DisposableType) + && c.Range.EndColumn > 26) + Assert.True( + badTypeSpans.Length = 0, + sprintf "No type-like classification on line 3 should extend past column 26 (end of 'MailboxProcessor'). Found: %A" + (badTypeSpans |> Array.map (fun c -> c.Range.StartColumn, c.Range.EndColumn, c.Type))) + + +// ===================================================================== +// Issue #19905 - Item 5 (open-ended slice trailing/leading bracket +// mis-classified as Method/Function) +// ===================================================================== + +/// (#19905 item 5) Open-ended slice `list[0..]` must not emit a Method/Function +/// classification whose range ends at the closing `]`. +[] +let ``19905 item 5 - open-ended slice does not classify closing bracket as function`` () = + let source = + """ +module Test +let list = [1; 2; 3] +let x = list[0..] +""" + let items = getClassifications source + // Line 4: "let x = list[0..]" - closing ']' is at column 17. + let badSpans = + items + |> Array.filter (fun c -> + c.Range.StartLine = 4 + && (c.Type = SemanticClassificationType.Method + || c.Type = SemanticClassificationType.Function) + && c.Range.EndColumn = 17) + Assert.True( + badSpans.Length = 0, + sprintf "No Method/Function classification on line 4 should end at column 17 (the ']'). Found: %A" + (badSpans |> Array.map (fun c -> c.Range.StartColumn, c.Range.EndColumn, c.Type))) + +/// (#19905 item 5 sibling) Open-ended lower slice `list[..2]` must not emit a +/// Method/Function classification whose range spans the whole `list[..2]`. +[] +let ``19905 item 5 - open-ended lower slice does not classify opening bracket as function`` () = + let source = + """ +module Test +let list = [1; 2; 3] +let x = list[..2] +""" + let items = getClassifications source + // Line 4: "let x = list[..2]" - opening '[' is at column 12; ']' at column 17. + let badSpans = + items + |> Array.filter (fun c -> + c.Range.StartLine = 4 + && (c.Type = SemanticClassificationType.Method + || c.Type = SemanticClassificationType.Function) + && c.Range.StartColumn <= 12 + && c.Range.EndColumn >= 17) + Assert.True( + badSpans.Length = 0, + sprintf "No Method/Function classification on line 4 should span the whole `list[..2]` expression. Found: %A" + (badSpans |> Array.map (fun c -> c.Range.StartColumn, c.Range.EndColumn, c.Type))) + +/// (#19905 item 5 negative) Closed slice `list[0..2]` must not emit a wide +/// Method/Function classification spanning `list[0..2]` either - guards against +/// the open-ended fix accidentally still allowing the closed-slice overrun. +[] +let ``19905 item 5 negative - closed slice classification unchanged`` () = + let source = + """ +module Test +let list = [1; 2; 3] +let x = list[0..2] +""" + let items = getClassifications source + Assert.True( + items |> Array.exists (fun c -> c.Range.StartLine = 4), + "Expected at least one classification on line 4 for the closed slice") + // Line 4: "let x = list[0..2]" - closing ']' is at column 18. + let badSpans = + items + |> Array.filter (fun c -> + c.Range.StartLine = 4 + && (c.Type = SemanticClassificationType.Method + || c.Type = SemanticClassificationType.Function) + && c.Range.EndColumn = 18) + Assert.True( + badSpans.Length = 0, + sprintf "No Method/Function classification on line 4 should end at column 18 (the ']'). Found: %A" + (badSpans |> Array.map (fun c -> c.Range.StartColumn, c.Range.EndColumn, c.Type))) + + +// ===================================================================== +// Issue #19905 - Item 2 (CE builder inside list/array comprehension +// painted as Value instead of ComputationExpression) +// ===================================================================== + +/// (#19905 item 2) Computation expression builder identifier used inside a list +/// comprehension must be classified as ComputationExpression on every +/// occurrence, not as Value/LocalValue. +[] +let ``19905 item 2 - CE inside list comp classified as ComputationExpression`` () = + let source = + """ +module Test +type OptionalBuilder() = + member _.Zero() = None + member _.Bind(x, f) = Option.bind f x + member _.Return(x) = Some x + member _.ReturnFrom(x) = x +let optional = OptionalBuilder() +let myList = [1; 2; 3] +let myNewList = [ + for i in myList do + optional { + return! Some(i+1) + } +] +""" + let items = getClassifications source + // Line 12: " optional {" + let ceLine = 12 + + let optionalClassifications = + items + |> Array.filter (fun c -> + c.Range.StartLine = ceLine + && substringOfRange source c.Range = "optional") + + Assert.True( + optionalClassifications + |> Array.exists (fun c -> c.Type = SemanticClassificationType.ComputationExpression), + sprintf "Expected `optional` on line %d to have a ComputationExpression classification, but got: %A" + ceLine (optionalClassifications |> Array.map (fun c -> c.Type))) + + // And ensure no Value/LocalValue paints over the same span (would still + // visually wash out the CE colour in VS). + Assert.True( + optionalClassifications + |> Array.forall (fun c -> + c.Type <> SemanticClassificationType.Value + && c.Type <> SemanticClassificationType.LocalValue), + sprintf "`optional` on line %d should not have a Value/LocalValue classification: %A" + ceLine (optionalClassifications |> Array.map (fun c -> c.Type))) + +/// (#19905 item 2 negative) Plain `async { .. }` outside a comprehension still +/// classifies as ComputationExpression. Regression guard: the generalised +/// `takeCustomBuilder` must not break the simple 2-CNR case it used to handle. +[] +let ``19905 item 2 negative - plain CE classification unchanged`` () = + let source = + """ +module Test +let _ = async { return 1 } +""" + let items = getClassifications source + Assert.True( + items + |> Array.exists (fun c -> + c.Range.StartLine = 3 + && substringOfRange source c.Range = "async" + && c.Type = SemanticClassificationType.ComputationExpression), + "`async` should still classify as ComputationExpression") + + + +// ===================================================================== +// Issue #19905 - Item 7 (open type incorrectly flagged unused when its +// imported members are referenced through the revealed-symbol path) +// ===================================================================== + +/// (#19905 item 7) `open type` whose imported member is referenced via the +/// revealed-symbol path must not be flagged unused. The Sprint 01 simple-case +/// pin covers the direct-symbol path; this test covers the revealed path that +/// requires walking symbolUses2 as well. +[] +let ``19905 item 7 - open type used via revealed symbol is not flagged unused`` () = + let source = + """ +module Test +module Inner = + type Helper = + static member val Counter = 0 with get, set + static member Greet (name: string) = sprintf "hi %s (count=%d)" name Helper.Counter +open type Inner.Helper +let _ = Greet "world" +let _ = Counter +""" + let fileName, snapshot, checker = singleFileChecker source + let results = checker.ParseAndCheckFileInProject(fileName, snapshot) |> Async.RunSynchronously + let checkResults = getTypeCheckResult results + let lines = source.Replace("\r\n", "\n").Split('\n') + let getSourceLineStr n = + if n >= 1 && n <= lines.Length then lines[n - 1] else "" + let unused = + UnusedOpens.getUnusedOpens(checkResults, getSourceLineStr) + |> Async.RunSynchronously + // 'open type Inner.Helper' is on line 7. + let unusedOnOpenLine = + unused + |> List.filter (fun r -> r.StartLine = 7) + Assert.True( + unusedOnOpenLine.IsEmpty, + sprintf "'open type Inner.Helper' (line 7) must not be flagged unused; got ranges: %A" + (unusedOnOpenLine |> List.map (fun r -> r.StartLine, r.StartColumn, r.EndColumn))) diff --git a/tests/FSharp.Compiler.Service.Tests/ProjectAnalysisTests.fs b/tests/FSharp.Compiler.Service.Tests/ProjectAnalysisTests.fs index 6b4f6d946f6..7fdf1d53041 100644 --- a/tests/FSharp.Compiler.Service.Tests/ProjectAnalysisTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/ProjectAnalysisTests.fs @@ -2086,7 +2086,7 @@ let ``Test Project11 all symbols`` () = ("int", "int", "file1", ((4, 57), (4, 60)), [], ["abbrev"]); ("Enumerator", "Enumerator", "file1", ((4, 62), (4, 72)), ["type"], ["valuetype"]); - ("member .ctor", "Enumerator", "file1", ((4, 15), (4, 72)), [], ["member"]); + ("member .ctor", "Enumerator", "file1", ((4, 62), (4, 72)), [], ["member"]); ("val enum", "enum", "file1", ((4, 4), (4, 8)), ["defn"], ["val"]); ("System", "System", "file1", ((5, 11), (5, 17)), [], ["namespace"]); ("Collections", "Collections", "file1", ((5, 18), (5, 29)), [], ["namespace"]); @@ -2798,8 +2798,7 @@ let ``Test Project17 all symbols`` () = ("FSharp", "FSharp", "file1", ((4, 18), (4, 24)), [], ["namespace"]); ("FSharpList`1", "List", "file1", ((4, 8), (4, 41)), [], ["union"]); ("int", "int", "file1", ((4, 42), (4, 45)), ["type"], ["abbrev"]); - ("FSharpList`1", "List", "file1", ((4, 8), (4, 46)), [], ["union"]); - ("property Empty", "Empty", "file1", ((4, 8), (4, 52)), [], ["member"; "prop"]); + ("property Empty", "Empty", "file1", ((4, 47), (4, 52)), [], ["member"; "prop"]); ("System", "System", "file1", ((6, 11), (6, 17)), [], ["namespace"]); ("Collections", "Collections", "file1", ((6, 18), (6, 29)), [], ["namespace"]); ("Generic", "Generic", "file1", ((6, 30), (6, 37)), [], ["namespace"]); @@ -2880,8 +2879,8 @@ let ``Test Project18 all symbols`` () = allUsesOfAllSymbols |> shouldEqual [|("list`1", "list", "file1", ((4, 8), (4, 12)), [], false); - ("list`1", "list", "file1", ((4, 8), (4, 15)), [], false); - ("property Empty", "Empty", "file1", ((4, 8), (4, 21)), [], false); + ("list`1", "list", "file1", ((4, 8), (4, 12)), [], false); + ("property Empty", "Empty", "file1", ((4, 16), (4, 21)), [], false); ("Impl", "Impl", "file1", ((2, 7), (2, 11)), ["defn"], false)|] diff --git a/tests/fsharp/typecheck/sigs/neg56_a.bsl b/tests/fsharp/typecheck/sigs/neg56_a.bsl index 7d477679f7b..696fbe5e7cb 100644 --- a/tests/fsharp/typecheck/sigs/neg56_a.bsl +++ b/tests/fsharp/typecheck/sigs/neg56_a.bsl @@ -1,4 +1,4 @@ neg56_a.fs(11,35,11,47): typecheck error FS1125: The instantiation of the generic type 'list1' is missing and can't be inferred from the arguments or return type of this member. Consider providing a type instantiation when accessing this type, e.g. 'list1<_>'. -neg56_a.fs(15,18,15,33): typecheck error FS3068: The function or member 'toList' is used in a way that requires further type annotations at its definition to ensure consistency of inferred types. The inferred signature is 'static member private list1.toList: ('a list1 -> 'a list)'. +neg56_a.fs(15,27,15,33): typecheck error FS3068: The function or member 'toList' is used in a way that requires further type annotations at its definition to ensure consistency of inferred types. The inferred signature is 'static member private list1.toList: ('a list1 -> 'a list)'.