Skip to content

fix: Emit string.Concat instead of Expression.Add for string + operator#2

Merged
koenbeuk merged 2 commits intomainfrom
fix/string-concatenation-emission
Mar 26, 2026
Merged

fix: Emit string.Concat instead of Expression.Add for string + operator#2
koenbeuk merged 2 commits intomainfrom
fix/string-concatenation-emission

Conversation

@koenbeuk
Copy link
Collaborator

Summary

  • String concatenation via the + operator was incorrectly emitting Expression.MakeBinary(ExpressionType.Add, ...) which is only valid for numeric types — at runtime this throws InvalidOperationException
  • Now emits Expression.Call(string.Concat, ...), matching the existing interpolation codepath
  • Picks Concat(string, string) when both operands are strings, Concat(object, object) for mixed types (e.g. int + string with boxing)
  • Also fixes the same latent bug in EmitCompoundAssignment for string +=
  • Safe for overloaded operators: only triggers when OperatorMethod is null (built-in compiler intrinsic)

Test plan

  • 3 new snapshot tests (StringPlusString, IntPlusString, SingleStringConcat)
  • 14 existing snapshot tests updated (extension member + constructor tests that used string +)
  • New integration test (Select_SummaryConcat_ReturnsCorrectValues) verifying runtime correctness
  • Full test suite passes (897 tests, 0 failures)

🤖 Generated with Claude Code

String concatenation via the + operator was incorrectly emitting
Expression.MakeBinary(ExpressionType.Add, ...) which is only valid for
numeric types. The built-in C# string + is a compiler intrinsic with no
OperatorMethod, so it fell through to the plain MakeBinary path. Now
emits Expression.Call(string.Concat, ...) matching the interpolation
codepath, with Concat(string,string) for homogeneous operands and
Concat(object,object) for mixed types. Also fixes the same latent bug
in EmitCompoundAssignment for string +=.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 26, 2026 00:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes expression-tree emission for C# string concatenation so that +/+= producing string are emitted as Expression.Call(string.Concat, ...) rather than Expression.MakeBinary(ExpressionType.Add, ...), preventing runtime InvalidOperationException for string addition scenarios and aligning with the interpolation emission path.

Changes:

  • Update generator emission for string + and string += to call string.Concat (string/string vs object/object overload selection).
  • Add new generator snapshot tests covering string concatenation variants.
  • Add an integration test scenario/property to validate runtime correctness when executing expanded expressions.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/ExpressiveSharp.Generator/Emitter/ExpressionTreeEmitter.cs Emits string.Concat for + / += when the result type is string and the operator is intrinsic.
tests/ExpressiveSharp.IntegrationTests/Scenarios/Store/Models/Order.cs Adds an [Expressive] property using + string concatenation for integration coverage.
tests/ExpressiveSharp.IntegrationTests/Scenarios/Common/Tests/StringOperationTests.cs Adds integration test validating SummaryConcat expansion/runtime execution.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/StringConcatenationTests.cs New snapshot tests for string concatenation emission.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/StringConcatenationTests.StringPlusString.verified.txt New verified snapshot asserting Concat(string,string) emission.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/StringConcatenationTests.IntPlusString.verified.txt New verified snapshot asserting Concat(object,object) + Concat(string,string) emission.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/StringConcatenationTests.SingleStringConcat.verified.txt New verified snapshot asserting Concat(string,string) emission.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExtensionMemberTests.ExtensionMemberWithMemberAccess.verified.txt Updates snapshot to use Expression.Call(string.Concat, ...) instead of ExpressionType.Add.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExtensionMemberTests.ExtensionMemberOnInterface.verified.txt Updates snapshot to use Expression.Call(string.Concat, ...) instead of ExpressionType.Add.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ConstructorTests.ProjectableConstructor_WithSwitchExpression_AndExtraProperty.verified.txt Updates snapshot string building to use string.Concat calls.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ConstructorTests.ProjectableConstructor_WithSwitchExpression.verified.txt Updates snapshot string building to use string.Concat calls.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ConstructorTests.ProjectableConstructor_WithMultipleLocalVariables.verified.txt Updates snapshot string building to use string.Concat calls.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ConstructorTests.ProjectableConstructor_WithLocalVariable.verified.txt Updates snapshot string building to use string.Concat calls.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ConstructorTests.ProjectableConstructor_WithFullObject.verified.txt Updates snapshot string building to use string.Concat calls (and renumbers cached MethodInfos).
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ConstructorTests.ProjectableConstructor_ThisInitializer_WithIfElseInDelegated.verified.txt Updates snapshot string building to use string.Concat calls.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ConstructorTests.ProjectableConstructor_ThisInitializer_WithBodyAfter.verified.txt Updates snapshot string building to use string.Concat calls.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ConstructorTests.ProjectableConstructor_ThisInitializer_ChainedThisAndBase.verified.txt Updates snapshot string building to use string.Concat calls.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ConstructorTests.ProjectableConstructor_ReferencingStaticConstMember.verified.txt Updates snapshot string building to use string.Concat calls.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ConstructorTests.ProjectableConstructor_ReferencingPreviouslyAssignedProperty.verified.txt Updates snapshot string building to use string.Concat calls.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ConstructorTests.ProjectableConstructor_ReferencingBasePropertyInDerivedBody.verified.txt Updates snapshot string building to use string.Concat calls.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The fallback paths set field names but never emitted corresponding
MethodInfo declarations, which would produce uncompilable generated code.
Since string.Concat(string,string) and string.Concat(object,object)
exist in all .NET versions, replace with InvalidOperationException.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@koenbeuk koenbeuk merged commit beb7b46 into main Mar 26, 2026
2 checks passed
@koenbeuk koenbeuk deleted the fix/string-concatenation-emission branch March 26, 2026 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants