fix(CA2023: Adds validation against invalid braces in logger message templates)

dotnet / roslyn-analyzers

MIT License

1.56k stars 462 forks source link

fix(CA2023: Adds validation against invalid braces in logger message templates) #7286

Open Kritner opened 3 months ago

Kritner commented 3 months ago

Fixes #7285

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 96.66667% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 96.48%. Comparing base (b07c100) to head (7a46e9f).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #7286 +/- ## ======================================= Coverage 96.48% 96.48% ======================================= Files 1443 1443 Lines 345394 345483 +89 Branches 11364 11378 +14 ======================================= + Hits 333240 333330 +90 + Misses 9275 9274 -1 Partials 2879 2879 ```

Kritner commented 3 months ago

https://dev.azure.com/dnceng-public/public/_build/results?buildId=639007&view=logs&j=f0f0520d-467b-5d1d-a95d-4e3a2581fe8a&t=c140c00f-6c0a-5356-d099-4e047a5525ed&l=221

getting this build error, both prior to and after making updates to the files mentioned. I'm not sure if this command is supposed to be run locally, or as a part of the build automatically; but i was getting errors attempting to run it locally.

going to revert changes to these two files mentioned and just double check the same error occurs:

One or more auto-generated documentation files were either edited manually, or not updated. Please revert changes made to the following files (if manually edited) and run `msbuild /t:pack` at the root of the repo to automatically update them:
      D:\a\_work\1\s\src\NetAnalyzers\Microsoft.CodeAnalysis.NetAnalyzers.md
      D:\a\_work\1\s\src\NetAnalyzers\Microsoft.CodeAnalysis.NetAnalyzers.sarif

Kritner commented 3 months ago

@buyaa-n i think this might be good to go now - though i'm a bit unsure of the git flow being used. I'm assuming i want to target my PR to the release/8.0.2xx branch such that it will get release under the next build of those revision of packages? Or should this be targeting main?

(I only tagged you because of your activity on #7285, lmk if i should tag someone else or just let this thing sit til it gets through some queue)

buyaa-n commented 3 months ago

I'm assuming i want to target my PR to the release/8.0.2xx branch such that it will get release under the next build of those revision of packages? Or should this be targeting main?

No this should target main, only servicing changes should target release branches

Kritner commented 2 months ago

Hey just wanted to bump this to check if anything is needed from me, and/or what next steps are?

tarekgh commented 2 months ago

@stephentoub could you please advise if we can handle this in already existing rule or do you think we need to introduce a new analysis rule with new diagnostic?

tarekgh commented 2 months ago

@Kritner thanks for helping with this issue.

If we'll continue having a new diagnostic we'll need to get approval for it before we proceed. Could you please log a new issue in the runtime repo to track reviewing it? here is some example issue https://github.com/dotnet/runtime/issues/78406 which you can mimic. meanwhile I converted this PR to draft till we finalize the process.

Kritner commented 2 months ago

from @tarekgh:

If we'll continue having a new diagnostic we'll need to get approval for it before we proceed. Could you please log a new issue in the runtime repo to track reviewing it? here is some example issue https://github.com/dotnet/runtime/issues/78406 which you can mimic. meanwhile I converted this PR to draft till we finalize the process.

added https://github.com/dotnet/runtime/issues/101698