From ebe6ff2f3aead18e8abd95b93db5efedeeabbdb7 Mon Sep 17 00:00:00 2001 From: Kazuki Yamada Date: Mon, 24 Nov 2025 21:37:29 +0900 Subject: [PATCH] fix(treeSitter): Fix Swift subscript parameter name collision bug Fixed a critical functional bug where multiple subscripts with the same parameter name (e.g., 'key') would create identifier collisions in the parsed output, making them indistinguishable. The fix changes the subscript capture pattern from capturing the parameter name to capturing the entire subscript_declaration node with @name.definition.method tags. This ensures each subscript is uniquely identified by its full signature (e.g., "subscript(key: String) -> Value?") rather than just the parameter name. Added comprehensive test cases: - Swift protocol/class with multiple subscripts to validate the fix - C# mixed inheritance (base class + multiple interfaces) for better coverage All 869 tests pass with this change. --- src/core/treeSitter/queries/querySwift.ts | 13 +--- .../core/treeSitter/parseFile.csharp.test.ts | 72 +++++++++++++++++++ tests/core/treeSitter/parseFile.swift.test.ts | 65 +++++++++++++++++ 3 files changed, 139 insertions(+), 11 deletions(-) diff --git a/src/core/treeSitter/queries/querySwift.ts b/src/core/treeSitter/queries/querySwift.ts index 6454062e..589c4825 100644 --- a/src/core/treeSitter/queries/querySwift.ts +++ b/src/core/treeSitter/queries/querySwift.ts @@ -12,13 +12,7 @@ export const querySwift = ` (function_declaration name: (simple_identifier) @name ) @definition.method - ; Note: Subscript currently captures parameter name (e.g., "key") rather than - ; the subscript itself. This may cause non-unique identifiers when multiple - ; subscripts use the same parameter name. Consider capturing the type signature - ; or the entire subscript declaration in future improvements. - (subscript_declaration - (parameter (simple_identifier) @name) - ) @definition.method + (subscript_declaration) @name.definition.method (init_declaration "init" @name) @definition.method (deinit_declaration "deinit" @name) @definition.method ] @@ -29,10 +23,7 @@ export const querySwift = ` (protocol_function_declaration name: (simple_identifier) @name ) @definition.method - ; Note: Same subscript parameter naming consideration as in class_body above - (subscript_declaration - (parameter (simple_identifier) @name) - ) @definition.method + (subscript_declaration) @name.definition.method (init_declaration "init" @name) @definition.method ] ) diff --git a/tests/core/treeSitter/parseFile.csharp.test.ts b/tests/core/treeSitter/parseFile.csharp.test.ts index 43d8cfae..febfcabd 100644 --- a/tests/core/treeSitter/parseFile.csharp.test.ts +++ b/tests/core/treeSitter/parseFile.csharp.test.ts @@ -256,4 +256,76 @@ describe('parseFile for C#', () => { expect(result).toContain(expectContent); } }); + + test('should parse mixed inheritance with base class and interfaces', async () => { + const fileContent = ` + // Base class + public class Animal { + public virtual void Speak() { + Console.WriteLine("Animal speaks"); + } + } + + // Interface for movement + public interface IMovable { + void Move(); + } + + // Interface for sound + public interface IAudible { + void Hear(); + } + + // Mixed inheritance: base class + multiple interfaces + public class Dog : Animal, IMovable, IAudible { + // Override Speak from Animal + public override void Speak() { + Console.WriteLine("Dog barks"); + } + + // Implementation of IMovable + public void Move() { + Console.WriteLine("Dog runs"); + } + + // Implementation of IAudible + public void Hear() { + Console.WriteLine("Dog listens"); + } + } + `; + const filePath = 'inheritance.cs'; + const config = {}; + const result = await parseFile(fileContent, filePath, createMockConfig(config)); + expect(typeof result).toBe('string'); + + const expectContents = [ + // Base class + 'Base class', + 'class Animal', + 'virtual void Speak()', + + // Interfaces + 'Interface for movement', + 'interface IMovable', + 'void Move()', + 'Interface for sound', + 'interface IAudible', + 'void Hear()', + + // Mixed inheritance class + 'Mixed inheritance: base class + multiple interfaces', + 'class Dog', + 'Animal', + 'IMovable', + 'IAudible', + 'void Move()', + 'void Hear()', + 'override void Speak()', + ]; + + for (const expectContent of expectContents) { + expect(result).toContain(expectContent); + } + }); }); diff --git a/tests/core/treeSitter/parseFile.swift.test.ts b/tests/core/treeSitter/parseFile.swift.test.ts index 89a113fd..29b11e75 100644 --- a/tests/core/treeSitter/parseFile.swift.test.ts +++ b/tests/core/treeSitter/parseFile.swift.test.ts @@ -319,4 +319,69 @@ describe('parseFile for Swift', () => { expect(result).toContain(expectContent); } }); + + test('should handle multiple subscripts with different parameter names', async () => { + const fileContent = ` + /// A storage protocol with multiple subscript accessors + protocol Storage { + /// Access by string key + subscript(key: String) -> Value? { get set } + + /// Access by integer index + subscript(index: Int) -> Value? { get set } + + /// Access by range + subscript(range: Range) -> [Value] { get } + } + + /// A dictionary-like class with multiple subscripts + class Dictionary { + /// Access by string key + subscript(key: String) -> Int? { + get { return nil } + set { } + } + + /// Access by integer index + subscript(index: Int) -> String? { + get { return nil } + set { } + } + + /// Access by key and default value + subscript(key: String, default defaultValue: Int) -> Int { + get { return defaultValue } + set { } + } + } + `; + const filePath = 'storage.swift'; + const config = {}; + const result = await parseFile(fileContent, filePath, createMockConfig(config)); + expect(typeof result).toBe('string'); + + const expectContents = [ + // Protocol + 'A storage protocol with multiple subscript accessors', + 'protocol Storage', + 'Access by string key', + 'subscript(key: String) -> Value?', + 'Access by integer index', + 'subscript(index: Int) -> Value?', + 'Access by range', + 'subscript(range: Range) -> [Value]', + + // Class + 'A dictionary-like class with multiple subscripts', + 'class Dictionary', + 'Access by string key', + 'Access by integer index', + 'Access by key and default value', + 'subscript(key: String, default defaultValue: Int) -> Int', + ]; + + for (const expectContent of expectContents) { + expect(result).toContain(expectContent); + } + }); });