Skip to content
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

C#: Introduce synthetic ToString calls where appropriate. #18442

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

C#: Introduce synthetic ToString calls where appropriate. #18442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions csharp/.vscode/launch.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"program": "${workspaceFolder}/extractor/Semmle.Extraction.CSharp.Standalone/bin/Debug/net9.0/Semmle.Extraction.CSharp.Standalone.dll",
"args": [],
// Set the path to the folder that should be extracted:
"cwd": "${workspaceFolder}/ql/test/library-tests/standalone/standalonemode",
"cwd": "${workspaceFolder}/ql/test/library-tests/testdataflow",
"env": {
"CODEQL_THREADS": "1",
"CODEQL_EXTRACTOR_CSHARP_OPTION_LOGGING_VERBOSITY": "progress+++",
Expand Down Expand Up @@ -68,9 +68,9 @@
"preLaunchTask": "dotnet: build",
"program": "${workspaceFolder}/extractor/Semmle.Extraction.CSharp.Driver/bin/Debug/net9.0/Semmle.Extraction.CSharp.Driver.dll",
// Set the path to the folder that should be extracted:
"cwd": "${workspaceFolder}/ql/test/library-tests/dataflow/local",
"cwd": "${workspaceFolder}/ql/test/library-tests/testdataflow",
"args": [
"LocalDataFlow.cs"
"stringinterpolated.cs"
],
"env": {},
"stopAtEntry": true,
Expand Down
6 changes: 3 additions & 3 deletions csharp/extractor/Semmle.Extraction.CSharp/Entities/Expression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public static Expression Create(Context cx, ExpressionSyntax node, IExpressionPa
return CreateFromNode(info);
}

public static Expression CreateFromNode(ExpressionNodeInfo info) => Expressions.ImplicitCast.Create(info);
public static Expression CreateFromNode(ExpressionNodeInfo info) => Expressions.Implicit.Create(info);

/// <summary>
/// Creates an expression from a syntax node.
Expand Down Expand Up @@ -208,7 +208,7 @@ private static bool ContainsPattern(SyntaxNode node) =>

if (type.SpecialType is SpecialType.None)
{
return ImplicitCast.CreateGeneratedConversion(cx, parent, childIndex, type, defaultValue, location);
return Implicit.CreateGeneratedConversion(cx, parent, childIndex, type, defaultValue, location);
}

if (type.SpecialType is SpecialType.System_DateTime)
Expand All @@ -220,7 +220,7 @@ private static bool ContainsPattern(SyntaxNode node) =>
type.SpecialType is SpecialType.System_IntPtr ||
type.SpecialType is SpecialType.System_UIntPtr)
{
return ImplicitCast.CreateGenerated(cx, parent, childIndex, type, defaultValue, location);
return Implicit.CreateGenerated(cx, parent, childIndex, type, defaultValue, location);
}

// const literal:
Expand Down
2 changes: 1 addition & 1 deletion 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 @@ -45,7 +45,7 @@ private static ExprKind AccessKind(Context cx, ISymbol symbol)
private Access(ExpressionNodeInfo info, ISymbol symbol, bool implicitThis, IEntity target)
: base(info.SetKind(AccessKind(info.Context, symbol)))
{
if (!(target is null))
if (target is not null)
{
Context.TrapWriter.Writer.expr_access(this, target);
}
Expand Down
63 changes: 42 additions & 21 deletions ...harp/Entities/Expressions/ImplicitCast.cs → ...n.CSharp/Entities/Expressions/Implicit.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,36 +5,30 @@

namespace Semmle.Extraction.CSharp.Entities.Expressions
{
internal sealed class ImplicitCast : Expression
internal sealed class Implicit : Expression
{
public Expression Expr
{
get;
private set;
}

private ImplicitCast(ExpressionNodeInfo info)
private Implicit(ExpressionNodeInfo info)
: base(new ExpressionInfo(info.Context, info.ConvertedType, info.Location, ExprKind.CAST, info.Parent, info.Child, isCompilerGenerated: true, info.ExprValue))
{
Expr = Factory.Create(new ExpressionNodeInfo(Context, info.Node, this, 0));
Factory.Create(new ExpressionNodeInfo(Context, info.Node, this, 0));
}

private ImplicitCast(ExpressionNodeInfo info, IMethodSymbol method)
: base(new ExpressionInfo(info.Context, info.ConvertedType, info.Location, ExprKind.OPERATOR_INVOCATION, info.Parent, info.Child, isCompilerGenerated: true, info.ExprValue))
private Implicit(ExpressionNodeInfo info, IMethodSymbol method, ExprKind kind, int child)
: base(new ExpressionInfo(info.Context, info.ConvertedType, info.Location, kind, info.Parent, info.Child, isCompilerGenerated: true, info.ExprValue))
{
Expr = Factory.Create(info.SetParent(this, 0));
Factory.Create(info.SetParent(this, child));

AddOperatorCall(method);
AddCall(method);
}

private ImplicitCast(ExpressionInfo info, IMethodSymbol method, object value) : base(info)
private Implicit(ExpressionInfo info, IMethodSymbol method, object value) : base(info)
{
Expr = Literal.CreateGenerated(Context, this, 0, method.Parameters[0].Type, value, info.Location);
Literal.CreateGenerated(Context, this, 0, method.Parameters[0].Type, value, info.Location);

AddOperatorCall(method);
AddCall(method);
}

private void AddOperatorCall(IMethodSymbol method)
private void AddCall(IMethodSymbol method)
{
var target = Method.Create(Context, method);
Context.TrapWriter.Writer.expr_call(this, target);
Expand Down Expand Up @@ -72,7 +66,7 @@ ExpressionInfo create(ExprKind kind, string? v) =>
if (method is not null)
{
var info = create(ExprKind.OPERATOR_INVOCATION, null);
return new ImplicitCast(info, method, value);
return new Implicit(info, method, value);
}
else
{
Expand All @@ -81,6 +75,21 @@ ExpressionInfo create(ExprKind kind, string? v) =>
}
}

/// <summary>
/// Gets the `ToString` method for the given type.
/// </summary>
private static IMethodSymbol? GetToStringMethod(ITypeSymbol type)
{
return type
.GetMembers()
.OfType<IMethodSymbol>()
.Where(method =>
method.GetName() == "ToString" &&
method.Parameters.Length == 0
)
.FirstOrDefault();
}

/// <summary>
/// Creates a new generated cast expression.
/// </summary>
Expand Down Expand Up @@ -130,7 +139,7 @@ info.Parent is ExplicitObjectCreation objectCreation &&
}

if (resolvedType.Symbol is not null)
return new ImplicitCast(info, conversion.MethodSymbol);
return new Implicit(info, conversion.MethodSymbol, ExprKind.OPERATOR_INVOCATION, 0);
}

var implicitUpcast = conversion.IsImplicit &&
Expand All @@ -144,7 +153,19 @@ resolvedType.Symbol is null ||

if (!conversion.IsIdentity && !implicitUpcast)
{
return new ImplicitCast(info);
return new Implicit(info);
}

// Implicit call to ToString.
if (!conversion.IsIdentity &&
resolvedType.Symbol is not null &&
implicitUpcast && // Maybe write the condition explicitly.
info.Parent is Expression par && // TODO: Only choose a specific set of parents (maybe BinaryExpression and StringInterpolation expressions?)
par.Type.HasValue && par.Type.Value.Symbol?.SpecialType == SpecialType.System_String)
{
return GetToStringMethod(resolvedType.Symbol) is IMethodSymbol toString
? new Implicit(info, toString, ExprKind.METHOD_INVOCATION, -1)
: Factory.Create(info);
}

if (conversion.IsIdentity && conversion.IsImplicit &&
Expand All @@ -153,7 +174,7 @@ convertedType.Symbol is IPointerTypeSymbol &&
{
// int[] -> int*
// string -> char*
return new ImplicitCast(info);
return new Implicit(info);
}

// Default: Just create the expression without a conversion.
Expand Down
Loading