Merge | Diagnostic Files

[benrr101]

Description: More work as part of the merge project. This one is pretty straightforward - the SqlDiagnostic* classes only belonged to the netcore project, so this PR:

• Moves the files over to the common project in a Diagnostics folder
• Split the files into one class per file
• Moved classes that weren't in the SqlClient.Diagnostics namespace into that namespace
• Moved the extension methods for DiagnosticsListener class into the DiagnosticListener class
  • I can revert this if there's significant backlash to the change, but considering the code is all internal, to me, there's no real reason to keep these as extension methods.
• Also created stubs for SqlCommand and SqlConnection in common project to make it a bit easier to work on the Diagnostic files.

Testing: No functional changes to the code, CI should be able to validate.

[codecov]

Codecov Report

Attention: Patch coverage is 35.74244% with 489 lines in your changes missing coverage. Please review.

Project coverage is 72.59%. Comparing base (1e59b88) to head (03b2459).
Report is 11 commits behind head on main.

Files with missing lines	Patch %	Lines
...lient/Diagnostics/SqlDiagnosticListener.netcore.cs	57.44%	100 Missing ⚠️
...stics/SqlClientTransactionRollbackError.netcore.cs	0.00%	36 Missing ⚠️
...agnostics/SqlClientConnectionCloseError.netcore.cs	0.00%	33 Missing ⚠️
...nostics/SqlClientTransactionCommitError.netcore.cs	0.00%	33 Missing ⚠️
...stics/SqlClientTransactionRollbackAfter.netcore.cs	0.00%	33 Missing ⚠️
...tics/SqlClientTransactionRollbackBefore.netcore.cs	0.00%	33 Missing ⚠️
...nostics/SqlClientTransactionCommitAfter.netcore.cs	0.00%	30 Missing ⚠️
...ostics/SqlClientTransactionCommitBefore.netcore.cs	0.00%	30 Missing ⚠️
...iagnostics/SqlClientConnectionOpenError.netcore.cs	36.36%	21 Missing ⚠️
...lient/Diagnostics/SqlClientCommandError.netcore.cs	39.39%	20 Missing ⚠️
... and 8 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3213      +/-   ##
==========================================
+ Coverage   72.58%   72.59%   +0.01%     
==========================================
  Files         289      304      +15     
  Lines       59503    59367     -136     
==========================================
- Hits        43188    43097      -91     
+ Misses      16315    16270      -45     

Flag	Coverage Δ
addons	92.58% <ø> (ø)
netcore	75.12% <35.74%> (+<0.01%)	⬆️
netfx	71.22% <ø> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[cheenamalhotra]

Is this needed in this PR for merging Diagnostics or can be done later?

[benrr101]

This is only added to aid the migration process - while I'm working within the common project, it resolves missing methods/classes so I can work without red squiggles everywhere. This will be removed when the sqlconnection class is merged.

[cheenamalhotra]

I don't see a need to split this class into subclasses since all of them have a single purpose and will not likely see any major works in foreseeable future.
This class could stay in single file targeted for netcore project.

[benrr101]

No, I disagree, it should be properly split into separate files. It messes with developer expectations to have multiple classes within a single file. C# standards suggest each class should get its own file. There are a lot of bad examples in this codebase where multiple classes have been jammed into the same file, and it makes it much more difficult to know where each class is supposed to be.
If it truly is only used within the confines of another class, maybe we consider making it a nested class.