From 5551aebaa9b57b908ebe7c5f1c54ae21e7554900 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 28 Feb 2025 15:02:41 +0100 Subject: [PATCH 1/9] C#: Add a primary ql class for UnknownType. --- csharp/ql/lib/semmle/code/csharp/Type.qll | 2 ++ 1 file changed, 2 insertions(+) diff --git a/csharp/ql/lib/semmle/code/csharp/Type.qll b/csharp/ql/lib/semmle/code/csharp/Type.qll index 6901fb806b19..9283bb3002a1 100644 --- a/csharp/ql/lib/semmle/code/csharp/Type.qll +++ b/csharp/ql/lib/semmle/code/csharp/Type.qll @@ -1214,6 +1214,8 @@ class ArglistType extends Type, @arglist_type { class UnknownType extends Type, @unknown_type { /** Holds if this is the canonical unknown type, and not a type that failed to extract properly. */ predicate isCanonical() { types(this, _, "") } + + override string getAPrimaryQlClass() { result = "UnknownType" } } /** From 9af170f60e9cb17fd5fd73f8acbcb7616803452a Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 28 Feb 2025 13:04:13 +0100 Subject: [PATCH 2/9] C#: Add BMN test using broken types. --- .../standalone/brokentypes/BrokenTypes.cs | 28 +++++++++++++++ .../brokentypes/brokenTypes.expected | 36 +++++++++++++++++++ .../standalone/brokentypes/brokenTypes.ql | 5 +++ .../standalone/brokentypes/options | 1 + 4 files changed, 70 insertions(+) create mode 100644 csharp/ql/test/library-tests/standalone/brokentypes/BrokenTypes.cs create mode 100644 csharp/ql/test/library-tests/standalone/brokentypes/brokenTypes.expected create mode 100644 csharp/ql/test/library-tests/standalone/brokentypes/brokenTypes.ql create mode 100644 csharp/ql/test/library-tests/standalone/brokentypes/options diff --git a/csharp/ql/test/library-tests/standalone/brokentypes/BrokenTypes.cs b/csharp/ql/test/library-tests/standalone/brokentypes/BrokenTypes.cs new file mode 100644 index 000000000000..22590630d5d3 --- /dev/null +++ b/csharp/ql/test/library-tests/standalone/brokentypes/BrokenTypes.cs @@ -0,0 +1,28 @@ +// Broken type without a name. +public class { } + +// Legal declaration, but we want don't want to use it. +public class var { } + +public class C +{ + public string Prop { get; set; } +} + + +public class Program +{ + public static void Main() + { + C x1 = new C(); + string y1 = x1.Prop; + + var x2 = new C(); // Has type `var` as this overrides the implicitly typed keyword `var`. + var y2 = x2.Prop; // Unknown type as `x2` har type `var`. + + C2 x3 = new C2(); // Unknown type. + var y3 = x3.Prop; // Unknown property of unknown type. + + string s = x1.Prop + x3.Prop; + } +} diff --git a/csharp/ql/test/library-tests/standalone/brokentypes/brokenTypes.expected b/csharp/ql/test/library-tests/standalone/brokentypes/brokenTypes.expected new file mode 100644 index 000000000000..57ffef4b604c --- /dev/null +++ b/csharp/ql/test/library-tests/standalone/brokentypes/brokenTypes.expected @@ -0,0 +1,36 @@ +| BrokenTypes.cs:2:14:2:13 | call to constructor Object | object | ObjectType | +| BrokenTypes.cs:5:14:5:16 | call to constructor Object | object | ObjectType | +| BrokenTypes.cs:7:14:7:14 | call to constructor Object | object | ObjectType | +| BrokenTypes.cs:13:14:13:20 | call to constructor Object | object | ObjectType | +| BrokenTypes.cs:17:11:17:12 | access to local variable x1 | C | Class | +| BrokenTypes.cs:17:11:17:22 | C x1 = ... | C | Class | +| BrokenTypes.cs:17:16:17:22 | object creation of type C | C | Class | +| BrokenTypes.cs:18:16:18:17 | access to local variable y1 | string | StringType | +| BrokenTypes.cs:18:16:18:27 | String y1 = ... | string | StringType | +| BrokenTypes.cs:18:21:18:22 | access to local variable x1 | C | Class | +| BrokenTypes.cs:18:21:18:27 | access to property Prop | string | StringType | +| BrokenTypes.cs:20:13:20:14 | access to local variable x2 | var | Class | +| BrokenTypes.cs:20:13:20:24 | var x2 = ... | var | Class | +| BrokenTypes.cs:20:18:20:24 | (...) ... | var | Class | +| BrokenTypes.cs:20:18:20:24 | object creation of type C | C | Class | +| BrokenTypes.cs:21:13:21:14 | access to local variable y2 | var | Class | +| BrokenTypes.cs:21:13:21:24 | var y2 = ... | var | Class | +| BrokenTypes.cs:21:18:21:19 | access to local variable x2 | var | Class | +| BrokenTypes.cs:21:18:21:24 | (...) ... | var | Class | +| BrokenTypes.cs:21:18:21:24 | access to property (unknown) | | Class | +| BrokenTypes.cs:23:12:23:13 | access to local variable x3 | | UnknownType | +| BrokenTypes.cs:23:12:23:24 | x3 = ... | | UnknownType | +| BrokenTypes.cs:23:17:23:24 | object creation of type | | UnknownType | +| BrokenTypes.cs:24:13:24:14 | access to local variable y3 | var | Class | +| BrokenTypes.cs:24:13:24:24 | var y3 = ... | var | Class | +| BrokenTypes.cs:24:18:24:19 | access to local variable x3 | | UnknownType | +| BrokenTypes.cs:24:18:24:24 | (...) ... | var | Class | +| BrokenTypes.cs:24:18:24:24 | access to property (unknown) | | Class | +| BrokenTypes.cs:26:16:26:16 | access to local variable s | string | StringType | +| BrokenTypes.cs:26:16:26:36 | String s = ... | string | StringType | +| BrokenTypes.cs:26:20:26:21 | access to local variable x1 | C | Class | +| BrokenTypes.cs:26:20:26:26 | access to property Prop | string | StringType | +| BrokenTypes.cs:26:20:26:36 | (...) ... | string | StringType | +| BrokenTypes.cs:26:20:26:36 | ... + ... | | Class | +| BrokenTypes.cs:26:30:26:31 | access to local variable x3 | | UnknownType | +| BrokenTypes.cs:26:30:26:36 | access to property (unknown) | | Class | diff --git a/csharp/ql/test/library-tests/standalone/brokentypes/brokenTypes.ql b/csharp/ql/test/library-tests/standalone/brokentypes/brokenTypes.ql new file mode 100644 index 000000000000..9ce360e3f788 --- /dev/null +++ b/csharp/ql/test/library-tests/standalone/brokentypes/brokenTypes.ql @@ -0,0 +1,5 @@ +import csharp + +from Expr e, Type t +where e.fromSource() and t = e.getType() +select e, t.toStringWithTypes(), t.getAPrimaryQlClass() diff --git a/csharp/ql/test/library-tests/standalone/brokentypes/options b/csharp/ql/test/library-tests/standalone/brokentypes/options new file mode 100644 index 000000000000..7ba3811b2afb --- /dev/null +++ b/csharp/ql/test/library-tests/standalone/brokentypes/options @@ -0,0 +1 @@ +semmle-extractor-options: --standalone From c2b835da4096b5c43a8a162ca681f2fc24cf86b1 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 28 Feb 2025 10:36:52 +0100 Subject: [PATCH 3/9] C#: Re-factor the check whether we are in standalone mode. --- .../extractor/Semmle.Extraction.CSharp/Entities/Assembly.cs | 4 ++-- .../Entities/Expressions/Invocation.cs | 2 +- .../extractor/Semmle.Extraction.CSharp/Extractor/Context.cs | 2 +- .../Semmle.Extraction.CSharp/Extractor/ExtractionContext.cs | 1 + 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Assembly.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Assembly.cs index cc36e41ff588..4ad05eea3833 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Assembly.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Assembly.cs @@ -31,7 +31,7 @@ public override void Populate(TextWriter trapFile) { if (assemblyPath is not null) { - var isBuildlessOutputAssembly = isOutputAssembly && Context.ExtractionContext.Mode.HasFlag(ExtractorMode.Standalone); + var isBuildlessOutputAssembly = isOutputAssembly && Context.ExtractionContext.IsStandalone; var identifier = isBuildlessOutputAssembly ? "" : assembly.ToString() ?? ""; @@ -72,7 +72,7 @@ public static Assembly CreateOutputAssembly(Context cx) public override void WriteId(EscapingTextWriter trapFile) { - if (isOutputAssembly && Context.ExtractionContext.Mode.HasFlag(ExtractorMode.Standalone)) + if (isOutputAssembly && Context.ExtractionContext.IsStandalone) { trapFile.Write("buildlessOutputAssembly"); } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Invocation.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Invocation.cs index 4db35fda985e..a6272974c22b 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Invocation.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Invocation.cs @@ -133,7 +133,7 @@ public IMethodSymbol? TargetSymbol .Where(method => method.Parameters.Length >= Syntax.ArgumentList.Arguments.Count) .Where(method => method.Parameters.Count(p => !p.HasExplicitDefaultValue) <= Syntax.ArgumentList.Arguments.Count); - return Context.ExtractionContext.Mode.HasFlag(ExtractorMode.Standalone) ? + return Context.ExtractionContext.IsStandalone ? candidates.FirstOrDefault() : candidates.SingleOrDefault(); } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs index 67bb2808ae62..a79341086363 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs @@ -376,7 +376,7 @@ private void ExtractionError(InternalError error) private void ReportError(InternalError error) { - if (!ExtractionContext.Mode.HasFlag(ExtractorMode.Standalone)) + if (!ExtractionContext.IsStandalone) throw error; ExtractionError(error); diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/ExtractionContext.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/ExtractionContext.cs index 262475ca5a16..0a2fc5258893 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/ExtractionContext.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/ExtractionContext.cs @@ -15,6 +15,7 @@ public class ExtractionContext public ExtractorMode Mode { get; } public string OutputPath { get; } public IEnumerable CompilationInfos { get; } + public bool IsStandalone => Mode.HasFlag(ExtractorMode.Standalone); /// /// Creates a new extractor instance for one compilation unit. From e835d8b1688a0f37d32c9df09c4644938e3adf3e Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 28 Feb 2025 10:37:47 +0100 Subject: [PATCH 4/9] C#: Change the populate logic context. It looks like a mistake that the only flag set is Standalone. --- csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs index a79341086363..f231c8238a96 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs @@ -267,7 +267,7 @@ private void Populate(ISymbol? optionalSymbol, Entities.CachedEntity entity) bool duplicationGuard, deferred; - if (ExtractionContext.Mode is ExtractorMode.Standalone) + if (ExtractionContext.IsStandalone) { duplicationGuard = false; deferred = false; From fc5a49ef84c64910aeafba8e9a066203fc2c34f7 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 28 Feb 2025 11:57:08 +0100 Subject: [PATCH 5/9] C#: Handle some broken types in BMN. --- .../Entities/Types/NamedType.cs | 4 +++- .../Entities/Types/Type.cs | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs index b2106febaff3..96c523d5bbdc 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/NamedType.cs @@ -166,7 +166,9 @@ private class UnderlyingTupleTypeFactory : CachedEntityFactory Symbol.TypeKind == TypeKind.Error || SymbolEqualityComparer.Default.Equals(Symbol.OriginalDefinition, Symbol); + private bool UsesTypeRef => + Symbol.TypeKind == TypeKind.Error || + SymbolEqualityComparer.Default.Equals(Symbol.OriginalDefinition, Symbol); public override Type TypeRef => UsesTypeRef ? (Type)NamedTypeRef.Create(Context, Symbol) : this; } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs index efd09409afd8..76a48d6929e9 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs @@ -25,6 +25,22 @@ public static bool ConstructedOrParentIsConstructed(INamedTypeSymbol symbol) symbol.ContainingType is not null && ConstructedOrParentIsConstructed(symbol.ContainingType); } + /// + /// Returns true in case we suspect this is broken type. + /// + /// Type symbol + private bool IsBrokenType(ITypeSymbol symbol) + { + if (!Context.ExtractionContext.IsStandalone || !symbol.FromSource()) + { + return false; + } + // (1) public class { ... } is a broken type and doesn't have a name. + // (2) public class var { ... } is a an allowed type, but it overrides the var keyword for all uses. + // It is probably a better heuristic to treat it as a broken type. + return string.IsNullOrEmpty(symbol.Name) || symbol.Name == "var"; + } + public Kinds.TypeKind GetTypeKind(Context cx, bool constructUnderlyingTupleType) { switch (Symbol.SpecialType) @@ -48,6 +64,9 @@ public Kinds.TypeKind GetTypeKind(Context cx, bool constructUnderlyingTupleType) if (Symbol.IsBoundNullable()) return Kinds.TypeKind.NULLABLE; + if (IsBrokenType(Symbol)) + return Kinds.TypeKind.UNKNOWN; + switch (Symbol.TypeKind) { case TypeKind.Class: return Kinds.TypeKind.CLASS; From 3b764b06407ed8aa91132d12d8c491620df440a3 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 28 Feb 2025 13:08:45 +0100 Subject: [PATCH 6/9] C#: Update test expected output. --- .../brokentypes/brokenTypes.expected | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/csharp/ql/test/library-tests/standalone/brokentypes/brokenTypes.expected b/csharp/ql/test/library-tests/standalone/brokentypes/brokenTypes.expected index 57ffef4b604c..bb3acba4f64c 100644 --- a/csharp/ql/test/library-tests/standalone/brokentypes/brokenTypes.expected +++ b/csharp/ql/test/library-tests/standalone/brokentypes/brokenTypes.expected @@ -9,28 +9,28 @@ | BrokenTypes.cs:18:16:18:27 | String y1 = ... | string | StringType | | BrokenTypes.cs:18:21:18:22 | access to local variable x1 | C | Class | | BrokenTypes.cs:18:21:18:27 | access to property Prop | string | StringType | -| BrokenTypes.cs:20:13:20:14 | access to local variable x2 | var | Class | -| BrokenTypes.cs:20:13:20:24 | var x2 = ... | var | Class | -| BrokenTypes.cs:20:18:20:24 | (...) ... | var | Class | +| BrokenTypes.cs:20:13:20:14 | access to local variable x2 | var | UnknownType | +| BrokenTypes.cs:20:13:20:24 | var x2 = ... | var | UnknownType | +| BrokenTypes.cs:20:18:20:24 | (...) ... | var | UnknownType | | BrokenTypes.cs:20:18:20:24 | object creation of type C | C | Class | -| BrokenTypes.cs:21:13:21:14 | access to local variable y2 | var | Class | -| BrokenTypes.cs:21:13:21:24 | var y2 = ... | var | Class | -| BrokenTypes.cs:21:18:21:19 | access to local variable x2 | var | Class | -| BrokenTypes.cs:21:18:21:24 | (...) ... | var | Class | -| BrokenTypes.cs:21:18:21:24 | access to property (unknown) | | Class | +| BrokenTypes.cs:21:13:21:14 | access to local variable y2 | var | UnknownType | +| BrokenTypes.cs:21:13:21:24 | var y2 = ... | var | UnknownType | +| BrokenTypes.cs:21:18:21:19 | access to local variable x2 | var | UnknownType | +| BrokenTypes.cs:21:18:21:24 | (...) ... | var | UnknownType | +| BrokenTypes.cs:21:18:21:24 | access to property (unknown) | | UnknownType | | BrokenTypes.cs:23:12:23:13 | access to local variable x3 | | UnknownType | | BrokenTypes.cs:23:12:23:24 | x3 = ... | | UnknownType | | BrokenTypes.cs:23:17:23:24 | object creation of type | | UnknownType | -| BrokenTypes.cs:24:13:24:14 | access to local variable y3 | var | Class | -| BrokenTypes.cs:24:13:24:24 | var y3 = ... | var | Class | +| BrokenTypes.cs:24:13:24:14 | access to local variable y3 | var | UnknownType | +| BrokenTypes.cs:24:13:24:24 | var y3 = ... | var | UnknownType | | BrokenTypes.cs:24:18:24:19 | access to local variable x3 | | UnknownType | -| BrokenTypes.cs:24:18:24:24 | (...) ... | var | Class | -| BrokenTypes.cs:24:18:24:24 | access to property (unknown) | | Class | +| BrokenTypes.cs:24:18:24:24 | (...) ... | var | UnknownType | +| BrokenTypes.cs:24:18:24:24 | access to property (unknown) | | UnknownType | | BrokenTypes.cs:26:16:26:16 | access to local variable s | string | StringType | | BrokenTypes.cs:26:16:26:36 | String s = ... | string | StringType | | BrokenTypes.cs:26:20:26:21 | access to local variable x1 | C | Class | | BrokenTypes.cs:26:20:26:26 | access to property Prop | string | StringType | | BrokenTypes.cs:26:20:26:36 | (...) ... | string | StringType | -| BrokenTypes.cs:26:20:26:36 | ... + ... | | Class | +| BrokenTypes.cs:26:20:26:36 | ... + ... | | UnknownType | | BrokenTypes.cs:26:30:26:31 | access to local variable x3 | | UnknownType | -| BrokenTypes.cs:26:30:26:36 | access to property (unknown) | | Class | +| BrokenTypes.cs:26:30:26:36 | access to property (unknown) | | UnknownType | From d5ee93dbbc4e536080e030044665b2bbc4de90ab Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 3 Mar 2025 14:09:38 +0100 Subject: [PATCH 7/9] C#: Anonymous types should not be considered unknown. --- .../Semmle.Extraction.CSharp/Entities/Types/Type.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs index 76a48d6929e9..4cf4729ca62e 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs @@ -31,10 +31,13 @@ public static bool ConstructedOrParentIsConstructed(INamedTypeSymbol symbol) /// Type symbol private bool IsBrokenType(ITypeSymbol symbol) { - if (!Context.ExtractionContext.IsStandalone || !symbol.FromSource()) + if (!Context.ExtractionContext.IsStandalone || + !symbol.FromSource() || + symbol.IsAnonymousType) { return false; } + // (1) public class { ... } is a broken type and doesn't have a name. // (2) public class var { ... } is a an allowed type, but it overrides the var keyword for all uses. // It is probably a better heuristic to treat it as a broken type. From 5c931fa897221dfe0282b8460b38945ae2e14808 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 3 Mar 2025 14:41:44 +0100 Subject: [PATCH 8/9] C#: Improve comments. --- .../Semmle.Extraction.CSharp/Entities/Types/Type.cs | 6 +++--- .../library-tests/standalone/brokentypes/BrokenTypes.cs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs index 4cf4729ca62e..6dd03ab97648 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs @@ -26,7 +26,7 @@ public static bool ConstructedOrParentIsConstructed(INamedTypeSymbol symbol) } /// - /// Returns true in case we suspect this is broken type. + /// Returns true in case we suspect this is a broken type. /// /// Type symbol private bool IsBrokenType(ITypeSymbol symbol) @@ -38,8 +38,8 @@ private bool IsBrokenType(ITypeSymbol symbol) return false; } - // (1) public class { ... } is a broken type and doesn't have a name. - // (2) public class var { ... } is a an allowed type, but it overrides the var keyword for all uses. + // (1) public class { ... } is a broken type as it doesn't have a name. + // (2) public class var { ... } is an allowed type, but it overrides the `var` keyword for all uses. // It is probably a better heuristic to treat it as a broken type. return string.IsNullOrEmpty(symbol.Name) || symbol.Name == "var"; } diff --git a/csharp/ql/test/library-tests/standalone/brokentypes/BrokenTypes.cs b/csharp/ql/test/library-tests/standalone/brokentypes/BrokenTypes.cs index 22590630d5d3..d78284d3dfde 100644 --- a/csharp/ql/test/library-tests/standalone/brokentypes/BrokenTypes.cs +++ b/csharp/ql/test/library-tests/standalone/brokentypes/BrokenTypes.cs @@ -18,7 +18,7 @@ public static void Main() string y1 = x1.Prop; var x2 = new C(); // Has type `var` as this overrides the implicitly typed keyword `var`. - var y2 = x2.Prop; // Unknown type as `x2` har type `var`. + var y2 = x2.Prop; // Unknown type as `x2` has type `var`. C2 x3 = new C2(); // Unknown type. var y3 = x3.Prop; // Unknown property of unknown type. From fb3ce464bea34a4dbf9e57998a805f6f803c56a0 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 6 Mar 2025 11:48:35 +0100 Subject: [PATCH 9/9] C#: Address review comments. --- .../Entities/Types/Type.cs | 19 +++++++++++++++++-- .../Extractor/BinaryLogExtractionContext.cs | 2 +- .../Extractor/ExtractionContext.cs | 1 + 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs index 6dd03ab97648..266fbfa5d606 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Types/Type.cs @@ -25,6 +25,20 @@ public static bool ConstructedOrParentIsConstructed(INamedTypeSymbol symbol) symbol.ContainingType is not null && ConstructedOrParentIsConstructed(symbol.ContainingType); } + + /// + /// A hashset containing the C# contextual keywords that could be confused with types (and typing). + /// + /// For the list of all contextual keywords, see + /// https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/keywords/#contextual-keywords + /// + private readonly HashSet ContextualKeywordTypes = [ + "dynamic", + "nint", + "nuint", + "var" + ]; + /// /// Returns true in case we suspect this is a broken type. /// @@ -40,8 +54,9 @@ private bool IsBrokenType(ITypeSymbol symbol) // (1) public class { ... } is a broken type as it doesn't have a name. // (2) public class var { ... } is an allowed type, but it overrides the `var` keyword for all uses. - // It is probably a better heuristic to treat it as a broken type. - return string.IsNullOrEmpty(symbol.Name) || symbol.Name == "var"; + // The same goes for other contextual keywords that could be used as type names. + // It is probably a better heuristic to treat these as broken types. + return string.IsNullOrEmpty(symbol.Name) || ContextualKeywordTypes.Contains(symbol.Name); } public Kinds.TypeKind GetTypeKind(Context cx, bool constructUnderlyingTupleType) diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/BinaryLogExtractionContext.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/BinaryLogExtractionContext.cs index e4ad5f83e2a2..a77e44f2456f 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/BinaryLogExtractionContext.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/BinaryLogExtractionContext.cs @@ -47,7 +47,7 @@ public BinaryLogExtractionContext(string cwd, string[] args, string outputPath, public static string? GetAdjustedPath(ExtractionContext extractionContext, string sourcePath) { - if (extractionContext.Mode.HasFlag(ExtractorMode.BinaryLog) + if (extractionContext.IsBinaryLog && extractionContext is BinaryLogExtractionContext binaryLogExtractionContext && binaryLogExtractionContext.GetAdjustedPath(sourcePath) is string adjustedPath) { diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/ExtractionContext.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/ExtractionContext.cs index 0a2fc5258893..899be99e028d 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/ExtractionContext.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/ExtractionContext.cs @@ -16,6 +16,7 @@ public class ExtractionContext public string OutputPath { get; } public IEnumerable CompilationInfos { get; } public bool IsStandalone => Mode.HasFlag(ExtractorMode.Standalone); + public bool IsBinaryLog => Mode.HasFlag(ExtractorMode.BinaryLog); /// /// Creates a new extractor instance for one compilation unit.