Skip to content

fix(objc): fold category/extension interfaces into the base class (#1556) - #2501

Open
xiongjianxu wants to merge 1 commit into
Graphify-Labs:v8from
xiongjianxu:fix/objc-category-folds-into-base-class
Open

fix(objc): fold category/extension interfaces into the base class (#1556)#2501
xiongjianxu wants to merge 1 commit into
Graphify-Labs:v8from
xiongjianxu:fix/objc-category-folds-into-base-class

Conversation

@xiongjianxu

Copy link
Copy Markdown

Problem

An Objective-C category (@interface Foo (Cat)) or class extension (@interface Foo ()) declares members of an existing class. But the extractor keyed the class node off the file stem:

name = _read(identifiers[0])
cls_nid = _make_id(stem, name)      # stem == "Foo+Cat" for Foo+Cat.h

So Foo.h and Foo+Cat.h each mint a node labelled Foo. _merge_decl_def_classes cannot fold them — it requires exactly one header per id-collision group, and category files reach it as two distinct headers.

The consequence is not a cosmetic duplicate; it destroys edges. Controlled experiment, same call site, only the location of the method declaration differs:

// Caller.m — unchanged in both runs
- (void)go { [Base useIt]; }
where -useIt is declared resulting calls edges
in Base.h's @interface Base -go -> -useIt EXTRACTED ✅
in Base+Extra.h's @interface Base (Extra) none

Measured on 0.9.34:

base_extra_base | Base | Base+Extra.h
base_base       | Base | Base.h
type_def_nids["base"] == ['base_extra_base', 'base_base']   -> len != 1 -> guard bails

Categories are pervasive in real ObjC (NSString+Trim.h, private extensions in every .m), so this hits ordinary code. grep -ic 'categor' graphify/extractors/objc.py0: they were never handled.

Fix

Two coordinated changes.

1. graphify/extractors/objc.py — a category/extension interface (and implementation) keys its class node off the base stem:

cls_stem = _objc_category_base_stem(stem) if _objc_is_category(node) else stem

_objc_is_category tests for the grammar's anonymous ( child, which appears only for a category or extension — a generic class (@interface Box<T>) uses parameterized_arguments, so it is not matched. Verified against the tree-sitter-objc grammar:

category        ['@interface', 'identifier', '(', 'identifier', ')', ...]
extension       ['@interface', 'identifier', '(', ')', ...]
generic class   ['@interface', 'identifier', 'parameterized_arguments', ':', 'identifier', ...]

Being keyed on syntax rather than filename is what makes it safe: Extra+Helpers.h declaring a plain @interface Helper keeps its own stem. _objc_category_base_stem additionally splits only a well-formed Name+Suffix pair, so C++Bridge.h and Foo+.h are left intact.

2. graphify/extractors/resolution.py_merge_decl_def_classes now folds a category header into the base header. It already strips + when computing sibling stems (_decldef_class_stem); it just refused to pick a keeper when several headers were present. It now keeps the base header (the stem with no +), or the lowest-sorting category header when the base class lives outside the corpus (NSString+Trim.h, NSString+JSON.h). Two non-category headers still bail to id-disambiguation, so an unrelated Foo.h / Foo.hpp pair behaves exactly as before.

Tests

New tests/test_objc_category_interfaces.py — each fix case paired with a scoping case that must not change:

test asserts
test_objc_category_method_is_reachable_from_another_class one Base node; -go -> -useIt EXTRACTED
test_objc_class_extension_folds_into_the_base_class anonymous @interface Base () in Base.m folds; -pub -> -priv
test_objc_non_category_interface_in_a_plus_named_file_is_untouched Extra+Helpers.h's plain @interface Helper keeps its own stem
test_objc_same_named_categories_in_different_directories_stay_distinct two Thing classes in a/ and b/ stay separate; [Thing act] stays ambiguous → zero edges

The two behavioral tests fail on v8 without the fix (verified by stashing both source changes: 2 failed, 2 passed).

Verification

uv run --frozen pytest tests/ -q --tb=short
  4048 passed, 3 skipped in 163.77s        (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 <changed files>
  All checks passed!

uv.lock untouched. Base branch: v8.

Notes

Issue creation is restricted for external contributors, so the reproduction is inline. This is a defect in the #1556 ObjC member-call pass, hence the reference.

Independent of my #2500 (protocol declarations as receiver types) — different files, no conflict — but the two compose: a corpus with both a category and a same-named protocol needs each fix to clear the god-node guard.

…aphify-Labs#1556)

`@interface Foo (Cat)` in `Foo+Cat.h` declares members of an EXISTING
class, but the class node was keyed off the `Foo+Cat` file stem — so
`Foo.h` and `Foo+Cat.h` produced TWO nodes labelled `Foo`. Every
`[Foo ...]` receiver then had two type-def candidates, tripped the
member-call resolver's single-definition god-node guard, and emitted
nothing: moving a method from a class body into a category silently
destroyed call edges the same corpus resolved fine before.

Category and class-extension interfaces (and implementations) now key
off the base stem, and `_merge_decl_def_classes` folds a category header
into the base header instead of bailing on "more than one header".

Scoping: the fold is keyed on the CATEGORY SYNTAX, not on a `+` in the
filename, so `Extra+Helpers.h` declaring a plain `@interface Helper`
keeps its own stem. Two non-category headers still bail to
id-disambiguation, and same-named classes in different directories stay
distinct (the id embeds the directory path).

@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 addresses ObjC category and class-extension interfaces (@interface Foo (Cat) / @interface Foo ()) that were keying their class node off the full file stem (e.g. Foo+Cat), which minted a second node for the same class. It adds helpers to detect category/extension syntax and strip the +-suffix down to a base stem, uses them when generating class/impl node ids in the ObjC extractor, and relaxes the decl/def class merge logic to fold multiple sibling headers (base + category) into a single keeper header. A new test file exercises category method resolution, class-extension folding, and cases where the fold should not apply, alongside changelog entries.

No blocking issues surfaced. 5 lower-confidence candidates did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 1474 functions depend on the 327 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: extract() — 370 callers, 39 callees
  • worse: walk() — 1 callers, 10 callees

Verification — 1474 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: 781 function(s) in the blast radius were not formally verified this run

· 2 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