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

add new .Net SDK analyzer rules based on mgmt SDK review experience #6905

Closed
archerzz opened this issue Sep 6, 2023 · 0 comments · Fixed by #6916
Closed

add new .Net SDK analyzer rules based on mgmt SDK review experience #6905

archerzz opened this issue Sep 6, 2023 · 0 comments · Fixed by #6916

Comments

@archerzz
Copy link
Member

archerzz commented Sep 6, 2023

Description

We've come up with a set of rules during the daily review of .Net mgmt SDK. See here for the list. We believe it would be beneficial to implement some of them into Azure.ClientSdk.Analyzers package, so that some issues could have been fixed before the SDK is sent for review.

The idea is that:

  • The rules are split into two categories:
    • Hard rules which is enabled by default. Those will break CI so that they must be fixed.
    • Soft rules/suggestion which is disabled by default. They can be enabled explicitly by SDK developers or in ApiView. The purpose of those rules is to provide prompts to improve SDK API signature.
  • The rules should be applied to both mgmt plane and data plane.
  • For existing SDKs, there will be many violations. So we provide a property to opt-out new rules.
@github-actions github-actions bot added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Sep 6, 2023
archerzz pushed a commit to archerzz/azure-sdk-tools that referenced this issue Sep 7, 2023
- add four rules checking the suffixes of model names
- upgrade Roslyn to latest version since new rules require some API
- minor refactor existing codes
- add test cases for the new rules

resolve Azure#6905
@kurtzeborn kurtzeborn added .NETClientSDKAnalyzers and removed needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels Sep 11, 2023
archerzz added a commit that referenced this issue Oct 26, 2023
- add four rules checking the suffixes of model names
- upgrade Roslyn to `4.4.0.0` since new rules require some API
- fix and suppress warnings due to upgrade of Roslyn
- temporarily suppress AD0001
- minor refactoring of existing codes
- add test cases for the new rules

resolve #6905

---------

Co-authored-by: Mingzhe Huang (from Dev Box) <[email protected]>
Co-authored-by: Christopher Scott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants