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

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

@@ -17,6 +17,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;
+    }
 }