Fix completion inconsistently filtering obsolete fields and events (#13512) (#19506)

* Fix completion inconsistently filtering obsolete fields and events

Add ILFieldInfoIsUnseen and EventInfoIsUnseen functions to filter obsolete
IL fields and events from completion, matching existing behavior for methods
and properties. Also update ItemIsUnseen to handle ILField and Event items.

Fixes #13693

Agent-Logs-Url: https://github.com/dotnet/fsharp/sessions/1d21d452-3f55-4d56-898c-0b50980050b5

Co-authored-by: abonie <20281641+abonie@users.noreply.github.com>

* Add C# interop tests for obsolete field/event/method/property filtering

Fix issue number to #13512. Add ObsoleteMembersClass to CSharp_Analysis
with obsolete and non-obsolete members. Add 6 C# interop completion tests
verifying all obsolete member types are consistently hidden.

Agent-Logs-Url: https://github.com/dotnet/fsharp/sessions/1d21d452-3f55-4d56-898c-0b50980050b5

Co-authored-by: abonie <20281641+abonie@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: abonie <20281641+abonie@users.noreply.github.com>

Author: Copilot

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

@@ -18,6 +18,7 @@
 * Fix `YieldFromFinal`/`ReturnFromFinal` being incorrectly called in non-tail positions (`for`, `use`, `use!`, `try/with` handler). ([Issue #19402](https://github.com/dotnet/fsharp/issues/19402), [PR #19403](https://github.com/dotnet/fsharp/pull/19403))
 * Fixed how the source ranges of warn directives are reported (as trivia) in the parser output (by not reporting leading spaces). ([Issue #19405](https://github.com/dotnet/fsharp/issues/19405), [PR #19408]((https://github.com/dotnet/fsharp/pull/19408)))
 * Fix UoM value type `ToString()` returning garbage values when `--checknulls+` is enabled, caused by double address-taking in codegen. ([Issue #19435](https://github.com/dotnet/fsharp/issues/19435), [PR #19440](https://github.com/dotnet/fsharp/pull/19440))
+* Fix completion inconsistently showing some obsolete members (fields and events) while hiding others (methods and properties). All obsolete members are now consistently hidden by default. ([Issue #13512](https://github.com/dotnet/fsharp/issues/13512), [PR #19506](https://github.com/dotnet/fsharp/pull/19506))
 
 ### Added
 

src/Compiler/Checking/AttributeChecking.fs

@@ -639,6 +639,27 @@ let PropInfoIsUnseen _m allowObsolete pinfo =
         CheckProvidedAttributesForUnseen (pi.PApply((fun st -> (st :> IProvidedCustomAttributeProvider)), m)) m
 #endif
 
+/// Indicate if an ILFieldInfo has 'Obsolete' attribute.
+/// Used to suppress the item in intellisense.
+let ILFieldInfoIsUnseen (finfo: ILFieldInfo) =
+    match finfo with
+    | ILFieldInfo(_, fdef) -> CheckILAttributesForUnseen fdef.CustomAttrs
+#if !NO_TYPEPROVIDERS
+    | ProvidedField(_amap, fi, m) ->
+        CheckProvidedAttributesForUnseen (fi.PApply((fun st -> (st :> IProvidedCustomAttributeProvider)), m)) m
+#endif
+
+/// Indicate if an EventInfo has 'Obsolete' or 'CompilerMessageAttribute'.
+/// Used to suppress the item in intellisense.
+let EventInfoIsUnseen allowObsolete (einfo: EventInfo) =
+    match einfo with
+    | ILEvent(ILEventInfo(_, ilEventDef)) -> CheckILAttributesForUnseen ilEventDef.CustomAttrs
+    | FSEvent(g, _, addValRef, _) -> CheckFSharpAttributesForUnseen g addValRef.Attribs allowObsolete
+#if !NO_TYPEPROVIDERS
+    | ProvidedEvent(_amap, ei, m) ->
+        CheckProvidedAttributesForUnseen (ei.PApply((fun st -> (st :> IProvidedCustomAttributeProvider)), m)) m
+#endif
+
 /// Check the attributes on a union case, returning errors and warnings as data.
 let CheckUnionCaseAttributes g (x:UnionCaseRef) m =
     trackErrors {

src/Compiler/Checking/AttributeChecking.fsi

@@ -101,6 +101,10 @@ val MethInfoIsUnseen: g: TcGlobals -> m: range -> ty: TType -> minfo: MethInfo -
 
 val PropInfoIsUnseen: _m: 'a -> allowObsolete: bool -> pinfo: PropInfo -> bool
 
+val ILFieldInfoIsUnseen: finfo: ILFieldInfo -> bool
+
+val EventInfoIsUnseen: allowObsolete: bool -> einfo: EventInfo -> bool
+
 val CheckEntityAttributes: g: TcGlobals -> tcref: TyconRef -> m: range -> OperationResult<unit>
 
 val CheckUnionCaseAttributes: g: TcGlobals -> x: UnionCaseRef -> m: range -> OperationResult<unit>

src/Compiler/Checking/NameResolution.fs

@@ -4393,6 +4393,8 @@ let ItemIsUnseen ad g amap m allowObsolete item =
         isUnseenNameOfOperator || IsValUnseen ad g m allowObsolete x
     | Item.UnionCase(x, _) -> IsUnionCaseUnseen ad g amap m allowObsolete x.UnionCaseRef
     | Item.ExnCase x -> IsTyconUnseen ad g amap m allowObsolete x
+    | Item.ILField finfo -> not allowObsolete && ILFieldInfoIsUnseen finfo
+    | Item.Event einfo -> not allowObsolete && EventInfoIsUnseen allowObsolete einfo
     | _ -> false
 
 let ItemOfTyconRef ncenv m (x: TyconRef) =
@@ -4467,7 +4469,8 @@ let ResolveCompletionsInType (ncenv: NameResolver) nenv (completionTargets: Reso
             ncenv.InfoReader.GetEventInfosOfType(None, ad, m, ty)
             |> List.filter (fun x ->
                 IsStandardEventInfo ncenv.InfoReader m ad x &&
-                x.IsStatic = statics)
+                x.IsStatic = statics &&
+                (allowObsolete || not (EventInfoIsUnseen allowObsolete x)))
         else []
 
     let nestedTypes =
@@ -4482,7 +4485,8 @@ let ResolveCompletionsInType (ncenv: NameResolver) nenv (completionTargets: Reso
         |> List.filter (fun x ->
             not x.IsSpecialName &&
             x.IsStatic = statics &&
-            IsILFieldInfoAccessible g amap m ad x)
+            IsILFieldInfoAccessible g amap m ad x &&
+            (allowObsolete || not (ILFieldInfoIsUnseen x)))
 
     let qinfos =
         ncenv.InfoReader.GetTraitInfosInType None ty

tests/FSharp.Compiler.Service.Tests/Checker.fs

@@ -162,6 +162,11 @@ module Checker =
         let parseResults, checkResults = getParseAndCheckResults context.Source
         checkResults.GetCodeCompletionSuggestions(context, parseResults, options)
 
+    let getCompletionInfoWithCompilerAndCompletionOptions (compilerOptions: string array) (completionOptions: FSharpCodeCompletionOptions) (markedSource: string) =
+        let context = getCompletionContext markedSource
+        let parseResults, checkResults = getParseAndCheckResultsWithOptions compilerOptions context.Source
+        checkResults.GetCodeCompletionSuggestions(context, parseResults, completionOptions)
+
     let getCompletionInfo markedSource =
         getCompletionInfoWithOptions FSharpCodeCompletionOptions.Default markedSource
 

tests/FSharp.Compiler.Service.Tests/CompletionTests.fs

@@ -691,6 +691,86 @@ exception E
 try () with E{caret}
 """
 
+        // https://github.com/dotnet/fsharp/issues/13512
+        [<Fact>]
+        let ``Event - Instance 01`` () =
+            assertItem "Ev" """
+type T() =
+    [<System.Obsolete; CLIEvent>]
+    member _.Ev = Event<System.EventHandler, _>().Publish
+
+T().{caret}
+"""
+
+        // https://github.com/dotnet/fsharp/issues/13512
+        [<Fact>]
+        let ``Event - Static 01`` () =
+            assertItem "Ev" """
+type T() =
+    [<System.Obsolete; CLIEvent>]
+    static member Ev = Event<System.EventHandler, _>().Publish
+
+T.{caret}
+"""
+
+        /// Helper to assert completion with a reference to the CSharp_Analysis assembly
+        let private assertCSharpInteropItem name source =
+            let csharpAssembly = PathRelativeToTestAssembly "CSharp_Analysis.dll"
+            let compilerOptions = [| $"-r:{csharpAssembly}" |]
+            [allowObsoleteOptions; disallowObsoleteOptions]
+            |> List.iter (fun completionOptions ->
+                let contains = completionOptions.SuggestObsoleteSymbols
+                let info = Checker.getCompletionInfoWithCompilerAndCompletionOptions compilerOptions completionOptions source
+                assertItemsWithNames contains [name] info
+            )
+
+        // https://github.com/dotnet/fsharp/issues/13512
+        [<Fact>]
+        let ``CSharp - Obsolete field is hidden`` () =
+            assertCSharpInteropItem "ObsoleteField" """
+open FSharp.Compiler.Service.Tests
+ObsoleteMembersClass.{caret}
+"""
+
+        // https://github.com/dotnet/fsharp/issues/13512
+        [<Fact>]
+        let ``CSharp - Obsolete method is hidden`` () =
+            assertCSharpInteropItem "ObsoleteMethod" """
+open FSharp.Compiler.Service.Tests
+ObsoleteMembersClass.{caret}
+"""
+
+        // https://github.com/dotnet/fsharp/issues/13512
+        [<Fact>]
+        let ``CSharp - Obsolete property is hidden`` () =
+            assertCSharpInteropItem "ObsoleteProperty" """
+open FSharp.Compiler.Service.Tests
+ObsoleteMembersClass.{caret}
+"""
+
+        // https://github.com/dotnet/fsharp/issues/13512
+        [<Fact>]
+        let ``CSharp - Obsolete event is hidden`` () =
+            assertCSharpInteropItem "ObsoleteEvent" """
+open FSharp.Compiler.Service.Tests
+ObsoleteMembersClass.{caret}
+"""
+
+        // https://github.com/dotnet/fsharp/issues/13512
+        [<Fact>]
+        let ``CSharp - Non-obsolete members are always shown`` () =
+            let csharpAssembly = PathRelativeToTestAssembly "CSharp_Analysis.dll"
+            let compilerOptions = [| $"-r:{csharpAssembly}" |]
+            let source = """
+open FSharp.Compiler.Service.Tests
+ObsoleteMembersClass.{caret}
+"""
+            [allowObsoleteOptions; disallowObsoleteOptions]
+            |> List.iter (fun completionOptions ->
+                let info = Checker.getCompletionInfoWithCompilerAndCompletionOptions compilerOptions completionOptions source
+                assertItemsWithNames true ["NonObsoleteField"; "NonObsoleteMethod"; "NonObsoleteProperty"; "NonObsoleteEvent"] info
+            )
+
 
     module PatternNameSuggestions =
         let private suggestPatternNames = { FSharpCodeCompletionOptions.Default with SuggestPatternNames = true }

tests/service/data/CSharp_Analysis/CSharpClass.cs

@@ -155,4 +155,25 @@ public class DummyClass
         {
         }
     }
+
+    /// <summary>
+    /// Class with obsolete members for testing completion filtering (issue #13512).
+    /// </summary>
+    public class ObsoleteMembersClass
+    {
+        [Obsolete("Field is obsolete")] public static readonly int ObsoleteField = 1;
+
+        [Obsolete("Method is obsolete")]
+        public static void ObsoleteMethod()
+        {
+        }
+
+        [Obsolete("Property is obsolete")] public static int ObsoleteProperty => 1;
+        [Obsolete("Event is obsolete")] public static event EventHandler ObsoleteEvent;
+
+        public static readonly int NonObsoleteField = 2;
+        public static void NonObsoleteMethod() { }
+        public static int NonObsoleteProperty => 2;
+        public static event EventHandler NonObsoleteEvent;
+    }
 }