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.

Sign up for GitHub

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

Exclude identifiers in misplaced attributes from identifier map #75100

Merged
merged 2 commits into from
Sep 17, 2024
Merged

Exclude identifiers in misplaced attributes from identifier map #75100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a3ad201
Exclude identifiers in attributes from identifier map
jcouv Sep 12, 2024
72803f5
Address feedback
jcouv Sep 13, 2024
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
11 changes: 11 additions & 0 deletions src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2010,6 +2010,8 @@ static void buildIdentifierMapOfBindIdentifierTargets(
}
}

// Logic in this lambda is based on Binder.IdentifierUsedAsValueFinder.CheckIdentifiersInNode.childrenNeedChecking.
// It can be more permissive (i.e. allow us to dive into more nodes), but should not be more restrictive
static void addIdentifiers(CSharpSyntaxNode? node, ConcurrentDictionary<IdentifierNameSyntax, int> identifierMap)
{
if (node is null)
Expand Down Expand Up @@ -2097,6 +2099,7 @@ static void assertBindIdentifierTargets(InMethodBinder? inMethodBinder, Concurre
{
inMethodBinder.IdentifierMap = null;

// In presence of errors, we're not guaranteed to have bound all identifiers, so we don't care about correctness of our prediction
if (!diagnostics.HasAnyResolvedErrors())
{
foreach (var (id, flags) in identifierMap)
Expand Down Expand Up @@ -2133,6 +2136,14 @@ static void assertBindIdentifierTargets(InMethodBinder? inMethodBinder, Concurre
{
continue;
}

// If an attribute is misplaced (invalid target), it is expected that identifiers within were not bound.
// In that case, we emit a warning and skip binding the attribute.
if (id.Ancestors(ascendOutOfTrivia: false).OfType<AttributeListSyntax>().Any() &&
diagnostics.DiagnosticBag!.AsEnumerable().Any(d => d.Code == (int)ErrorCode.WRN_AttributeLocationOnBadDeclaration))
{
continue;
}
}

Debug.Assert(false);
Expand Down
201 changes: 201 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10387,5 +10387,206 @@ int LocalFunc(string s) {}

Assert.Equal("System.Int32 LocalFunc(System.String s)", methodSymbol.ToTestDisplayString());
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/73905")]
public void IdentifierInNameofInAttributeOnLocalFunctionInAccessor()
{
var src = """
using System;

[AttributeUsage(AttributeTargets.All)]
class A : Attribute
{
public A(string s) { }
}

class C
{
event EventHandler E
{
add
{
[param: A(nameof(p))] void F(int p) { }
}
remove
{
}
}
}
""";
var comp = CreateCompilation(src);
comp.VerifyEmitDiagnostics(
// (15,14): warning CS0657: 'param' is not a valid attribute location for this declaration. Valid attribute locations for this declaration are 'method, return'. All attributes in this block will be ignored.
// [param: A(nameof(p))] void F(int p) { }
Diagnostic(ErrorCode.WRN_AttributeLocationOnBadDeclaration, "param").WithArguments("param", "method, return").WithLocation(15, 14),
// (15,40): warning CS8321: The local function 'F' is declared but never used
// [param: A(nameof(p))] void F(int p) { }
Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "F").WithArguments("F").WithLocation(15, 40));

var tree = comp.SyntaxTrees.Single();
var model = comp.GetSemanticModel(tree);
var nameof = GetSyntax<InvocationExpressionSyntax>(tree, "nameof(p)");
var p = nameof.ArgumentList.Arguments[0].Expression;
Assert.Equal("System.Int32", model.GetTypeInfo(p).Type.ToTestDisplayString());
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/73905")]
public void IdentifierInNameofInAttributeOnLocalFunctionInMethod()
{
var src = """
using System;

[AttributeUsage(AttributeTargets.All)]
class A : Attribute
{
public A(string s) { }
}

class C
{
void M()
{
[param: A(nameof(p))] void F(int p) { }
}
}
""";
var comp = CreateCompilation(src);
comp.VerifyEmitDiagnostics(
// (13,10): warning CS0657: 'param' is not a valid attribute location for this declaration. Valid attribute locations for this declaration are 'method, return'. All attributes in this block will be ignored.
// [param: A(nameof(p))] void F(int p) { }
Diagnostic(ErrorCode.WRN_AttributeLocationOnBadDeclaration, "param").WithArguments("param", "method, return").WithLocation(13, 10),
// (13,36): warning CS8321: The local function 'F' is declared but never used
// [param: A(nameof(p))] void F(int p) { }
Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "F").WithArguments("F").WithLocation(13, 36));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/73905")]
public void IdentifierInNameofInAttributeOnLocalFunctionInMethodWithParameter()
{
var src = """
using System;

[AttributeUsage(AttributeTargets.All)]
class A : Attribute
{
public A(string s) { }
}

class C
{
void M(int p)
{
[param: A(nameof(p))] void F(int p2) { }
}
}
""";
var comp = CreateCompilation(src);
comp.VerifyEmitDiagnostics(
// (13,10): warning CS0657: 'param' is not a valid attribute location for this declaration. Valid attribute locations for this declaration are 'method, return'. All attributes in this block will be ignored.
// [param: A(nameof(p))] void F(int p2) { }
Diagnostic(ErrorCode.WRN_AttributeLocationOnBadDeclaration, "param").WithArguments("param", "method, return").WithLocation(13, 10),
// (13,36): warning CS8321: The local function 'F' is declared but never used
// [param: A(nameof(p))] void F(int p2) { }
Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "F").WithArguments("F").WithLocation(13, 36));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/73905")]
public void IdentifierInNameofInParamAttributeOnLocalFunctionInPrimaryConstructorType()
{
var src = """
using System;

[AttributeUsage(AttributeTargets.All)]
class A : Attribute
{
public A(string s) { }
}

class C(int p)
{
void M()
{
[param: A(nameof(p))] void F(int p2) { }
}
}
""";
var comp = CreateCompilation(src);
comp.VerifyEmitDiagnostics(
// (9,13): warning CS9113: Parameter 'p' is unread.
// class C(int p)
Diagnostic(ErrorCode.WRN_UnreadPrimaryConstructorParameter, "p").WithArguments("p").WithLocation(9, 13),
// (13,10): warning CS0657: 'param' is not a valid attribute location for this declaration. Valid attribute locations for this declaration are 'method, return'. All attributes in this block will be ignored.
// [param: A(nameof(p))] void F(int p2) { }
Diagnostic(ErrorCode.WRN_AttributeLocationOnBadDeclaration, "param").WithArguments("param", "method, return").WithLocation(13, 10),
// (13,36): warning CS8321: The local function 'F' is declared but never used
// [param: A(nameof(p))] void F(int p2) { }
Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "F").WithArguments("F").WithLocation(13, 36));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/73905")]
public void IdentifierInNameofInAttributeOnLocalFunctionInPrimaryConstructorType()
{
var src = """
using System;

[AttributeUsage(AttributeTargets.All)]
class A : Attribute
{
public A(string s) { }
}

class C(int p)
{
void M()
{
[A(nameof(p))] void F() { }
}
}
""";
var comp = CreateCompilation(src);
comp.VerifyEmitDiagnostics(
// (9,13): warning CS9113: Parameter 'p' is unread.
// class C(int p)
Diagnostic(ErrorCode.WRN_UnreadPrimaryConstructorParameter, "p").WithArguments("p").WithLocation(9, 13),
// (13,29): warning CS8321: The local function 'F' is declared but never used
// [A(nameof(p))] void F() { }
Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "F").WithArguments("F").WithLocation(13, 29));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/73905")]
public void IdentifierInAttributeOnLocalFunctionInPrimaryConstructorType()
{
var src = """
using System;

[AttributeUsage(AttributeTargets.All)]
class A : Attribute
{
public A(string s) { }
}

class C(string p)
{
void M()
{
[A(p)] void F() { }
}
}
""";
var comp = CreateCompilation(src);
comp.VerifyEmitDiagnostics(
// (9,16): warning CS9113: Parameter 'p' is unread.
// class C(string p)
Diagnostic(ErrorCode.WRN_UnreadPrimaryConstructorParameter, "p").WithArguments("p").WithLocation(9, 16),
// (13,12): error CS9105: Cannot use primary constructor parameter 'string p' in this context.
// [A(p)] void F() { }
Diagnostic(ErrorCode.ERR_InvalidPrimaryConstructorParameterReference, "p").WithArguments("string p").WithLocation(13, 12),
// (13,12): error CS0182: An attribute argument must be a constant expression, typeof expression or array creation expression of an attribute parameter type
// [A(p)] void F() { }
Diagnostic(ErrorCode.ERR_BadAttributeArgument, "p").WithLocation(13, 12),
// (13,21): warning CS8321: The local function 'F' is declared but never used
// [A(p)] void F() { }
Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "F").WithArguments("F").WithLocation(13, 21));
}
}
}