Skip to content

Honour a non-public [JsonConstructor] when deciding constructor binding [patch] - #86

Merged
matt-edmondson merged 4 commits into
mainfrom
fix/55-non-public-json-constructor
Oct 31, 2026
Merged

matt-edmondson merged 4 commits into
mainfrom
fix/55-non-public-json-constructor

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #55

What was wrong

RequirementRuleCompiler.SelectDeserializationConstructor looked only at type.GetConstructors(), which returns public constructors only. System.Text.Json 8.0 and later also honour [JsonConstructor] on a private or internal constructor, which is the usual shape of an immutable model. For such a type:

  • Its get-only properties never counted as constructor-bound, so the holder was never claimed.
  • Each decorated child was then validated on its own.
  • Violations lost their path prefix: Tuning instead of Child.Tuning.
  • Only the first failing child was reported: B.Tuning never appeared next to A.Tuning.

Fix

  • SelectDeserializationConstructor now also searches non-public instance constructors for [JsonConstructor], but only when the running System.Text.Json honours them.
  • That is decided by a one-time runtime probe, NonPublicJsonConstructorSupport. It deserializes a private type whose only constructor is a private [JsonConstructor]. The library uses no conditional compilation, so a runtime check is the only option. net7.0's in-box System.Text.Json ignores such constructors, so it keeps the old public-only behaviour.
  • The probe uses the existing FieldAwareProbeOptions, which carry an explicit DefaultJsonTypeInfoResolver, so it does not depend on reflection being enabled by default.
  • The method's remarks, which called this a known divergence, are rewritten. CLAUDE.md is updated to match.

Tests

Three new tests in EligibilityTests, as the issue's acceptance criteria ask:

  • A class with a private [JsonConstructor] reports Child.Tuning.
  • A class with an internal two-parameter [JsonConstructor] reports both A.Tuning and B.Tuning in one exception.
  • A struct with a private [JsonConstructor] reports Child.Tuning.

Results:

  • Before the fix: all three fail as the issue describes. Two return ["Tuning"] instead of ["Child.Tuning"], and one returns ["Tuning"] instead of ["A.Tuning", "B.Tuning"].
  • After the fix: all three pass. The full suite passes 278/278 on the net10.0 leg.
  • Not run here: the net9.0 and net8.0 legs (this container has only the .NET 10 runtime), and the net7.0 path where the probe answers false. CI runs net10.0, net9.0 and net8.0.
  • Release build: all six library target frameworks build clean.

This touches the same method that #44 changed earlier, through #71 (constructor chosen from the deserialized type). That change is already on main, and this one builds on it.


Generated by Claude Code

…ng [patch]

SelectDeserializationConstructor looked only at public constructors, but System.Text.Json 8.0 and
later honour [JsonConstructor] on a private or internal one, the usual shape of an immutable model.
Its get-only properties never counted as populated, so the holder was not claimed and each decorated
child validated on its own: violations lost their path prefix and every one after the first was
dropped.

Non-public constructors are now considered when a one-time probe finds the running
System.Text.Json honours them, so net7.0's in-box copy keeps its public-only behaviour.

Fixes #55

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW4hDvfx3eQasG41fzL
The probe's refusal branches only ran on net7.0, which no test leg covers. CanConstruct now takes
the probe type, so tests reach both refusals on every leg: a type with no usable constructor, and
one whose [JsonConstructor] is ambiguous. A further test pins that every tested leg honours a
private [JsonConstructor].

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW4hDvfx3eQasG41fzL
…n-constructor

# Conflicts:
#	JsonRequiredConditionally.Test/TestModels.cs
…n-constructor

# Conflicts:
#	CLAUDE.md
#	JsonRequiredConditionally.Test/TestModels.cs
@sonarqubecloud

Copy link
Copy Markdown

@matt-edmondson
matt-edmondson merged commit 1921b4a into main Oct 31, 2026
15 checks passed
@matt-edmondson
matt-edmondson deleted the fix/55-non-public-json-constructor branch October 31, 2026 03:28
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.

A holder built through a non-public [JsonConstructor] loses the path prefix on its violations and drops every violation after the first

2 participants