From ed8957710f43e9865977ffec6226c6a26c0baca5 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Tue, 20 Oct 2026 01:43:14 +0000 Subject: [PATCH] fix(objc): a file-scope constant or global is indexed, in an @implementation too (#2699) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Objective-C read a file-scope declaration through the generic variable fallback, which takes only a declarator that is a bare name (`int count;`). Its most common globals were never indexed: `NSString *const SDWebImageErrorDomain = @"…";`, `static const NSInteger kMaxRetries = 3;`, `static dispatch_queue_t queues[4];`. A declaration written inside an @implementation, file-scope C in the language, was not read at all, because the class counts as an enclosing scope. - tree-sitter.ts: Objective-C takes C's declarator branch in extractVariable (initialized, pointer and array declarators, with the initializer as the signature), keeping the bare names it always read. A declaration whose parent is an implementation_definition takes the variable branch; its globals are qualified under the class, as the C functions written there already are. A block or function-pointer variable reads as a prototype, as in C, and stays out. - c-cpp.ts: isConstDeclaration, C's isConst, is shared, and objc.ts uses it, so `static const NSInteger kMaxRetries` is a constant. - name-matcher.ts: a global that a .m or .mm file defines joins the C types that #2690 made file-local (OBJC_FILE_LOCAL_KINDS). A new isObjcFileLocalElsewhere drops such a candidate in the exact-match and fuzzy chains, before one is picked, and isVisibleAcrossFiles applies it to the pick. Before the pick, so that a header's `extern NSString *MJConfigFile;` stands for the definition in variables.m. Rejecting only the pick left Hammerspoon's MJConfigFile references unresolved. Without the rule, React Native's completionHandler(nil, image) block-parameter calls reached a completionHandler in another .mm, and #include a .mm's vector. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/objc-file-scope-variables.test.ts | 153 ++++++++++++++++++++ __tests__/objc-implementation-docs.test.ts | 1 + src/extraction/languages/c-cpp.ts | 19 ++- src/extraction/languages/objc.ts | 3 + src/extraction/tree-sitter.ts | 19 ++- src/resolution/name-matcher.ts | 36 ++++- 7 files changed, 214 insertions(+), 18 deletions(-) create mode 100644 __tests__/objc-file-scope-variables.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 053c09151..717d34ead 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -185,6 +185,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In Objective-C, a macro written alone on its line, like `NS_ASSUME_NONNULL_BEGIN`, `OF_SUBCLASSING_RESTRICTED` or React Native's `RCT_EXTERN_C_BEGIN`, no longer hides the declaration below it. Before, the parser read such a macro as the start of a declaration, so an `@interface` under it could come out as a function and a `typedef` under it as two variables: many of ObjFW's classes, with their properties and methods and the classes they extend and protocols they adopt, were missing, and so were classes and protocols in React Native and KSCrash headers. A message to `super` from a class whose `@interface` follows an attribute macro like `NS_SWIFT_NAME(…)` still reaches the superclass. Re-index Objective-C projects after upgrading. - In Objective-C, and in the C and C++ headers that sit beside it, an enum declared with Foundation's macros, like `typedef NS_ENUM(NSInteger, SDImageCacheType) { … };`, `NS_OPTIONS`, `NS_CLOSED_ENUM`, `NS_ERROR_ENUM` or CoreFoundation's `CF_ENUM` and `CF_OPTIONS`, is now indexed with its values. Before, the parser read the macro as a broken typedef, so these enums were missing (SDWebImage had none at all) and a type alias named after the first value stood in their place. A Swift annotation naming such an enum, like KSCrash's `let unwindMethods: KSBacktraceUnwindMethods`, now links to it. Re-index Objective-C, C and C++ projects after upgrading. - In Objective-C, the doc comment above a method, function or static written inside an `@implementation` is now read as its description, as the one above an `@interface` property already was. Before, the parser's wrapper around each item of an `@implementation` hid the comment, so search and `codegraph_explore` showed these methods without their documentation. Re-index Objective-C projects after upgrading. +- In Objective-C, a file-scope constant or global, like `NSString *const SDWebImageErrorDomain = @"SDWebImageErrorDomain";` or `static const NSInteger kMaxRetries = 3;`, is now indexed with its value, and so is one written inside an `@implementation`. Before, only a global declared with a bare name, like `int count;`, was, so error domains, notification names, keys and shared queues were missing from search and `codegraph_explore`. A global that a `.m` file defines is reached from other files through the header that declares it, and a call of a block parameter, like `completionHandler(nil, image)`, no longer links to a namesake that another `.m` file keeps. Re-index Objective-C projects after upgrading. - In TypeScript React code, a styled-components or emotion component declared with a type argument, like `const Wrapper = styled.div` followed by its CSS, or `styled(Base)`, is now indexed as a component, as one written without a type argument already was. Before, it was indexed as a plain constant, so `codegraph callers` and impact found none of the code that renders it, and its tags could link to another file's component of the same name instead: in outline/outline, `NudeButton` and `HStack` had no callers, and `` tags linked to a different `Text` than the one each file imports. Re-index TypeScript React projects after upgrading. - `codegraph sync` and the file watcher now move a React Router route to its page when the router file loads that page lazily into a value of its own, like `const Docs = lazy(() => import('./pages/Docs'))` rendered by `} />`, and the page's file is added after the router was indexed, comes back after being deleted, or gets or changes its default export in a later edit. Before, the route kept pointing at the `Docs` declaration, or at the page's old component, until the router file itself changed or the project was re-indexed, unless the page's component happened to share the declaration's name. The same now holds for a `lazy: () => import('./pages/Docs')` route whose page switches its default export to another component. Re-index React projects after upgrading to correct routes an earlier version left behind. - `codegraph sync` and the file watcher now link a component to the JSX that renders it, like ``, when the component is added in a file with no JSX of its own, as in `export default function Team() { return null; }`. Before, the link waited for an unrelated change to a file with JSX, or for a re-index, so the new component looked unused. diff --git a/__tests__/objc-file-scope-variables.test.ts b/__tests__/objc-file-scope-variables.test.ts new file mode 100644 index 000000000..b7159a302 --- /dev/null +++ b/__tests__/objc-file-scope-variables.test.ts @@ -0,0 +1,153 @@ +/** + * Objective-C read a file-scope variable only when its declarator was a bare + * name (`int plain;`), so its most common globals were never indexed: + * + * NSString *const SDWebImageErrorDomain = @"SDWebImageErrorDomain"; + * static const NSInteger kMaxRetries = 3; + * @implementation SDImageCache + * static NSString *const kKey = @"…"; // file-scope C inside @implementation + * + * Objective-C now reads a declaration's declarators as C does (initialized, + * pointer and array ones), keeps its bare names, and reads a declaration + * written inside an `@implementation` too. A global that a `.m` file defines + * is that file's, as C makes it: another file reaches it through the header + * that declares it, and never through a namesake in another `.m`, like a + * block parameter called `completionHandler(…)`. + */ +import { describe, it, expect, beforeAll, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import CodeGraph from '../src/index'; +import { extractFromSource } from '../src/extraction'; +import { initGrammars, loadGrammarsForLanguages } from '../src/extraction/grammars'; + +describe('an Objective-C file-scope variable', () => { + beforeAll(async () => { + await initGrammars(); + await loadGrammarsForLanguages(['objc']); + }); + + for (const crlf of [false, true]) { + const eol = (s: string) => (crlf ? s.replace(/\n/g, '\r\n') : s); + const label = crlf ? ' (CRLF)' : ''; + + it(`is indexed with its initializer, at file scope and inside an @implementation${label}`, () => { + const source = [ + '#import ', + '', + '/// The error domain.', + 'NSString *const SDWebImageErrorDomain = @"SDWebImageErrorDomain";', + 'static const NSInteger kMaxRetries = 3;', + 'static int counter = 0;', + 'int plain;', + 'static dispatch_queue_t queues[4];', + 'extern NSString *const SDExternDecl;', + 'static void (^gHandler)(void) = ^{ };', + '', + '@implementation SDImageCache', + 'static NSString *const kKey = @"key";', + '- (void)run', + '{', + '\tint local = 1;', + '\t(void)local;', + '}', + '@end', + '', + ].join('\n'); + const result = extractFromSource('src/SDImageCache.m', eol(source), 'objc'); + const symbols = result.nodes + .filter((n) => n.kind !== 'file' && n.kind !== 'import') + .map((n) => `${n.kind} ${n.qualifiedName}${n.signature ? ` ${n.signature}` : ''}`); + // A block or function-pointer variable reads as a prototype, as in C. + expect(symbols).toEqual([ + 'variable SDWebImageErrorDomain = @"SDWebImageErrorDomain"', + 'constant kMaxRetries = 3', + 'variable counter = 0', + 'variable plain', + 'variable queues', + 'variable SDExternDecl', + 'class SDImageCache', + 'variable SDImageCache::kKey = @"key"', + 'method SDImageCache::run', + ]); + expect(result.nodes.find((n) => n.name === 'SDWebImageErrorDomain')?.docstring).toBe('The error domain.'); + }); + } +}); + +describe('an indexed Objective-C project with globals', () => { + let root = ''; + let cg: CodeGraph | undefined; + + afterEach(() => { + cg?.destroy(); + cg = undefined; + if (root) fs.rmSync(root, { recursive: true, force: true }); + root = ''; + }); + + /** Index `files` and return what a method's edges, `contains` aside, reach. */ + async function indexProject(files: Record): Promise<(method: string) => string[]> { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-objc-globals-')); + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + const graph = (cg = await CodeGraph.init(root, { index: true })); + return (method: string) => { + const node = graph.getNodesByName(method).find((n) => n.kind === 'method')!; + expect(node, method).toBeDefined(); + return graph + .getOutgoingEdges(node.id) + .filter((e) => e.kind !== 'contains') + .map((e) => { + const t = graph.getNode(e.target)!; + return `${e.kind} ${t.kind} ${t.name} ${t.filePath}`; + }); + }; + } + + it('reaches a global through the header that declares it', async () => { + const targetsOf = await indexProject({ + 'Hammerspoon/variables.h': ['#import ', '', 'extern NSString *MJConfigFile;', ''].join('\n'), + 'Hammerspoon/variables.m': ['#import "variables.h"', '', 'NSString *MJConfigFile = @"~/.hammerspoon/init.lua";', ''].join('\n'), + 'Hammerspoon/MJConfigUtils.m': [ + '#import "variables.h"', + '', + '@implementation MJConfigUtils', + '- (NSString *)configPath', + '{', + '\treturn [MJConfigFile stringByStandardizingPath];', + '}', + '@end', + '', + ].join('\n'), + }); + expect(targetsOf('configPath')).toEqual(['references variable MJConfigFile Hammerspoon/variables.h']); + }, 60_000); + + it('never reaches a global another .m file defines', async () => { + const targetsOf = await indexProject({ + 'Libraries/RCTPushNotificationManager.m': [ + '#import ', + '', + 'static NSString *completionHandler = nil;', + '', + ].join('\n'), + 'Libraries/RCTBundleAssetImageLoader.m': [ + '#import ', + '', + '@implementation RCTBundleAssetImageLoader', + '- (void)loadWithCompletion:(void (^)(id image))completionHandler', + '{', + '\tcompletionHandler(nil);', + '}', + '@end', + '', + ].join('\n'), + }); + // The call is the block parameter's. + expect(targetsOf('loadWithCompletion:')).toEqual([]); + }, 60_000); +}); diff --git a/__tests__/objc-implementation-docs.test.ts b/__tests__/objc-implementation-docs.test.ts index c4fff6c84..d7aa866db 100644 --- a/__tests__/objc-implementation-docs.test.ts +++ b/__tests__/objc-implementation-docs.test.ts @@ -65,6 +65,7 @@ describe('a doc comment inside an Objective-C @implementation', () => { expect(docs).toEqual([ 'property Tests::runs: The number of runs.', 'struct Tests::g_callbackData: Flags the callbacks set.', + 'variable Tests::g_callbackData: Flags the callbacks set.', 'method Tests::testIt: Runs it.', 'function Tests::helper: A helper.', 'method Tests::shared: The shared instance.', diff --git a/src/extraction/languages/c-cpp.ts b/src/extraction/languages/c-cpp.ts index 8e75f9454..d2f211ec3 100644 --- a/src/extraction/languages/c-cpp.ts +++ b/src/extraction/languages/c-cpp.ts @@ -32,6 +32,17 @@ export function cDeclaratorIdentifier(node: SyntaxNode | null): SyntaxNode | nul return null; } +/** + * Whether a C or Objective-C declaration is a constant: a `const` or + * `static const` file-scope declaration carries a `type_qualifier` child + * reading "const". Extracted as `constant`, plain globals as `variable`. A + * `const` inside a pointer declarator (`char *const p`) qualifies the + * pointer, and the declaration stays a variable. + */ +export function isConstDeclaration(node: SyntaxNode): boolean { + return node.namedChildren.some((c: SyntaxNode) => c.type === 'type_qualifier' && c.text === 'const'); +} + /** C and C++ specifiers that define a class-like type, or an enum, when they carry a body (C has no `class_specifier`). */ const CLASS_LIKE_SPECIFIERS = new Set(['class_specifier', 'struct_specifier', 'union_specifier', 'enum_specifier']); @@ -431,13 +442,7 @@ export const cExtractor: LanguageExtractor = { nameField: 'declarator', bodyField: 'body', paramsField: 'parameters', - // A `const`/`static const` file-scope declaration carries a `type_qualifier` - // child reading "const" — extract those as `constant`, plain globals as - // `variable`. - isConst: (node) => - node.namedChildren.some( - (c: SyntaxNode) => c.type === 'type_qualifier' && c.text === 'const' - ), + isConst: isConstDeclaration, getReturnType: extractCppReturnType, resolveTypeAliasKind: (node, _source) => { // C typedef: `typedef enum { ... } name;` or `typedef struct { ... } name;` diff --git a/src/extraction/languages/objc.ts b/src/extraction/languages/objc.ts index 88fdc9292..d55588566 100644 --- a/src/extraction/languages/objc.ts +++ b/src/extraction/languages/objc.ts @@ -4,6 +4,7 @@ import type { ExtractorContext, LanguageExtractor } from '../tree-sitter-types'; import { blankLoneMacroLines, definingDeclaration, + isConstDeclaration, OBJC_LONE_MACROS, rewriteFoundationEnumMacros, unnamedTypeVariableName, @@ -129,6 +130,8 @@ export const objcExtractor: LanguageExtractor = { bodyField: 'body', paramsField: 'parameters', getReturnType: extractObjcReturnType, + // `static const NSInteger kMaxRetries = 3;` is a constant, as in C. + isConst: isConstDeclaration, // A method is named by its selector. An unnamed struct, union or enum // defined in a declaration's type takes the first variable's name, as in C: // `static struct { … } placeholder;` is the struct `placeholder`. diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index cf07a4080..27d222c32 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -1515,9 +1515,14 @@ export class TreeSitterExtractor { // languages) keep almost exclusively inside a class/module. A Ruby `CONST = // …` has a `constant`-typed LHS; other languages don't put one here, so this // is effectively Ruby-only and doesn't disturb their class-internal locals. + // An Objective-C declaration written in an `@implementation` + // (`static NSString *const kKey = @"…";`) is file-scope C, though it sits + // between `@implementation` and `@end`. else if ( this.extractor.variableTypes.includes(nodeType) && - (!this.isInsideClassLikeNode() || this.isClassScopeConstantAssignment(node)) + (!this.isInsideClassLikeNode() || + this.isClassScopeConstantAssignment(node) || + (this.language === 'objc' && node.parent?.type === 'implementation_definition')) ) { // C, C++ and Objective-C: a class, struct, union or enum defined in the // declaration's type (`struct Foo { … } foo;`, `static struct { … } @@ -3709,8 +3714,8 @@ export class TreeSitterExtractor { const initSignature = initValue ? `= ${initValue}${initValue.length >= 100 ? '...' : ''}` : undefined; this.createNode(kind, target.name, nameNode, { docstring, signature: initSignature, isExported }); }); - } else if (this.language === 'c') { - // C: a `declaration` node's name nests inside the `declarator` field — + } else if (this.language === 'c' || this.language === 'objc') { + // C and Objective-C: a `declaration` node's name nests inside the `declarator` field — // `init_declarator` (with value) or bare/pointer/array declarators (no // value); a `function_declarator` is a prototype, not a variable. The // generic fallback below only finds a *direct* identifier child, which C @@ -3733,11 +3738,15 @@ export class TreeSitterExtractor { // a spurious type-named global for every macro-prefixed prototype in a // header. Those misparses are always bare identifiers; real // consts/tables always carry an initializer. The only legit loss is - // uninitialized scalar globals (`static int g;`). + // uninitialized scalar globals (`static int g;`). Objective-C keeps + // them, as it did before it shared this branch: until then it read + // nothing but bare identifiers, and missed every + // `NSString *const kKey = @"…";`. if ( child.type !== 'init_declarator' && child.type !== 'pointer_declarator' && - child.type !== 'array_declarator' + child.type !== 'array_declarator' && + !(this.language === 'objc' && child.type === 'identifier') ) { continue; } diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 88f0fee1d..ad1d3f1c2 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -2002,8 +2002,23 @@ const C_STATIC_MEMO = new WeakMap>(); */ const C_SOURCE_EXT = /\.(c|cc|cpp|cxx|c\+\+|m|mm)$/i; -/** The C types an Objective-C file can define, as opposed to its classes and protocols. */ -const OBJC_C_TYPE_KINDS: ReadonlySet = new Set(['struct', 'union', 'enum', 'type_alias']); +/** + * What an Objective-C source file defines for itself, as C does: its C types + * and its globals, as opposed to its classes and protocols. + */ +const OBJC_FILE_LOCAL_KINDS: ReadonlySet = new Set(['struct', 'union', 'enum', 'type_alias', 'variable', 'constant']); + +/** + * Whether `n` is a C type or global that an Objective-C source file other + * than `ref`'s defines, which no other file can name. It leaves the + * candidates before one is picked, so a global's declaration in a header + * (Hammerspoon's `extern NSString *MJConfigFile;` in variables.h) stands for + * the definition in variables.m. isVisibleAcrossFiles applies the same rule + * to the pipeline's pick. + */ +function isObjcFileLocalElsewhere(n: Node, ref: UnresolvedRef): boolean { + return n.language === 'objc' && n.filePath !== ref.filePath && OBJC_FILE_LOCAL_KINDS.has(n.kind) && C_SOURCE_EXT.test(n.filePath); +} /** Per context: a C/C++ type's qualified name → its declarations. */ const CPP_TYPE_DECLS = new WeakMap>(); @@ -3145,8 +3160,9 @@ function isGoNameVisible(candidate: Node, ref: UnresolvedRef): boolean { * whichever USB class file ranked first. So is what a source file's * unnamed namespace declares, and a type only source files declare, with * its members (isCppTranslationUnitLocal). - * - **Objective-C**: a struct, union, enum or typedef a `.m` / `.mm` source - * file defines is local to that file, as in C. A class is not. + * - **Objective-C**: a struct, union, enum, typedef or global a `.m` / `.mm` + * source file defines is local to that file, as in C; other files reach a + * global through its header's `extern` declaration. A class is not. * - **Kotlin, Java, C#, Swift, Scala, Dart, PHP**: `private` is class- or * file-local. An Android `editor.apply()` resolved onto an unrelated class's * `private fun apply`. @@ -3220,8 +3236,14 @@ export function isVisibleAcrossFiles(candidate: Node, ref: UnresolvedRef, contex // An Objective-C source file is a translation unit too: a C type it defines // is its own. ObjFW's runtime keeps `static struct Lock { … } *locks;` in // synchronized.m, and the AmigaOS `Lock(path, mode)` calls in other files - // make no such struct. (A class is the runtime's, wherever it is written.) - if (lang === 'objc' && OBJC_C_TYPE_KINDS.has(candidate.kind) && C_SOURCE_EXT.test(candidate.filePath)) return false; + // make no such struct. So is a global it defines: another file reaches one + // through the header that declares it (`extern OFStdIOStream *OFStdOut;`), + // whose node stands for it there. React Native's + // `completionHandler(nil, image)` calls a block parameter, not the + // `completionHandler` another .mm file keeps, and `#include ` names + // no `.mm` file's `vector`. (A class is the runtime's, wherever it is + // written.) + if (isObjcFileLocalElsewhere(candidate, ref)) return false; if (lang === 'go') return isGoNameVisible(candidate, ref); if (lang === 'rust') { if (candidate.visibility !== 'private') return true; @@ -8437,6 +8459,7 @@ export function matchByExactName( !(objcShape === 'c-call' && OBJC_MEMBER_KINDS.has(n.kind)) && !(objcShape === 'self-send' && !isObjcSelfSendTarget(n, ref, context)) && !(objcShape === 'super-send' && !isObjcSelfSendTarget(n, ref, context, true)) && + !isObjcFileLocalElsewhere(n, ref) && !(vbReceiver !== null && !isVbMemberReachable(n, vbReceiver)) && !(vbReceiver && !/^(?:me|mybase|myclass)$/i.test(vbReceiver) && !isVbTypeQualifiedBy(n, vbReceiver, ref.filePath, context)) && !(vbScoped && !isVbMemberInScope(n, ref, context)) && @@ -18146,6 +18169,7 @@ export function matchFuzzy( !(objcShape === 'c-call' && OBJC_MEMBER_KINDS.has(n.kind)) && !(objcShape === 'self-send' && !isObjcSelfSendTarget(n, ref, context)) && !(objcShape === 'super-send' && !isObjcSelfSendTarget(n, ref, context, true)) && + !isObjcFileLocalElsewhere(n, ref) && !(csharpBare && !isCsharpMemberInScope(n, ref, context)) && !(scalaBare && !isScalaMemberInScope(n, ref, context)) && !(rustGoShape && !isRustGoCallTarget(n, rustGoShape)) &&