Description
bridge::Translator's handle_hover, handle_definition, and other position-based handlers take line: u32 and character: u32 as adjacent bare-u32 positional arguments. Every MCP tool handler in mcp/server.rs destructures a PositionParams { file_path, line, character } and forwards line, character positionally into the matching handle_* call. Because both are the same primitive type in the same position, a copy-paste or refactor mistake that swaps the two arguments at a call site compiles cleanly and fails only at runtime (wrong hover/definition/etc. result for a given cursor position), with no compiler assistance.
This was identified during the #301 boilerplate-dedup refactor (PR #319): the PR explicitly states in its description that #301's originally cited failure mode (a destructure/call argument mismatch "compiles fine but can silently return the wrong shape") is not actually closed by that refactor -- collapsing the destructure/call/wrap pattern does not remove the swap hazard, since both the old per-tool handlers and the new centralized dispatch still pass line/character as two adjacent bare u32s. Newtypes (e.g. LineNumber(u32)/CharacterOffset(u32), or a single Position { line: u32, character: u32 } struct passed by name) are the only mechanism that would make a swap a compile error rather than a silent behavioral bug.
Reproduction Steps
grep -n "fn handle_hover\|fn handle_definition" crates/mcpls-core/src/bridge/translator.rs -- observe line: u32, character: u32 as adjacent same-typed parametersgrep -n "handle_hover(file_path, line, character)" crates/mcpls-core/src/mcp/server.rs -- observe the call site passes them positionally- Hypothetically swap the two arguments at any call site: the code still compiles,
cargo clippy -- -D warnings does not flag it, and only a runtime/integration test exercising a non-symmetric position (line != character) would catch the regression
Expected Behavior
A swapped line/character argument at any Translator::handle_* call site should be a compile-time type error, not a silent runtime behavior change.
Actual Behavior
line/character are both bare u32 in the same argument position across all position-taking Translator::handle_* methods and all their MCP tool call sites; a swap is undetectable by the type system.
Environment
Notes
Filed as a separate, deliberately out-of-scope follow-up from PR #319 -- fixing this requires changing Translator::handle_*'s public method signatures (a larger, more invasive change than the boilerplate dedup PR #319 addressed), and was explicitly flagged by that PR as the only real fix for the swap-hazard concern in #301's original issue body.
Description
bridge::Translator'shandle_hover,handle_definition, and other position-based handlers takeline: u32andcharacter: u32as adjacent bare-u32positional arguments. Every MCP tool handler inmcp/server.rsdestructures aPositionParams { file_path, line, character }and forwardsline, characterpositionally into the matchinghandle_*call. Because both are the same primitive type in the same position, a copy-paste or refactor mistake that swaps the two arguments at a call site compiles cleanly and fails only at runtime (wrong hover/definition/etc. result for a given cursor position), with no compiler assistance.This was identified during the #301 boilerplate-dedup refactor (PR #319): the PR explicitly states in its description that #301's originally cited failure mode (a destructure/call argument mismatch "compiles fine but can silently return the wrong shape") is not actually closed by that refactor -- collapsing the destructure/call/wrap pattern does not remove the swap hazard, since both the old per-tool handlers and the new centralized dispatch still pass
line/characteras two adjacent bareu32s. Newtypes (e.g.LineNumber(u32)/CharacterOffset(u32), or a singlePosition { line: u32, character: u32 }struct passed by name) are the only mechanism that would make a swap a compile error rather than a silent behavioral bug.Reproduction Steps
grep -n "fn handle_hover\|fn handle_definition" crates/mcpls-core/src/bridge/translator.rs-- observeline: u32, character: u32as adjacent same-typed parametersgrep -n "handle_hover(file_path, line, character)" crates/mcpls-core/src/mcp/server.rs-- observe the call site passes them positionallycargo clippy -- -D warningsdoes not flag it, and only a runtime/integration test exercising a non-symmetric position (line != character) would catch the regressionExpected Behavior
A swapped
line/characterargument at anyTranslator::handle_*call site should be a compile-time type error, not a silent runtime behavior change.Actual Behavior
line/characterare both bareu32in the same argument position across all position-takingTranslator::handle_*methods and all their MCP tool call sites; a swap is undetectable by the type system.Environment
Notes
Filed as a separate, deliberately out-of-scope follow-up from PR #319 -- fixing this requires changing
Translator::handle_*'s public method signatures (a larger, more invasive change than the boilerplate dedup PR #319 addressed), and was explicitly flagged by that PR as the only real fix for the swap-hazard concern in #301's original issue body.