fix: resolve all PR #494 review findings (10 items)

CRITICAL:
- parse-worker.ts: classNode: any → SyntaxNode on getFieldInfo
  and findEnclosingClassNode; removed redundant as number casts
- parsing-processor.ts: classNode: any → SyntaxNode on seqGetFieldInfo

HIGH:
- ruby.ts: attr_accessor now extracts ALL symbol arguments via
  extractNames hook in generic factory (was firstNamedChild only)
- typescript.ts: added JSDoc explaining why hand-written extractor
  coexists with config-based typescript-javascript.ts

MEDIUM:
- field-types.ts: FieldVisibility union type replaces string
  ('public'|'private'|'protected'|'internal'|'package'|'fileprivate'|'open')
  Propagated through field-extractor.ts, generic.ts, all 7 config files
- typescript.ts: extractFullType collapsed from 12 branches to 3 lines
- generic.ts: added extractNames? optional hook + buildField refactor

LOW:
- ruby.ts: extractVisibility(node) → extractVisibility(_node)
- python.ts: fixed misleading isStatic comment

TypeScript compiles cleanly.
This commit is contained in:
Gergo Magyar 2026-03-26 16:10:44 +00:00
parent fc49fd2e72
commit d6e8b03464
15 changed files with 150 additions and 119 deletions

View file

@ -2,9 +2,10 @@
import type { SyntaxNode } from './utils/ast-helpers.js';
import { SupportedLanguages } from '../../config/supported-languages.js';
import type {
FieldExtractorContext,
ExtractedFields
import type {
FieldExtractorContext,
ExtractedFields,
FieldVisibility,
} from './field-types.js';
/**
@ -56,5 +57,5 @@ export abstract class BaseFieldExtractor implements FieldExtractor {
return typeName;
}
protected abstract extractVisibility(node: SyntaxNode): string;
protected abstract extractVisibility(node: SyntaxNode): FieldVisibility;
}

View file

@ -5,12 +5,13 @@ import type { FieldExtractionConfig } from '../generic.js';
import { hasKeyword } from './helpers.js';
import { extractSimpleTypeName } from '../../type-extractors/shared.js';
import type { SyntaxNode } from '../../utils/ast-helpers.js';
import type { FieldVisibility } from '../../field-types.js';
/**
* Detect C++ access specifier (public:/private:/protected:) by walking
* backwards from the field node through siblings.
*/
function cppAccessSpecifier(node: SyntaxNode): string | undefined {
function cppAccessSpecifier(node: SyntaxNode): FieldVisibility | undefined {
let sibling = node.previousNamedSibling;
while (sibling) {
if (sibling.type === 'access_specifier') {

View file

@ -4,8 +4,9 @@ import { SupportedLanguages } from '../../../../config/supported-languages.js';
import type { FieldExtractionConfig } from '../generic.js';
import { findVisibility, hasKeyword, hasModifier } from './helpers.js';
import { extractSimpleTypeName } from '../../type-extractors/shared.js';
import type { FieldVisibility } from '../../field-types.js';
const CSHARP_VIS = new Set(['public', 'private', 'protected', 'internal']);
const CSHARP_VIS = new Set<FieldVisibility>(['public', 'private', 'protected', 'internal']);
/**
* C# field extraction config.

View file

@ -7,6 +7,7 @@
import type { SyntaxNode } from '../../utils/ast-helpers.js';
import { extractSimpleTypeName } from '../../type-extractors/shared.js';
import type { FieldVisibility } from '../../field-types.js';
// ---------------------------------------------------------------------------
// Modifier scanning
@ -48,14 +49,15 @@ export function hasModifier(node: SyntaxNode, modifierType: string, keyword: str
*/
export function findVisibility(
node: SyntaxNode,
keywords: ReadonlySet<string>,
defaultVis: string,
keywords: ReadonlySet<FieldVisibility>,
defaultVis: FieldVisibility,
modifierNodeType?: string,
): string {
): FieldVisibility {
// Direct keyword children
for (let i = 0; i < node.childCount; i++) {
const child = node.child(i);
if (child && keywords.has(child.text.trim())) return child.text.trim();
const text = child?.text.trim() as FieldVisibility | undefined;
if (text && (keywords as ReadonlySet<string>).has(text)) return text;
}
// Modifier wrapper
if (modifierNodeType) {
@ -64,7 +66,8 @@ export function findVisibility(
if (child && child.type === modifierNodeType) {
for (let j = 0; j < child.childCount; j++) {
const mod = child.child(j);
if (mod && keywords.has(mod.text.trim())) return mod.text.trim();
const modText = mod?.text.trim() as FieldVisibility | undefined;
if (modText && (keywords as ReadonlySet<string>).has(modText)) return modText;
}
}
}

View file

@ -4,12 +4,13 @@ import { SupportedLanguages } from '../../../../config/supported-languages.js';
import type { FieldExtractionConfig } from '../generic.js';
import { findVisibility, hasKeyword, hasModifier, typeFromField } from './helpers.js';
import { extractSimpleTypeName } from '../../type-extractors/shared.js';
import type { FieldVisibility } from '../../field-types.js';
// ---------------------------------------------------------------------------
// Java
// ---------------------------------------------------------------------------
const JAVA_VIS = new Set(['public', 'private', 'protected']);
const JAVA_VIS = new Set<FieldVisibility>(['public', 'private', 'protected']);
export const javaConfig: FieldExtractionConfig = {
language: SupportedLanguages.Java,
@ -70,7 +71,7 @@ export const javaConfig: FieldExtractionConfig = {
// Kotlin
// ---------------------------------------------------------------------------
const KOTLIN_VIS = new Set(['public', 'private', 'protected', 'internal']);
const KOTLIN_VIS = new Set<FieldVisibility>(['public', 'private', 'protected', 'internal']);
export const kotlinConfig: FieldExtractionConfig = {
language: SupportedLanguages.Kotlin,

View file

@ -4,8 +4,9 @@ import { SupportedLanguages } from '../../../../config/supported-languages.js';
import type { FieldExtractionConfig } from '../generic.js';
import { findVisibility, hasKeyword } from './helpers.js';
import { extractSimpleTypeName } from '../../type-extractors/shared.js';
import type { FieldVisibility } from '../../field-types.js';
const PHP_VIS = new Set(['public', 'private', 'protected']);
const PHP_VIS = new Set<FieldVisibility>(['public', 'private', 'protected']);
/**
* PHP field extraction config.

View file

@ -85,8 +85,8 @@ export const pythonConfig: FieldExtractionConfig = {
},
isStatic(_node) {
// Class-level variables in Python are effectively static;
// instance variables (self.x) live in __init__ and are not extracted here.
// Reports syntactic static keyword — Python class variables don't use explicit static keyword.
// Instance variables (self.x) live in __init__ and are not extracted here.
return false;
},

View file

@ -2,6 +2,33 @@
import { SupportedLanguages } from '../../../../config/supported-languages.js';
import type { FieldExtractionConfig } from '../generic.js';
import type { SyntaxNode } from '../../utils/ast-helpers.js';
/**
* Collect all field names declared by an `attr_accessor`, `attr_reader`, or
* `attr_writer` call node. A single call may list multiple symbols:
* attr_accessor :foo, :bar, :baz
*/
function extractAttrNames(node: SyntaxNode): string[] {
const method = node.childForFieldName('method');
if (!method) return [];
const methodName = method.text;
if (methodName !== 'attr_accessor' && methodName !== 'attr_reader'
&& methodName !== 'attr_writer') {
return [];
}
const args = node.childForFieldName('arguments');
if (!args) return [];
const names: string[] = [];
for (let i = 0; i < args.namedChildCount; i++) {
const arg = args.namedChild(i);
if (!arg) continue;
// simple_symbol text is :name — strip the leading colon
const text = arg.text;
names.push(text.startsWith(':') ? text.slice(1) : text);
}
return names;
}
/**
* Ruby field extraction config.
@ -25,22 +52,13 @@ export const rubyConfig: FieldExtractionConfig = {
defaultVisibility: 'public',
extractName(node) {
// call node: method = attr_accessor / attr_reader / attr_writer
const method = node.childForFieldName('method');
if (!method) return undefined;
const methodName = method.text;
if (methodName !== 'attr_accessor' && methodName !== 'attr_reader'
&& methodName !== 'attr_writer') {
return undefined;
}
// arguments: argument_list > simple_symbol (:name)
const args = node.childForFieldName('arguments');
if (!args) return undefined;
const firstArg = args.firstNamedChild;
if (!firstArg) return undefined;
// simple_symbol text is :name — strip the colon
const text = firstArg.text;
return text.startsWith(':') ? text.slice(1) : text;
// Returns the first symbol name for interface compatibility.
// Use extractNames to obtain all names from a single attr_* call.
return extractAttrNames(node)[0];
},
extractNames(node) {
return extractAttrNames(node);
},
extractType(_node) {
@ -48,7 +66,7 @@ export const rubyConfig: FieldExtractionConfig = {
return undefined;
},
extractVisibility(node) {
extractVisibility(_node) {
// attr_accessor/attr_writer fields are effectively public
// attr_reader fields are read-only from outside but still public
return 'public';

View file

@ -4,8 +4,9 @@ import { SupportedLanguages } from '../../../../config/supported-languages.js';
import type { FieldExtractionConfig } from '../generic.js';
import { hasKeyword, findVisibility } from './helpers.js';
import { extractSimpleTypeName } from '../../type-extractors/shared.js';
import type { FieldVisibility } from '../../field-types.js';
const SWIFT_VIS = new Set(['public', 'private', 'fileprivate', 'internal', 'open']);
const SWIFT_VIS = new Set<FieldVisibility>(['public', 'private', 'fileprivate', 'internal', 'open']);
/**
* Swift field extraction config.

View file

@ -3,8 +3,9 @@
import { SupportedLanguages } from '../../../../config/supported-languages.js';
import type { FieldExtractionConfig } from '../generic.js';
import { hasKeyword, findVisibility, typeFromAnnotation } from './helpers.js';
import type { FieldVisibility } from '../../field-types.js';
const VISIBILITY_KEYWORDS = new Set(['public', 'private', 'protected']);
const VISIBILITY_KEYWORDS = new Set<FieldVisibility>(['public', 'private', 'protected']);
const shared: Omit<FieldExtractionConfig, 'language'> = {
typeDeclarationNodes: [
@ -43,7 +44,7 @@ const shared: Omit<FieldExtractionConfig, 'language'> = {
for (let i = 0; i < node.namedChildCount; i++) {
const child = node.namedChild(i);
if (child && child.type === 'accessibility_modifier') {
const t = child.text.trim();
const t = child.text.trim() as FieldVisibility;
if (VISIBILITY_KEYWORDS.has(t)) return t;
}
}

View file

@ -12,7 +12,7 @@ import type { SyntaxNode } from '../utils/ast-helpers.js';
import { SupportedLanguages } from '../../../config/supported-languages.js';
import { BaseFieldExtractor } from '../field-extractor.js';
import type { FieldExtractor } from '../field-extractor.js';
import type { FieldExtractorContext, ExtractedFields, FieldInfo } from '../field-types.js';
import type { FieldExtractorContext, ExtractedFields, FieldInfo, FieldVisibility } from '../field-types.js';
// ---------------------------------------------------------------------------
// Config interface
@ -27,13 +27,23 @@ export interface FieldExtractionConfig {
/** AST node type(s) for the class body container (e.g., 'class_body', 'declaration_list') */
bodyNodeTypes: string[];
/** Default visibility when no modifier is present */
defaultVisibility: string;
/** Extract field name from a field declaration node */
defaultVisibility: FieldVisibility;
/**
* Extract field name from a field declaration node.
* Use this for nodes that declare exactly one field.
*/
extractName: (node: SyntaxNode) => string | undefined;
/**
* Extract multiple field names from a single declaration node.
* Optional override for languages where one AST node can declare
* several fields (e.g. Ruby `attr_accessor :foo, :bar`).
* When present, the factory uses this instead of `extractName`.
*/
extractNames?: (node: SyntaxNode) => string[];
/** Extract type annotation from a field declaration node */
extractType: (node: SyntaxNode) => string | undefined;
/** Extract visibility from a field declaration node */
extractVisibility: (node: SyntaxNode) => string;
extractVisibility: (node: SyntaxNode) => FieldVisibility;
/** Check if a field is static */
isStatic: (node: SyntaxNode) => boolean;
/** Check if a field is readonly/final/const */
@ -59,7 +69,7 @@ export function createFieldExtractor(config: FieldExtractionConfig): FieldExtrac
return typeDeclarationSet.has(node.type);
}
protected extractVisibility(node: SyntaxNode): string {
protected extractVisibility(node: SyntaxNode): FieldVisibility {
return config.extractVisibility(node);
}
@ -117,8 +127,17 @@ export function createFieldExtractor(config: FieldExtractionConfig): FieldExtrac
if (!child) continue;
if (fieldNodeSet.has(child.type)) {
const field = this.extractSingleField(child, context);
if (field) out.push(field);
if (config.extractNames) {
// Multi-name path: one node may declare several fields (e.g. Ruby attr_accessor)
const names = config.extractNames(child);
for (const name of names) {
const field = this.buildField(child, name, context);
if (field) out.push(field);
}
} else {
const field = this.extractSingleField(child, context);
if (field) out.push(field);
}
}
}
}
@ -129,6 +148,15 @@ export function createFieldExtractor(config: FieldExtractionConfig): FieldExtrac
): FieldInfo | null {
const name = config.extractName(node);
if (!name) return null;
return this.buildField(node, name, context);
}
private buildField(
node: SyntaxNode,
name: string,
context: FieldExtractorContext,
): FieldInfo | null {
if (!name) return null;
let type: string | null = config.extractType(node) ?? null;
if (type) {

View file

@ -3,17 +3,19 @@
import type { SyntaxNode } from '../utils/ast-helpers.js';
import { SupportedLanguages } from '../../../config/supported-languages.js';
import { BaseFieldExtractor } from '../field-extractor.js';
import type { FieldExtractorContext, ExtractedFields, FieldInfo } from '../field-types.js';
import type { FieldExtractorContext, ExtractedFields, FieldInfo, FieldVisibility } from '../field-types.js';
/**
* TypeScript field extractor for class and interface declarations.
*
* Handles:
* - Class fields with visibility modifiers (public/private/protected)
* - Interface properties with optional markers (?:)
* - Static and readonly modifiers
* - Complex generic types
* - Property signatures in interface bodies
* Hand-written TypeScript field extractor.
*
* This exists alongside the config-based extractor in configs/typescript-javascript.ts
* (used for JavaScript) because TypeScript has unique requirements:
* 1. type_alias_declaration with object type literals (e.g., type Config = { key: string })
* 2. Optional property detection appending '| undefined' to types
* 3. Nested type discovery within class/interface bodies
*
* The config-based extractor cannot express these TS-specific capabilities.
* JavaScript uses the config-based version since it lacks type syntax.
*/
export class TypeScriptFieldExtractor extends BaseFieldExtractor {
language = SupportedLanguages.TypeScript;
@ -40,7 +42,7 @@ export class TypeScriptFieldExtractor extends BaseFieldExtractor {
/**
* Visibility modifiers in TypeScript
*/
private static readonly VISIBILITY_MODIFIERS = new Set([
private static readonly VISIBILITY_MODIFIERS = new Set<FieldVisibility>([
'public',
'private',
'protected',
@ -56,12 +58,12 @@ export class TypeScriptFieldExtractor extends BaseFieldExtractor {
/**
* Extract visibility modifier from a field node
*/
protected extractVisibility(node: SyntaxNode): string {
protected extractVisibility(node: SyntaxNode): FieldVisibility {
// Check for accessibility_modifier named child (tree-sitter typescript)
for (let i = 0; i < node.namedChildCount; i++) {
const child = node.namedChild(i);
if (child && child.type === 'accessibility_modifier') {
const text = child.text.trim();
const text = child.text.trim() as FieldVisibility;
if (TypeScriptFieldExtractor.VISIBILITY_MODIFIERS.has(text)) {
return text;
}
@ -72,7 +74,7 @@ export class TypeScriptFieldExtractor extends BaseFieldExtractor {
for (let i = 0; i < node.childCount; i++) {
const child = node.child(i);
if (child && !child.isNamed) {
const text = child.text.trim();
const text = child.text.trim() as FieldVisibility;
if (TypeScriptFieldExtractor.VISIBILITY_MODIFIERS.has(text)) {
return text;
}
@ -84,8 +86,9 @@ export class TypeScriptFieldExtractor extends BaseFieldExtractor {
if (modifiers) {
for (let i = 0; i < modifiers.childCount; i++) {
const modifier = modifiers.child(i);
if (modifier && TypeScriptFieldExtractor.VISIBILITY_MODIFIERS.has(modifier.text)) {
return modifier.text;
const modText = modifier?.text.trim() as FieldVisibility | undefined;
if (modText && TypeScriptFieldExtractor.VISIBILITY_MODIFIERS.has(modText)) {
return modText;
}
}
}
@ -164,65 +167,19 @@ export class TypeScriptFieldExtractor extends BaseFieldExtractor {
}
/**
* Extract the full type text, handling complex generic types
* Extract the full type text, handling complex generic types.
*
* type_annotation nodes wrap the literal ': SomeType' only that branch
* needs special handling to unwrap the inner child and skip the colon.
* All other node kinds are already the type text itself, so normalizeType
* is applied directly.
*/
private extractFullType(typeNode: SyntaxNode | null): string | null {
if (!typeNode) return null;
// For type_annotation, get the inner type (skip the ':')
if (typeNode.type === 'type_annotation') {
const innerType = typeNode.firstNamedChild;
if (innerType) {
return this.normalizeType(innerType.text);
}
return innerType ? this.normalizeType(innerType.text) : null;
}
// Handle predefined_type (string, number, boolean, etc.)
if (typeNode.type === 'predefined_type') {
return typeNode.text;
}
// Handle type_identifier (custom types)
if (typeNode.type === 'type_identifier') {
return typeNode.text;
}
// Handle generic_type (Array<User>, Map<string, User>, etc.)
if (typeNode.type === 'generic_type') {
return this.normalizeType(typeNode.text);
}
// Handle array_type (User[])
if (typeNode.type === 'array_type') {
return this.normalizeType(typeNode.text);
}
// Handle union_type (User | null)
if (typeNode.type === 'union_type') {
return this.normalizeType(typeNode.text);
}
// Handle intersection_type (A & B)
if (typeNode.type === 'intersection_type') {
return this.normalizeType(typeNode.text);
}
// Handle object_type ({ name: string; age: number })
if (typeNode.type === 'object_type') {
return this.normalizeType(typeNode.text);
}
// Handle literal types
if (typeNode.type === 'literal_type') {
return this.normalizeType(typeNode.text);
}
// Handle nullable_type (string | null shorthand)
if (typeNode.type === 'nullable_type') {
return this.normalizeType(typeNode.text);
}
// Fallback: use the full text and normalize
return this.normalizeType(typeNode.text);
}

View file

@ -4,6 +4,23 @@ import type { TypeEnvironment } from './type-env.js';
import type { SymbolTable } from './symbol-table.js';
import { SupportedLanguages } from '../../config/supported-languages.js';
/**
* Visibility levels used across all supported languages.
* - public / private / protected: universal modifiers
* - internal: C#, Kotlin (assembly/module scope)
* - package: Java (package-private, no keyword)
* - fileprivate: Swift (file scope)
* - open: Swift (subclassable across modules)
*/
export type FieldVisibility =
| 'public'
| 'private'
| 'protected'
| 'internal'
| 'package'
| 'fileprivate'
| 'open';
/**
* Represents a field or property within a class/struct/interface
*/
@ -12,8 +29,8 @@ export interface FieldInfo {
name: string;
/** Resolved type (may be primitive, FQN, or generic) */
type: string | null;
/** Visibility: public, private, protected, internal */
visibility: string;
/** Visibility modifier */
visibility: FieldVisibility;
/** Is this a static member? */
isStatic: boolean;
/** Is this readonly/const? */

View file

@ -7,7 +7,7 @@ import { SymbolTable } from './symbol-table.js';
import { ASTCache } from './ast-cache.js';
import { getLanguageFromFilename } from './utils/language-detection.js';
import { yieldToEventLoop } from './utils/event-loop.js';
import { getDefinitionNodeFromCaptures, findEnclosingClassId, extractMethodSignature, getLabelFromCaptures, CLASS_CONTAINER_TYPES } from './utils/ast-helpers.js';
import { getDefinitionNodeFromCaptures, findEnclosingClassId, extractMethodSignature, getLabelFromCaptures, CLASS_CONTAINER_TYPES, type SyntaxNode } from './utils/ast-helpers.js';
import { detectFrameworkFromAST } from './framework-detection.js';
import { buildTypeEnv } from './type-env.js';
import type { FieldInfo, FieldExtractorContext } from './field-types.js';
@ -177,12 +177,12 @@ const NOOP_SYMBOL_TABLE_SEQ: any = {
};
function seqGetFieldInfo(
classNode: any,
classNode: SyntaxNode,
provider: LanguageProvider,
context: FieldExtractorContext,
): Map<string, FieldInfo> | undefined {
if (!provider.fieldExtractor) return undefined;
const cacheKey = classNode.startIndex as number;
const cacheKey = classNode.startIndex;
let cached = seqFieldInfoCache.get(cacheKey);
if (cached) return cached;
const extracted = provider.fieldExtractor.extract(classNode, context);

View file

@ -38,6 +38,7 @@ import {
extractMethodSignature,
findDescendant,
extractStringContent,
type SyntaxNode,
} from '../utils/ast-helpers.js';
import {
countCallArguments,
@ -309,7 +310,7 @@ const fieldInfoCache = new Map<number, Map<string, FieldInfo>>();
* Walk up from a definition node to find the nearest enclosing class/struct/interface
* AST node. Returns the SyntaxNode itself (not an ID) for passing to FieldExtractor.
*/
function findEnclosingClassNode(node: any): any | null {
function findEnclosingClassNode(node: SyntaxNode): SyntaxNode | null {
let current = node.parent;
while (current) {
if (CLASS_CONTAINER_TYPES.has(current.type)) {
@ -337,13 +338,13 @@ const NOOP_SYMBOL_TABLE: any = {
* or the class yielded no fields.
*/
function getFieldInfo(
classNode: any,
classNode: SyntaxNode,
provider: LanguageProvider,
context: FieldExtractorContext,
): Map<string, FieldInfo> | undefined {
if (!provider.fieldExtractor) return undefined;
const cacheKey = classNode.startIndex as number;
const cacheKey = classNode.startIndex;
let cached = fieldInfoCache.get(cacheKey);
if (cached) return cached;