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/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..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,40 @@ 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. + /// + /// Type symbol + private bool IsBrokenType(ITypeSymbol symbol) + { + if (!Context.ExtractionContext.IsStandalone || + !symbol.FromSource() || + symbol.IsAnonymousType) + { + return false; + } + + // (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. + // 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) { switch (Symbol.SpecialType) @@ -48,6 +82,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; 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/Context.cs b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/Context.cs index 67bb2808ae62..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; @@ -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..899be99e028d 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Extractor/ExtractionContext.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Extractor/ExtractionContext.cs @@ -15,6 +15,8 @@ public class ExtractionContext public ExtractorMode Mode { get; } 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. 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" } } /** 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..d78284d3dfde --- /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` has 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..bb3acba4f64c --- /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 | 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 | 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 | 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 | 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 | ... + ... | | UnknownType | +| BrokenTypes.cs:26:30:26:31 | access to local variable x3 | | UnknownType | +| BrokenTypes.cs:26:30:26:36 | access to property (unknown) | | UnknownType | 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