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

Update analyzer to use FeatureGuardAttribute #99340

Merged
merged 4 commits into from
Mar 7, 2024

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Mar 6, 2024

This replaces FeatureCheckAttribute with FeatureGuardAttribute in the form that was approved in #96859 (comment).

Removes FeatureDependsOnAttribute and replaces it with FeatureGuardAttribute on the property. Since it is not allowed on types, feature requirements must be "bubbled up" to the guard property, so the tests are updated to do so.

@sbomer sbomer requested review from agocke, LakshanF and jtschuster March 6, 2024 00:49
@sbomer sbomer requested a review from marek-safar as a code owner March 6, 2024 00:49
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-Tools-ILLink .NET linker development as well as trimming analyzers label Mar 6, 2024
@ghost ghost assigned sbomer Mar 6, 2024
@ghost
Copy link

ghost commented Mar 6, 2024

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details

This replaces FeatureCheckAttribute with FeatureGuardAttribute in the form that was approved in #96859 (comment).

Removes FeatureDependsOnAttribute and replaces it with FeatureGuardAttribute on the property. Since it is not allowed on types, feature requirements must be "bubbled up" to the guard property, so the tests are updated to do so.

Author: sbomer
Assignees: -
Labels:

area-Tools-ILLink

Milestone: -

@stephentoub
Copy link
Member

What's the plan for generalizing the analyzer? Presumably with these new attributes being added to the core libraries, we'll be shipping an analyzer for them that's not tied to illink?

@sbomer
Copy link
Member Author

sbomer commented Mar 7, 2024

In the API review we decided that there aren't enough use cases for a generalized analyzer yet (that's why RequiresFeatureAttribute wasn't part of the approved API). So for now FeatureGuard will only be used by the illink analyzer to guard RequiresUnreferencedCode, RequiresDynamicCode, or RequiresAssemblyFiles. Other analyzers could also decide to understand it as a guard for analyzer-specific warnings.

Copy link
Member

@jtschuster jtschuster left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@sbomer sbomer merged commit 33cdab9 into dotnet:main Mar 7, 2024
74 of 76 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants