Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -447,6 +447,124 @@ await VerifyExpectedItemsAsync(code, [
]);
}

[Fact]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot use [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/7640")] on all tests you add

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/7640")] to all 22 attribute filtering tests. (commit bd67526)

public async Task AttributeTargetFiltering_AssemblyAttribute()
{
var code = """
[assembly: $$]

namespace TestNamespace
{
class Program
{
static void Main(string[] args)
{
}
}
}

[System.AttributeUsage(System.AttributeTargets.Assembly)]
public class AssemblyOnlyAttribute : System.Attribute
{
}

[System.AttributeUsage(System.AttributeTargets.Class)]
public class ClassOnlyAttribute : System.Attribute
{
}
""";

await VerifyExpectedItemsAsync(code, [
ItemExpectation.Exists("AssemblyOnly"),
ItemExpectation.Absent("ClassOnly")
]);
}

[Fact]
public async Task AttributeTargetFiltering_ClassAttribute()
{
var code = """
[$$]
class TestClass
{
}

[System.AttributeUsage(System.AttributeTargets.Assembly)]
public class AssemblyOnlyAttribute : System.Attribute
{
}

[System.AttributeUsage(System.AttributeTargets.Class)]
public class ClassOnlyAttribute : System.Attribute
{
}
""";

await VerifyExpectedItemsAsync(code, [
ItemExpectation.Absent("AssemblyOnly"),
ItemExpectation.Exists("ClassOnly")
]);
}

[Fact]
public async Task AttributeTargetFiltering_MethodAttribute()
{
var code = """
class TestClass
{
[$$]
void TestMethod()
{
}
}

[System.AttributeUsage(System.AttributeTargets.Method)]
public class MethodOnlyAttribute : System.Attribute
{
}

[System.AttributeUsage(System.AttributeTargets.Property)]
public class PropertyOnlyAttribute : System.Attribute
{
}
""";

await VerifyExpectedItemsAsync(code, [
ItemExpectation.Exists("MethodOnly"),
ItemExpectation.Absent("PropertyOnly")
]);
}

[Fact]
public async Task AttributeTargetFiltering_ExplicitTargetSpecifier()
{
var code = """
class TestClass
{
[return: $$]
int TestMethod()
{
return 0;
}
}

[System.AttributeUsage(System.AttributeTargets.ReturnValue)]
public class ReturnValueOnlyAttribute : System.Attribute
{
}

[System.AttributeUsage(System.AttributeTargets.Method)]
public class MethodOnlyAttribute : System.Attribute
{
}
""";

await VerifyExpectedItemsAsync(code, [
ItemExpectation.Exists("ReturnValueOnly"),
ItemExpectation.Absent("MethodOnly")
]);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot have tests for all the possible nodes that you compute different AttributeTargets for. Also, have tests for all the possible target identifiers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comprehensive tests for all syntax node types (struct, interface, enum, delegate, record class, record struct, constructor, property, field, event, parameter, type parameter, indexer) and all target specifiers (assembly, module, type, method, field, property, event, param, return, typevar). Total of 22 new tests added. (commit bd67526)


[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/7213")]
public Task NamespaceName_EmptyNameSpan_TopLevel()
=> VerifyItemExistsAsync(@"namespace $$ { }", "System", sourceCodeKind: SourceCodeKind.Regular);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ internal bool ShouldIncludeSymbol(ISymbol symbol)
return symbol.IsOrContainsAccessibleAttribute(
_context.SemanticModel.GetEnclosingNamedType(_context.LeftToken.SpanStart, _cancellationToken)!,
_context.SemanticModel.Compilation.Assembly,
_context.ValidAttributeTargets,
_cancellationToken);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,10 @@ public static bool IsStaticType([NotNullWhen(true)] this ISymbol? symbol)

public static bool IsOrContainsAccessibleAttribute(
[NotNullWhen(true)] this ISymbol? symbol, ISymbol withinType, IAssemblySymbol withinAssembly, CancellationToken cancellationToken)
=> IsOrContainsAccessibleAttribute(symbol, withinType, withinAssembly, AttributeTargets.All, cancellationToken);

public static bool IsOrContainsAccessibleAttribute(
[NotNullWhen(true)] this ISymbol? symbol, ISymbol withinType, IAssemblySymbol withinAssembly, AttributeTargets validTargets, CancellationToken cancellationToken)
{
var namespaceOrType = symbol is IAliasSymbol alias ? alias.Target : symbol as INamespaceOrTypeSymbol;
if (namespaceOrType == null)
Expand All @@ -548,13 +552,49 @@ public static bool IsOrContainsAccessibleAttribute(
{
if (type.IsAttribute() && type.IsAccessibleWithin(withinType ?? withinAssembly))
{
// Check if the attribute is valid for the specified targets
if (validTargets != AttributeTargets.All && !IsAttributeValidForTargets(type, validTargets))
{
continue;
}

return true;
}
}

return false;
}

private static bool IsAttributeValidForTargets(INamedTypeSymbol attributeType, AttributeTargets validTargets)
{
// Get the AttributeUsageAttribute applied to this attribute type
var attributeUsageAttribute = attributeType.GetAttributes()
.FirstOrDefault(attr => attr.AttributeClass?.Name == "AttributeUsageAttribute" &&
attr.AttributeClass.ContainingNamespace?.Name == "System" &&
attr.AttributeClass.ContainingNamespace.ContainingNamespace?.IsGlobalNamespace == true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot use a pattern to simplify the name/containingnamespace/etc. check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplified using nested property pattern: attr.AttributeClass is { Name: "AttributeUsageAttribute", ContainingNamespace: { Name: "System", ContainingNamespace.IsGlobalNamespace: true } } (commit bd67526)


if (attributeUsageAttribute == null)
{
// If no AttributeUsage is specified, the default is AttributeTargets.All
return true;
}

// The first constructor argument is the AttributeTargets value
if (attributeUsageAttribute.ConstructorArguments.Length > 0)
{
var constructorArg = attributeUsageAttribute.ConstructorArguments[0];
if (constructorArg.Value is int targetsValue)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot replacce with something like if (attributeUsageAttribute.ConstructorArguments is [{ Value: int targetsValue }, ..]) to simplify things.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced with collection pattern: if (attributeUsageAttribute.ConstructorArguments is [{ Value: int targetsValue }, ..]) (commit bd67526)

{
var attributeTargets = (AttributeTargets)targetsValue;
// Check if there's any overlap between the attribute's targets and the valid targets
return (attributeTargets & validTargets) != 0;
}
}

// Default to allowing the attribute if we can't determine the targets
return true;
}

public static IEnumerable<IPropertySymbol> GetValidAnonymousTypeProperties(this ISymbol symbol)
{
Contract.ThrowIfFalse(symbol.IsNormalAnonymousType());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Threading;
Expand Down Expand Up @@ -177,6 +178,8 @@
this.PrecedingModifiers = precedingModifiers;
}

public override AttributeTargets ValidAttributeTargets => ComputeValidAttributeTargets();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot this looks expensive to compute. Can you store this in a AttributeTargets ? in teh class, and compute and cache the value when first requested.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added lazy caching using a nullable AttributeTargets? field. The value is computed only once per CSharpSyntaxContext instance on first access. (commit a95e595)


public static CSharpSyntaxContext CreateContext(Document document, SemanticModel semanticModel, int position, CancellationToken cancellationToken)
=> CreateContextWorker(document, semanticModel, position, cancellationToken);

Expand Down Expand Up @@ -512,4 +515,75 @@

return false;
}

private AttributeTargets ComputeValidAttributeTargets()
{
// If we're not in an attribute context, return All to allow all attributes
if (!IsAttributeNameContext)
return AttributeTargets.All;

// Find the attribute list that contains the current position
var token = TargetToken;
var attributeList = token.Parent?.FirstAncestorOrSelf<AttributeListSyntax>();
if (attributeList == null)
return AttributeTargets.All;

// Check if there's an explicit target specifier (e.g., "assembly:", "return:", etc.)
if (attributeList.Target != null)
{
var targetIdentifier = attributeList.Target.Identifier.ValueText;
return targetIdentifier switch
{
"assembly" or "module" => AttributeTargets.Assembly | AttributeTargets.Module,
"type" => AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Enum | AttributeTargets.Interface | AttributeTargets.Delegate,
"method" => AttributeTargets.Method,
"field" => AttributeTargets.Field,
"property" => AttributeTargets.Property,
"event" => AttributeTargets.Event,
"param" => AttributeTargets.Parameter,
"return" => AttributeTargets.ReturnValue,
"typevar" => AttributeTargets.GenericParameter,
_ => AttributeTargets.All
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot you should be able to use SyntaxKind here. Like SyntaxKind.TypeVarKeyword.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to use SyntaxKind enums instead of string literals. Using attributeList.Target.Identifier.Kind() with values like SyntaxKind.AssemblyKeyword, SyntaxKind.TypeVarKeyword, etc. (commit bd67526)

};
}

// No explicit target, determine from context
// Walk up to find what the attribute is attached to
var parentNode = attributeList.Parent;
if (parentNode == null)
return AttributeTargets.All;

return parentNode switch
{
// Type declarations
ClassDeclarationSyntax => AttributeTargets.Class,
StructDeclarationSyntax => AttributeTargets.Struct,
InterfaceDeclarationSyntax => AttributeTargets.Interface,
EnumDeclarationSyntax => AttributeTargets.Enum,
DelegateDeclarationSyntax => AttributeTargets.Delegate,
RecordDeclarationSyntax record => record.ClassOrStructKeyword.IsKind(SyntaxKind.StructKeyword)

Check failure on line 564 in src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/CSharpSyntaxContext.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/CSharpSyntaxContext.cs#L564

src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/CSharpSyntaxContext.cs(564,107): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)

Check failure on line 564 in src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/CSharpSyntaxContext.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/CSharpSyntaxContext.cs#L564

src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/CSharpSyntaxContext.cs(564,107): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)

Check failure on line 564 in src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/CSharpSyntaxContext.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/CSharpSyntaxContext.cs#L564

src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/CSharpSyntaxContext.cs(564,107): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)
? AttributeTargets.Struct

Check failure on line 565 in src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/CSharpSyntaxContext.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/CSharpSyntaxContext.cs#L565

src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/CSharpSyntaxContext.cs(565,42): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)

Check failure on line 565 in src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/CSharpSyntaxContext.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/CSharpSyntaxContext.cs#L565

src/Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ContextQuery/CSharpSyntaxContext.cs(565,42): error IDE0055: (NETCORE_ENGINEERING_TELEMETRY=Build) Fix formatting (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/style-rules/ide0055)
: AttributeTargets.Class,

// Member declarations
MethodDeclarationSyntax => AttributeTargets.Method,
ConstructorDeclarationSyntax => AttributeTargets.Constructor,
PropertyDeclarationSyntax => AttributeTargets.Property,
EventDeclarationSyntax => AttributeTargets.Event,
EventFieldDeclarationSyntax => AttributeTargets.Event,
FieldDeclarationSyntax => AttributeTargets.Field,
IndexerDeclarationSyntax => AttributeTargets.Property,

// Parameters
ParameterSyntax => AttributeTargets.Parameter,

// Type parameters
TypeParameterSyntax => AttributeTargets.GenericParameter,

// Assembly/module level
CompilationUnitSyntax => AttributeTargets.Assembly | AttributeTargets.Module,

_ => AttributeTargets.All
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Immutable;
using System.Threading;
using Microsoft.CodeAnalysis.Host;
Expand Down Expand Up @@ -96,6 +97,13 @@ internal abstract class SyntaxContext(

public ImmutableArray<ITypeSymbol> InferredTypes { get; } = document.GetRequiredLanguageService<ITypeInferenceService>().InferTypes(semanticModel, position, cancellationToken);

/// <summary>
/// Gets the valid attribute targets for the current attribute context.
/// Returns <see cref="AttributeTargets.All"/> if not in an attribute context or if language-specific
/// filtering is not implemented.
/// </summary>
public virtual AttributeTargets ValidAttributeTargets => AttributeTargets.All;

public TService? GetLanguageService<TService>() where TService : class, ILanguageService
=> Document.GetLanguageService<TService>();

Expand Down
Loading