-
Notifications
You must be signed in to change notification settings - Fork 14
fix(parity): align WASM/native extraction for swift and jelly-micro fixtures #1594
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
592da5d
b0f5fd0
0301a61
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,7 @@ function walkSwiftNode(node: TreeSitterNode, ctx: ExtractorOutput): void { | |
| handleSwiftCallExpression(node, ctx); | ||
| break; | ||
| case 'property_declaration': | ||
| seedSwiftPropertyTypeMap(node, ctx); | ||
| handleSwiftPropertyDecl(node, ctx); | ||
| break; | ||
| } | ||
|
|
@@ -250,11 +251,26 @@ function handleSwiftCallExpression(node: TreeSitterNode, ctx: ExtractorOutput): | |
| if (!funcNode) return; | ||
| const call: Call = { name: '', line: node.startPosition.row + 1 }; | ||
| if (funcNode.type === 'navigation_expression') { | ||
| // obj.method(...) | ||
| // obj.method(...) — Swift's tree-sitter grammar wraps the suffix in a | ||
| // `navigation_suffix` node: navigation_expression > [simple_identifier, navigation_suffix]. | ||
| // We must descend into navigation_suffix to get the bare method name. | ||
| // Mirrors Rust match_swift_node which reads node_text of the last child directly | ||
| // (which equals ".method") and the receiver from child(0). | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The comment says the TypeScript "Mirrors Rust Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed — the comment now says both TS and Rust descend into |
||
| const lastChild = funcNode.child(funcNode.childCount - 1); | ||
| const firstChild = funcNode.child(0); | ||
| if (lastChild && lastChild.type === 'simple_identifier' && firstChild) { | ||
| call.name = lastChild.text; | ||
| if (lastChild && firstChild) { | ||
| // Resolve the method name: descend into navigation_suffix to find the | ||
| // simple_identifier, or fall back to stripping the leading dot from the text. | ||
| let methodName: string; | ||
| if (lastChild.type === 'simple_identifier') { | ||
| methodName = lastChild.text; | ||
| } else if (lastChild.type === 'navigation_suffix') { | ||
| const inner = findChild(lastChild, 'simple_identifier'); | ||
| methodName = inner ? inner.text : lastChild.text.replace(/^\./, ''); | ||
| } else { | ||
| methodName = lastChild.text; | ||
| } | ||
| call.name = methodName; | ||
| call.receiver = firstChild.text; | ||
| } | ||
| } else if (funcNode.type === 'simple_identifier') { | ||
|
|
@@ -265,6 +281,33 @@ function handleSwiftCallExpression(node: TreeSitterNode, ctx: ExtractorOutput): | |
| if (call.name) ctx.calls.push(call); | ||
| } | ||
|
|
||
| /** | ||
| * Seed the typeMap for a property_declaration with a type annotation. | ||
| * This runs for ALL property_declaration nodes (including class-body ones) | ||
| * so that `repo.method()` calls can be resolved to the correct class. | ||
| * Mirrors Rust match_swift_type_map which walks all nodes unconditionally. | ||
| */ | ||
| function seedSwiftPropertyTypeMap(node: TreeSitterNode, ctx: ExtractorOutput): void { | ||
| const typeAnn = findChild(node, 'type_annotation'); | ||
| if (!typeAnn) return; | ||
| // type_annotation: ":" <user_type | simple_identifier | ...> | ||
| // The last child is the actual type node. | ||
| const lastChild = typeAnn.child(typeAnn.childCount - 1); | ||
| if (!lastChild) return; | ||
| // For "user_type > type_identifier", grab the inner identifier text; | ||
| // for a plain simple_identifier, use it directly. | ||
| const typeNode = | ||
| lastChild.type === 'user_type' ? findChild(lastChild, 'type_identifier') : lastChild; | ||
| if (!typeNode) return; | ||
| const typeName = typeNode.text; | ||
| if (!typeName) return; | ||
| const pattern = findChild(node, 'pattern'); | ||
| if (!pattern) return; | ||
| const varName = findChild(pattern, 'simple_identifier')?.text ?? pattern.text; | ||
| if (!varName) return; | ||
| ctx.typeMap.set(varName, { type: typeName, confidence: 0.9 }); | ||
| } | ||
|
|
||
| function handleSwiftPropertyDecl(node: TreeSitterNode, ctx: ExtractorOutput): void { | ||
| // Only handle top-level properties (class properties are handled inline) | ||
| if ( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR summary says "Native extractor missed object-spread property extraction —
javascript.rsupdated to handle{ ...obj }spread patterns." However, this diff contains no changes tocrates/codegraph-core/src/extractors/javascript.rs, and the actual code here handles method definitions insidelet/varobject literals ({ m() {} },{ k: () => {} }), not{ ...obj }spread patterns. Ifjavascript.rsalready hadmatch_js_objlit_qualified_method_defsbefore this PR, it would be helpful to confirm that explicitly; if it still needs to be added, that fix appears to be missing. Didcrates/codegraph-core/src/extractors/javascript.rsalready havematch_js_objlit_qualified_method_defsforlet/varobject literals before this PR? The description impliesjavascript.rswas changed here, but no diff for that file is present. If Rust was already correct, the description is just wrong; if Rust still needs the fix, it's missing from this PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Investigated —
match_js_objlit_qualified_method_defsalready exists injavascript.rs(no change needed there). The PR description's mention of spread patterns andjavascript.rschanges was inaccurate. The TS functionextractLetVarObjLiteralDeclaratorsmirrors the existing Rust equivalent. Updated the JSDoc to say exactly that: it mirrorsmatch_js_objlit_qualified_method_defsinjavascript.rs. Commit: 0301a61.