Skip to content

Commit 3e93aa9

Browse files
hvitvedhvitved
committed
C#: Address review comments
- Undo split of `localvars` relation. - Properly extract tuple declarations in `is` expressions.
1 parent a062d7d commit 3e93aa9

File tree

13 files changed

+339
-3888
lines changed

13 files changed

+339
-3888
lines changed

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/IsPattern.cs‎

Lines changed: 2 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public static Expression CreatePattern(this Context cx, PatternSyntax syntax, IE
4848
{
4949
var type = Type.Create(cx, symbol.GetAnnotatedType());
5050

51-
return VariableDeclaration.Create(cx, symbol, type, null, cx.Create(syntax.GetLocation()), false, parent, child);
51+
return VariableDeclaration.Create(cx, symbol, type, null, cx.Create(syntax.GetLocation()), true, parent, child);
5252
}
5353
else
5454
{
@@ -138,38 +138,10 @@ private IsPattern(ExpressionNodeInfo info) : base(info.SetKind(ExprKind.IS))
138138
{
139139
}
140140

141-
private void PopulatePattern(PatternSyntax pattern, TypeSyntax optionalType, SyntaxToken varKeyword, VariableDesignationSyntax designation)
142-
{
143-
var isVar = optionalType is null;
144-
if (!(designation is null) && cx.GetModel(pattern).GetDeclaredSymbol(designation) is ILocalSymbol symbol)
145-
{
146-
var type = Entities.Type.Create(cx, symbol.GetAnnotatedType());
147-
VariableDeclaration.Create(cx, symbol, type, optionalType, cx.Create(pattern.GetLocation()), isVar, this, 1);
148-
}
149-
else if (!isVar)
150-
Expressions.TypeAccess.Create(cx, optionalType, this, 1);
151-
}
152-
153141
protected override void PopulateExpression(TextWriter trapFile)
154142
{
155143
Create(cx, Syntax.Expression, this, 0);
156-
switch (Syntax.Pattern)
157-
{
158-
case ConstantPatternSyntax constantPattern:
159-
Create(cx, constantPattern.Expression, this, 1);
160-
return;
161-
case VarPatternSyntax varPattern:
162-
PopulatePattern(varPattern, null, varPattern.VarKeyword, varPattern.Designation);
163-
return;
164-
case DeclarationPatternSyntax declPattern:
165-
PopulatePattern(declPattern, declPattern.Type, default(SyntaxToken), declPattern.Designation);
166-
return;
167-
case RecursivePatternSyntax recPattern:
168-
new RecursivePattern(cx, recPattern, this, 1);
169-
return;
170-
default:
171-
throw new InternalError(Syntax, "Is pattern not handled");
172-
}
144+
cx.CreatePattern(Syntax.Pattern, this, 1);
173145
}
174146

175147
public static Expression Create(ExpressionNodeInfo info) => new IsPattern(info).TryPopulate();

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/VariableDeclaration.cs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public static VariableDeclaration Create(Context cx, ISymbol symbol, AnnotatedTy
1717
cx.Try(null, null, () =>
1818
{
1919
var l = LocalVariable.Create(cx, symbol);
20-
l.PopulateInfo(ret, isVar);
20+
l.PopulateManual(ret, isVar);
2121
if (optionalSyntax != null)
2222
TypeMention.Create(cx, optionalSyntax, parent, type);
2323
});
@@ -38,7 +38,7 @@ static VariableDeclaration CreateSingle(Context cx, DeclarationExpressionSyntax
3838
cx.Try(null, null, () =>
3939
{
4040
var l = LocalVariable.Create(cx, variableSymbol);
41-
l.PopulateInfo(ret, node.Type.IsVar);
41+
l.PopulateManual(ret, node.Type.IsVar);
4242
});
4343
return ret;
4444
}
@@ -81,7 +81,7 @@ public static Expression CreateParenthesized(Context cx, VarPatternSyntax varPat
8181
{
8282
var decl = Create(cx, variable, Entities.Type.Create(cx, local.GetAnnotatedType()), tuple, child0++);
8383
var l = LocalVariable.Create(cx, local);
84-
l.PopulateInfo(decl, true);
84+
l.PopulateManual(decl, true);
8585
}
8686
else
8787
{
@@ -133,7 +133,7 @@ public static VariableDeclaration Create(Context cx, CatchDeclarationSyntax d, b
133133
{
134134
var declSymbol = cx.GetModel(d).GetDeclaredSymbol(d);
135135
var l = LocalVariable.Create(cx, declSymbol);
136-
l.PopulateInfo(ret, isVar);
136+
l.PopulateManual(ret, isVar);
137137
TypeMention.Create(cx, d.Type, ret, type);
138138
});
139139
return ret;
@@ -146,7 +146,7 @@ public static VariableDeclaration CreateDeclarator(Context cx, VariableDeclarato
146146
{
147147
var declSymbol = cx.GetModel(d).GetDeclaredSymbol(d);
148148
var localVar = LocalVariable.Create(cx, declSymbol);
149-
localVar.PopulateInfo(ret, isVar);
149+
localVar.PopulateManual(ret, isVar);
150150

151151
if (d.Initializer != null)
152152
{

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,29 +15,29 @@ public override void WriteId(TextWriter trapFile)
1515
trapFile.Write(";localvar");
1616
}
1717

18-
public override void Populate(TextWriter trapFile)
18+
public override void Populate(TextWriter trapFile) { }
19+
20+
public void PopulateManual(Expression parent, bool isVar)
1921
{
22+
var trapFile = Context.TrapWriter.Writer;
23+
var (kind, type) =
24+
symbol is ILocalSymbol l ?
25+
(l.IsRef ? 3 : l.IsConst ? 2 : 1, Type.Create(Context, l.GetAnnotatedType())) :
26+
(1, parent.Type);
27+
trapFile.localvars(this, kind, symbol.Name, isVar ? 1 : 0, type.Type.TypeRef, parent);
28+
2029
if (symbol is ILocalSymbol local)
2130
{
2231
PopulateNullability(trapFile, local.GetAnnotatedType());
2332
if (local.IsRef)
2433
trapFile.type_annotation(this, Kinds.TypeAnnotation.Ref);
2534
}
2635

27-
var kind = symbol is ILocalSymbol l ? (l.IsRef ? 3 : l.IsConst ? 2 : 1) : 1;
28-
trapFile.localvars(this, kind, symbol.Name);
29-
3036
trapFile.localvar_location(this, Location);
3137

3238
DefineConstantValue(trapFile);
3339
}
3440

35-
public void PopulateInfo(Expression parent, bool isVar)
36-
{
37-
var type = symbol is ILocalSymbol l ? Type.Create(Context, l.GetAnnotatedType()) : parent.Type;
38-
Context.TrapWriter.Writer.localvar_info(this, isVar ? 1 : 0, type.Type.TypeRef, parent);
39-
}
40-
4141
public static LocalVariable Create(Context cx, ISymbol local)
4242
{
4343
return LocalVariableFactory.Instance.CreateEntity(cx, local);

‎csharp/extractor/Semmle.Extraction.CSharp/Tuples.cs‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -326,14 +326,9 @@ internal static void localvar_location(this TextWriter trapFile, LocalVariable v
326326
trapFile.WriteTuple("localvar_location", var, location);
327327
}
328328

329-
internal static void localvars(this TextWriter trapFile, LocalVariable key, int @const, string name)
329+
internal static void localvars(this TextWriter trapFile, LocalVariable key, int @const, string name, int @var, Type type, Expression expr)
330330
{
331-
trapFile.WriteTuple("localvars", key, @const, name);
332-
}
333-
334-
internal static void localvar_info(this TextWriter trapFile, LocalVariable key, int @var, Type type, Expression expr)
335-
{
336-
trapFile.WriteTuple("localvar_info", key, @var, type, expr);
331+
trapFile.WriteTuple("localvars", key, @const, name, @var, type, expr);
337332
}
338333

339334
public static void metadata_handle(this TextWriter trapFile, IEntity entity, Location assembly, int handleValue)

‎csharp/ql/src/semmle/code/csharp/Variable.qll‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -314,22 +314,22 @@ class LocalVariable extends LocalScopeVariable, @local_variable {
314314
* }
315315
* ```
316316
*/
317-
predicate isImplicitlyTyped() { localvar_info(this, 1, _, _) }
317+
predicate isImplicitlyTyped() { localvars(this, _, _, 1, _, _) }
318318

319319
/** Gets the enclosing callable of this local variable. */
320320
Callable getEnclosingCallable() { result = this.getVariableDeclExpr().getEnclosingCallable() }
321321

322322
override Callable getCallable() { result = getEnclosingCallable() }
323323

324-
override predicate isRef() { localvars(this, 3, _) }
324+
override predicate isRef() { localvars(this, 3, _, _, _, _) }
325325

326326
override ValueOrRefType getDeclaringType() {
327327
result = this.getVariableDeclExpr().getEnclosingCallable().getDeclaringType()
328328
}
329329

330-
override string getName() { localvars(this, _, result) }
330+
override string getName() { localvars(this, _, result, _, _, _) }
331331

332-
override Type getType() { localvar_info(this, _, getTypeRef(result), _) }
332+
override Type getType() { localvars(this, _, _, _, getTypeRef(result), _) }
333333

334334
override Location getALocation() { localvar_location(this, result) }
335335
}

‎csharp/ql/src/semmle/code/csharp/exprs/Expr.qll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ private predicate isDynamicElementAccess(@dynamic_element_access_expr e) { any()
117117
*/
118118
class LocalVariableDeclExpr extends Expr, @local_var_decl_expr {
119119
/** Gets the local variable being declared. */
120-
LocalVariable getVariable() { localvar_info(result, _, _, this) }
120+
LocalVariable getVariable() { localvars(result, _, _, _, _, this) }
121121

122122
/** Gets the name of the variable being declared. */
123123
string getName() { result = this.getVariable().getName() }
@@ -157,7 +157,7 @@ class LocalVariableDeclExpr extends Expr, @local_var_decl_expr {
157157
class LocalConstantDeclExpr extends LocalVariableDeclExpr {
158158
LocalConstantDeclExpr() { super.getVariable() instanceof LocalConstant }
159159

160-
override LocalConstant getVariable() { localvar_info(result, _, _, this) }
160+
override LocalConstant getVariable() { localvars(result, _, _, _, _, this) }
161161
}
162162

163163
/**

‎csharp/ql/src/semmlecode.csharp.dbscheme‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,10 @@ field_location(
727727
localvars(
728728
unique int id: @local_variable,
729729
int kind: int ref,
730-
string name: string ref);
730+
string name: string ref,
731+
int implicitly_typed: int ref /* 0 = no, 1 = yes */,
732+
int type_id: @type_or_ref ref,
733+
int parent_id: @local_var_decl_expr ref);
731734

732735
case @local_variable.kind of
733736
1 = @addressable_local_variable
@@ -739,12 +742,6 @@ localvar_location(
739742
unique int id: @local_variable ref,
740743
int loc: @location ref);
741744

742-
localvar_info(
743-
unique int id: @local_variable ref,
744-
int implicitly_typed: int ref /* 0 = no, 1 = yes */,
745-
int type_id: @type_or_ref ref,
746-
int parent_id: @local_var_decl_expr ref);
747-
748745
@parameterizable = @callable | @delegate_type | @indexer;
749746

750747
#keyset[name, parent_id]

0 commit comments

Comments
 (0)