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

Enable generated code analysis but analyze razor/cshtml pages only #5976

Closed
wants to merge 6 commits into from

Conversation

buyaa-n
Copy link
Member

@buyaa-n buyaa-n commented Apr 26, 2022

Fixes #5971

CA1416 is disabled for generated code, but we need to enable them for razor/shtml pages

As proposed in dotnet/roslyn#39758: Roslyn already recognizes <auto-generated /> in a comment on the beginning of source files. We could extend this with an optional type attribute:

// <auto-generated type="Razor" />

We also want to support *.cshtml files, that means the type name might change

  • Finalize the type name
  • Add CS test
    - [ ] Add VB test VB not supported
  • Apply all code review feedback
  • End to end testing when the source generators start adding the type="Razor" attribute to the <auto-generated/> comment

Creating the PR as a draft until all checks fixed The PR now ready for review

@buyaa-n buyaa-n requested a review from a team as a code owner April 26, 2022 05:04
@@ -126,13 +125,15 @@ public sealed partial class PlatformCompatibilityAnalyzer : DiagnosticAnalyzer
isPortedFxCopRule: false,
isDataflowRule: false);

protected abstract bool IsSingleLineComment(SyntaxTrivia trivia);
Copy link
Member Author

Choose a reason for hiding this comment

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

We might want to move the language specific code into the Utilities instead

@buyaa-n buyaa-n marked this pull request as draft April 26, 2022 05:10
@buyaa-n buyaa-n marked this pull request as ready for review July 7, 2022 22:00
@buyaa-n buyaa-n requested a review from carlossanlop July 7, 2022 22:08
@codecov
Copy link

codecov bot commented Jul 8, 2022

Codecov Report

Merging #5976 (122fc55) into main (7e77cc9) will decrease coverage by 0.00%.
The diff coverage is 87.23%.

@@            Coverage Diff             @@
##             main    #5976      +/-   ##
==========================================
- Coverage   96.03%   96.03%   -0.01%     
==========================================
  Files        1338     1341       +3     
  Lines      307197   307463     +266     
  Branches     9788     9814      +26     
==========================================
+ Hits       295014   295258     +244     
- Misses       9805     9827      +22     
  Partials     2378     2378              

Copy link
Member

@mavasani mavasani left a comment

Choose a reason for hiding this comment

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

Please add VB tests and also confirm this does not affect build performance, given this analyzer is on by default on command line.

@buyaa-n buyaa-n marked this pull request as draft September 9, 2022 00:10
@buyaa-n
Copy link
Member Author

buyaa-n commented Sep 9, 2022

Moving to draft until dotnet/roslyn#7578 resolved and the type="Razor" attribute added into razor/cshtml pages

@mavasani
Copy link
Member

@buyaa-n dotnet/roslyn#7578 has been implemented, so we should soon have a public API IsGeneratedCode on the various analysis contexts and you wouldn't need to clone the GeneratedCodeUtilities helpers into the repo.

@jeffhandley
Copy link
Member

Closing out this old draft PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CA1416 Enable generated code analysis on razor pages
3 participants