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

[Kritner]

• Fixes Malformed message template string should throw warning or compiler error as a preference over runtime exceptions #7285

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.49%. Comparing base (3211f48) to head (d962a08).
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7286      +/-   ##
==========================================
- Coverage   96.49%   96.49%   -0.01%     
==========================================
  Files        1443     1443              
  Lines      345885   345946      +61     
  Branches    11374    11383       +9     
==========================================
+ Hits       333765   333823      +58     
+ Misses       9241     9240       -1     
- Partials     2879     2883       +4     

[Kritner]

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]

@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]

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

[tarekgh]

@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 dotnet/runtime#78406 which you can mimic. meanwhile I converted this PR to draft till we finalize the process.

[Kritner]

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 dotnet/runtime#78406 which you can mimic. meanwhile I converted this PR to draft till we finalize the process.

added dotnet/runtime#101698

[Kritner]

Think i handled all the comments from the PR review, wasn't sure if i should be the one "resolving conversations" or if you'd be doing that @tarekgh. Thanks for the help!

[tarekgh]

LGTM. Thanks @Kritner!

[tarekgh]

@buyaa-n could you please have a quick look and merge if it looks good to you too?

[buyaa-n]

@buyaa-n could you please have a quick look and merge if it looks good to you too?

Is this analyzer needed for v9? I would wait until the main open for v10.

[tarekgh]

@buyaa-n could you please track it to merge when the main open for v10 merges?

[buyaa-n]

@buyaa-n could you please track it to merge when the main open for v10 merges?

I plan to go through all PRs and review/merge when main opens for v10

[Kritner]

Added a few more covering tests based on the new implementation

[buyaa-n]

Looks good, thank you @Kritner!