Skip to content

Commit 992a4df

Browse files
tamasvajktamasvajk
authored
Merge pull request github#5619 from tamasvajk/feature/fix-default-argument-value-extraction
C# Improve default argument value extraction
2 parents 652e8b4 + 46197e6 commit 992a4df

File tree

24 files changed

+213
-37
lines changed

24 files changed

+213
-37
lines changed

‎csharp/change-notes/2021-04-09-default-argument-values.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* The extractor has been improved to store default argument values for parameters that are extracted from referenced assemblies.

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,39 @@ private static bool ContainsPattern(SyntaxNode node) =>
164164
}
165165
}
166166

167+
/// <summary>
168+
/// Creates a generated expression for a default argument value.
169+
/// </summary>
170+
public static Expression? CreateGenerated(Context cx, IParameterSymbol parameter, IExpressionParentEntity parent,
171+
int childIndex, Extraction.Entities.Location location)
172+
{
173+
if (!parameter.HasExplicitDefaultValue)
174+
{
175+
return null;
176+
}
177+
178+
var defaultValue = parameter.ExplicitDefaultValue;
179+
180+
if (parameter.Type is INamedTypeSymbol nt && nt.EnumUnderlyingType is not null)
181+
{
182+
// = (MyEnum)1, = MyEnum.Value1, = default(MyEnum), = new MyEnum()
183+
// we're generating a (MyEnum)value cast expression:
184+
defaultValue ??= 0;
185+
Action<Expression, int> createChild = (parent, index) => Literal.CreateGenerated(cx, parent, index, nt.EnumUnderlyingType, defaultValue, location);
186+
return Cast.CreateGenerated(cx, parent, childIndex, parameter.Type, defaultValue, createChild, location);
187+
}
188+
189+
if (defaultValue is null)
190+
{
191+
// = null, = default, = default(T), = new MyStruct()
192+
// we're generating a default expression:
193+
return Default.CreateGenerated(cx, parent, childIndex, location, parameter.Type.IsReferenceType ? ValueAsString(null) : null);
194+
}
195+
196+
// const literal:
197+
return Literal.CreateGenerated(cx, parent, childIndex, parameter.Type, defaultValue, location);
198+
}
199+
167200
/// <summary>
168201
/// Adapt the operator kind depending on whether it's a dynamic call or a user-operator call.
169202
/// </summary>

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,20 @@ protected override void PopulateExpression(TextWriter trapFile)
1414
{
1515
TypeAccess.Create(Context, Syntax.Type, this, 0);
1616
}
17+
18+
public static Expression CreateGenerated(Context cx, IExpressionParentEntity parent, int childIndex, Extraction.Entities.Location location, string? value)
19+
{
20+
var info = new ExpressionInfo(
21+
cx,
22+
null,
23+
location,
24+
ExprKind.DEFAULT,
25+
parent,
26+
childIndex,
27+
true,
28+
value);
29+
30+
return new Expression(info);
31+
}
1732
}
1833
}

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

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using Microsoft.CodeAnalysis.CSharp.Syntax;
55
using Semmle.Extraction.Entities;
66
using System.IO;
7+
using System;
78

89
namespace Semmle.Extraction.CSharp.Entities
910
{
@@ -124,6 +125,17 @@ Symbol.ContainingSymbol is IMethodSymbol ms &&
124125
trapFile.param_location(this, Context.CreateLocation());
125126
}
126127

128+
if (Symbol.HasExplicitDefaultValue && Context.Defines(Symbol))
129+
{
130+
var defaultValueSyntax = GetDefaultValueFromSyntax(Symbol);
131+
132+
Action defaultValueExpressionCreation = defaultValueSyntax is not null
133+
? () => Expression.Create(Context, defaultValueSyntax.Value, this, 0)
134+
: () => Expression.CreateGenerated(Context, Symbol, this, 0, Location);
135+
136+
Context.PopulateLater(defaultValueExpressionCreation);
137+
}
138+
127139
if (!IsSourceDeclaration || !Symbol.FromSource())
128140
return;
129141

@@ -139,36 +151,28 @@ Symbol.ContainingSymbol is IMethodSymbol ms &&
139151
TypeMention.Create(Context, syntax.Type!, this, type);
140152
}
141153
}
154+
}
142155

143-
if (Symbol.HasExplicitDefaultValue && Context.Defines(Symbol))
156+
private static EqualsValueClauseSyntax? GetDefaultValueFromSyntax(IParameterSymbol symbol)
157+
{
158+
// This is a slight bug in the dbscheme
159+
// We should really define param_default(param, string)
160+
// And use parameter child #0 to encode the default expression.
161+
var defaultValue = GetParameterDefaultValue(symbol);
162+
if (defaultValue is null)
144163
{
145-
// This is a slight bug in the dbscheme
146-
// We should really define param_default(param, string)
147-
// And use parameter child #0 to encode the default expression.
148-
var defaultValue = GetParameterDefaultValue(Symbol);
149-
if (defaultValue is null)
150-
{
151-
// In case this parameter belongs to an accessor of an indexer, we need
152-
// to get the default value from the corresponding parameter belonging
153-
// to the indexer itself
154-
var method = (IMethodSymbol)Symbol.ContainingSymbol;
155-
if (method is not null)
156-
{
157-
var i = method.Parameters.IndexOf(Symbol);
158-
var indexer = (IPropertySymbol?)method.AssociatedSymbol;
159-
if (indexer is not null)
160-
defaultValue = GetParameterDefaultValue(indexer.Parameters[i]);
161-
}
162-
}
163-
164-
if (defaultValue is not null)
164+
// In case this parameter belongs to an accessor of an indexer, we need
165+
// to get the default value from the corresponding parameter belonging
166+
// to the indexer itself
167+
if (symbol.ContainingSymbol is IMethodSymbol method)
165168
{
166-
Context.PopulateLater(() =>
167-
{
168-
Expression.Create(Context, defaultValue.Value, this, 0);
169-
});
169+
var i = method.Parameters.IndexOf(symbol);
170+
if (method.AssociatedSymbol is IPropertySymbol indexer)
171+
defaultValue = GetParameterDefaultValue(indexer.Parameters[i]);
170172
}
171173
}
174+
175+
return defaultValue;
172176
}
173177

174178
public override bool IsSourceDeclaration => Symbol.IsSourceDeclaration();

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,6 @@ class Element extends DotNet::Element, @element {
2727
/** Gets a location of this element, including sources and assemblies. */
2828
override Location getALocation() { none() }
2929

30-
/** Holds if this element is from an assembly. */
31-
predicate fromLibrary() { this.getFile().fromLibrary() }
32-
3330
/** Gets the parent of this element, if any. */
3431
Element getParent() { result.getAChild() = this }
3532

‎csharp/ql/src/semmle/code/dotnet/Element.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ class Element extends @dotnet_element {
2828
/** Holds if this element is from source code. */
2929
predicate fromSource() { this.getFile().fromSource() }
3030

31+
/** Holds if this element is from an assembly. */
32+
predicate fromLibrary() { this.getFile().fromLibrary() }
33+
3134
/**
3235
* Gets the "language" of this program element, as defined by the extension of the filename.
3336
* For example, C# has language "cs", and Visual Basic has language "vb".

‎csharp/ql/test/library-tests/controlflow/graph/Common.qll

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,15 @@ class SourceControlFlowElement extends ControlFlowElement {
1111
}
1212

1313
class SourceControlFlowNode extends ControlFlow::Node {
14-
SourceControlFlowNode() { not this.getLocation().getFile() instanceof StubFile }
14+
SourceControlFlowNode() {
15+
not this.getLocation().getFile() instanceof StubFile and
16+
not this.getLocation().getFile().fromLibrary()
17+
}
1518
}
1619

1720
class SourceBasicBlock extends ControlFlow::BasicBlock {
18-
SourceBasicBlock() { not this.getLocation().getFile() instanceof StubFile }
21+
SourceBasicBlock() {
22+
not this.getLocation().getFile() instanceof StubFile and
23+
not this.getLocation().getFile().fromLibrary()
24+
}
1925
}

‎csharp/ql/test/library-tests/controlflow/splits/SplittingStressTest.ql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import csharp
22

33
query predicate countSplits(ControlFlowElement cfe, int i) {
4+
not cfe.fromLibrary() and
45
i = strictcount(ControlFlow::Nodes::ElementNode n | n.getElement() = cfe)
56
}
67

‎csharp/ql/test/library-tests/csharp7.1/DefaultLiterals.ql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import csharp
22

33
from DefaultValueExpr l
4+
where l.fromSource()
45
select l, l.getValue()

‎csharp/ql/test/library-tests/csharp7/LocalTaintFlow.ql

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,7 @@ import csharp
22
import semmle.code.csharp.dataflow.TaintTracking
33

44
from DataFlow::Node pred, DataFlow::Node succ
5-
where TaintTracking::localTaintStep(pred, succ)
5+
where
6+
TaintTracking::localTaintStep(pred, succ) and
7+
not pred.asExpr().fromLibrary()
68
select pred, succ

0 commit comments

Comments
 (0)