Skip to content

Commit 29f88fb

Browse files
cristian-ambrosini-sonarsourcecristian-ambrosini-sonarsource
authored
Fix S2183 FN: Support unsigned right-shift operator (>>>) (#6358)
1 parent 7984f27 commit 29f88fb

4 files changed

Lines changed: 42 additions & 54 deletions

File tree

‎analyzers/src/SonarAnalyzer.CFG/ShimLayer/SyntaxKindEx.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ public static class SyntaxKindEx
99
{
1010
public const SyntaxKind DotDotToken = (SyntaxKind)8222;
1111
public const SyntaxKind QuestionQuestionEqualsToken = (SyntaxKind)8284;
12+
public const SyntaxKind GreaterThanGreaterThanGreaterThanToken = (SyntaxKind)8286;
13+
public const SyntaxKind GreaterThanGreaterThanGreaterThanEqualsToken = (SyntaxKind)8287;
1214
public const SyntaxKind ManagedKeyword = (SyntaxKind)8445;
1315
public const SyntaxKind UnmanagedKeyword = (SyntaxKind)8446;
1416
public const SyntaxKind NullableKeyword = (SyntaxKind)8486;

‎analyzers/src/SonarAnalyzer.CSharp/Rules/DoNotShiftByZeroOrIntSize.cs‎

Lines changed: 35 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,16 @@ namespace SonarAnalyzer.Rules.CSharp
3535
public sealed class DoNotShiftByZeroOrIntSize : SonarDiagnosticAnalyzer
3636
{
3737
private const string DiagnosticId = "S2183";
38-
private const string MessageFormat_UseLargerTypeOrPromote = "Either promote shift target to a larger integer type or shift by {0} instead.";
39-
private const string MessageFormat_ShiftTooLarge = "Correct this shift; shift by {0} instead.";
40-
private const string MessageFormat_RightShiftTooLarge = "Correct this shift; '{0}' is larger than the type size.";
41-
private const string MessageFormat_UselessShift = "Remove this useless shift by {0}.";
38+
private const string MessageFormatUseLargerTypeOrPromote = "Either promote shift target to a larger integer type or shift by {0} instead.";
39+
private const string MessageFormatShiftTooLarge = "Correct this shift; shift by {0} instead.";
40+
private const string MessageFormatRightShiftTooLarge = "Correct this shift; '{0}' is larger than the type size.";
41+
private const string MessageFormatUselessShift = "Remove this useless shift by {0}.";
4242

4343
private static readonly DiagnosticDescriptor Rule = DescriptorFactory.Create(DiagnosticId, "{0}");
4444

4545
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
4646

47-
private static ImmutableDictionary<KnownType, int> mapKnownTypesToIntegerBitSize
47+
private static readonly ImmutableDictionary<KnownType, int> MapKnownTypesToIntegerBitSize
4848
= new Dictionary<KnownType, int>
4949
{
5050
[KnownType.System_Int64] = 64,
@@ -60,10 +60,13 @@ private static ImmutableDictionary<KnownType, int> mapKnownTypesToIntegerBitSize
6060
[KnownType.System_SByte] = 32
6161
}.ToImmutableDictionary();
6262

63-
private enum Shift { Left, Right };
64-
65-
protected override void Initialize(SonarAnalysisContext context)
63+
private enum Shift
6664
{
65+
Left,
66+
Right
67+
}
68+
69+
protected override void Initialize(SonarAnalysisContext context) =>
6770
context.RegisterSyntaxNodeActionInNonGenerated(
6871
c =>
6972
{
@@ -97,17 +100,13 @@ protected override void Initialize(SonarAnalysisContext context)
97100
},
98101
SyntaxKind.MethodDeclaration,
99102
SyntaxKind.PropertyDeclaration);
100-
}
101103

102-
private static bool ContainsShiftExpressionWithinTwoLines(HashSet<int> linesWithShiftOperations,
103-
int lineNumber)
104-
{
105-
return linesWithShiftOperations.Contains(lineNumber - 2) ||
106-
linesWithShiftOperations.Contains(lineNumber - 1) ||
107-
linesWithShiftOperations.Contains(lineNumber) ||
108-
linesWithShiftOperations.Contains(lineNumber + 1) ||
109-
linesWithShiftOperations.Contains(lineNumber + 2);
110-
}
104+
private static bool ContainsShiftExpressionWithinTwoLines(HashSet<int> linesWithShiftOperations, int lineNumber) =>
105+
linesWithShiftOperations.Contains(lineNumber - 2)
106+
|| linesWithShiftOperations.Contains(lineNumber - 1)
107+
|| linesWithShiftOperations.Contains(lineNumber)
108+
|| linesWithShiftOperations.Contains(lineNumber + 1)
109+
|| linesWithShiftOperations.Contains(lineNumber + 2);
111110

112111
private static Tuple<Shift, ExpressionSyntax> GetRhsArgumentOfShiftNode(SyntaxNode node)
113112
{
@@ -117,7 +116,7 @@ private static Tuple<Shift, ExpressionSyntax> GetRhsArgumentOfShiftNode(SyntaxNo
117116
return new Tuple<Shift, ExpressionSyntax>(Shift.Left, binaryExpression.Right);
118117
}
119118

120-
if (binaryExpression?.OperatorToken.IsKind(SyntaxKind.GreaterThanGreaterThanToken) ?? false)
119+
if (binaryExpression?.OperatorToken.IsAnyKind(SyntaxKind.GreaterThanGreaterThanToken, SyntaxKindEx.GreaterThanGreaterThanGreaterThanToken) ?? false)
121120
{
122121
return new Tuple<Shift, ExpressionSyntax>(Shift.Right, binaryExpression.Right);
123122
}
@@ -128,7 +127,7 @@ private static Tuple<Shift, ExpressionSyntax> GetRhsArgumentOfShiftNode(SyntaxNo
128127
return new Tuple<Shift, ExpressionSyntax>(Shift.Left, assignmentExpession.Right);
129128
}
130129

131-
if (assignmentExpession?.OperatorToken.IsKind(SyntaxKind.GreaterThanGreaterThanEqualsToken) ?? false)
130+
if (assignmentExpession?.OperatorToken.IsAnyKind(SyntaxKind.GreaterThanGreaterThanEqualsToken, SyntaxKindEx.GreaterThanGreaterThanGreaterThanEqualsToken) ?? false)
132131
{
133132
return new Tuple<Shift, ExpressionSyntax>(Shift.Right, assignmentExpession.Right);
134133
}
@@ -139,8 +138,8 @@ private static Tuple<Shift, ExpressionSyntax> GetRhsArgumentOfShiftNode(SyntaxNo
139138
private static bool TryGetConstantValue(ExpressionSyntax expression, out int value)
140139
{
141140
value = 0;
142-
return expression?.RemoveParentheses() is LiteralExpressionSyntax literalExpression &&
143-
int.TryParse(literalExpression?.Token.ValueText, out value);
141+
return expression.RemoveParentheses() is LiteralExpressionSyntax literalExpression
142+
&& int.TryParse(literalExpression.Token.ValueText, out value);
144143
}
145144

146145
private static ShiftInstance FindShiftInstance(SyntaxNode node, SemanticModel semanticModel)
@@ -151,41 +150,29 @@ private static ShiftInstance FindShiftInstance(SyntaxNode node, SemanticModel se
151150
return null;
152151
}
153152

154-
if (!TryGetConstantValue(tuple.Item2, out var shiftByCount))
155-
{
156-
return new ShiftInstance(node);
157-
}
158-
159-
var typeSymbol = semanticModel.GetTypeInfo(node).ConvertedType;
160-
if (typeSymbol == null)
161-
{
162-
return new ShiftInstance(node);
163-
}
164-
165-
var variableBitLength = FindTypeSizeOrDefault(typeSymbol);
166-
if (variableBitLength == 0)
153+
if (!TryGetConstantValue(tuple.Item2, out var shiftByCount)
154+
|| semanticModel.GetTypeInfo(node).ConvertedType is not { } typeSymbol
155+
|| (FindTypeSizeOrDefault(typeSymbol) is var variableBitLength && variableBitLength == 0))
167156
{
168157
return new ShiftInstance(node);
169158
}
170159

171-
var issueDescription = FindProblemDescription(variableBitLength, shiftByCount, tuple.Item1, out bool isLiteralZero);
160+
var issueDescription = FindProblemDescription(variableBitLength, shiftByCount, tuple.Item1, out var isLiteralZero);
172161
return issueDescription == null ? new ShiftInstance(node) : new ShiftInstance(issueDescription, isLiteralZero, node);
173162
}
174163

175-
private static int FindTypeSizeOrDefault(ITypeSymbol typeSymbol)
176-
{
177-
return mapKnownTypesToIntegerBitSize
164+
private static int FindTypeSizeOrDefault(ITypeSymbol typeSymbol) =>
165+
MapKnownTypesToIntegerBitSize
178166
.Where(kv => typeSymbol.Is(kv.Key))
179167
.Select(kv => kv.Value)
180168
.FirstOrDefault();
181-
}
182169

183170
private static string FindProblemDescription(int typeSizeInBits, int shiftBy, Shift shiftDirection, out bool isLiteralZero)
184171
{
185172
if (shiftBy == 0)
186173
{
187174
isLiteralZero = true;
188-
return string.Format(MessageFormat_UselessShift, 0);
175+
return string.Format(MessageFormatUselessShift, 0);
189176
}
190177

191178
isLiteralZero = false;
@@ -197,37 +184,35 @@ private static string FindProblemDescription(int typeSizeInBits, int shiftBy, Sh
197184

198185
if (shiftDirection == Shift.Right)
199186
{
200-
return string.Format(MessageFormat_RightShiftTooLarge, shiftBy);
187+
return string.Format(MessageFormatRightShiftTooLarge, shiftBy);
201188
}
202189

203190
var shiftSuggestion = shiftBy % typeSizeInBits;
204191

205192
if (typeSizeInBits == 64)
206193
{
207194
return shiftSuggestion == 0
208-
? string.Format(MessageFormat_UselessShift, shiftBy)
209-
: string.Format(MessageFormat_ShiftTooLarge, shiftSuggestion);
195+
? string.Format(MessageFormatUselessShift, shiftBy)
196+
: string.Format(MessageFormatShiftTooLarge, shiftSuggestion);
210197
}
211198

212199
if (shiftSuggestion == 0)
213200
{
214-
return string.Format(MessageFormat_UseLargerTypeOrPromote,
201+
return string.Format(MessageFormatUseLargerTypeOrPromote,
215202
"less than " + typeSizeInBits);
216203
}
217204

218-
return string.Format(MessageFormat_UseLargerTypeOrPromote, shiftSuggestion);
205+
return string.Format(MessageFormatUseLargerTypeOrPromote, shiftSuggestion);
219206
}
220207

221-
private class ShiftInstance
208+
private sealed class ShiftInstance
222209
{
223210
public Diagnostic Diagnostic { get; }
224211
public bool IsLiteralZero { get; }
225212
public int Line { get; }
226213

227-
public ShiftInstance(SyntaxNode node)
228-
{
214+
public ShiftInstance(SyntaxNode node) =>
229215
Line = node.GetLineNumberToReport();
230-
}
231216

232217
public ShiftInstance(string description, bool isLieralZero, SyntaxNode node)
233218
: this(node)

‎analyzers/tests/SonarAnalyzer.UnitTest/TestCases/DoNotShiftByZeroOrIntSize.CSharp11.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@ class Program
44
{
55
void UnsignedRightShiftOperator(int i, ulong q)
66
{
7-
i = i >>> 60; // FN
7+
i = i >>> 60; // Noncompliant
88
i = i >>> 31; // Compliant
9-
i >>>= 40; // FN
9+
i >>>= 40; // Noncompliant
1010

11-
q = q >>> 64; // FN
11+
q = q >>> 64; // Noncompliant
1212
q = q >>> 0; // Compliant
13-
q >>>= 70; // FN
13+
q >>>= 70; // Noncompliant
1414
}
1515
}

‎analyzers/tests/SonarAnalyzer.UnitTest/TestCases/DoNotShiftByZeroOrIntSize.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ private void Test()
2828

2929
ul <<= 0;
3030
ul <<= 1025; // Noncompliant {{Correct this shift; shift by 1 instead.}}
31+
ul <<= "I am not an integer"; // Error [CS0019]
3132

3233
b <<= 16;
3334
b <<= 17;

0 commit comments

Comments
 (0)