C#: Address review comments part 1.

This commit is contained in:
Calum Grant
2019-06-24 18:26:40 +01:00
parent 35ecb948fc
commit 620ecc8128
23 changed files with 71 additions and 61 deletions

View File

@@ -16,7 +16,7 @@
* The new class `AnnotatedType` models types with type annotations, including nullability information, return kinds (`ref` and `readonly ref`), and parameter kinds (`in`, `out`, and `ref`)
- The new predicate `Assignable.getAnnotatedType()` gets the annotated type of an assignable (such as a variable or a property)
- The new predicate `Callable.getAnnotatedReturnType()` and `DelegateType.getAnnotatedReturnType()` get the annotated type of the return value
- The new predicates `Callable.getAnnotatedReturnType()` and `DelegateType.getAnnotatedReturnType()` get the annotated type of the return value
- The new predicate `ArrayType.getAnnotatedElementType()` gets the annotated type of the array element
- The new predicate `ConstructedGeneric.getAnnotatedTypeArgument()` gets the annotated type of a type argument
- The new predicate `TypeParameterConstraints.getAnAnnotatedTypeConstraint()` gets a type constraint with type annotations

View File

@@ -57,7 +57,7 @@ namespace Semmle.Extraction.CSharp.Entities
}
var initInfo = new ExpressionInfo(Context,
new AnnotatedType(initializerType, NullableAnnotation.NotAnnotated),
new AnnotatedType(initializerType, Kinds.TypeAnnotation.NotAnnotated),
Context.Create(initializer.ThisOrBaseKeyword.GetLocation()),
Kinds.ExprKind.CONSTRUCTOR_INIT,
this,

View File

@@ -42,7 +42,7 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
var info = new ExpressionInfo(
cx,
new AnnotatedType(Entities.Type.Create(cx, cx.Compilation.GetSpecialType(Microsoft.CodeAnalysis.SpecialType.System_Int32)), NullableAnnotation.NotAnnotated),
new AnnotatedType(Entities.Type.Create(cx, cx.Compilation.GetSpecialType(Microsoft.CodeAnalysis.SpecialType.System_Int32)), Kinds.TypeAnnotation.NotAnnotated),
Location,
ExprKind.INT_LITERAL,
this,

View File

@@ -20,7 +20,7 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
{
// Type conversion
OperatorCall(Syntax);
TypeMention.Create(cx, Syntax.Type, this, Type.Type);
TypeMention.Create(cx, Syntax.Type, this, Type);
}
}

View File

@@ -73,7 +73,7 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
}
}
TypeMention.Create(cx, Syntax.Type, this, Type.Type);
TypeMention.Create(cx, Syntax.Type, this, Type);
}
static SyntaxToken? GetDynamicName(CSharpSyntaxNode name)

View File

@@ -71,7 +71,7 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
TypeSyntax declTypeSyntax = null;
if (getElement)
{
if(node is FromClauseSyntax from && from.Type != null)
if (node is FromClauseSyntax from && from.Type != null)
{
declTypeSyntax = from.Type;
declType = Type.Create(cx, cx.GetType(from.Type));

View File

@@ -8,7 +8,7 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
This(IExpressionInfo info) : base(info) { }
public static This CreateImplicit(Context cx, Type @class, Location loc, IExpressionParentEntity parent, int child) =>
new This(new ExpressionInfo(cx, new AnnotatedType(@class, Microsoft.CodeAnalysis.NullableAnnotation.NotAnnotated), loc, Kinds.ExprKind.THIS_ACCESS, parent, child, true, null));
new This(new ExpressionInfo(cx, new AnnotatedType(@class, Kinds.TypeAnnotation.NotAnnotated), loc, Kinds.ExprKind.THIS_ACCESS, parent, child, true, null));
public static This CreateExplicit(ExpressionNodeInfo info) => new This(info.SetKind(ExprKind.THIS_ACCESS));
}

View File

@@ -17,17 +17,17 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
if (Type.Type.ContainingType == null)
{
// namespace qualifier
TypeMention.Create(cx, maes.Name, this, Type.Type, Syntax.GetLocation());
TypeMention.Create(cx, maes.Name, this, Type, Syntax.GetLocation());
}
else
{
// type qualifier
TypeMention.Create(cx, maes.Name, this, Type.Type);
TypeMention.Create(cx, maes.Name, this, Type);
Create(cx, maes.Expression, this, -1);
}
return;
default:
TypeMention.Create(cx, (TypeSyntax)Syntax, this, Type.Type);
TypeMention.Create(cx, (TypeSyntax)Syntax, this, Type);
return;
}
}

View File

@@ -18,7 +18,7 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
{
LocalVariable.Create(cx, symbol, ret, isVar, declLocation);
if (optionalSyntax != null)
TypeMention.Create(cx, optionalSyntax, parent, type.Type);
TypeMention.Create(cx, optionalSyntax, parent, type);
});
return ret;
}
@@ -135,7 +135,7 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
var declSymbol = cx.Model(d).GetDeclaredSymbol(d);
var location = cx.Create(id.GetLocation());
LocalVariable.Create(cx, declSymbol, ret, isVar, location);
TypeMention.Create(cx, d.Type, ret, type.Type);
TypeMention.Create(cx, d.Type, ret, type);
});
return ret;
}
@@ -161,7 +161,7 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions
var decl = d.Parent as VariableDeclarationSyntax;
if (decl != null)
TypeMention.Create(cx, decl.Type, ret, type.Type);
TypeMention.Create(cx, decl.Type, ret, type);
});
return ret;
}

View File

@@ -82,7 +82,7 @@ namespace Semmle.Extraction.CSharp.Entities
foreach (var syntax in symbol.DeclaringSyntaxReferences.
Select(d => d.GetSyntax()).OfType<VariableDeclaratorSyntax>().
Select(d => d.Parent).OfType<VariableDeclarationSyntax>())
TypeMention.Create(Context, syntax.Type, this, Type.Type);
TypeMention.Create(Context, syntax.Type, this, Type);
}
readonly Lazy<AnnotatedType> type;

View File

@@ -122,13 +122,11 @@ namespace Semmle.Extraction.CSharp.Entities
else
{
tb.Append("<");
tb.BuildList(",", m.symbol.TypeArguments, (ta, tb0) => AddSignatureTypeToId(m.Context, tb0, m.symbol, ta));
// Encode the nullability of the type arguments in the label.
// Type arguments with different nullability can result in
// a constructed method with different nullability of its parameters and return type,
// so we need to create a distinct database entity for it.
tb.BuildList("", m.symbol.TypeArgumentsNullableAnnotations, (a, tb1) => tb.Append((int)a));
tb.BuildList(",", m.symbol.GetAnnotatedTypeArguments(), (ta, tb0) => { AddSignatureTypeToId(m.Context, tb0, m.symbol, ta.Symbol); tb.Append((int)ta.Nullability); });
tb.Append(">");
}
}
@@ -345,8 +343,9 @@ namespace Semmle.Extraction.CSharp.Entities
foreach (var tp in symbol.GetAnnotatedTypeArguments())
{
Context.Emit(Tuples.type_arguments(Type.Create(Context, tp.Symbol), child, this));
if(tp.Nullability.NeedsExtraction())
Context.Emit(Tuples.type_argument_annotation(this, child, (Kinds.TypeAnnotation)symbol.TypeArgumentsNullableAnnotations[child]));
var ta = tp.Nullability.GetTypeAnnotation();
if (ta != Kinds.TypeAnnotation.None)
Context.Emit(Tuples.type_argument_annotation(this, child, ta));
child++;
}
}

View File

@@ -29,7 +29,7 @@ namespace Semmle.Extraction.CSharp.Entities.Statements
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.Type);
TypeMention.Create(cx, Stmt.Type, this, type);
Statement.Create(cx, Stmt.Statement, this, 2);
}

View File

@@ -29,8 +29,9 @@ namespace Semmle.Extraction.CSharp.Entities
protected void ExtractNullability(NullableAnnotation annotation)
{
if (annotation.NeedsExtraction())
Context.Emit(Tuples.type_annotation(this, (Kinds.TypeAnnotation)annotation));
var ta = annotation.GetTypeAnnotation();
if (ta != Kinds.TypeAnnotation.None)
Context.Emit(Tuples.type_annotation(this, ta));
}
protected void ExtractRefKind(RefKind kind)

View File

@@ -30,7 +30,7 @@ namespace Semmle.Extraction.CSharp.Entities
var ats = (ArrayTypeSyntax)Syntax;
var at = (ArrayType)Type;
Emit(Loc ?? Syntax.GetLocation(), Parent, Type);
Create(cx, ats.ElementType, this, at.ElementType.Type);
Create(cx, ats.ElementType, this, at.ElementType);
return;
case SyntaxKind.NullableType:
var nts = (NullableTypeSyntax)Syntax;
@@ -48,7 +48,7 @@ namespace Semmle.Extraction.CSharp.Entities
var tts = (TupleTypeSyntax)Syntax;
var tt = (TupleType)Type;
Emit(Loc ?? Syntax.GetLocation(), Parent, Type);
tts.Elements.Zip(tt.TupleElements, (s, t) => Create(cx, s.Type, this, t.Type.Type)).Enumerate();
tts.Elements.Zip(tt.TupleElements, (s, t) => Create(cx, s.Type, this, t.Type)).Enumerate();
return;
case SyntaxKind.PointerType:
var pts = (PointerTypeSyntax)Syntax;
@@ -94,6 +94,9 @@ namespace Semmle.Extraction.CSharp.Entities
return ret;
}
public static TypeMention Create(Context cx, TypeSyntax syntax, IEntity parent, AnnotatedType type, Microsoft.CodeAnalysis.Location loc = null) =>
Create(cx, syntax, parent, type.Type, loc);
public override TrapStackBehaviour TrapStackBehaviour => TrapStackBehaviour.OptionalLabel;
}
}

View File

@@ -56,7 +56,9 @@ namespace Semmle.Extraction.CSharp.Entities
for (int i = 0; i < symbol.TypeArguments.Length; ++i)
{
Context.Emit(Tuples.type_argument_annotation(this, i, (Kinds.TypeAnnotation)symbol.TypeArgumentsNullableAnnotations[i]));
var ta = symbol.TypeArgumentsNullableAnnotations[i].GetTypeAnnotation();
if (ta != Kinds.TypeAnnotation.None)
Context.Emit(Tuples.type_argument_annotation(this, i, ta));
Context.Emit(Tuples.type_arguments(TypeArguments[i].TypeRef, i, this));
}
}

View File

@@ -23,7 +23,7 @@ namespace Semmle.Extraction.CSharp.Entities
return obj != null && obj.GetType() == typeof(NullType);
}
public static AnnotatedType Create(Context cx) => new AnnotatedType(NullTypeFactory.Instance.CreateEntity(cx, null), NullableAnnotation.NotApplicable);
public static AnnotatedType Create(Context cx) => new AnnotatedType(NullTypeFactory.Instance.CreateEntity(cx, null), Kinds.TypeAnnotation.None);
class NullTypeFactory : ICachedEntityFactory<ITypeSymbol, NullType>
{

View File

@@ -13,14 +13,14 @@ namespace Semmle.Extraction.CSharp.Entities
/// </summary>
public struct AnnotatedType
{
public AnnotatedType(Type t, NullableAnnotation a)
public AnnotatedType(Type t, Kinds.TypeAnnotation a)
{
Type = t;
Annotation = a;
}
public Type Type;
public NullableAnnotation Annotation;
public Kinds.TypeAnnotation Annotation;
}
public abstract class Type : CachedSymbol<ITypeSymbol>
@@ -291,7 +291,7 @@ namespace Semmle.Extraction.CSharp.Entities
}
public static AnnotatedType Create(Context cx, AnnotatedTypeSymbol type) =>
new AnnotatedType(Create(cx, type.Symbol), type.Nullability);
new AnnotatedType(Create(cx, type.Symbol), type.Nullability.GetTypeAnnotation());
public virtual int Dimension => 0;

View File

@@ -50,8 +50,8 @@ namespace Semmle.Extraction.CSharp.Entities
baseType = abase.Symbol;
var t = Create(Context, abase.Symbol);
Context.Emit(Tuples.specific_type_parameter_constraints(constraints, t.TypeRef));
if (abase.Nullability.NeedsExtraction())
Context.Emit(Tuples.specific_type_parameter_annotation(constraints, t.TypeRef, abase.Nullability));
if (abase.Nullability.GetTypeAnnotation() != Kinds.TypeAnnotation.None)
Context.Emit(Tuples.specific_type_parameter_annotation(constraints, t.TypeRef, abase.Nullability.GetTypeAnnotation()));
}
Context.Emit(Tuples.types(this, Semmle.Extraction.Kinds.TypeKind.TYPE_PARAMETER, symbol.Name));

View File

@@ -1,13 +1,15 @@
namespace Semmle.Extraction.Kinds
using System;
namespace Semmle.Extraction.Kinds
{
enum TypeAnnotation
[Flags]
public enum TypeAnnotation
{
NotApplicable,
Disabled,
NotAnnotated,
Annotated,
ReadonlyRef,
Ref,
Out
None = 0,
NotAnnotated = 4,
Annotated = 8,
ReadonlyRef = 16,
Ref = 32,
Out = 64
}
}

View File

@@ -227,13 +227,11 @@ namespace Semmle.Extraction.CSharp
{
subTermAction(cx, tb, named.ConstructedFrom);
tb.Append("<");
tb.BuildList(",", named.TypeArguments, (ta, tb0) => subTermAction(cx, tb0, ta));
// Encode the nullability of the type arguments in the label.
// Type arguments with different nullability can result in
// a constructed type with different nullability of its members and methods,
// so we need to create a distinct database entity for it.
tb.BuildList("", named.TypeArgumentsNullableAnnotations, (a, tb1) => tb.Append((int)a));
tb.BuildList(",", named.GetAnnotatedTypeArguments(), (ta, tb0) => { subTermAction(cx, tb0, ta.Symbol); tb.Append((int)ta.Nullability); });
tb.Append(">");
}
}
@@ -492,7 +490,7 @@ namespace Semmle.Extraction.CSharp
public static AnnotatedTypeSymbol GetAnnotatedType(this IPropertySymbol symbol) => new AnnotatedTypeSymbol(symbol.Type, symbol.NullableAnnotation);
/// <summary>
/// Gets the annotated type of an ILocalSymbol.
/// Gets the annotated type of an IFieldSymbol.
/// This has not yet been exposed on the public API.
/// </summary>
public static AnnotatedTypeSymbol GetAnnotatedType(this IFieldSymbol symbol) => new AnnotatedTypeSymbol(symbol.Type, symbol.NullableAnnotation);
@@ -504,11 +502,20 @@ namespace Semmle.Extraction.CSharp
public static AnnotatedTypeSymbol GetAnnotatedReturnType(this IMethodSymbol symbol) => new AnnotatedTypeSymbol(symbol.ReturnType, symbol.ReturnNullableAnnotation);
/// <summary>
/// Holds if the annotation should be extracted.
/// The "Disabled" and "NotApplicable" annotations can be inferred so
/// do not need to be stored in the database.
/// Gets the type annotation for a NullableAnnotation.
/// </summary>
public static bool NeedsExtraction(this NullableAnnotation info) => info == NullableAnnotation.Annotated || info == NullableAnnotation.NotAnnotated;
public static Kinds.TypeAnnotation GetTypeAnnotation(this NullableAnnotation na)
{
switch(na)
{
case NullableAnnotation.Annotated:
return Kinds.TypeAnnotation.Annotated;
case NullableAnnotation.NotAnnotated:
return Kinds.TypeAnnotation.NotAnnotated;
default:
return Kinds.TypeAnnotation.None;
}
}
/// <summary>
/// Gets the annotated element type of an IArrayTypeSymbol.

View File

@@ -192,7 +192,7 @@ namespace Semmle.Extraction.CSharp
internal static Tuple specific_type_parameter_constraints(TypeParameterConstraints constraints, Type baseType) => new Tuple("specific_type_parameter_constraints", constraints, baseType);
internal static Tuple specific_type_parameter_annotation(TypeParameterConstraints constraints, Type baseType, Microsoft.CodeAnalysis.NullableAnnotation annotation) => new Tuple("specific_type_parameter_annotation", constraints, baseType, (int)annotation);
internal static Tuple specific_type_parameter_annotation(TypeParameterConstraints constraints, Type baseType, TypeAnnotation annotation) => new Tuple("specific_type_parameter_annotation", constraints, baseType, (int)annotation);
internal static Tuple successors(IEntity from, IEntity to) => new Tuple("successors", from, to);

View File

@@ -147,21 +147,17 @@ private int getBitMask(int bit) {
}
private int getElementTypeFlags(@has_type_annotation element) {
result = sum(int b | type_annotation(element, b) | getBitMask(b))
result = sum(int b | type_annotation(element, b) | b)
}
private int getTypeArgumentFlags(ConstructedGeneric generic, int argument) {
exists(generic.getTypeArgument(argument)) and
result = sum(int b | type_argument_annotation(generic, argument, b) | getBitMask(b))
result = sum(int b | type_argument_annotation(generic, argument, b) | b)
}
private int getTypeParameterFlags(TypeParameterConstraints constraints, Type type) {
specific_type_parameter_annotation(constraints, getTypeRef(type), _) and
result = sum(int b |
specific_type_parameter_annotation(constraints, getTypeRef(type), b)
|
getBitMask(b)
)
result = sum(int b | specific_type_parameter_annotation(constraints, getTypeRef(type), b) | b)
}
private newtype TAnnotatedType =

View File

@@ -3,16 +3,16 @@ class Element extends @element {
int getAnnotation() {
exists(int mode | params(this, _, _, _, mode, _, _) |
mode = 1 and result = 5 // ref
mode = 1 and result = 32 // ref
or
mode = 2 and result = 6 // out
mode = 2 and result = 64 // out
or
mode = 5 and result = 4 // in
mode = 5 and result = 16 // in
)
or
ref_returns(this) and result = 5
ref_returns(this) and result = 32
or
ref_readonly_returns(this) and result = 4
ref_readonly_returns(this) and result = 16
}
}