Report a call as unresolvable when nothing matched at all

[siegfriedpammer]

Decompiling FSharp.DataFrame from nuget.org crashes with a NullReferenceException in CallBuilder.IsAppropriateCallTarget.

OverloadResolution.BestCandidateErrors returns None for an empty candidate set - there is no best candidate to attach an error to - so IsUnambiguousCall read the null result of GetBestCandidateWithSubstitutedTypeArguments() as success and passed it on to be dereferenced. Only callvirt call sites crash: for the other opcodes the null is compared before it is dereferenced.

This PR is that guard: an empty candidate set is reported as unresolvable, so the call keeps its explicit form instead of failing the whole method.

What produces the empty candidate set in that assembly is a separate defect. F# compiles its comparison members to instance methods carrying operator metadata names (DelayedSource<'T,'U>.op_LessThan(a, b)), and the operator candidate search looks at the operand types rather than at the receiver type the member belongs to.

The new InstanceOperatorCall IL-pretty fixture is built from that shape - a generic instance op_LessThan called via callvirt, plus a direct call variant. Without the guard the callvirt method decompiles to the exception text; with it, decompilation completes. The expected output still shows the misclassification: the instance methods render as operator < / operator > and the call sites drop the receiver, so the fixture will be updated when that is fixed - it pins the crash, not the rendering.

Full ICSharpCode.Decompiler.Tests sweep: 3376 total, 0 failed, 46 skipped.

🤖 Generated with Claude Code

[christophwille]

Review summary

The diagnosis is right and the three pieces fit together well:

• MetadataMethod: requiring MethodAttributes.Static before assigning SymbolKind.Operator matches C#'s rule and matches the sibling explicit-interface-implementation branch a few lines below, which already required Static. It also makes MemberLookup stop hiding these members, which is what lets s.op_LessThan(a, b) resolve as an ordinary call.
• CallBuilder.IsUnambiguousCall: the null guard is a genuine pre-existing bug fix. With an empty candidate set BestCandidateErrors is None and IsAmbiguous is false, so GetBestCandidateWithSubstitutedTypeArguments() returned null and IsAppropriateCallTarget dereferenced it at actualTarget.IsOverride (only on the callvirt path; the call path happened to short-circuit). IsUnambiguousAccess already carries the same foundMember != null && guard, so this is consistent.
• ReplaceMethodCallsWithOperators: a gate here is definitely required. I verified that with only the MetadataMethod change and no gate, s.op_LessThan(a, b) still decompiles to a < b, silently dropping the receiver.

One issue with the gate's predicate - details inline.

The .il/.cs fixture and the csproj registration look correct (sortTypes: true puts CallSite before Source<T>, and .ver 1:0:0:0 producing no assembly attribute matches Issue2443/Issue1325).

Nit: the csproj change also adds a stray UTF-8 BOM to ICSharpCode.Decompiler.Tests.csproj line 1. No other .csproj in the repo has one - probably unintended editor churn worth dropping from the diff.

[christophwille]

IsOperator is a broader gate than this rewrite needs, and it regresses unresolved assembly references.

When the operator's declaring type comes from an assembly that could not be resolved, MetadataModule.ResolveMethodReference falls through to CreateFakeMethod, which only ever assigns SymbolKind.Constructor or SymbolKind.Method - never SymbolKind.Operator. So IsOperator is false for every operator call into an unresolved reference, and this early return now skips the rewrite for all of them. (Same for operators emitted without specialname, which some non-Roslyn producers do.)

Repro - IL calling op_Addition on a type from an .assembly extern UnknownAssembly that is not on disk, decompiled with ilspycmd:

// master
public static Vector3 Add(Vector3 a, Vector3 b)
{
    return a + b;
}

// this PR
public static Vector3 Add(Vector3 a, Vector3 b)
{
    return Vector3.op_Addition(a, b);
}

That scenario is not exotic - it is whenever someone opens a DLL in ILSpy without its dependencies loaded, and it is already an explicitly supported case here (TestCases/ILPretty/UnknownTypes.il). The existing operator fixtures all reference mscorlib/netstandard, so nothing in the suite catches this.

What the rewrite actually requires is that the arguments are the full operand list and there is no receiver to drop - i.e. IsStatic, not IsOperator. UserDefinedCompoundAssign.IsIncrementOrDecrement already pairs the two (method.IsOperator && method.IsStatic) for the same reason.

if (!method.IsStatic)
{
    // The rewrites below key off the metadata name alone and treat the arguments as the
    // operands. A method that only carries an operator name - an instance method from
    // another language, say - is an ordinary call whose receiver would be dropped.
    return;
}

I built this and confirmed it keeps InstanceOperatorCall decompiling exactly as the new fixture expects (s.op_LessThan(a, b) / s.op_GreaterThan(a, b)) while restoring a + b for the unresolved-reference case.