Skip to content

C#: Fix various extraction errors #1555

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hvitved merged 9 commits into from
Aug 30, 2019
Merged

C#: Fix various extraction errors #1555

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ae36359
C#: Fix typementions of arrays.
calumgrant Jul 4, 2019
8aeeec0
C#: Add namespace_access_expr because of nameof(Namespace)
calumgrant Jul 4, 2019
0e62377
C#: Treat _ as an ordinary variable in a foreach.
calumgrant Jul 4, 2019
5613769
C#: Add NamespaceAccess, and make namespaces declarations.
calumgrant Jul 5, 2019
dfbb194
C#: Minor edits
calumgrant Jul 5, 2019
83ab044
C#: Update expected test output.
calumgrant Jul 8, 2019
83d1e0e
C#: Name DotNet::Namespace a declaration.
calumgrant Aug 23, 2019
424ab3e
C#: Analysis change notes.
calumgrant Aug 29, 2019
4dd4167
C#: Update test output (following merge).
calumgrant Aug 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions change-notes/1.23/analysis-csharp.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Improvements to C# analysis

The following changes in version 1.23 affect C# analysis in all applications.

## Changes to existing queries

| **Query** | **Expected impact** | **Change** |
|------------------------------|------------------------|-----------------------------------|

## Removal of old queries

## Changes to code extraction

* `nameof` expressions are now extracted correctly when the name is a namespace.

## Changes to QL libraries

* The new class `NamespaceAccess` models accesses to namespaces, for example in `nameof` expressions.

## Changes to autobuilder

3 changes: 3 additions & 0 deletions csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Access.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ static ExprKind AccessKind(Context cx, ISymbol symbol)
case SymbolKind.Parameter:
return ExprKind.PARAMETER_ACCESS;

case SymbolKind.Namespace:
return ExprKind.NAMESPACE_ACCESS;

default:
cx.ModelError(symbol, $"Unhandled access kind '{symbol.Kind}'");
return ExprKind.UNKNOWN;
Expand Down
3 changes: 3 additions & 0 deletions csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Name.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ public static Expression Create(ExpressionNodeInfo info)
case SymbolKind.Parameter:
return Access.Create(info, target, false, Parameter.GetAlreadyCreated(info.Context, (IParameterSymbol)target));

case SymbolKind.Namespace:
return Access.Create(info, target, false, Namespace.Create(info.Context, (INamespaceSymbol)target));

default:
throw new InternalError(info.Node, $"Unhandled identifier kind '{target.Kind}'");
}
Expand Down
5 changes: 1 addition & 4 deletions csharp/extractor/Semmle.Extraction.CSharp/Entities/Statements/ForEach.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,7 @@ protected override void Populate()

var location = cx.Create(Stmt.Identifier.GetLocation());

if (typeSymbol.Name != "_")
Expressions.VariableDeclaration.Create(cx, typeSymbol, type, Stmt.Type, location, location, Stmt.Type.IsVar, this, 0);
else
TypeMention.Create(cx, Stmt.Type, this, type);
Expressions.VariableDeclaration.Create(cx, typeSymbol, type, Stmt.Type, location, location, Stmt.Type.IsVar, this, 0);

Statement.Create(cx, Stmt.Statement, this, 2);
}
Expand Down
30 changes: 27 additions & 3 deletions csharp/extractor/Semmle.Extraction.CSharp/Entities/TypeMention.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,39 @@ class TypeMention : FreshEntity
Loc = loc;
}

static TypeSyntax GetElementType(TypeSyntax type)
{
switch (type)
{
case ArrayTypeSyntax ats:
return GetElementType(ats.ElementType);
case NullableTypeSyntax nts:
return GetElementType(nts.ElementType);
default:
return type;
}
}

static Type GetElementType(Type type)
{
switch (type)
{
case ArrayType at:
return GetElementType(at.ElementType.Type);
case NamedType nt when nt.symbol.IsBoundNullable():
return nt.TypeArguments.Single();
default:
return type;
}
}

void Populate()
{
switch (Syntax.Kind())
{
case SyntaxKind.ArrayType:
var ats = (ArrayTypeSyntax)Syntax;
var at = (ArrayType)Type;
Emit(Loc ?? Syntax.GetLocation(), Parent, Type);
Create(cx, ats.ElementType, this, at.ElementType);
Create(cx, GetElementType(Syntax), this, GetElementType(Type));
return;
case SyntaxKind.NullableType:
var nts = (NullableTypeSyntax)Syntax;
Expand Down
3 changes: 2 additions & 1 deletion csharp/extractor/Semmle.Extraction.CSharp/Kinds/ExprKind.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ public enum ExprKind
POSITIONAL_PATTERN = 117,
SWITCH_CASE = 118,
ASSIGN_COALESCE = 119,
SUPPRESS_NULLABLE_WARNING = 120
SUPPRESS_NULLABLE_WARNING = 120,
NAMESPACE_ACCESS = 121
}
}
6 changes: 5 additions & 1 deletion csharp/ql/src/semmle/code/csharp/Namespace.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class TypeContainer extends DotNet::NamedElement, Element, @type_container { }
* }
* ```
*/
class Namespace extends DotNet::Namespace, TypeContainer, @namespace {
class Namespace extends DotNet::Namespace, TypeContainer, Declaration, @namespace {
/** Gets the simple name of this namespace, for example `IO` in `System.IO`. */
override string getName() { namespaces(this, result) }

Expand Down Expand Up @@ -117,6 +117,10 @@ class Namespace extends DotNet::Namespace, TypeContainer, @namespace {
NamespaceDeclaration getADeclaration() { result.getNamespace() = this }

override Location getALocation() { result = this.getADeclaration().getALocation() }

override string toString() { result = DotNet::Namespace.super.toString() }

override predicate hasQualifiedName(string a, string b) { DotNet::Namespace.super.hasQualifiedName(a, b) }
}

/**
Expand Down
9 changes: 9 additions & 0 deletions csharp/ql/src/semmle/code/csharp/exprs/Access.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -826,3 +826,12 @@ class ArrayRead extends ArrayAccess, AssignableRead { }
* ```
*/
class ArrayWrite extends ArrayAccess, AssignableWrite { }

/**
* An access to a namespace, for example `System` in `nameof(System)`.
*/
class NamespaceAccess extends Access, @namespace_access_expr {
override Namespace getTarget() { expr_access(this, result) }

override string toString() { result = "access to namespace " + this.getTarget().getName() }
}
4 changes: 2 additions & 2 deletions csharp/ql/src/semmle/code/dotnet/Namespace.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
* Provides the `Namespace` class to represent .Net namespaces.
*/

private import Element
private import Declaration

/** A namespace. */
class Namespace extends NamedElement, @namespace {
class Namespace extends Declaration, @namespace {
/**
* Gets the parent namespace, if any. For example the parent namespace of `System.IO`
* is `System`. The parent namespace of `System` is the global namespace.
Expand Down
7 changes: 4 additions & 3 deletions csharp/ql/src/semmlecode.csharp.dbscheme
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ tokens(
| @using_directive | @type_parameter_constraints | @external_element
| @xmllocatable | @asp_element | @namespace;

@declaration = @callable | @generic | @assignable;
@declaration = @callable | @generic | @assignable | @namespace;

@named_element = @namespace | @declaration;

Expand Down Expand Up @@ -982,6 +982,7 @@ case @expr.kind of
| 118 = @switch_case_expr
| 119 = @assign_coalesce_expr
| 120 = @suppress_nullable_warning_expr
| 121 = @namespace_access_expr
;

@switch = @switch_stmt | @switch_expr;
Expand All @@ -1004,7 +1005,7 @@ case @expr.kind of

@member_access_expr = @field_access_expr | @property_access_expr | @indexer_access_expr | @event_access_expr
| @method_access_expr | @type_access_expr | @dynamic_member_access_expr;
@access_expr = @member_access_expr | @this_access_expr | @base_access_expr | @assignable_access_expr;
@access_expr = @member_access_expr | @this_access_expr | @base_access_expr | @assignable_access_expr | @namespace_access_expr;
@element_access_expr = @indexer_access_expr | @array_access_expr | @dynamic_element_access_expr;

@local_variable_access = @local_variable_access_expr | @local_var_decl_expr;
Expand Down Expand Up @@ -1084,7 +1085,7 @@ expr_access(
unique int accesser_id: @access_expr ref,
int target_id: @accessible ref);

@accessible = @method | @assignable | @local_function;
@accessible = @method | @assignable | @local_function | @namespace;

expr_location(
unique int id: @expr ref,
Expand Down
4 changes: 4 additions & 0 deletions csharp/ql/src/semmlecode.csharp.dbscheme.stats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,10 @@
<v>7589</v>
</e>
<e>
<k>@namespace_access_expr</k>
<v>1000</v>
</e>
<e>
<k>@interpolated_string_expr</k>
<v>3470</v>
</e>
Expand Down
1 change: 1 addition & 0 deletions csharp/ql/test/library-tests/assignables/AssignableDefinition.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
| Discards.cs:20:17:20:17 | y | Discards.cs:20:17:20:17 | Double y | Discards.cs:20:17:20:17 | <none> | Discards.cs:20:17:20:17 | <none> | certain |
| Discards.cs:20:32:20:32 | z | Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:32:20:32 | <none> | certain |
| Discards.cs:23:27:23:30 | args | Discards.cs:23:27:23:30 | args | Discards.cs:23:27:23:30 | <none> | Discards.cs:23:27:23:30 | <none> | certain |
| Discards.cs:25:22:25:22 | _ | Discards.cs:25:22:25:22 | String _ | Discards.cs:25:22:25:22 | <none> | Discards.cs:25:22:25:22 | <none> | certain |
| Discards.cs:30:24:30:24 | o | Discards.cs:30:24:30:24 | o | Discards.cs:30:24:30:24 | <none> | Discards.cs:30:24:30:24 | <none> | certain |
| Finally.cs:5:16:5:16 | b | Finally.cs:5:16:5:16 | b | Finally.cs:5:16:5:16 | <none> | Finally.cs:5:16:5:16 | <none> | certain |
| Finally.cs:7:13:7:13 | i | Finally.cs:7:13:7:17 | Int32 i = ... | Finally.cs:7:13:7:13 | access to local variable i | Finally.cs:7:17:7:17 | 0 | certain |
Expand Down
1 change: 1 addition & 0 deletions csharp/ql/test/library-tests/assignables/AssignableDefinitionNode.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@
| Discards.cs:20:17:20:17 | Double y | Discards.cs:20:17:20:17 | Double y |
| Discards.cs:20:32:20:32 | Boolean z | Discards.cs:20:22:20:33 | call to method f |
| Discards.cs:23:27:23:30 | args | Discards.cs:23:10:23:16 | enter Foreach |
| Discards.cs:25:22:25:22 | String _ | Discards.cs:25:22:25:22 | String _ |
| Discards.cs:30:24:30:24 | o | Discards.cs:30:10:30:15 | enter Switch |
| Finally.cs:5:16:5:16 | b | Finally.cs:5:9:5:9 | enter M |
| Finally.cs:7:13:7:17 | Int32 i = ... | Finally.cs:7:13:7:17 | Int32 i = ... |
Expand Down
1 change: 1 addition & 0 deletions csharp/ql/test/library-tests/assignables/Assignables.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
| Discards.cs:20:17:20:17 | y |
| Discards.cs:20:32:20:32 | z |
| Discards.cs:23:27:23:30 | args |
| Discards.cs:25:22:25:22 | _ |
| Discards.cs:30:24:30:24 | o |
| Finally.cs:5:16:5:16 | b |
| Finally.cs:7:13:7:13 | i |
Expand Down
6 changes: 3 additions & 3 deletions csharp/ql/test/library-tests/controlflow/graph/BasicBlock.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@
| Conditions.cs:67:16:67:16 | access to local variable y | Conditions.cs:57:9:57:10 | exit M5 | 3 |
| Conditions.cs:70:9:70:10 | enter M6 | Conditions.cs:74:27:74:28 | access to parameter ss | 12 |
| Conditions.cs:74:9:80:9 | foreach (... ... in ...) ... | Conditions.cs:74:9:80:9 | foreach (... ... in ...) ... | 1 |
| Conditions.cs:75:9:80:9 | {...} | Conditions.cs:76:17:76:17 | access to local variable b | 3 |
| Conditions.cs:74:22:74:22 | String _ | Conditions.cs:76:17:76:17 | access to local variable b | 4 |
| Conditions.cs:77:17:77:20 | ...; | Conditions.cs:77:17:77:19 | ...++ | 3 |
| Conditions.cs:78:13:79:26 | if (...) ... | Conditions.cs:78:17:78:21 | ... > ... | 4 |
| Conditions.cs:79:17:79:26 | ...; | Conditions.cs:79:17:79:25 | ... = ... | 3 |
Expand All @@ -185,7 +185,7 @@
| Conditions.cs:83:16:83:16 | access to local variable x | Conditions.cs:70:9:70:10 | exit M6 | 3 |
| Conditions.cs:86:9:86:10 | enter M7 | Conditions.cs:90:27:90:28 | access to parameter ss | 12 |
| Conditions.cs:90:9:98:9 | foreach (... ... in ...) ... | Conditions.cs:90:9:98:9 | foreach (... ... in ...) ... | 1 |
| Conditions.cs:91:9:98:9 | {...} | Conditions.cs:92:17:92:17 | access to local variable b | 3 |
| Conditions.cs:90:22:90:22 | String _ | Conditions.cs:92:17:92:17 | access to local variable b | 4 |
| Conditions.cs:93:17:93:20 | ...; | Conditions.cs:93:17:93:19 | ...++ | 3 |
| Conditions.cs:94:13:95:26 | if (...) ... | Conditions.cs:94:17:94:21 | ... > ... | 4 |
| Conditions.cs:95:17:95:26 | ...; | Conditions.cs:95:17:95:25 | ... = ... | 3 |
Expand Down Expand Up @@ -258,7 +258,7 @@
| Foreach.cs:12:10:12:11 | enter M2 | Foreach.cs:14:27:14:30 | access to parameter args | 3 |
| Foreach.cs:12:10:12:11 | exit M2 | Foreach.cs:12:10:12:11 | exit M2 | 1 |
| Foreach.cs:14:9:15:13 | foreach (... ... in ...) ... | Foreach.cs:14:9:15:13 | foreach (... ... in ...) ... | 1 |
| Foreach.cs:15:13:15:13 | ; | Foreach.cs:15:13:15:13 | ; | 1 |
| Foreach.cs:14:22:14:22 | String _ | Foreach.cs:15:13:15:13 | ; | 2 |
| Foreach.cs:18:10:18:11 | enter M3 | Foreach.cs:20:27:20:27 | access to parameter e | 4 |
| Foreach.cs:18:10:18:11 | exit M3 | Foreach.cs:18:10:18:11 | exit M3 | 1 |
| Foreach.cs:20:9:21:11 | foreach (... ... in ...) ... | Foreach.cs:20:9:21:11 | foreach (... ... in ...) ... | 1 |
Expand Down
10 changes: 5 additions & 5 deletions csharp/ql/test/library-tests/controlflow/graph/Condition.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,24 +167,24 @@ conditionBlock
| Conditions.cs:61:9:64:9 | {...} | Conditions.cs:65:9:66:16 | [b (line 57): true] if (...) ... | true |
| Conditions.cs:63:17:63:20 | [b (line 57): true] ...; | Conditions.cs:61:9:64:9 | [b (line 57): true] {...} | true |
| Conditions.cs:63:17:63:20 | [b (line 57): true] ...; | Conditions.cs:65:9:66:16 | [b (line 57): true] if (...) ... | false |
| Conditions.cs:74:9:80:9 | foreach (... ... in ...) ... | Conditions.cs:75:9:80:9 | {...} | false |
| Conditions.cs:74:9:80:9 | foreach (... ... in ...) ... | Conditions.cs:74:22:74:22 | String _ | false |
| Conditions.cs:74:9:80:9 | foreach (... ... in ...) ... | Conditions.cs:77:17:77:20 | ...; | false |
| Conditions.cs:74:9:80:9 | foreach (... ... in ...) ... | Conditions.cs:78:13:79:26 | if (...) ... | false |
| Conditions.cs:74:9:80:9 | foreach (... ... in ...) ... | Conditions.cs:79:17:79:26 | ...; | false |
| Conditions.cs:74:9:80:9 | foreach (... ... in ...) ... | Conditions.cs:81:9:82:16 | if (...) ... | true |
| Conditions.cs:74:9:80:9 | foreach (... ... in ...) ... | Conditions.cs:82:13:82:16 | ...; | true |
| Conditions.cs:74:9:80:9 | foreach (... ... in ...) ... | Conditions.cs:83:16:83:16 | access to local variable x | true |
| Conditions.cs:75:9:80:9 | {...} | Conditions.cs:77:17:77:20 | ...; | true |
| Conditions.cs:74:22:74:22 | String _ | Conditions.cs:77:17:77:20 | ...; | true |
| Conditions.cs:78:13:79:26 | if (...) ... | Conditions.cs:79:17:79:26 | ...; | true |
| Conditions.cs:81:9:82:16 | if (...) ... | Conditions.cs:82:13:82:16 | ...; | true |
| Conditions.cs:90:9:98:9 | foreach (... ... in ...) ... | Conditions.cs:91:9:98:9 | {...} | false |
| Conditions.cs:90:9:98:9 | foreach (... ... in ...) ... | Conditions.cs:90:22:90:22 | String _ | false |
| Conditions.cs:90:9:98:9 | foreach (... ... in ...) ... | Conditions.cs:93:17:93:20 | ...; | false |
| Conditions.cs:90:9:98:9 | foreach (... ... in ...) ... | Conditions.cs:94:13:95:26 | if (...) ... | false |
| Conditions.cs:90:9:98:9 | foreach (... ... in ...) ... | Conditions.cs:95:17:95:26 | ...; | false |
| Conditions.cs:90:9:98:9 | foreach (... ... in ...) ... | Conditions.cs:96:13:97:20 | if (...) ... | false |
| Conditions.cs:90:9:98:9 | foreach (... ... in ...) ... | Conditions.cs:97:17:97:20 | ...; | false |
| Conditions.cs:90:9:98:9 | foreach (... ... in ...) ... | Conditions.cs:99:16:99:16 | access to local variable x | true |
| Conditions.cs:91:9:98:9 | {...} | Conditions.cs:93:17:93:20 | ...; | true |
| Conditions.cs:90:22:90:22 | String _ | Conditions.cs:93:17:93:20 | ...; | true |
| Conditions.cs:94:13:95:26 | if (...) ... | Conditions.cs:95:17:95:26 | ...; | true |
| Conditions.cs:96:13:97:20 | if (...) ... | Conditions.cs:97:17:97:20 | ...; | true |
| Conditions.cs:102:12:102:13 | enter M8 | Conditions.cs:106:13:106:20 | [b (line 102): true] ...; | true |
Expand Down Expand Up @@ -217,7 +217,7 @@ conditionBlock
| Foreach.cs:8:9:9:13 | foreach (... ... in ...) ... | Foreach.cs:6:10:6:11 | exit M1 | true |
| Foreach.cs:8:9:9:13 | foreach (... ... in ...) ... | Foreach.cs:8:22:8:24 | String arg | false |
| Foreach.cs:14:9:15:13 | foreach (... ... in ...) ... | Foreach.cs:12:10:12:11 | exit M2 | true |
| Foreach.cs:14:9:15:13 | foreach (... ... in ...) ... | Foreach.cs:15:13:15:13 | ; | false |
| Foreach.cs:14:9:15:13 | foreach (... ... in ...) ... | Foreach.cs:14:22:14:22 | String _ | false |
| Foreach.cs:18:10:18:11 | enter M3 | Foreach.cs:20:29:20:38 | call to method ToArray | false |
| Foreach.cs:20:9:21:11 | foreach (... ... in ...) ... | Foreach.cs:18:10:18:11 | exit M3 | true |
| Foreach.cs:20:9:21:11 | foreach (... ... in ...) ... | Foreach.cs:20:22:20:22 | String x | false |
Expand Down
Loading