microsoft / microsoft/typespec
[http-client-csharp] Model factory overload disambiguation should use C# overload-resolution analysis
- Dominant language
- Java
- Stars
- 5.9k
- Forks
- 394
- Avg merge
- 1d 23h
- Merged PRs (30d)
- 104
Description
## Problem
`MethodSignatureHelper.GetMinimumRequiredParameterCount` decides how many leading parameters a generated model factory overload must require so it cannot collide with a published compatibility overload or a custom overload.
It reasons about **argument counts**, while C# overload resolution reasons about **argument types and forms**. Making the applicable argument counts disjoint is sufficient in the common case, but it cannot express several situations where two overloads stay ambiguous.
## Known gaps
### 1. Unordered implicit conversions
A positional type difference does not imply the position selects an overload:
```csharp
M(float count = default, string description = default);
M(decimal size, bool? enabled = default); // one-parameter prefix, as produced today
int value = 1;
M(value); // CS0121
```
`int` converts implicitly to both `float` and `decimal` and neither conversion is better.
Brute-forcing all 66 built-in numeric pairs against the compiler shows `decimal`↔`float` and `decimal`↔`double` are the only affected built-in pairs — every other pair is ordered either by an implicit conversion (`int`→`long`) or by the signed/unsigned better-conversion-target rules (`int` beats `uint`, `short` beats `ushort`). User-defined implicit conversions are unbounded and invisible to the current type-name comparison.
### 2. Equal-arity overloads
Two overloads with the same parameter count cannot be disambiguated by optionality at all, because no required-prefix length separates them:
```csharp
A(string id, string description, string text, bool? isRegex); // generated
A(string id = default, string description = default,
string text = default, string isRegex = default); // custom
A("a", "b", "c", null); // CS0121 regardless of the prefix required
```
### 3. Named arguments
Named arguments can keep both candidates applicable no matter which prefix is required, since they bypass positional matching:
```csharp
M(id: "i", name: "n", kind: "k"); // CS0121
```
#### Verified end-to-end example
This shape is reachable from the generator today. Given a last contract of
```csharp
public static CompatibilityModel CompatibilityModel(string id = default, int count = default);
```
and a current model exposing a required `Count` (`int`) plus an optional `Description` (`string`), the generator emits:
```csharp
public static CompatibilityModel CompatibilityModel(int count, string description = default)
[EditorBrowsable(EditorBrowsableState.Never)]
public static CompatibilityModel CompatibilityModel(string id = default, int count = 0)
```
Compiling a consumer against that pair:
| Call | Result |
| --- | --- |
| `CompatibilityModel(count: 1)` | **CS0121 ambiguous** |
| `CompatibilityModel(id: "x")` | ok |
| `CompatibilityModel("x", 1)` | ok |
| `CompatibilityModel(1)` | ok |
Only the named form breaks: each overload omits exactly one optional parameter, so neither wins the "fewer omitted optional parameters" tie-break.
The required-prefix computation is not what creates this. `count` is required because the property is required, and `GetMinimumRequiredParameterCount` returns the same value the natural signature would have had. The pre-PR shape (`M(string id = default, int count = default)` versus `M(int count = default, string description = default)`) is ambiguous for `M(count: 1)` as well.
Making `description` required too does resolve this particular call, but that direction -- requiring parameters that correspond to newly added optional properties -- regresses existing back-compat expectations, which is why it needs the conversion-aware analysis described below rather than a targeted tweak.
### 4. Parameter modifiers
`ref`/`out` and `params` affect applicability and are only partially accounted for.
## Proposed work
Replace the count-based heuristic with conversion-aware applicability and better-function-member analysis, ideally driven by Roslyn's `Compilation.ClassifyConversion` rather than a hand-maintained conversion table. Where an ambiguity cannot be resolved by adjusting optionality (gap 2), the generator should detect it and report a diagnostic instead of silently emitting overloads that do not compile for some call shapes.
Suggested acceptance criteria:
- Consumer-compilation regressions covering unordered numeric conversions, named arguments, `null` literals, equal-arity overloads, and parameter modifiers.
- A diagnostic when two emitted overloads cannot be made unambiguous.
## Context
Found while reviewing #11703, which introduced the required-prefix logic to preserve published model factory optionality. These gaps are **not regressions from that PR** — before it, the competing overloads were fully optional and the same calls were already ambiguous. #11703 narrows the problem but does not close it.
Relevant code: `packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs` (`GetMinimumRequiredParameterCount`, `RequireMinimumParameterPrefix`).
Contributor guide
Research direction
Start in packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/src/Shared/MethodSignatureHelper.cs, reading GetMinimumRequiredParameterCount and RequireMinimumParameterPrefix. Trace the generated model-factory overloads and consumer-compilation coverage for numeric conversions, named arguments, null literals, equal arity, and parameter modifiers. Done means ambiguous emitted overloads are detected with a diagnostic and the listed regressions are covered.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend-api-design, compilers, testing-qa
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100