Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix inaccessible protected constructors being shown in sig help #75642

Merged
merged 3 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 ',';

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading