Use string interpolation in System.Text.Json

[Copilot]

Converts two string concatenation expressions to string interpolation in ReflectionEmitMemberAccessor.cs, where DynamicMethod names are constructed from a runtime variable.

Description

• ReflectionEmitMemberAccessor.cs: Replace memberName + "Getter" / memberName + "Setter" with $"{memberName}Getter" / $"{memberName}Setter"

// Before
new DynamicMethod(memberName + "Getter", ...)
new DynamicMethod(memberName + "Setter", ...)

// After
new DynamicMethod($"{memberName}Getter", ...)
new DynamicMethod($"{memberName}Setter", ...)

Note: JsonValue.cs has a superficially similar const + string concatenation inside an [RequiresUnreferencedCode] attribute — this cannot be converted since attribute arguments must be compile-time constants.

---

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

[github-actions]

🔒 Security Review — PR #14

Security-relevant changes detected: No

This PR replaces two instances of string concatenation (memberName + "Getter" / memberName + "Setter") with string interpolation ($"{memberName}Getter" / $"{memberName}Setter") in ReflectionEmitMemberAccessor.cs. The change is purely cosmetic — no behavioral difference, no new code paths, no changes to input handling, access control, or memory safety.

Findings

No findings.

Verdict: PASS ✅

Reasoning: This is a trivial style refactor (string concatenation → string interpolation) with identical runtime semantics. No security-relevant code is introduced or modified.

Generated by Security Review · ◷

[github-actions]

🔍 API Surface Review — PR #14

Public API changes detected: No

Findings

✅ No Public API Changes

This PR replaces two string concatenation expressions (memberName + "Getter" / memberName + "Setter") with string interpolation ($"{memberName}Getter" / $"{memberName}Setter") in the private methods CreateGetterMethod and CreateSetterMethod within ReflectionEmitMemberAccessor.cs. These are internal implementation details with no impact on the public API surface.

No ref assembly update, API approval, or breaking-change analysis is required.

Verdict

PASS ✅ — No public API surface affected.

Generated by API Surface Review · ◷

[github-actions]

🤖 Copilot Code Review — PR #14

Holistic Assessment

Motivation: The PR replaces two instances of string concatenation (memberName + "Getter", memberName + "Setter") with string interpolation ($"{memberName}Getter", $"{memberName}Setter") in ReflectionEmitMemberAccessor.cs. This is a pure style/readability change with zero behavioral impact. String interpolation is the generally preferred modern C# style, but the existing concatenation was perfectly clear and functional.

Approach: The approach is correct — the two substitutions are the only string concatenation sites in the file, and the rest of the Metadata/ directory already uses interpolation. The change is consistent.

Summary: ⚠️ Needs Human Review. The code change is correct and harmless, but this is a style-only PR with no functional impact. dotnet/runtime maintainers have historically pushed back on pure style changes ("Don't change existing code for style alone"), so a human should decide whether to accept this.

---

Detailed Findings

✅ Correctness — String interpolation is functionally equivalent

The change from memberName + "Getter" to $"{memberName}Getter" (and the same for "Setter") is semantically identical. The C# compiler lowers both to string.Concat(memberName, "Getter"). The DynamicMethod name parameter is used only for debugging/reflection metadata — no behavior changes.

Verified: both call sites are in private static helper methods (CreateGetterMethod, CreateSetterMethod) that construct DynamicMethod objects. No callers depend on the exact name format.

✅ No public API surface changes

No ref/ assembly changes, no new public members. API approval verification is not required.

⚠️ Value & Justification — Very low-value style-only change

This PR changes 2 lines for purely cosmetic reasons. While string interpolation is idiomatic modern C#, the existing concatenation was clear and the DynamicMethod name string is not user-facing. dotnet/runtime conventions note: "Match existing style in modified files" and "Do not change existing code for style alone." A maintainer should decide if this meets the bar for a merge.

✅ Branch divergence — Workflow file diffs are not part of this PR

The diff against main shows ~1,400 lines of changes in .github/workflows/ and .agentic/ files. These are not intentional modifications — the copilot branch was created from commit 1d7981da, and main has since received 20+ workflow hardening commits. The workflow differences are purely branch divergence. No security-relevant workflow changes are introduced by this PR.

💡 Suggestion — Consider consistency sweep if accepted

If this change is accepted, the same memberName + "..." pattern does not appear elsewhere in System.Text.Json, so no additional consistency sweep is needed. The two changed lines were the only instances.

Generated by Code Review · ◷

[github-actions]

Aggregated Review Summary

``

PR #14 Review Synthesis

Code Review

• Correctness: The change from string concatenation to string interpolation in ReflectionEmitMemberAccessor.cs is functionally equivalent and correct. No behavioral impact.
• Value: This is a style-only change. Maintainers may push back on pure style PRs; human review is recommended to determine if it meets the bar for merge.
• API Surface: No public API changes. No ref assembly or breaking change analysis required.
• Branch Divergence: Workflow file diffs are unrelated to this PR and are due to branch divergence.
• Suggestion: If accepted, no further consistency sweep is needed—these were the only instances.

Security Review

• Findings: No security-relevant changes. The refactor is cosmetic only.
• Verdict: PASS

API Surface Review

• Findings: No public API changes. Internal-only, no impact on public surface.
• Verdict: PASS

---

Labels Applied

• ai:high-confidence — All reviews agree the change is correct, safe, and has no impact beyond style.
• ai:ready-for-human — No blocking findings; human review is recommended for style-only PRs.

``

Note

This summary and label application were generated by an AI Copilot agent based on specialist reviewer outputs. Please verify before merging.

Generated by Review Aggregator · ◷

[Copilot]

Pull request overview

Updates DynamicMethod name construction in System.Text.Json’s Reflection.Emit member accessor to use string interpolation instead of string concatenation for getter/setter method names.

Changes:

• Replaced memberName + "Getter" with $"{memberName}Getter".
• Replaced memberName + "Setter" with $"{memberName}Setter".

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[danmoseley]

@copilot please address the review feedback on this PR.

[Copilot]

@copilot please address the review feedback on this PR.

The automated review generated no new comments on the changed file — there's no feedback to address. The two string concatenation changes in ReflectionEmitMemberAccessor.cs were reviewed and approved as-is.

[MihaZupan]

@MihuBot