Skip to content

Commit 82b7a19

Browse files
michaelnebelmichaelnebel
authored
Merge pull request #18894 from michaelnebel/csharp/garbagetypes
C#: Handle some BMN garbage types.
2 parents 5c3f21b + fb3ce46 commit 82b7a19

File tree

12 files changed

+120
-7
lines changed

12 files changed

+120
-7
lines changed

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public override void Populate(TextWriter trapFile)
3131
{
3232
if (assemblyPath is not null)
3333
{
34-
var isBuildlessOutputAssembly = isOutputAssembly && Context.ExtractionContext.Mode.HasFlag(ExtractorMode.Standalone);
34+
var isBuildlessOutputAssembly = isOutputAssembly && Context.ExtractionContext.IsStandalone;
3535
var identifier = isBuildlessOutputAssembly
3636
? ""
3737
: assembly.ToString() ?? "";
@@ -72,7 +72,7 @@ public static Assembly CreateOutputAssembly(Context cx)
7272

7373
public override void WriteId(EscapingTextWriter trapFile)
7474
{
75-
if (isOutputAssembly && Context.ExtractionContext.Mode.HasFlag(ExtractorMode.Standalone))
75+
if (isOutputAssembly && Context.ExtractionContext.IsStandalone)
7676
{
7777
trapFile.Write("buildlessOutputAssembly");
7878
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public IMethodSymbol? TargetSymbol
133133
.Where(method => method.Parameters.Length >= Syntax.ArgumentList.Arguments.Count)
134134
.Where(method => method.Parameters.Count(p => !p.HasExplicitDefaultValue) <= Syntax.ArgumentList.Arguments.Count);
135135

136-
return Context.ExtractionContext.Mode.HasFlag(ExtractorMode.Standalone) ?
136+
return Context.ExtractionContext.IsStandalone ?
137137
candidates.FirstOrDefault() :
138138
candidates.SingleOrDefault();
139139
}

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs

+3-1
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,9 @@ private class UnderlyingTupleTypeFactory : CachedEntityFactory<INamedTypeSymbol,
166166
// Create typerefs for constructed error types in case they are fully defined elsewhere.
167167
// We cannot use `!this.NeedsPopulation` because this would not be stable as it would depend on
168168
// the assembly that was being extracted at the time.
169-
private bool UsesTypeRef => Symbol.TypeKind == TypeKind.Error || SymbolEqualityComparer.Default.Equals(Symbol.OriginalDefinition, Symbol);
169+
private bool UsesTypeRef =>
170+
Symbol.TypeKind == TypeKind.Error ||
171+
SymbolEqualityComparer.Default.Equals(Symbol.OriginalDefinition, Symbol);
170172

171173
public override Type TypeRef => UsesTypeRef ? (Type)NamedTypeRef.Create(Context, Symbol) : this;
172174
}

‎csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs

+37
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,40 @@ public static bool ConstructedOrParentIsConstructed(INamedTypeSymbol symbol)
2525
symbol.ContainingType is not null && ConstructedOrParentIsConstructed(symbol.ContainingType);
2626
}
2727

28+
29+
/// <summary>
30+
/// A hashset containing the C# contextual keywords that could be confused with types (and typing).
31+
///
32+
/// For the list of all contextual keywords, see
33+
/// https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/#contextual-keywords
34+
/// </summary>
35+
private readonly HashSet<string> ContextualKeywordTypes = [
36+
"dynamic",
37+
"nint",
38+
"nuint",
39+
"var"
40+
];
41+
42+
/// <summary>
43+
/// Returns true in case we suspect this is a broken type.
44+
/// </summary>
45+
/// <param name="symbol">Type symbol</param>
46+
private bool IsBrokenType(ITypeSymbol symbol)
47+
{
48+
if (!Context.ExtractionContext.IsStandalone ||
49+
!symbol.FromSource() ||
50+
symbol.IsAnonymousType)
51+
{
52+
return false;
53+
}
54+
55+
// (1) public class { ... } is a broken type as it doesn't have a name.
56+
// (2) public class var { ... } is an allowed type, but it overrides the `var` keyword for all uses.
57+
// The same goes for other contextual keywords that could be used as type names.
58+
// It is probably a better heuristic to treat these as broken types.
59+
return string.IsNullOrEmpty(symbol.Name) || ContextualKeywordTypes.Contains(symbol.Name);
60+
}
61+
2862
public Kinds.TypeKind GetTypeKind(Context cx, bool constructUnderlyingTupleType)
2963
{
3064
switch (Symbol.SpecialType)
@@ -48,6 +82,9 @@ public Kinds.TypeKind GetTypeKind(Context cx, bool constructUnderlyingTupleType)
4882
if (Symbol.IsBoundNullable())
4983
return Kinds.TypeKind.NULLABLE;
5084

85+
if (IsBrokenType(Symbol))
86+
return Kinds.TypeKind.UNKNOWN;
87+
5188
switch (Symbol.TypeKind)
5289
{
5390
case TypeKind.Class: return Kinds.TypeKind.CLASS;

‎csharp/extractor/Semmle.Extraction.CSharp/Extractor/BinaryLogExtractionContext.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public BinaryLogExtractionContext(string cwd, string[] args, string outputPath,
4747

4848
public static string? GetAdjustedPath(ExtractionContext extractionContext, string sourcePath)
4949
{
50-
if (extractionContext.Mode.HasFlag(ExtractorMode.BinaryLog)
50+
if (extractionContext.IsBinaryLog
5151
&& extractionContext is BinaryLogExtractionContext binaryLogExtractionContext
5252
&& binaryLogExtractionContext.GetAdjustedPath(sourcePath) is string adjustedPath)
5353
{

‎csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ private void Populate(ISymbol? optionalSymbol, Entities.CachedEntity entity)
267267

268268
bool duplicationGuard, deferred;
269269

270-
if (ExtractionContext.Mode is ExtractorMode.Standalone)
270+
if (ExtractionContext.IsStandalone)
271271
{
272272
duplicationGuard = false;
273273
deferred = false;
@@ -376,7 +376,7 @@ private void ExtractionError(InternalError error)
376376

377377
private void ReportError(InternalError error)
378378
{
379-
if (!ExtractionContext.Mode.HasFlag(ExtractorMode.Standalone))
379+
if (!ExtractionContext.IsStandalone)
380380
throw error;
381381

382382
ExtractionError(error);

‎csharp/extractor/Semmle.Extraction.CSharp/Extractor/ExtractionContext.cs

+2
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ public class ExtractionContext
1515
public ExtractorMode Mode { get; }
1616
public string OutputPath { get; }
1717
public IEnumerable<CompilationInfo> CompilationInfos { get; }
18+
public bool IsStandalone => Mode.HasFlag(ExtractorMode.Standalone);
19+
public bool IsBinaryLog => Mode.HasFlag(ExtractorMode.BinaryLog);
1820

1921
/// <summary>
2022
/// Creates a new extractor instance for one compilation unit.

‎csharp/ql/lib/semmle/code/csharp/Type.qll

+2
Original file line numberDiff line numberDiff line change
@@ -1214,6 +1214,8 @@ class ArglistType extends Type, @arglist_type {
12141214
class UnknownType extends Type, @unknown_type {
12151215
/** Holds if this is the canonical unknown type, and not a type that failed to extract properly. */
12161216
predicate isCanonical() { types(this, _, "<unknown type>") }
1217+
1218+
override string getAPrimaryQlClass() { result = "UnknownType" }
12171219
}
12181220

12191221
/**

‎csharp/ql/test/library-tests/standalone/brokentypes/BrokenTypes.cs

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Broken type without a name.
2+
public class { }
3+
4+
// Legal declaration, but we want don't want to use it.
5+
public class var { }
6+
7+
public class C
8+
{
9+
public string Prop { get; set; }
10+
}
11+
12+
13+
public class Program
14+
{
15+
public static void Main()
16+
{
17+
C x1 = new C();
18+
string y1 = x1.Prop;
19+
20+
var x2 = new C(); // Has type `var` as this overrides the implicitly typed keyword `var`.
21+
var y2 = x2.Prop; // Unknown type as `x2` has type `var`.
22+
23+
C2 x3 = new C2(); // Unknown type.
24+
var y3 = x3.Prop; // Unknown property of unknown type.
25+
26+
string s = x1.Prop + x3.Prop;
27+
}
28+
}

‎csharp/ql/test/library-tests/standalone/brokentypes/brokenTypes.expected

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
| BrokenTypes.cs:2:14:2:13 | call to constructor Object | object | ObjectType |
2+
| BrokenTypes.cs:5:14:5:16 | call to constructor Object | object | ObjectType |
3+
| BrokenTypes.cs:7:14:7:14 | call to constructor Object | object | ObjectType |
4+
| BrokenTypes.cs:13:14:13:20 | call to constructor Object | object | ObjectType |
5+
| BrokenTypes.cs:17:11:17:12 | access to local variable x1 | C | Class |
6+
| BrokenTypes.cs:17:11:17:22 | C x1 = ... | C | Class |
7+
| BrokenTypes.cs:17:16:17:22 | object creation of type C | C | Class |
8+
| BrokenTypes.cs:18:16:18:17 | access to local variable y1 | string | StringType |
9+
| BrokenTypes.cs:18:16:18:27 | String y1 = ... | string | StringType |
10+
| BrokenTypes.cs:18:21:18:22 | access to local variable x1 | C | Class |
11+
| BrokenTypes.cs:18:21:18:27 | access to property Prop | string | StringType |
12+
| BrokenTypes.cs:20:13:20:14 | access to local variable x2 | var | UnknownType |
13+
| BrokenTypes.cs:20:13:20:24 | var x2 = ... | var | UnknownType |
14+
| BrokenTypes.cs:20:18:20:24 | (...) ... | var | UnknownType |
15+
| BrokenTypes.cs:20:18:20:24 | object creation of type C | C | Class |
16+
| BrokenTypes.cs:21:13:21:14 | access to local variable y2 | var | UnknownType |
17+
| BrokenTypes.cs:21:13:21:24 | var y2 = ... | var | UnknownType |
18+
| BrokenTypes.cs:21:18:21:19 | access to local variable x2 | var | UnknownType |
19+
| BrokenTypes.cs:21:18:21:24 | (...) ... | var | UnknownType |
20+
| BrokenTypes.cs:21:18:21:24 | access to property (unknown) | | UnknownType |
21+
| BrokenTypes.cs:23:12:23:13 | access to local variable x3 | <unknown type> | UnknownType |
22+
| BrokenTypes.cs:23:12:23:24 | <unknown type> x3 = ... | <unknown type> | UnknownType |
23+
| BrokenTypes.cs:23:17:23:24 | object creation of type <unknown type> | <unknown type> | UnknownType |
24+
| BrokenTypes.cs:24:13:24:14 | access to local variable y3 | var | UnknownType |
25+
| BrokenTypes.cs:24:13:24:24 | var y3 = ... | var | UnknownType |
26+
| BrokenTypes.cs:24:18:24:19 | access to local variable x3 | <unknown type> | UnknownType |
27+
| BrokenTypes.cs:24:18:24:24 | (...) ... | var | UnknownType |
28+
| BrokenTypes.cs:24:18:24:24 | access to property (unknown) | | UnknownType |
29+
| BrokenTypes.cs:26:16:26:16 | access to local variable s | string | StringType |
30+
| BrokenTypes.cs:26:16:26:36 | String s = ... | string | StringType |
31+
| BrokenTypes.cs:26:20:26:21 | access to local variable x1 | C | Class |
32+
| BrokenTypes.cs:26:20:26:26 | access to property Prop | string | StringType |
33+
| BrokenTypes.cs:26:20:26:36 | (...) ... | string | StringType |
34+
| BrokenTypes.cs:26:20:26:36 | ... + ... | | UnknownType |
35+
| BrokenTypes.cs:26:30:26:31 | access to local variable x3 | <unknown type> | UnknownType |
36+
| BrokenTypes.cs:26:30:26:36 | access to property (unknown) | | UnknownType |

‎csharp/ql/test/library-tests/standalone/brokentypes/brokenTypes.ql

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import csharp
2+
3+
from Expr e, Type t
4+
where e.fromSource() and t = e.getType()
5+
select e, t.toStringWithTypes(), t.getAPrimaryQlClass()

‎csharp/ql/test/library-tests/standalone/brokentypes/options

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
semmle-extractor-options: --standalone

0 commit comments

Comments
 (0)