Skip to content

Commit 7da26a6

Browse files
sharwellsharwell
committed
Update SA1139 to preserve numeric form except for the type suffix
Fixes #2275
1 parent a46d2a5 commit 7da26a6

5 files changed

Lines changed: 94 additions & 114 deletions

File tree

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1139CodeFixProvider.cs‎

Lines changed: 7 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -24,22 +24,6 @@ namespace StyleCop.Analyzers.ReadabilityRules
2424
[Shared]
2525
internal class SA1139CodeFixProvider : CodeFixProvider
2626
{
27-
private static readonly Dictionary<SyntaxKind, string> LiteralSyntaxKindToSuffix = new Dictionary<SyntaxKind, string>()
28-
{
29-
{ SyntaxKind.IntKeyword, string.Empty },
30-
{ SyntaxKind.LongKeyword, "L" },
31-
{ SyntaxKind.ULongKeyword, "UL" },
32-
{ SyntaxKind.UIntKeyword, "U" },
33-
{ SyntaxKind.FloatKeyword, "F" },
34-
{ SyntaxKind.DoubleKeyword, "D" },
35-
{ SyntaxKind.DecimalKeyword, "M" },
36-
};
37-
38-
private static readonly char[] LettersAllowedInLiteralSuffix = LiteralSyntaxKindToSuffix.Values
39-
.SelectMany(s => s.ToCharArray()).Distinct()
40-
.SelectMany(c => new[] { char.ToLowerInvariant(c), c })
41-
.ToArray();
42-
4327
/// <inheritdoc/>
4428
public override ImmutableArray<string> FixableDiagnosticIds { get; } =
4529
ImmutableArray.Create(SA1139UseLiteralSuffixNotationInsteadOfCasting.DiagnosticId);
@@ -73,50 +57,22 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
7357
var replacementNode = GenerateReplacementNode(node);
7458
var newSyntaxRoot = syntaxRoot.ReplaceNode(node, replacementNode);
7559
var newDocument = document.WithSyntaxRoot(newSyntaxRoot);
76-
var newSemanticModel = await newDocument.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
77-
var newNode = newSemanticModel.SyntaxTree.GetRoot().FindNode(
78-
span: new TextSpan(start: node.FullSpan.Start, length: replacementNode.FullSpan.Length),
79-
getInnermostNodeForTie: true);
80-
81-
var oldConstantValue = oldSemanticModel.GetConstantValue(node).Value;
82-
var newConstantValueOption = newSemanticModel.GetConstantValue(newNode, cancellationToken);
83-
if (newConstantValueOption.HasValue && oldConstantValue.Equals(newConstantValueOption.Value))
84-
{
85-
return newDocument;
86-
}
87-
else
88-
{
89-
var newNodeBasedOnValue = GenerateReplacementNodeBasedOnValue(node, oldConstantValue);
90-
newSyntaxRoot = syntaxRoot.ReplaceNode(node, newNodeBasedOnValue);
91-
return document.WithSyntaxRoot(newSyntaxRoot);
92-
}
60+
return newDocument;
9361
}
9462

9563
private static SyntaxNode GenerateReplacementNode(CastExpressionSyntax node)
9664
{
97-
var plusMinusSyntax = node.Expression.WalkDownParentheses() as PrefixUnaryExpressionSyntax;
9865
var literalExpressionSyntax =
99-
plusMinusSyntax == null ?
66+
!(node.Expression.WalkDownParentheses() is PrefixUnaryExpressionSyntax plusMinusSyntax) ?
10067
(LiteralExpressionSyntax)node.Expression.WalkDownParentheses() :
10168
(LiteralExpressionSyntax)plusMinusSyntax.Operand.WalkDownParentheses();
10269
var typeToken = node.Type.GetFirstToken();
103-
var prefix = plusMinusSyntax == null
104-
? string.Empty
105-
: plusMinusSyntax.OperatorToken.Text;
106-
var literalWithoutSuffix = literalExpressionSyntax.StripLiteralSuffix();
107-
var correspondingSuffix = LiteralSyntaxKindToSuffix[typeToken.Kind()];
108-
var fixedCodePreservingText = SyntaxFactory.ParseExpression(prefix + literalWithoutSuffix + correspondingSuffix);
109-
110-
return fixedCodePreservingText.WithTriviaFrom(node);
111-
}
112-
113-
private static SyntaxNode GenerateReplacementNodeBasedOnValue(CastExpressionSyntax node, object desiredValue)
114-
{
115-
var typeToken = node.Type.GetFirstToken();
116-
var correspondingSuffix = LiteralSyntaxKindToSuffix[typeToken.Kind()];
117-
var fixedCodePreservingText = SyntaxFactory.ParseExpression(desiredValue + correspondingSuffix);
70+
var replacementLiteral = literalExpressionSyntax.WithLiteralSuffix(typeToken.Kind());
71+
var replacementNode = node.Expression.ReplaceNode(literalExpressionSyntax, replacementLiteral)
72+
.WithLeadingTrivia(node.GetLeadingTrivia().Concat(node.CloseParenToken.TrailingTrivia).Concat(node.Expression.GetLeadingTrivia()))
73+
.WithTrailingTrivia(node.Expression.GetTrailingTrivia().Concat(node.GetTrailingTrivia()));
11874

119-
return fixedCodePreservingText.WithTriviaFrom(node);
75+
return replacementNode;
12076
}
12177
}
12278
}

‎StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/ReadabilityRules/SA1139CSharp7UnitTests.cs‎

Lines changed: 10 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -213,19 +213,18 @@ public void Method()
213213
}
214214

215215
/// <summary>
216-
/// Verifies that using casts in unchecked environment produces diagnostics with a correct code fix.
216+
/// Verifies that casts in unchecked environment do not get replaced with incorrect values.
217217
/// </summary>
218218
/// <param name="castExpression">A cast which can be performed in unchecked environment</param>
219-
/// <param name="correctLiteral">The corresponding literal with suffix</param>
220219
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
221220
[Theory]
222-
[InlineData("(ulong)-0_1L", "18446744073709551615UL")]
223-
[InlineData("(int)1_000_000_000_000_000_000L", "-1486618624")]
224-
[InlineData("(int)0xFFFF_FFFF_FFFF_FFFFL", "-1")]
225-
[InlineData("(uint)0xFFFF_FFFF_FFFF_FFFFL", "4294967295U")]
226-
[InlineData("(int)0b11111111_11111111_11111111_11111111_11111111_11111111_11111111_11111111L", "-1")]
227-
[InlineData("(uint)0b11111111_11111111_11111111_11111111_11111111_11111111_11111111_11111111L", "4294967295U")]
228-
public async Task TestCastsWithSeparatorsInUncheckedEnviromentShouldPreserveValueAsync(string castExpression, string correctLiteral)
221+
[InlineData("(ulong)-0_1L")]
222+
[InlineData("(int)1_000_000_000_000_000_000L")]
223+
[InlineData("(int)0xFFFF_FFFF_FFFF_FFFFL")]
224+
[InlineData("(uint)0xFFFF_FFFF_FFFF_FFFFL")]
225+
[InlineData("(int)0b11111111_11111111_11111111_11111111_11111111_11111111_11111111_11111111L")]
226+
[InlineData("(uint)0b11111111_11111111_11111111_11111111_11111111_11111111_11111111_11111111L")]
227+
public async Task TestCastsWithSeparatorsInUncheckedEnviromentShouldPreserveValueAsync(string castExpression)
229228
{
230229
var testCode = $@"
231230
class ClassName
@@ -240,26 +239,8 @@ public void Method()
240239
}}
241240
}}
242241
";
243-
var fixedCode = $@"
244-
class ClassName
245-
{{
246-
public void Method()
247-
{{
248-
unchecked
249-
{{
250-
var x = {correctLiteral};
251-
}}
252-
var y = unchecked({correctLiteral});
253-
}}
254-
}}
255-
";
256-
DiagnosticResult[] expectedDiagnosticResult =
257-
{
258-
this.CSharpDiagnostic().WithLocation(8, 21),
259-
this.CSharpDiagnostic().WithLocation(10, 27),
260-
};
261-
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnosticResult, CancellationToken.None).ConfigureAwait(false);
262-
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
242+
243+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
263244
}
264245
}
265246
}

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1139UnitTests.cs‎

Lines changed: 22 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,10 @@ public void Method()
7272
[InlineData("long", "1", "1L")]
7373
[InlineData("long", "+1", "+1L")]
7474
[InlineData("long", "-1", "-1L")]
75-
[InlineData("long", "(+1)", "+1L")]
76-
[InlineData("long", "(-1)", "-1L")]
77-
[InlineData("long", "(1)", "1L")]
78-
[InlineData("long", "(-(1))", "-1L")]
75+
[InlineData("long", "(+1)", "(+1L)")]
76+
[InlineData("long", "(-1)", "(-1L)")]
77+
[InlineData("long", "(1)", "(1L)")]
78+
[InlineData("long", "(-(1))", "(-(1L))")]
7979
[InlineData("ulong", "1", "1UL")]
8080
[InlineData("ulong", "+1", "+1UL")]
8181
[InlineData("uint", "1", "1U")]
@@ -96,6 +96,10 @@ public void Method()
9696
[InlineData("ulong", "1l", "1UL")]
9797
[InlineData("ulong", "1U", "1UL")]
9898
[InlineData("ulong", "1u", "1UL")]
99+
[InlineData("long", "0xF", "0xFL")]
100+
[InlineData("long", "0x1", "0x1L")]
101+
[InlineData("ulong", "0x1", "0x1UL")]
102+
[InlineData("long", "-0x1", "-0x1L")]
99103
public async Task TestUsingCastsProducesDiagnosticAndCorrectCodefixAsync(string literalType, string castedLiteral, string correctLiteral)
100104
{
101105
var testCode = $@"
@@ -183,9 +187,14 @@ public void Method()
183187
[InlineData("(bool)true")]
184188
[InlineData("(bool)(false)")]
185189
[InlineData("(long)~+1")]
190+
[InlineData("(long)(~+1)")]
191+
[InlineData("unchecked((int)0x80000000)")]
192+
[InlineData("unchecked((int)0xFFFFFFFF)")]
193+
[InlineData("unchecked((ulong)-1)")]
186194
[InlineData("unchecked((byte)-1)")]
187195
[InlineData("(sbyte)-1")]
188196
[InlineData("(int)-1")]
197+
[InlineData("unchecked((ulong)-1L)")]
189198
public async Task TestOtherTypesOfCastsShouldNotTriggerDiagnosticAsync(string correctCastExpression)
190199
{
191200
var testCode = $@"
@@ -230,17 +239,16 @@ public void Method()
230239
}
231240

232241
/// <summary>
233-
/// Verifies that using casts in unchecked enviroment produces diagnostics with a correct codefix.
242+
/// Verifies that casts in unchecked environment do not get replaced with incorrect values.
234243
/// </summary>
235-
/// <param name="castExpression">A cast which can be performend in unchecked enviroment</param>
236-
/// <param name="correctLiteral">The corresponding literal with suffix</param>
244+
/// <param name="castExpression">A cast which can be performed in unchecked environment</param>
237245
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
238246
[Theory]
239-
[InlineData("(ulong)-1L", "18446744073709551615UL")]
240-
[InlineData("(int)1000000000000000000L", "-1486618624")]
241-
[InlineData("(int)0xFFFFFFFFFFFFFFFFL", "-1")]
242-
[InlineData("(uint)0xFFFFFFFFFFFFFFFFL", "4294967295U")]
243-
public async Task TestCastsInUncheckedEnviromentShouldPreserveValueAsync(string castExpression, string correctLiteral)
247+
[InlineData("(ulong)-1L")]
248+
[InlineData("(int)1000000000000000000L")]
249+
[InlineData("(int)0xFFFFFFFFFFFFFFFFL")]
250+
[InlineData("(uint)0xFFFFFFFFFFFFFFFFL")]
251+
public async Task TestCastsInUncheckedEnviromentShouldPreserveValueAsync(string castExpression)
244252
{
245253
var testCode = $@"
246254
class ClassName
@@ -255,26 +263,8 @@ public void Method()
255263
}}
256264
}}
257265
";
258-
var fixedCode = $@"
259-
class ClassName
260-
{{
261-
public void Method()
262-
{{
263-
unchecked
264-
{{
265-
var x = {correctLiteral};
266-
}}
267-
var y = unchecked({correctLiteral});
268-
}}
269-
}}
270-
";
271-
DiagnosticResult[] expectedDiagnosticResult =
272-
{
273-
this.CSharpDiagnostic().WithLocation(8, 21),
274-
this.CSharpDiagnostic().WithLocation(10, 27),
275-
};
276-
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnosticResult, CancellationToken.None).ConfigureAwait(false);
277-
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
266+
267+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
278268
}
279269

280270
protected override CodeFixProvider GetCSharpCodeFixProvider()

‎StyleCop.Analyzers/StyleCop.Analyzers/Helpers/LiteralExpressionHelpers.cs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
namespace StyleCop.Analyzers.Helpers
55
{
6+
using Microsoft.CodeAnalysis.CSharp;
67
using Microsoft.CodeAnalysis.CSharp.Syntax;
78

89
internal static class LiteralExpressionHelpers
@@ -45,5 +46,44 @@ internal static string StripLiteralSuffix(this LiteralExpressionSyntax literalEx
4546
// If this is reached literalText does not contain a literal
4647
return string.Empty;
4748
}
49+
50+
internal static LiteralExpressionSyntax WithLiteralSuffix(this LiteralExpressionSyntax literalExpression, SyntaxKind syntaxKindKeyword)
51+
{
52+
string textWithoutSuffix = literalExpression.StripLiteralSuffix();
53+
54+
string suffix;
55+
switch (syntaxKindKeyword)
56+
{
57+
case SyntaxKind.UIntKeyword:
58+
suffix = "U";
59+
break;
60+
61+
case SyntaxKind.ULongKeyword:
62+
suffix = "UL";
63+
break;
64+
65+
case SyntaxKind.LongKeyword:
66+
suffix = "L";
67+
break;
68+
69+
case SyntaxKind.FloatKeyword:
70+
suffix = "F";
71+
break;
72+
73+
case SyntaxKind.DoubleKeyword:
74+
suffix = "D";
75+
break;
76+
77+
case SyntaxKind.DecimalKeyword:
78+
suffix = "M";
79+
break;
80+
81+
default:
82+
suffix = string.Empty;
83+
break;
84+
}
85+
86+
return literalExpression.WithToken(SyntaxFactory.ParseToken(textWithoutSuffix + suffix).WithTriviaFrom(literalExpression.Token));
87+
}
4888
}
4989
}

‎StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1139UseLiteralSuffixNotationInsteadOfCasting.cs‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,13 @@ private static void HandleCastExpression(SyntaxNodeAnalysisContext context)
9191
return;
9292
}
9393

94-
if (context.SemanticModel.GetTypeInfo(castedElementTypeSyntax).Type
95-
== context.SemanticModel.GetTypeInfo(castExpressionSyntax).Type)
94+
var targetType = context.SemanticModel.GetTypeInfo(castExpressionSyntax).Type;
95+
if (targetType is null)
96+
{
97+
return;
98+
}
99+
100+
if (targetType.Equals(context.SemanticModel.GetTypeInfo(castedElementTypeSyntax).Type))
96101
{
97102
// cast is redundant which is reported by another diagnostic.
98103
return;
@@ -104,6 +109,14 @@ private static void HandleCastExpression(SyntaxNodeAnalysisContext context)
104109
return;
105110
}
106111

112+
var speculativeExpression = castExpressionSyntax.Expression.ReplaceNode(castedElementTypeSyntax, castedElementTypeSyntax.WithLiteralSuffix(syntaxKindKeyword));
113+
var speculativeTypeInfo = context.SemanticModel.GetSpeculativeTypeInfo(castExpressionSyntax.SpanStart, speculativeExpression, SpeculativeBindingOption.BindAsExpression);
114+
if (!targetType.Equals(speculativeTypeInfo.Type))
115+
{
116+
// Suffix notation would change the type of the expression
117+
return;
118+
}
119+
107120
context.ReportDiagnostic(Diagnostic.Create(Descriptor, castExpressionSyntax.GetLocation()));
108121
}
109122
}

0 commit comments

Comments
 (0)