CA1802: Don't report diagnostic for preview feature constant interpolated strings #4741
Conversation
| @@ -30,7 +30,7 @@ | |||
| <!-- Roslyn --> | |||
| <MicrosoftCodeAnalysisVersion>3.3.1</MicrosoftCodeAnalysisVersion> | |||
| <MicrosoftCodeAnalysisForRoslynDiagnosticsAnalyzersVersion>3.7.0</MicrosoftCodeAnalysisForRoslynDiagnosticsAnalyzersVersion> | |||
| <MicrosoftCodeAnalysisVersionForTests>3.8.0</MicrosoftCodeAnalysisVersionForTests> | |||
| <MicrosoftCodeAnalysisVersionForTests>3.9.0-3.final</MicrosoftCodeAnalysisVersionForTests> | |||
There was a problem hiding this comment.
Version 3.8.0 doesn't produce operation tree where interpolated string has constant value. I had to update to be able to test.
| private static bool IsConstantInterpolatedStringSupported() | ||
| { | ||
| // TODO: When constant interpolated string is supported in a stable language version (most likely C# 10), this method should be updated. | ||
| // The feature is currently available for preview only. | ||
| return false; | ||
| } |
There was a problem hiding this comment.
I didn't return true for preview version based on #4732 (comment)
When this is implemented later, it will need to be abstract with language-specific implementation.
A possible another approach would be to introduce a new service that gives information about features.
I'd love to be able to do the following (or similar):
var service = compilation.GetService<ILanguageFeatureSupportService>();
if (service.IsConstantInterpolationSupported(compilation)
{
// do something.
}
|
@mavasani Test failure is for another analyzer CA1834. This happened because the operation tree changed after updating the compiler used in tests. Can you give it a look? Both behaviors can be reasoned about, so I don't know what will be your preference. |
|
|
||
| private static bool IsConstantInterpolatedStringSupported() | ||
| { | ||
| // TODO: When constant interpolated string is supported in a stable language version (most likely C# 10), this method should be updated. |
There was a problem hiding this comment.
Can you instead check the LanguageVersion from the Parse options and get rid of the TODO? You would need to add analyzer sub-types to C# and VB layers to access the parse options.
Youssef1313
force-pushed
the
ca1802-fp-const-interpolated
branch
from
d6eba7b to
b02c297
Compare
|
@Youssef1313 tests are failing |
|
@mavasani Failing test is for another analyzer. Could you check what should be the correct behavior for it? |
@Youssef1313 You need to update the failing test to expect an analyzer diagnostic given that with the new MS.CA package for testing, interpolated string that is made up of constant constituent parts is also considered a constant now. |
| } | ||
| } | ||
| }"; | ||
| await VerifyCS.VerifyAnalyzerAsync(interpolatedString_cs); | ||
|
|
||
| await VerifyCS.VerifyCodeFixAsync(interpolatedString_cs, interpolatedString_cs); |
There was a problem hiding this comment.
@mavasani No fix is offered for this edge case. Is this behavior okay or should I file an issue?
Codecov Report
@@ Coverage Diff @@
## main #4741 +/- ##
==========================================
- Coverage 95.52% 95.52% -0.01%
==========================================
Files 1196 1199 +3
Lines 274480 274432 -48
Branches 16594 16595 +1
==========================================
- Hits 262191 262143 -48
- Misses 10102 10111 +9
+ Partials 2187 2178 -9 |
Fixes #4732