Merge pull request #75642 from CyrusNajmabadi/protectedSigHelp

src/EditorFeatures/CSharpTest/SignatureHelp/ObjectCreationExpressionSignatureHelpProviderTests.cs

@@ -921,11 +921,80 @@ void M()
             }
             """;
 
-        var expectedOrderedItems = new List<SignatureHelpTestItem>
-        {
-            new SignatureHelpTestItem($"Program(string s, string s2)", currentParameterIndex: expectedParameterIndex, isSelected: true),
-        };
+        await TestAsync(markup.Replace("ARGUMENTS", arguments),
+            [new($"Program(string s, string s2)", currentParameterIndex: expectedParameterIndex, isSelected: true)]);
+    }
 
-        await TestAsync(markup.Replace("ARGUMENTS", arguments), expectedOrderedItems);
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70235")]
+    public async Task ProtectedConstructor1()
+    {
+        var markup = """
+            public class Derived:BaseClass
+            {
+                public void Do()
+                {
+                    var baseInstance = new BaseClass($$);
+                }
+            }
+
+            public class BaseClass
+            {
+                public BaseClass(int val) { }
+                protected BaseClass(int val, int val1) { } 
+            }
+            """;
+
+        await TestAsync(markup,
+            [new SignatureHelpTestItem("BaseClass(int val)", currentParameterIndex: 0)],
+            usePreviousCharAsTrigger: true);
+    }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70235")]
+    public async Task ProtectedConstructor2()
+    {
+        var markup = """
+            public class BaseClass
+            {
+                public BaseClass(int val) { }
+                protected BaseClass(int val, int val1) { } 
+
+                public class Nested
+                {
+                    public void Do()
+                    {
+                        var baseInstance = new BaseClass($$);
+                    }
+                }
+            }
+            """;
+
+        await TestAsync(markup, [
+                new SignatureHelpTestItem("BaseClass(int val)", currentParameterIndex: 0),
+                new SignatureHelpTestItem("BaseClass(int val, int val1)", currentParameterIndex: 0),
+            ],
+            usePreviousCharAsTrigger: true);
+    }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70235")]
+    public async Task ProtectedConstructor3()
+    {
+        var markup = """
+            public class BaseClass
+            {
+                public BaseClass(int val) { }
+                protected BaseClass(int val, int val1) { } 
+
+                public void Do()
+                {
+                    var baseInstance = new BaseClass($$);
+                }
+            }
+            """;
+
+        await TestAsync(markup, [
+                new SignatureHelpTestItem("BaseClass(int val)", currentParameterIndex: 0),
+                new SignatureHelpTestItem("BaseClass(int val, int val1)", currentParameterIndex: 0),
+            ],
+            usePreviousCharAsTrigger: true);
     }
 }

src/Features/CSharp/Portable/SignatureHelp/AbstractCSharpSignatureHelpProvider.cs

@@ -15,10 +15,6 @@ internal abstract partial class AbstractCSharpSignatureHelpProvider : AbstractSi
     private static readonly SymbolDisplayFormat s_allowDefaultLiteralFormat = SymbolDisplayFormat.MinimallyQualifiedFormat
         .AddMiscellaneousOptions(SymbolDisplayMiscellaneousOptions.AllowDefaultLiteral);
 
-    protected AbstractCSharpSignatureHelpProvider()
-    {
-    }
-
     protected static SymbolDisplayPart Keyword(SyntaxKind kind)
         => new SymbolDisplayPart(SymbolDisplayPartKind.Keyword, null, SyntaxFacts.GetText(kind));
 

src/Features/CSharp/Portable/SignatureHelp/ObjectCreationExpressionSignatureHelpProvider.cs

@@ -5,7 +5,6 @@
 using System;
 using System.Composition;
 using System.Diagnostics;
-using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -19,14 +18,10 @@
 namespace Microsoft.CodeAnalysis.CSharp.SignatureHelp;
 
 [ExportSignatureHelpProvider("ObjectCreationExpressionSignatureHelpProvider", LanguageNames.CSharp), Shared]
-internal sealed partial class ObjectCreationExpressionSignatureHelpProvider : AbstractCSharpSignatureHelpProvider
+[method: ImportingConstructor]
+[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
+internal sealed partial class ObjectCreationExpressionSignatureHelpProvider() : AbstractCSharpSignatureHelpProvider
 {
-    [ImportingConstructor]
-    [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
-    public ObjectCreationExpressionSignatureHelpProvider()
-    {
-    }
-
     public override bool IsTriggerCharacter(char ch)
         => ch is '(' or ',';
 
@@ -80,17 +75,18 @@ private static bool IsArgumentListToken(BaseObjectCreationExpressionSyntax expre
         if (within == null)
             return null;
 
-        var symbolDisplayService = document.GetLanguageService<ISymbolDisplayService>();
         if (type.TypeKind == TypeKind.Delegate)
             return await GetItemsWorkerForDelegateAsync(document, position, objectCreationExpression, type, cancellationToken).ConfigureAwait(false);
 
-        // get the candidate methods
+        // Get the candidate methods.  Consider the constructor's containing type to be the "through type" instance
+        // (which matches the compiler's logic in Binder.IsConstructorAccessible), to ensure that we do not see
+        // protected constructors in derived types (but continue to see them in nested types).
         var methods = type.InstanceConstructors
-            .WhereAsArray(c => c.IsAccessibleWithin(within))
+            .WhereAsArray(c => c.IsAccessibleWithin(within: within, throughType: c.ContainingType))
             .WhereAsArray(s => s.IsEditorBrowsable(options.HideAdvancedMembers, semanticModel.Compilation))
             .Sort(semanticModel, objectCreationExpression.SpanStart);
 
-        if (!methods.Any())
+        if (methods.IsEmpty)
             return null;
 
         // guess the best candidate if needed and determine parameter index

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/ISymbolExtensions_Accessibility.cs

@@ -150,22 +150,16 @@ private static bool IsSymbolAccessibleCore(
                 }
 
                 // If this is a synthesized operator of dynamic, it's always accessible.
-                if (symbol.IsKind(SymbolKind.Method) &&
-                    ((IMethodSymbol)symbol).MethodKind == MethodKind.BuiltinOperator &&
-                    symbol.ContainingSymbol.IsKind(SymbolKind.DynamicType))
+                if (symbol is IMethodSymbol { MethodKind: MethodKind.BuiltinOperator })
                 {
-                    return true;
-                }
+                    if (symbol.ContainingSymbol.IsKind(SymbolKind.DynamicType))
+                        return true;
 
-                // If it's a synthesized operator on a pointer, use the pointer's PointedAtType.
-                // Note: there are currently no synthesized operators on function pointer types. If that
-                // ever changes, updated the below assert and fix the code
-                Debug.Assert(!(symbol.IsKind(SymbolKind.Method) && ((IMethodSymbol)symbol).MethodKind == MethodKind.BuiltinOperator && symbol.ContainingSymbol.IsKind(SymbolKind.FunctionPointerType)));
-                if (symbol.IsKind(SymbolKind.Method) &&
-                    ((IMethodSymbol)symbol).MethodKind == MethodKind.BuiltinOperator &&
-                    symbol.ContainingSymbol.IsKind(SymbolKind.PointerType))
-                {
-                    return IsSymbolAccessibleCore(((IPointerTypeSymbol)symbol.ContainingSymbol).PointedAtType, within, null, out failedThroughTypeCheck);
+                    // If it's a synthesized operator on a pointer, use the pointer's PointedAtType.
+                    // Note: there are currently no synthesized operators on function pointer types. If that
+                    // ever changes, updated the below assert and fix the code
+                    if (symbol.ContainingSymbol is IPointerTypeSymbol pointerType)
+                        return IsSymbolAccessibleCore(pointerType.PointedAtType, within, null, out failedThroughTypeCheck);
                 }
 
                 return IsMemberAccessible(symbol.ContainingType, symbol.DeclaredAccessibility, within, throughType, out failedThroughTypeCheck);