Make SqlConnectionEncryptOption string parser public

[cheenamalhotra]

It would be useful to have these parsers public to be able to parse string values directly when needed, instead of consumer apps writing their own.

[codecov]

Codecov Report

Base: 71.82% // Head: 71.41% // Decreases project coverage by -0.40% ⚠️

Coverage data is based on head (a55245c) compared to base (7d348eb).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1771      +/-   ##
==========================================
- Coverage   71.82%   71.41%   -0.41%     
==========================================
  Files         293      293              
  Lines       61376    61383       +7     
==========================================
- Hits        44082    43836     -246     
- Misses      17294    17547     +253     

Flag	Coverage Δ
addons	92.38% <ø> (ø)
netcore	75.03% <100.00%> (-0.41%)	⬇️
netfx	69.22% <100.00%> (-0.33%)	⬇️

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

Impacted Files	Coverage Δ
...osoft/Data/SqlClient/SqlConnectionEncryptOption.cs	93.10% <100.00%> (+6.73%)	⬆️
...ActiveDirectoryAuthenticationTimeoutRetryHelper.cs	56.81% <0.00%> (-18.19%)	⬇️
...nt/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs	78.57% <0.00%> (-17.86%)	⬇️
...ata/SqlClient/SqlConnectionTimeoutErrorInternal.cs	30.35% <0.00%> (-11.61%)	⬇️
...re/src/Microsoft/Data/SqlClient/SNI/SNINpHandle.cs	73.77% <0.00%> (-4.92%)	⬇️
...e/src/Microsoft/Data/SqlClient/SNI/SNITcpHandle.cs	63.49% <0.00%> (-3.18%)	⬇️
...Client/Reliability/Common/SqlRetryLogicProvider.cs	89.13% <0.00%> (-2.18%)	⬇️
...c/Microsoft/Data/SqlClient/TdsParserStateObject.cs	81.58% <0.00%> (-1.96%)	⬇️
...c/Microsoft/Data/SqlClient/TdsParserStateObject.cs	81.65% <0.00%> (-1.70%)	⬇️
...core/src/Microsoft/Data/SqlClient/SqlConnection.cs	77.31% <0.00%> (-1.21%)	⬇️
... and 14 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[DavoudEshtehari]

(SqlConnectionEncryptOption)null returns True. Just raising it for more discussion.
If it's by design it'd better be mentioned in the documentation, plus extending the code coverage.

cc @David-Engel

[David-Engel]

(SqlConnectionEncryptOption)null returns True. Just raising it for more discussion. If it's by design it'd better be mentioned in the documentation, plus extending the code coverage.

@DavoudEshtehari

Do you have a use case for that scenario? It wasn't intentional (it wasn't even considered), but since that's the behavior, we should maintain it by adding it to the tests.

[DavoudEshtehari]

Do you have a use case for that scenario? It wasn't intentional (it wasn't even considered), but since that's the behavior, we should maintain it by adding it to the tests.

No, just a random test.

I'd suggest these 2 options to improve this part:
1- Overriding nullable bool instead of a bool type to support null:

public static implicit operator bool?(SqlConnectionEncryptOption value) => !Optional?.Equals(value);

2- Keep it as it is instead of interpreting null to false not true:

public static implicit operator bool(SqlConnectionEncryptOption value) => value is not null && !Optional.Equals(value);

[lcheunglci]

I realize I missed the discussion before approving the changes, but the changes is relatively safe and tests were included to validate them so perhaps the suggestion from @DavoudEshtehari could be handled in separate PR.