Skip to content

Commit c26075e

Browse files
sharwellsharwell
authored
Merge pull request #2723 from sharwell/relax-literal-enforcement
Relax literal enforcement
2 parents 6434b20 + 7da26a6 commit c26075e

File tree

6 files changed

+127
-118
lines changed

6 files changed

+127
-118
lines changed

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

Lines changed: 9 additions & 53 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 as PrefixUnaryExpressionSyntax;
9865
var literalExpressionSyntax =
99-
plusMinusSyntax == null ?
100-
(LiteralExpressionSyntax)node.Expression :
101-
(LiteralExpressionSyntax)plusMinusSyntax.Operand;
102-
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-
{
66+
!(node.Expression.WalkDownParentheses() is PrefixUnaryExpressionSyntax plusMinusSyntax) ?
67+
(LiteralExpressionSyntax)node.Expression.WalkDownParentheses() :
68+
(LiteralExpressionSyntax)plusMinusSyntax.Operand.WalkDownParentheses();
11569
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: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +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))")]
7579
[InlineData("ulong", "1", "1UL")]
7680
[InlineData("ulong", "+1", "+1UL")]
7781
[InlineData("uint", "1", "1U")]
@@ -92,6 +96,10 @@ public void Method()
9296
[InlineData("ulong", "1l", "1UL")]
9397
[InlineData("ulong", "1U", "1UL")]
9498
[InlineData("ulong", "1u", "1UL")]
99+
[InlineData("long", "0xF", "0xFL")]
100+
[InlineData("long", "0x1", "0x1L")]
101+
[InlineData("ulong", "0x1", "0x1UL")]
102+
[InlineData("long", "-0x1", "-0x1L")]
95103
public async Task TestUsingCastsProducesDiagnosticAndCorrectCodefixAsync(string literalType, string castedLiteral, string correctLiteral)
96104
{
97105
var testCode = $@"
@@ -175,9 +183,18 @@ public void Method()
175183
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
176184
[Theory]
177185
[InlineData("(long)~1")]
186+
[InlineData("(long)(~1)")]
178187
[InlineData("(bool)true")]
179188
[InlineData("(bool)(false)")]
180189
[InlineData("(long)~+1")]
190+
[InlineData("(long)(~+1)")]
191+
[InlineData("unchecked((int)0x80000000)")]
192+
[InlineData("unchecked((int)0xFFFFFFFF)")]
193+
[InlineData("unchecked((ulong)-1)")]
194+
[InlineData("unchecked((byte)-1)")]
195+
[InlineData("(sbyte)-1")]
196+
[InlineData("(int)-1")]
197+
[InlineData("unchecked((ulong)-1L)")]
181198
public async Task TestOtherTypesOfCastsShouldNotTriggerDiagnosticAsync(string correctCastExpression)
182199
{
183200
var testCode = $@"
@@ -222,17 +239,16 @@ public void Method()
222239
}
223240

224241
/// <summary>
225-
/// 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.
226243
/// </summary>
227-
/// <param name="castExpression">A cast which can be performend in unchecked enviroment</param>
228-
/// <param name="correctLiteral">The corresponding literal with suffix</param>
244+
/// <param name="castExpression">A cast which can be performed in unchecked environment</param>
229245
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
230246
[Theory]
231-
[InlineData("(ulong)-1L", "18446744073709551615UL")]
232-
[InlineData("(int)1000000000000000000L", "-1486618624")]
233-
[InlineData("(int)0xFFFFFFFFFFFFFFFFL", "-1")]
234-
[InlineData("(uint)0xFFFFFFFFFFFFFFFFL", "4294967295U")]
235-
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)
236252
{
237253
var testCode = $@"
238254
class ClassName
@@ -247,26 +263,8 @@ public void Method()
247263
}}
248264
}}
249265
";
250-
var fixedCode = $@"
251-
class ClassName
252-
{{
253-
public void Method()
254-
{{
255-
unchecked
256-
{{
257-
var x = {correctLiteral};
258-
}}
259-
var y = unchecked({correctLiteral});
260-
}}
261-
}}
262-
";
263-
DiagnosticResult[] expectedDiagnosticResult =
264-
{
265-
this.CSharpDiagnostic().WithLocation(8, 21),
266-
this.CSharpDiagnostic().WithLocation(10, 27),
267-
};
268-
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnosticResult, CancellationToken.None).ConfigureAwait(false);
269-
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
266+
267+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
270268
}
271269

272270
protected override CodeFixProvider GetCSharpCodeFixProvider()

‎StyleCop.Analyzers/StyleCop.Analyzers/Helpers/ExpressionSyntaxHelpers.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
2+
// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information.
3+
4+
namespace StyleCop.Analyzers.Helpers
5+
{
6+
using Microsoft.CodeAnalysis.CSharp.Syntax;
7+
8+
internal static class ExpressionSyntaxHelpers
9+
{
10+
public static ExpressionSyntax WalkDownParentheses(this ExpressionSyntax expression)
11+
{
12+
var result = expression;
13+
while (result is ParenthesizedExpressionSyntax parenthesizedExpression)
14+
{
15+
result = parenthesizedExpression.Expression;
16+
}
17+
18+
return result;
19+
}
20+
}
21+
}

‎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: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ internal class SA1139UseLiteralSuffixNotationInsteadOfCasting : DiagnosticAnalyz
3131

3232
private static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor(DiagnosticId, Title, MessageFormat, AnalyzerCategory.ReadabilityRules, DiagnosticSeverity.Warning, AnalyzerConstants.EnabledByDefault, Description, HelpLink);
3333
private static readonly Action<CompilationStartAnalysisContext> CompilationStartAction = HandleCompilationStart;
34-
private static readonly Action<SyntaxNodeAnalysisContext> GenericNameAction = HandleGenericName;
34+
private static readonly Action<SyntaxNodeAnalysisContext> CastExpressionAction = HandleCastExpression;
3535

3636
/// <inheritdoc/>
3737
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
@@ -46,10 +46,10 @@ public override void Initialize(AnalysisContext context)
4646

4747
private static void HandleCompilationStart(CompilationStartAnalysisContext context)
4848
{
49-
context.RegisterSyntaxNodeAction(GenericNameAction, SyntaxKind.CastExpression);
49+
context.RegisterSyntaxNodeAction(CastExpressionAction, SyntaxKind.CastExpression);
5050
}
5151

52-
private static void HandleGenericName(SyntaxNodeAnalysisContext context)
52+
private static void HandleCastExpression(SyntaxNodeAnalysisContext context)
5353
{
5454
var castExpressionSyntax = (CastExpressionSyntax)context.Node;
5555

@@ -58,7 +58,7 @@ private static void HandleGenericName(SyntaxNodeAnalysisContext context)
5858
return;
5959
}
6060

61-
var unaryExpressionSyntax = castExpressionSyntax.Expression as PrefixUnaryExpressionSyntax;
61+
var unaryExpressionSyntax = castExpressionSyntax.Expression.WalkDownParentheses() as PrefixUnaryExpressionSyntax;
6262
if (unaryExpressionSyntax != null)
6363
{
6464
if (unaryExpressionSyntax.Kind() != SyntaxKind.UnaryPlusExpression
@@ -70,8 +70,8 @@ private static void HandleGenericName(SyntaxNodeAnalysisContext context)
7070
}
7171

7272
var castedElementTypeSyntax = unaryExpressionSyntax == null
73-
? castExpressionSyntax.Expression as LiteralExpressionSyntax
74-
: unaryExpressionSyntax.Operand as LiteralExpressionSyntax;
73+
? castExpressionSyntax.Expression.WalkDownParentheses() as LiteralExpressionSyntax
74+
: unaryExpressionSyntax.Operand.WalkDownParentheses() as LiteralExpressionSyntax;
7575

7676
if (castedElementTypeSyntax == null)
7777
{
@@ -91,8 +91,13 @@ private static void HandleGenericName(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 HandleGenericName(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)