Skip to content

Commit 93edaa7

Browse files
hvitvedhvitved
authored
Merge pull request #4309 from tamasvajk/feature/enum-value-init
Extract constant value of enum member equal clauses
2 parents 75262dd + a635503 commit 93edaa7

File tree

12 files changed

+91
-17
lines changed

12 files changed

+91
-17
lines changed

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Field.cs‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,12 @@ public override void Populate(TextWriter trapFile)
6868
Context.PopulateLater(() =>
6969
{
7070
var loc = Context.Create(initializer.GetLocation());
71-
var simpleAssignExpr = new Expression(new ExpressionInfo(Context, Type, loc, ExprKind.SIMPLE_ASSIGN, this, child++, false, null));
72-
Expression.CreateFromNode(new ExpressionNodeInfo(Context, initializer.Initializer.Value, simpleAssignExpr, 0));
73-
var access = new Expression(new ExpressionInfo(Context, Type, Location, ExprKind.FIELD_ACCESS, simpleAssignExpr, 1, false, null));
74-
trapFile.expr_access(access, this);
71+
72+
var fieldAccess = AddInitializerAssignment(trapFile, initializer.Initializer.Value, loc, null, ref child);
73+
7574
if (!symbol.IsStatic)
7675
{
77-
This.CreateImplicit(Context, Entities.Type.Create(Context, symbol.ContainingType), Location, access, -1);
76+
This.CreateImplicit(Context, Entities.Type.Create(Context, symbol.ContainingType), Location, fieldAccess, -1);
7877
}
7978
});
8079
}
@@ -85,8 +84,13 @@ public override void Populate(TextWriter trapFile)
8584
Where(n => n.EqualsValue != null))
8685
{
8786
// Mark fields that have explicit initializers.
88-
var expr = new Expression(new ExpressionInfo(Context, Type, Context.Create(initializer.EqualsValue.Value.FixedLocation()), Kinds.ExprKind.FIELD_ACCESS, this, child++, false, null));
89-
trapFile.expr_access(expr, this);
87+
var constValue = symbol.HasConstantValue
88+
? Expression.ValueAsString(symbol.ConstantValue)
89+
: null;
90+
91+
var loc = Context.Create(initializer.GetLocation());
92+
93+
AddInitializerAssignment(trapFile, initializer.EqualsValue.Value, loc, constValue, ref child);
9094
}
9195

9296
if (IsSourceDeclaration)
@@ -96,6 +100,16 @@ public override void Populate(TextWriter trapFile)
96100
TypeMention.Create(Context, syntax.Type, this, Type);
97101
}
98102

103+
private Expression AddInitializerAssignment(TextWriter trapFile, ExpressionSyntax initializer, Extraction.Entities.Location loc,
104+
string constValue, ref int child)
105+
{
106+
var simpleAssignExpr = new Expression(new ExpressionInfo(Context, Type, loc, ExprKind.SIMPLE_ASSIGN, this, child++, false, constValue));
107+
Expression.CreateFromNode(new ExpressionNodeInfo(Context, initializer, simpleAssignExpr, 0));
108+
var access = new Expression(new ExpressionInfo(Context, Type, Location, ExprKind.FIELD_ACCESS, simpleAssignExpr, 1, false, constValue));
109+
trapFile.expr_access(access, this);
110+
return access;
111+
}
112+
99113
readonly Lazy<AnnotatedType> type;
100114
public AnnotatedType Type => type.Value;
101115

‎csharp/ql/src/Likely Bugs/SelfAssignment.ql‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ class StructuralComparisonConfig extends StructuralComparisonConfiguration {
2020
exists(AssignExpr ae |
2121
// Member initializers are never self-assignments, in particular
2222
// not initializers such as `new C { F = F };`
23-
not ae instanceof MemberInitializer
23+
not ae instanceof MemberInitializer and
24+
// Enum field initializers are never self assignments. `enum E { A = 42 }`
25+
not ae.getParent().(Field).getDeclaringType() instanceof Enum
2426
|
2527
ae.getLValue() = x and
2628
ae.getRValue() = y

‎csharp/ql/test/library-tests/dataflow/signanalysis/MissingSign.expected‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +0,0 @@
1-
| SignAnalysis.cs:428:23:428:24 | access to constant A |

‎csharp/ql/test/library-tests/dataflow/signanalysis/SignAnalysis.expected‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,9 @@
211211
| SignAnalysis.cs:414:13:414:13 | access to local variable i | strictlyPositive |
212212
| SignAnalysis.cs:415:31:415:31 | access to local variable i | strictlyPositive |
213213
| SignAnalysis.cs:424:31:424:31 | access to local variable x | strictlyNegative |
214+
| SignAnalysis.cs:428:19:428:19 | access to constant A | strictlyPositive |
215+
| SignAnalysis.cs:428:19:428:24 | ... = ... | strictlyPositive |
216+
| SignAnalysis.cs:428:23:428:24 | 12 | strictlyPositive |
214217
| SignAnalysis.cs:434:38:434:38 | access to local variable i | strictlyNegative |
215218
| SignAnalysis.cs:443:38:443:38 | access to local variable x | strictlyNegative |
216219
| SignAnalysis.cs:446:31:446:32 | 10 | strictlyPositive |

‎csharp/ql/test/library-tests/definitions/PrintAst.expected‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,13 @@ definitions.cs:
88
# 9| 4: [BlockStmt] {...}
99
# 13| 2: [Enum] Enumeration
1010
# 15| 5: [Field] e1
11-
# 15| 1: [MemberConstantAccess] access to constant e1
11+
# 15| 1: [AssignExpr] ... = ...
12+
# 15| 0: [IntLiteral] 1
13+
# 15| 1: [MemberConstantAccess] access to constant e1
1214
# 15| 6: [Field] e2
13-
# 15| 1: [MemberConstantAccess] access to constant e2
15+
# 15| 1: [AssignExpr] ... = ...
16+
# 15| 0: [IntLiteral] 2
17+
# 15| 1: [MemberConstantAccess] access to constant e2
1418
# 15| 7: [Field] e3
1519
# 18| 3: [Class] C1
1620
# 20| 4: [InstanceConstructor] C1

‎csharp/ql/test/library-tests/enums/Enums11.expected‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
| enums.cs:28:18:28:18 | (...) ... | 1 |
2+
| enums.cs:29:20:29:20 | (...) ... | 2 |
3+
| enums.cs:30:20:30:20 | (...) ... | 4 |
4+
| enums.cs:38:17:38:18 | 10 | 10 |
5+
| enums.cs:40:23:40:32 | ... + ... | 11 |

‎csharp/ql/test/library-tests/enums/Enums11.ql‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/**
2+
* @name Test for enums
3+
*/
4+
5+
import csharp
6+
7+
from Expr e
8+
where
9+
exists(Assignment a | a.getRValue() = e |
10+
a.getParent().(Field).getDeclaringType() instanceof Enum
11+
)
12+
select e, e.getValue()

‎csharp/ql/test/library-tests/enums/PrintAst.expected‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,35 @@ enums.cs:
1111
# 23| 3: [Enum] E
1212
# 25| 4: [Enum] ValueColor
1313
# 28| 5: [Field] OneRed
14-
# 28| 1: [MemberConstantAccess] access to constant OneRed
14+
# 28| 1: [AssignExpr] ... = ...
15+
# 28| 0: [CastExpr] (...) ...
16+
# 28| 0: [IntLiteral] 1
17+
# 28| 1: [MemberConstantAccess] access to constant OneRed
1518
# 29| 6: [Field] TwoGreen
16-
# 29| 1: [MemberConstantAccess] access to constant TwoGreen
19+
# 29| 1: [AssignExpr] ... = ...
20+
# 29| 0: [CastExpr] (...) ...
21+
# 29| 0: [IntLiteral] 2
22+
# 29| 1: [MemberConstantAccess] access to constant TwoGreen
1723
# 30| 7: [Field] FourBlue
18-
# 30| 1: [MemberConstantAccess] access to constant FourBlue
24+
# 30| 1: [AssignExpr] ... = ...
25+
# 30| 0: [CastExpr] (...) ...
26+
# 30| 0: [IntLiteral] 4
27+
# 30| 1: [MemberConstantAccess] access to constant FourBlue
1928
# 34| 5: [Enum] SparseColor
2029
# 37| 5: [Field] Red
2130
# 38| 6: [Field] Green
22-
# 38| 1: [MemberConstantAccess] access to constant Green
31+
# 38| 1: [AssignExpr] ... = ...
32+
# 38| 0: [IntLiteral] 10
33+
# 38| 1: [MemberConstantAccess] access to constant Green
2334
# 39| 7: [Field] Blue
2435
# 40| 8: [Field] AnotherBlue
25-
# 40| 1: [MemberConstantAccess] access to constant AnotherBlue
36+
# 40| 1: [AssignExpr] ... = ...
37+
# 40| 0: [AddExpr] ... + ...
38+
# 40| 0: [CastExpr] (...) ...
39+
# 40| 0: [MemberConstantAccess] access to constant Blue
40+
# 40| 1: [CastExpr] (...) ...
41+
# 40| 0: [MemberConstantAccess] access to constant Red
42+
# 40| 1: [MemberConstantAccess] access to constant AnotherBlue
2643
# 44| 6: [Class] Test
2744
# 47| 5: [Method] Main
2845
# 48| 4: [BlockStmt] {...}

‎csharp/ql/test/library-tests/enums/enums.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ enum SparseColor
3737
Red,
3838
Green = 10,
3939
Blue,
40-
AnotherBlue = Blue
40+
AnotherBlue = Blue + Red
4141

4242
}
4343

‎csharp/ql/test/query-tests/Language Abuse/UselessCastToSelf/UselessCastToSelf.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,13 @@ void f()
1818
var good6 = (Action<int>)(delegate (int x) { });
1919
var good7 = (Action<int>)((int x) => { });
2020
}
21+
22+
enum Enum
23+
{
24+
A = 2,
25+
B = 1 | A,
26+
C = 1 | (int)A, // BAD
27+
D = 9 | (32 << A),
28+
E = 9 | (32 << (int)A) // BAD
29+
}
2130
}

0 commit comments

Comments
 (0)