Skip to content

[ILLink analyzer] Add analyzer support for feature checks #94944

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

Jump to bottom
Merged
merged 11 commits into from
Feb 29, 2024
Merged

[ILLink analyzer] Add analyzer support for feature checks #94944

merged 11 commits into from
Feb 29, 2024

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Nov 18, 2023 •
edited
Loading

Adds support for annotating static boolean properties with [FeatureCheckAttribute(typeof(RequiresDynamicCodeAttribute))], causing the property to be treated as a guard for analyzer warnings about the corresponding Requires attribute.

Adds two new warnings for:

  • Invalid use of FeatureCheckAttribute applied to a non-static or non-bool property
  • Implementation of the property doesn't obviously satisfy the "guard property" (it should return false whenever the guarded feature is disabled).

See #94625 for notes on the design.
See #96859 for the API proposal.

@sbomer sbomer requested a review from vitek-karas November 18, 2023 00:54
@sbomer sbomer requested a review from marek-safar as a code owner November 18, 2023 00:54
@ghost ghost added linkable-framework Issues associated with delivering a linker friendly framework area-Tools-ILLink .NET linker development as well as trimming analyzers labels Nov 18, 2023
@ghost ghost assigned sbomer Nov 18, 2023
@ghost
Copy link

ghost commented Nov 18, 2023

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr, @marek-safar
See info in area-owners.md if you want to be subscribed.

Issue Details

Adds support for annotating static boolean properties with [FeatureGuardAttribute(typeof(RequiresDynamicCodeAttribute))], causing the property to be treated as a guard for analyzer warnings about the corresponding Requires attribute.

Adds two new warnings for:

  • Invalid use of FeatureGuardAttribute applied to a non-static or non-bool property
  • Implementation of the property doesn't obviously satisfy the "guard property" (it should return false whenever the guarded feature is disabled).

See #94625 for notes on the design.

Author: sbomer
Assignees: -
Labels:

linkable-framework
Milestone: -

@build-analysis build-analysis bot mentioned this pull request Feb 24, 2024
Closed
@sbomer sbomer requested a review from jtschuster February 27, 2024 18:13
@sbomer sbomer changed the title [ILLink analyzer] Add analyzer support for feature guards [ILLink analyzer] Add analyzer support for feature checks Feb 27, 2024
@sbomer sbomer requested a review from agocke February 27, 2024 18:14
@sbomer sbomer requested a review from LakshanF February 27, 2024 19:41
This was referenced Feb 28, 2024
Closed
Closed
@build-analysis build-analysis bot mentioned this pull request Feb 28, 2024
Closed
jtschuster
jtschuster approved these changes Feb 28, 2024
View reviewed changes
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.

Thank you!

sbomer and others added 3 commits February 28, 2024 15:29
@sbomer @jtschuster
Update src/tools/illink/src/ILLink.RoslynAnalyzer/DataFlow/FeatureChe…
45337ce 
…cksValue.cs

Co-authored-by: Jackson Schuster <[email protected]>
@sbomer @jtschuster
Update src/tools/illink/src/ILLink.Shared/SharedStrings.resx
57f5e4e 
Co-authored-by: Jackson Schuster <[email protected]>
@sbomer
PR feedback
c9c244b 
- Add more detail to feature check validation message
- Use existing attribute helper to get AttributeData by attribute name
- Replace unnecessary ImmutableArray Builder
- Invert some if conditions to return early
- Rename TrimAnalysisReturnValuePattern -> FeatureCheckReturnValuePattern
@sbomer sbomer merged commit 756141c into dotnet:main Feb 29, 2024
@vitek-karas vitek-karas mentioned this pull request Feb 29, 2024
Closed
@sbomer sbomer mentioned this pull request Mar 4, 2024
Merged
@sbomer sbomer mentioned this pull request Mar 12, 2024
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 31, 2024
@sbomer sbomer deleted the featureGuardAnalyzerSupport branch August 7, 2025 17:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jtschuster jtschuster jtschuster approved these changes

@vitek-karas vitek-karas Awaiting requested review from vitek-karas

@marek-safar marek-safar Awaiting requested review from marek-safar marek-safar is a code owner

@agocke agocke Awaiting requested review from agocke

@LakshanF LakshanF Awaiting requested review from LakshanF

Assignees

@sbomer sbomer

Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sbomer @jtschuster