Skip to content

fix(objc): don't treat @protocol declarations as receiver types (#1556) - #2500

Open
xiongjianxu wants to merge 1 commit into
Graphify-Labs:v8from
xiongjianxu:fix/objc-protocol-not-a-type
Open

fix(objc): don't treat @protocol declarations as receiver types (#1556)#2500
xiongjianxu wants to merge 1 commit into
Graphify-Labs:v8from
xiongjianxu:fix/objc-protocol-not-a-type

Conversation

@xiongjianxu

Copy link
Copy Markdown

Problem

Objective-C keeps protocol names and class names in separate namespaces — Foundation itself ships both @protocol NSObject and @interface NSObject. A same-named pair is ordinary ObjC, not a mistake.

The ObjC extractor labels a protocol declaration <Name> (graphify/extractors/objc.py:264), and _resolve_objc_member_calls's index key strips non-alphanumerics:

def _key(label: str) -> str:
    return re.sub(r"[^a-zA-Z0-9]+", "", str(label)).lower()

So <Locking> and Locking collapse to the same key locking, and a protocol becomes a receiver-typing candidate. Both outcomes are wrong:

1. Protocol only in the corpus — a WRONG edge at confidence 1.0.

// Reload.h
@protocol Reload <NSObject>
- (void)reload;
@end

// Use.m
@implementation Use
- (void)go { [Reload reload]; }
@end

No class named Reload exists, so the receiver is untypable and the god-node guard should bail. Instead:

-go -> -reload   calls  EXTRACTED  (target: reload_reload_reload, Reload.h)

The edge points at the protocol's method declaration. A protocol is a contract; it is never a message receiver.

2. Protocol AND class present — a real edge is destroyed.

// Locking.h
@protocol Locking <NSObject>
- (void)lock;
@end

// LockingImpl.h / .m
@interface Locking : NSObject
+ (void)lock;
@end

// Worker.m
- (void)run { [Locking lock]; }

type_def_nids["locking"] == ['locking_locking' (<Locking>), 'lockingimpl_locking' (Locking)]len(type_defs) != 1 → the single-definition guard bails, and -run -> +lock is never emitted (measured: 0 call edges).

Fix

Exclude protocol declarations from the receiver-type index in _resolve_objc_member_calls only. The <Name> label shape is unique to ObjC protocols — grep -rn 'f"<{' graphify/ returns exactly one hit, extractors/objc.py:264 — so the check is unambiguous, and keeping it local to this resolver leaves _is_type_like_definition (7 call sites, 6 other languages) untouched.

Protocols remain valid implements targets and valid _rewire_unique_stub_nodes destinations; only receiver typing ignores them. Verified: Widget implements <Reload> still resolves to the real protocol node.

Tests

New tests/test_objc_member_calls.py, following the test_csharp_member_calls.py / test_java_member_calls.py convention — every case pairs the positive assertion with a decoy that must get no edge:

test asserts
test_objc_protocol_only_receiver_emits_no_call_edge protocol-only receiver → zero call edges (decoy: the protocol's own -reload)
test_objc_class_resolves_past_a_same_named_protocol -run -> +lock EXTRACTED present, -run -> -lock absent
test_objc_protocol_stays_a_valid_implements_target Widget implements <Reload> unaffected

Both new behavioral tests fail on v8 without the fix (verified by stashing the extract.py change: 2 failed, 1 passed) and pass with it.

Verification

uv run --frozen pytest tests/ -q --tb=short
  4047 passed, 3 skipped in 161.52s        (v8 baseline: 4044 passed, 3 skipped)

uv run --frozen python -m tools.skillgen --check --audit-coverage --schema-singleton \
                                          --monolith-roundtrip --always-on-roundtrip
  all 5 OK

uv run --frozen ruff check --config pyproject.toml graphify/extract.py tests/test_objc_member_calls.py
  All checks passed!

uv.lock untouched. Base branch: v8.

Notes

No open issue covers this; issue creation is restricted for external contributors, so the reproduction is inline above. It is a defect in the #1556 ObjC member-call pass, hence the reference.

This is the first of a few small, independent ObjC precision fixes I have measured on 0.9.34; the others (category/class-extension interfaces minting a duplicate class node, and @property/ivar receivers never being typed) are separate PRs so each can be reviewed on its own.

…hify-Labs#1556)

ObjC keeps protocol and class names in separate namespaces, but the
member-call resolver's index key strips the extractor's `<Name>` label
for protocols, so a same-named protocol/class pair collapsed to one key:

  * protocol only in corpus -> `[Reload reload]` bound to the PROTOCOL's
    method declaration at confidence 1.0 (a wrong edge, not a missing one);
  * protocol AND class present -> two candidates tripped the
    single-definition god-node guard, so a call that should resolve
    produced no edge at all.

Exclude protocol declarations from the receiver-type index. Protocols stay
valid `implements` targets; only receiver typing ignores them.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).

Graphify reviewed this change.

Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).


Graphify review — findings

This PR modifies the Objective-C member-call resolver in graphify/extract.py so that @protocol declarations (labeled <Name> by the extractor) are excluded from the receiver-type index used for resolving message-send calls. A new _is_protocol_declaration helper detects the angle-bracket label and filters those nodes out when building type_def_nids, while leaving protocols available as implements targets. It also adds a CHANGELOG entry (#1556) and a new test file tests/test_objc_member_calls.py covering three scenarios: a protocol-only receiver producing no call edge, a class resolving despite a same-named protocol, and a protocol still serving as a valid implements target.

No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 1463 functions depend on the 381 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: extract() — 369 callers, 39 callees

Verification — 1463 functions in the blast radius were not formally verified this run (proofs are advisory here).

Gate & verification

graphify gate

PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.

Advisory (not blocking):

  • verification_scope: 1333 function(s) in the blast radius were not formally verified this run

· 1 more finding(s) on lines outside this diff (see the check run).

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.

1 participant