Skip to content

fix(objc): resolve dot-syntax property access and @selector() calls (#1475)#1543

Closed
guyoron1 wants to merge 1 commit into
Graphify-Labs:v8from
guyoron1:fix/objc-dot-syntax-and-selector-1475
Closed

fix(objc): resolve dot-syntax property access and @selector() calls (#1475)#1543
guyoron1 wants to merge 1 commit into
Graphify-Labs:v8from
guyoron1:fix/objc-dot-syntax-and-selector-1475

Conversation

@guyoron1

Copy link
Copy Markdown
Contributor

Summary

  • Add field_expression handler in walk_calls so dot-syntax property access (self.name, self.product.name) produces calls edges — previously invisible because the grammar emits field_identifier, not a message send
  • Add selector_expression handler so @selector(method) compile-time references produce calls edges — previously unhandled node type
  • Both use the same suffix-match resolution strategy as the existing message_expression handler

Closes the remaining two sub-bugs from #1475 (Bug 5: dot-syntax, Bug 6: @selector).

Test plan

  • test_objc_dot_syntax_property_calls_edge — verifies self.name resolves to a calls edge
  • test_objc_selector_expression_calls_edge — verifies @selector(fetch) resolves to a calls edge
  • All 17 ObjC tests pass
  • Full test suite: 269 passed, 13 skipped, 0 failures

@safishamsi safishamsi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling the two deferred ObjC follow-ups. A few changes needed before this can land.

1. Drop the #1308 commit from this PR. It re-includes the workspace exports-map work, but that already shipped on v8 (e8dabad, from your #1541) — and the merged version has two things this copy is missing: a package-dir containment guard (so an exports target like "./x": "../../../etc/..." can't resolve outside the package) and default consulted last in the condition order. The duplicate is also what's causing all the merge conflicts here. Please rebase on current v8 and reduce this PR to the ObjC changes only.

2. The dot-syntax / @selector resolution needs a scoping guard — this is the blocker. These two were deferred specifically because resolving an untyped name globally floods false edges. Right now the resolution suffix-matches the property/selector name against every method id, with no receiver-type restriction and no single-definition check. On a fixture with two classes that each declare a property named name, one self.product.name access fans out to both -name methods (plus the product class node) — four edges from one access. @selector(doThing) with two doThing methods resolves to both. That's the god-node inflation (#543/#1219) the deferral was meant to avoid.

Please resolve only to an unambiguous, in-scope definition:

  • for self.<prop>, restrict to the current class's own declared properties / the known receiver type;
  • for @selector(...), emit only when exactly one method matches;
  • skip emission entirely when 0 or >1 candidates match (and don't match property names against class node ids).

3. Use accesses for dot-syntax, not calls. The issue scopes self.product.name to a property-access edge; the PR currently emits calls.

4. Add a fan-out regression test. The current tests are happy-path (one uniquely-named method, any(... in ...) assertions) and wouldn't catch the flooding above. Please add a two-same-named-property fixture asserting the exact edge count (no fan-out), plus an unresolvable-property case asserting zero edges, and the same for a two-same-named @selector.

For reference, the alloc/init reference edges that just landed (#1475 on v8) use the unique-stub guard (ensure_named_node + _rewire_unique_stub_nodes, which only resolves when exactly one class of that name exists) — the same "resolve only when unambiguous" idea applies here. The new walk_calls branches otherwise coexist fine with that change, so once the guard and the rebase are in, this should be a clean ObjC-only PR. Thanks again.

@guyoron1 guyoron1 force-pushed the fix/objc-dot-syntax-and-selector-1475 branch from 5bd5d0b to 9142946 Compare June 29, 2026 15:17
@guyoron1

guyoron1 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

Hey !!!
Thank you so much the detailed review!
All four points are now addressed:

  1. Dropped TypeScript workspace imports do not resolve package subpath exports #1308 commit — rebased on current v8. The PR is now a single ObjC-only commit.

  2. Scoping guard added — the blocker is fixed:

    • self.<prop> (field_expression): resolution is restricted to sibling methods within the same class (via a class_method_nids index built from the method_bodies container). Emits only when exactly 1 sibling matches; skips on 0 or >1.
    • @selector(...) (selector_expression): emits only when exactly 1 method matches across the entire file; skips on 0 or >1.
    • Neither handler matches property names against class node IDs.
  3. accesses for dot-syntax — field_expression edges use relation: "accesses" (not calls). @selector keeps calls with context: "call".

  4. Fan-out regression tests added — five new tests:

    • test_objc_dot_syntax_property_accesses_edge — happy path, exactly 1 accesses edge
    • test_objc_dot_syntax_no_fanout_two_same_named_properties — two classes with -name, asserts exactly 2 scoped accesses (one per class), no cross-class fan-out
    • test_objc_dot_syntax_unresolvable_property_zero_edgesself.missing → 0 edges
    • test_objc_selector_expression_calls_edge — unique @selector(fetch) → 1 calls edge
    • test_objc_selector_no_fanout_two_same_named_methods@selector(doThing) with 2 classes having -doThing → 0 edges

All tests pass.

Graphify-Labs#1475)

Add field_expression handler in walk_calls so dot-syntax property access
(self.name) produces an `accesses` edge scoped to the current class — only
resolves when exactly one sibling method matches, preventing god-node
fan-out when multiple classes declare the same property name.

Add selector_expression handler so @selector(method) compile-time
references produce a `calls` edge — only when exactly one method matches
across the entire file, skipping ambiguous names entirely.

Both guards follow the same "resolve only when unambiguous" principle as
the alloc/init unique-stub guard from Graphify-Labs#1475.

Five new tests cover happy-path, fan-out regression (two same-named
properties/selectors), and unresolvable-property edge cases.
@guyoron1 guyoron1 force-pushed the fix/objc-dot-syntax-and-selector-1475 branch from 9142946 to 476fac1 Compare June 29, 2026 15:20
safishamsi pushed a commit that referenced this pull request Jun 30, 2026
…1475, #1543)

`self.product.name` dot-syntax now emits an `accesses` edge and
`@selector(method)` emits a `calls` edge, both resolved only to an
unambiguous in-scope definition (a sibling method of the same class for
dot-syntax; exactly one method by exact selector name for @selector) so
no false-edge fan-out occurs when multiple classes share a name.

Hardened over the original PR: resolution now matches the method node id
EXACTLY (a method id is _make_id(container, name)) rather than by
`endswith` suffix. The substring match would mis-resolve `self.name` to a
sibling `-surname` (false positive) and, when a substring-colliding
sibling existed, suppress the correct edge (false negative); exact
matching fixes both. Adds substring-collision regression tests
(`-name`/`-surname`, `-doThing`/`-reallyDoThing`).

Completes the #1475 ObjC follow-ups (Bug 5 dot-syntax accesses, Bug 6b
@selector target-action).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@safishamsi

Copy link
Copy Markdown
Collaborator

Merged into v8 (0792b41) with your authorship — thanks for the rework. The scoping (sibling-only for dot-syntax, unique-match for @selector) and the move to accesses are exactly right.

One change I made on top: the resolver matched the method node id by endswith (suffix substring), which had two bugs your fixtures didn't cover — self.name would falsely resolve to a sibling -surname (...surname ends with name), and a substring-colliding sibling would suppress the correct edge. Since a method id is _make_id(container, name), I switched both branches to exact-id matching and added substring-collision regression tests (-name/-surname, -doThing/-reallyDoThing). Verified end-to-end. Ships next release.

@safishamsi safishamsi closed this Jun 30, 2026
safishamsi added a commit that referenced this pull request Jun 30, 2026
README: document that `reflect --graph` writes the .graphify_learning.json
overlay and that explain/query surface a Lesson hint (with the
code-changed staleness flag).

CHANGELOG: add an Unreleased section for the post-0.9.2 work — the
work-memory overlay (#1441/#1542), this.field.method() injected-field
resolution (#1316), TS wildcard path aliases (#1544), JS namespace
re-exports (#1552), and the ObjC dot-syntax/@selector edges (#1475/#1543).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants