Merge pull request #19089 from michaelnebel/csharp/improvestringinterpolation

C#: Extract string interpolation alignment and format.

Author: michaelnebel

csharp/downgrades/66044cfa5bbf2ecfabd06ead25e91db2bdd79764/old.dbscheme

@@ -0,0 +1,39 @@
+class Expr extends @expr {
+  string toString() { none() }
+}
+
+class TypeOrRef extends @type_or_ref {
+  string toString() { none() }
+}
+
+class InterpolatedStringInsertExpr extends Expr, @interpolated_string_insert_expr { }
+
+private predicate remove_expr(Expr e) {
+  exists(InterpolatedStringInsertExpr ie |
+    e = ie
+    or
+    // Alignment
+    expr_parent(e, 1, ie)
+    or
+    // Format
+    expr_parent(e, 2, ie)
+  )
+}
+
+query predicate new_expressions(Expr e, int kind, TypeOrRef t) {
+  expressions(e, kind, t) and
+  // Remove the syntheetic intert expression and previously un-extracted children
+  not remove_expr(e)
+}
+
+query predicate new_expr_parent(Expr e, int child, Expr parent) {
+  expr_parent(e, child, parent) and
+  not remove_expr(e) and
+  not remove_expr(parent)
+  or
+  // Use the string interpolation as parent instead of the synthetic insert expression
+  exists(InterpolatedStringInsertExpr ie |
+    expr_parent(e, 0, ie) and
+    expr_parent(ie, child, parent)
+  )
+}

csharp/downgrades/66044cfa5bbf2ecfabd06ead25e91db2bdd79764/semmlecode.csharp.dbscheme

@@ -0,0 +1,4 @@
+description: Remove `interpolated_string_insert_expr` kind.
+compatibility: backwards
+expressions.rel: run string_interpol_insert.qlo new_expressions
+expr_parent.rel: run string_interpol_insert.qlo new_expr_parent

csharp/downgrades/66044cfa5bbf2ecfabd06ead25e91db2bdd79764/string_interpol_insert.ql

@@ -29,6 +29,15 @@ public AnnotatedTypeSymbol(ITypeSymbol? symbol, NullableAnnotation nullability)
             symbol is null ? (AnnotatedTypeSymbol?)null : new AnnotatedTypeSymbol(symbol, NullableAnnotation.None);
     }
 
+    internal static class AnnotatedTypeSymbolExtensions
+    {
+        /// <summary>
+        /// Returns true if the type is a string type.
+        /// </summary>
+        public static bool IsStringType(this AnnotatedTypeSymbol? type) =>
+            type.HasValue && type.Value.Symbol?.SpecialType == SpecialType.System_String;
+    }
+
     internal static class SymbolExtensions
     {
         /// <summary>

csharp/downgrades/66044cfa5bbf2ecfabd06ead25e91db2bdd79764/upgrade.properties

@@ -18,7 +18,7 @@ private Expression CreateChild(Context cx, ExpressionSyntax node, int child)
         {
             // If this is a "+" expression we might need to wrap the child expressions
             // in ToString calls
-            return Kind == ExprKind.ADD
+            return Kind == ExprKind.ADD && Type.IsStringType()
                 ? ImplicitToString.Create(cx, node, this, child)
                 : Create(cx, node, this, child);
         }

csharp/extractor/Semmle.Extraction.CSharp/CodeAnalysisExtensions/SymbolExtensions.cs

@@ -39,16 +39,13 @@ private ImplicitToString(ExpressionNodeInfo info, IMethodSymbol toString) : base
             Context.TrapWriter.Writer.expr_call(this, target);
         }
 
-        private static bool IsStringType(AnnotatedTypeSymbol? type) =>
-            type.HasValue && type.Value.Symbol?.SpecialType == SpecialType.System_String;
-
         /// <summary>
         /// Creates a new expression, adding a compiler generated `ToString` call if required.
         /// </summary>
-        public static Expression Create(Context cx, ExpressionSyntax node, Expression parent, int child)
+        public static Expression Create(Context cx, ExpressionSyntax node, IExpressionParentEntity parent, int child)
         {
             var info = new ExpressionNodeInfo(cx, node, parent, child);
-            return CreateFromNode(info.SetImplicitToString(IsStringType(parent.Type) && !IsStringType(info.Type)));
+            return CreateFromNode(info.SetImplicitToString(!info.Type.IsStringType()));
         }
 
         /// <summary>

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/Binary.cs

@@ -1,5 +1,4 @@
 using System.IO;
-using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Semmle.Extraction.Kinds;
@@ -21,15 +20,7 @@ protected override void PopulateExpression(TextWriter trapFile)
                 {
                     case SyntaxKind.Interpolation:
                         var interpolation = (InterpolationSyntax)c;
-                        var exp = interpolation.Expression;
-                        if (Context.GetTypeInfo(exp).Type is ITypeSymbol type && !type.ImplementsIFormattable())
-                        {
-                            ImplicitToString.Create(Context, exp, this, child++);
-                        }
-                        else
-                        {
-                            Create(Context, exp, this, child++);
-                        }
+                        new InterpolatedStringInsert(Context, interpolation, this, child++);
                         break;
                     case SyntaxKind.InterpolatedStringText:
                         // Create a string literal

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/ImplicitToString.cs

@@ -0,0 +1,41 @@
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Semmle.Extraction.Kinds;
+
+namespace Semmle.Extraction.CSharp.Entities.Expressions
+{
+    internal class InterpolatedStringInsert : Expression
+    {
+        public InterpolatedStringInsert(Context cx, InterpolationSyntax syntax, Expression parent, int child) :
+            base(new ExpressionInfo(cx, null, cx.CreateLocation(syntax.GetLocation()), ExprKind.INTERPOLATED_STRING_INSERT, parent, child, isCompilerGenerated: false, null))
+        {
+            var exp = syntax.Expression;
+            if (parent.Type.IsStringType() &&
+                cx.GetTypeInfo(exp).Type is ITypeSymbol type &&
+                !type.ImplementsIFormattable())
+            {
+                ImplicitToString.Create(cx, exp, this, 0);
+            }
+            else
+            {
+                Create(cx, exp, this, 0);
+            }
+
+            // Hardcode the child number of the optional alignment clause to 1 and format clause to 2.
+            // This simplifies the logic in QL.
+            if (syntax.AlignmentClause?.Value is ExpressionSyntax alignment)
+            {
+                Create(cx, alignment, this, 1);
+            }
+
+            if (syntax.FormatClause is InterpolationFormatClauseSyntax format)
+            {
+                var f = format.FormatStringToken.ValueText;
+                var t = AnnotatedTypeSymbol.CreateNotAnnotated(cx.Compilation.GetSpecialType(SpecialType.System_String));
+                new Expression(new ExpressionInfo(cx, t, cx.CreateLocation(format.GetLocation()), ExprKind.UTF16_STRING_LITERAL, this, 2, isCompilerGenerated: false, f));
+            }
+
+        }
+    }
+
+}

csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/InterpolatedString.cs

@@ -132,6 +132,7 @@ public enum ExprKind
         UTF8_STRING_LITERAL = 135,
         COLLECTION = 136,
         SPREAD_ELEMENT = 137,
+        INTERPOLATED_STRING_INSERT = 138,
         DEFINE_SYMBOL = 999,
     }
 }