Skip to content

Invoke conversion methods that take an IFormatProvider or optional parameters - #96

Merged
matt-edmondson merged 1 commit into
mainfrom
claude/optimistic-clarke-cy7cby-95
Sep 27, 2026
Merged

matt-edmondson merged 1 commit into
mainfrom
claude/optimistic-clarke-cy7cby-95

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #95

Problem

FindStringConversionMethod accepted any FromString/Parse/Create/Convert method whose first parameter is a string. Read and ReadAsPropertyName then always called it with exactly one argument. So CanConvert claimed types it could not deserialize, and deserializing them threw TargetParameterCountException. Two common shapes hit this:

  • types with only the IParsable<T> shape, Parse(string, IFormatProvider?)
  • factories with an optional second parameter

Change

  • Overload choice: within each supported method name, a single-string overload is now preferred. Before, whichever overload GetMethods() returned first was used, and that order is unspecified.
  • Which methods are accepted (IsUsableConversionMethod): a method qualifies only if every parameter after the string is an IFormatProvider or optional. For any other shape, such as Create(string, int) with a required int, CanConvert now returns false.
  • How the method is called (BuildArguments): an IFormatProvider parameter gets CultureInfo.InvariantCulture, and an optional parameter gets Type.Missing, which makes the runtime use its declared default. Read and ReadAsPropertyName both use this.
  • New ExtraParameterConversionTests:
    • an IParsable-shaped round trip, as a value and as a dictionary key
    • a Create(string, int weight = 7) round trip that gets the default weight
    • rejection of a required extra parameter
    • the single-string overload winning over the IFormatProvider overload

Verification

  • With the library change reverted, all 5 new tests fail, four of them with TargetParameterCountException. With it in place, they pass. The overload-preference test also failed on main, which confirms the GetMethods() ordering problem is real.
  • Full suite: 101/101 passed on net10.0.

Not included

The triage suggested culture-invariant IFormattable serialization on the write side as a follow-up. It changes the output format, so I left it out of this PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_011wnB34hMv7jTvqsaysM9MS


Generated by Claude Code

…rameters [patch]

FindStringConversionMethod accepted any FromString/Parse/Create/Convert
whose first parameter is a string, but Read always invoked it with one
argument, so types with only the IParsable<T> shape
Parse(string, IFormatProvider?) or a factory with an optional second
parameter threw TargetParameterCountException on deserialization.

A single-string overload is now preferred. Otherwise a method is
accepted only when every extra parameter is an IFormatProvider (passed
the invariant culture) or optional (passed its default), and any other
shape is rejected so CanConvert returns false.

Fixes #95

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wnB34hMv7jTvqsaysM9MS
@sonarqubecloud

Copy link
Copy Markdown

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.

Deserialization throws TargetParameterCountException for types whose Parse/Create takes extra parameters (e.g. IParsable&lt;T&gt;)

1 participant