Type-check pins when creating a link

[matt-edmondson]

Fixes #472

The defect

NodeEditorEngine.TryCreateLink validated direction, duplicates, connection limits and self-links, but never compared the pins' DataType. A string output connected to an int input and the result reported Success, leaving the graph holding a link whose value cannot survive the trip — and leaving a host with nothing to surface, since the message said the link was created.

The issue is accurate about the pieces already existing: PinTypeUtilities.CanConnect(Type, Type) in ktsu.NodeGraph implements the rules (exact match, assignability, numeric conversion, string), Pin already carries Type? DataType, and ImGui.NodeEditor already project-references NodeGraph. So this is a use of both rather than a new policy.

The change

One check, placed with the direction check and ahead of the graph's own state, because an incompatible pair is refused on its own terms rather than because of what the graph currently holds:

if (outputPin.DataType is Type outputType
    && inputPin.DataType is Type inputType
    && !PinTypeUtilities.CanConnect(outputType, inputType))
{
    return new LinkCreationResult(false, $"Cannot connect {outputType.Name} output '{outputPin.EffectiveDisplayName}' to {inputType.Name} input '{inputPin.EffectiveDisplayName}'");
}

The message names both types and both pin names, which is what the issue asks for — a host can say which end is wrong, not merely that the drag failed. Measured against the library's own nodes:

Cannot connect NumberData output 'Result' to Double input 'a'

NodeEditorHistory.TryCreateLink delegates to this method, so the undo/redo path is covered without a second change.

Only a pair that both declare a type is checked

This is the one judgement worth arguing, so the reasoning in full.

CanConnect answers false for a null argument rather than "unknown". Pin.DataType is Type? and PinSpec's own doc comment describes null as "an untyped pin" — a supported way to describe one, not a defect. So handing an untyped pin to CanConnect would refuse every link that pin appears in: that is most of the existing test suite and every caller of the name-only CreateNode overloads, which is the entire untyped API.

Hence the is Type pattern on both ends. Untyped stays permissive; a declared pair is checked. A fix that skipped this reads as smaller and breaks 200+ call sites.

Tests

Four, in NodeEditorEngineTests beside the existing TryCreateLink cases.

test	covers
TryCreateLink_RefusesPinsWhoseDeclaredTypesCannotConnect	the reported defect — string output into an int input, and that the message names both types and both pin names
TryCreateLink_RefusesExecutionFlowIntoADataPin	a void execution pin into a data input, which CanConnect allows only against another void
TryCreateLink_ConnectsPinsWhoseDeclaredTypesAreCompatible	the over-correction — a widening numeric conversion must stay connectable
TryCreateLink_DoesNotTypeCheckAnUntypedPin	the other over-correction — the null-DataType reasoning above, one end declared and neither end declared

The split matters: the last two pass in both configurations, which is correct for no-regression guards. They are what a fix refusing every typed pair, or one passing nulls straight to CanConnect, would fail.

Proved failing without the fix. Reverting only ImGui.NodeEditor/NodeEditorEngine.cs and keeping the tests:

failed TryCreateLink_RefusesPinsWhoseDeclaredTypesCannotConnect (17ms)
  Assertion failed. Expected condition to be false.
  A string output should not feed an int input.
  actual: true
failed TryCreateLink_RefusesExecutionFlowIntoADataPin (0ms)
  Assertion failed. Expected condition to be false.
  Execution flow should not feed a data pin.
  actual: true

  total: 266   failed: 2   succeeded: 264

Both fail on the reported symptom — the link being created — not on a message that merely changed shape.

Verification

• dotnet build ImGui.sln -c Release — succeeded, 0 warnings, 0 errors (this repo runs analyzers as errors)
• dotnet test tests/ImGui.NodeEditor.Tests -c Release — 266 total, 266 passed, 0 failed, 0 skipped
• dotnet test tests/NodeGraph.Tests -c Release — 106 total, 106 passed, 0 failed
• Same NodeEditor suite against reverted NodeEditorEngine.cs — 2 of 266 failed, as above

Baseline on unmodified main was 262 passed for the NodeEditor suite, so the 262 pre-existing tests are unaffected in both directions. Run on .NET SDK 10.0.401, Linux.

The check caught two miswired demo links

Worth reading before reviewing the third file in the diff, because it is a behaviour change rather than a tidy-up.

CleanImNodesDemo fed makeNumber1/makeNumber2's output — a NumberData structure — straight into MathOperations.Add(double a, double b). Those two links previously succeeded and now do not:

make1.Out0 -> add.In0 => False : Cannot connect NumberData output 'Result' to Double input 'a'
make2.Out0 -> add.In1 => False : Cannot connect NumberData output 'Result' to Double input 'b'

They were wrong before this change; nothing reported it. Leaving them would ship a demo whose graph is silently missing two edges, so each operand is routed through a Split (whose Value output is a double), which is the Make → Split → Operate pattern the surrounding comments already describe. That needed one added SplitNumberNode for makeNumber2's chain, since MakeNumberNode exposes no scalar output.

I verified the rest of the demo graph rather than assuming: the vector chain is unaffected, because float → double and double → float are both allowed numeric conversions, and every Vector2Data edge is an exact match. The demo UI tests assert no link counts, and the full solution builds clean.

Tagged [minor] on the commit subject: a caller whose typed pins were incompatible got a link before and gets a refusal now.

Not in this change

Nothing widens CanConnect itself. typeof(object) outputs, which AttributeBasedNodeFactory produces for an attribute-declared pin with no explicit type, are only assignable to object and string under the existing rules — so such a pin is now narrower than an untyped one. That asymmetry is pre-existing in CanConnect and is a policy question for the type system rather than for this wiring; nothing in the repository currently produces it on a connected pin.

Type coercion at execution time is also untouched. This refuses the link; it does not convert a value that crosses a compatible-but-not-identical edge.

🤖 Generated with Claude Code

https://claude.ai/code/session_01J9LhTGkoRSYpyGzriwfeZd

---

Generated by Claude Code

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud