Accept leading :: in Graph#[] lookups#814
Conversation
|
Friendly check-in on this one. It's a small surgical fix (strip an optional leading |
|
Closing this for now. The fix is small (accept an optional leading |
|
Sorry about that. We didn't review because the CLA was needing to be signed, but the change looks ok. I'll happily merge if the CLA is signed. |
|
CLA: Unexpected error has occurred, please re-run the workflow. We are sorry for the inconvenience, due to GitHub actions limitations this requires a manual intervention. There are few ways to do it:
If the issue persists, please contact the maintainers of this repo. |
CLA has been signed already. Thanks! |
|
CLA: Unexpected error has occurred, please re-run the workflow. We are sorry for the inconvenience, due to GitHub actions limitations this requires a manual intervention. There are few ways to do it:
If the issue persists, please contact the maintainers of this repo. |
`Rubydex::Graph.new["::Object"]` returned `nil` because the FFI lookup hashed the string verbatim, while the stored declaration is keyed on the unqualified name. Strip an optional leading `::` at the lookup boundary in both `Graph::get` and `rdx_graph_get_declaration` so the root-scope marker resolves to the same declaration as the bare name. Fixes Shopify#813
e9b2d4f to
0cf7dd3
Compare
|
Rebased onto the latest |
By moving the logic for stripping the leading `::` root-scope marker into a single function, we ensure that all declaration name lookups are normalized consistently. This prevents potential mismatches when looking up declarations with or without the leading `::`.
We don't define functions on type aliases for the `DefinitionId`, so let's not define a function on the `DeclarationId` type alias either.
3 participants
Closes #813
Rubydex::Graph.new["::Object"]returnednilbecause the FFI lookup hashed the input string verbatim, while declarations are keyed on the unqualified name (e.g.DeclarationId::from("Object")inbuilt_in.rs). The two affected entry points areGraph::getand the FFIrdx_graph_get_declarationthatGraph#[]forwards through.Stripping an optional leading
::at both lookup sites lets the root-scope marker resolve to the same declaration as the bare name, matching the way nested constants likeThread::Backtracealready work. Added a Rust unit test covering built-ins, top-level, nested, and unknown-name cases, plus a Ruby integration test that exercises the full FFI path.This is the lookup quirk koic hit in the Rubydex/RuboCop integration.