From c22644fb3608c4622b42f096a35b49231de075d9 Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Fri, 25 Jul 2025 14:59:47 +0200 Subject: [PATCH] [c] Improved null-analysis.ts, handle exclusions, resolve XXXGeneric to XXX in Java. --- spine-c/codegen/src/null-analysis.ts | 280 +++++++++++++++++++++------ 1 file changed, 226 insertions(+), 54 deletions(-) diff --git a/spine-c/codegen/src/null-analysis.ts b/spine-c/codegen/src/null-analysis.ts index 3254ced45..8ed343e35 100644 --- a/spine-c/codegen/src/null-analysis.ts +++ b/spine-c/codegen/src/null-analysis.ts @@ -2,7 +2,9 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { fileURLToPath } from 'node:url'; +import { execSync } from 'node:child_process'; import { extractTypes } from './type-extractor'; +import { loadExclusions, isTypeExcluded, isMethodExcluded } from './exclusions'; import type { ClassOrStruct, Method, Type } from './types'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); @@ -48,13 +50,24 @@ function analyzeNullableMethods(): void { console.log('Extracting type information...'); const allTypes = extractTypes(); + console.log('Loading exclusions...'); + const exclusions = loadExclusions(path.join(__dirname, '../exclusions.txt')); + const nullableMethods: Array<{ filename: string; line: number; signature: string; - reason: string; + reasons: string[]; }> = []; + // Map to group methods by signature + const methodMap = new Map(); + // Process each type for (const type of allTypes) { if (type.kind === 'enum') continue; @@ -62,6 +75,11 @@ function analyzeNullableMethods(): void { const classType = type as ClassOrStruct; if (!classType.members) continue; + // Skip excluded types + if (isTypeExcluded(classType.name, exclusions)) { + continue; + } + // Get the source file name relative to the nullable file location const filename = `../../spine-cpp/include/spine/${classType.name}.h`; @@ -70,18 +88,24 @@ function analyzeNullableMethods(): void { if (member.kind !== 'method') continue; const method = member as Method; + + // Skip inherited methods - we only want methods declared in this class + if (method.fromSupertype) continue; + + // Skip excluded methods + if (isMethodExcluded(classType.name, method.name, exclusions, method)) { + continue; + } + const signature = buildMethodSignature(classType.name, method); + const reasons: string[] = []; + // Check return type - if it returns a pointer to a class if (method.returnType) { const cleanReturnType = method.returnType.replace(/\bconst\b/g, '').trim(); if (isPointerToClass(cleanReturnType, allTypes)) { - nullableMethods.push({ - filename, - line: method.loc.line, - signature, - reason: `returns nullable pointer: ${method.returnType}` - }); + reasons.push(`returns nullable pointer: ${method.returnType}`); } } @@ -89,20 +113,32 @@ function analyzeNullableMethods(): void { if (method.parameters) { for (const param of method.parameters) { if (isPointerToClass(param.type, allTypes)) { - nullableMethods.push({ - filename, - line: method.loc.line, - signature, - reason: `takes nullable parameter '${param.name}': ${param.type}` - }); - break; // Only report once per method + reasons.push(`takes nullable parameter '${param.name}': ${param.type}`); } } } + + // If method has nullability issues, add or update in map + if (reasons.length > 0) { + const key = `${filename}:${method.loc.line}`; + if (methodMap.has(key)) { + // Merge reasons if method already exists + const existing = methodMap.get(key)!; + existing.reasons.push(...reasons); + } else { + methodMap.set(key, { + filename, + line: method.loc.line, + signature, + reasons + }); + } + } } } - // Sort by filename and line + // Convert map to array and sort by filename and line + nullableMethods.push(...Array.from(methodMap.values())); nullableMethods.sort((a, b) => { if (a.filename !== b.filename) { return a.filename.localeCompare(b.filename); @@ -117,36 +153,7 @@ function analyzeNullableMethods(): void { ## Instructions -**Phase 1: Enrich nullable.md (if implementations not yet inlined)** -If checkboxes don't contain concrete implementations: -1. Use parallel Task agents to find implementations (agents do NOT write to file) -2. Each agent researches 10-15 methods and returns structured data: - \`\`\` - METHOD: [method signature] - CPP_HEADER: [file:line] [declaration] - CPP_IMPL: [file:line] [implementation code] - JAVA_IMPL: [file:line] [java method code] - --- - \`\`\` -3. Collect all agent results and do ONE MultiEdit to update nullable.md -4. Inline implementations BELOW each existing checkbox (keep original checkbox text): - \`\`\` - - [ ] [keep original checkbox line exactly as is] - **C++ Implementation:** - \`\`\`cpp - // Header: [file:line] - [declaration] - // Implementation: [file:line] - [implementation body] - \`\`\` - **Java Implementation:** - \`\`\`java - // [file:line] - [java method body] - \`\`\` - \`\`\` - -**Phase 2: Review and Update** +**Review and Update Process** For each unchecked checkbox (now with implementations inlined): 1. **Present both implementations** from the checkbox 2. **Ask if we need to change the C++ signature** based on Java nullability patterns (y/n) @@ -163,9 +170,40 @@ For each unchecked checkbox (now with implementations inlined): `; - const methodsList = nullableMethods.map(m => - `- [ ] [${m.filename}:${m.line}](${m.filename}#L${m.line}) ${m.signature} // ${m.reason}` - ).join('\n'); + console.log('Finding implementations for methods...'); + const enrichedMethods = nullableMethods.map((m, index) => { + if (index % 20 === 0) { + console.log(`Processing method ${index + 1}/${nullableMethods.length}...`); + } + + const { cppImpl, javaImpl } = findImplementations(m.filename, m.line, m.signature, allTypes); + + const reasonsText = m.reasons.join('; '); + let methodEntry = `- [ ] [${m.filename}:${m.line}](${m.filename}#L${m.line}) ${m.signature} // ${reasonsText}`; + + // Add implementations if found + if (cppImpl !== 'NOT FOUND' || javaImpl !== 'NOT FOUND') { + methodEntry += '\n ```cpp'; + if (cppImpl !== 'NOT FOUND') { + methodEntry += '\n ' + cppImpl.replace(/\n/g, '\n '); + } else { + methodEntry += '\n // NOT FOUND'; + } + methodEntry += '\n ```'; + + methodEntry += '\n ```java'; + if (javaImpl !== 'NOT FOUND') { + methodEntry += '\n ' + javaImpl.replace(/\n/g, '\n '); + } else { + methodEntry += '\n // NOT FOUND'; + } + methodEntry += '\n ```'; + } + + return methodEntry; + }); + + const methodsList = enrichedMethods.join('\n'); fs.writeFileSync(outputPath, instructions + methodsList + '\n'); @@ -173,16 +211,17 @@ For each unchecked checkbox (now with implementations inlined): console.log(`Results written to: ${outputPath}`); // Print summary statistics - const byReason = new Map(); + let returnCount = 0; + let paramCount = 0; for (const method of nullableMethods) { - const reasonType = method.reason.startsWith('returns') ? 'nullable return' : 'nullable parameter'; - byReason.set(reasonType, (byReason.get(reasonType) || 0) + 1); + if (method.reasons.some(r => r.startsWith('returns'))) returnCount++; + if (method.reasons.some(r => r.startsWith('takes'))) paramCount++; } console.log('\nSummary:'); - for (const [reason, count] of byReason) { - console.log(` ${reason}: ${count} methods`); - } + console.log(` methods with nullable returns: ${returnCount}`); + console.log(` methods with nullable parameters: ${paramCount}`); + console.log(` methods with both: ${returnCount + paramCount - nullableMethods.length}`); } /** @@ -194,6 +233,139 @@ function buildMethodSignature(className: string, method: Method): string { return `${method.returnType || 'void'} ${className}::${method.name}(${params})${constStr}`; } +/** + * Finds concrete subclasses of a given class + */ +function findConcreteSubclasses(className: string, allTypes: Type[]): string[] { + const subclasses: string[] = []; + for (const type of allTypes) { + if (type.kind === 'enum') continue; + const classType = type as ClassOrStruct; + if (classType.superTypes?.includes(className) && !classType.isAbstract) { + subclasses.push(classType.name); + } + } + return subclasses; +} + +/** + * Finds C++ and Java implementations for a method + */ +function findImplementations(filename: string, line: number, signature: string, allTypes: Type[]): { + cppImpl: string; + javaImpl: string; +} { + // Extract header filename (e.g., "AnimationState.h" from "../../spine-cpp/include/spine/AnimationState.h") + const headerFile = path.basename(filename); + const className = headerFile.replace('.h', ''); + + // For Java search, remove "Generic" suffix if present (C++ template pattern) + const javaClassName = className.endsWith('Generic') ? className.slice(0, -7) : className; + + // Extract method name from signature + const methodMatch = signature.match(/\w+::(\w+)\(/); + const methodName = methodMatch?.[1] || ''; + + // Check if this class is abstract + const classType = allTypes.find(t => t.kind !== 'enum' && (t as ClassOrStruct).name === className) as ClassOrStruct; + const isAbstract = classType?.isAbstract; + + // Map to C++ implementation file + const cppFile = filename.replace('/include/', '/src/').replace('.h', '.cpp'); + + let cppImpl = 'NOT FOUND'; + let javaImpl = 'NOT FOUND'; + + // Find C++ implementation + const grepPattern = `${className}::${methodName}`; + try { + if (fs.existsSync(cppFile)) { + const cppResult = execSync(`rg -n -A 9 "${grepPattern}" "${cppFile}"`, { encoding: 'utf8' }); + // Take only the first match and limit to 10 lines + const lines = cppResult.trim().split('\n').slice(0, 10); + const lineNum = lines[0].split(':')[0]; + cppImpl = `${cppFile}:${lineNum}\n${lines.join('\n')}`; + } else if (isAbstract) { + const subclasses = findConcreteSubclasses(className, allTypes); + if (subclasses.length > 0) { + const subclassFiles = subclasses.map(sc => `../../spine-cpp/src/spine/${sc}.cpp`).join(', '); + cppImpl = `PURE VIRTUAL - concrete implementations in: ${subclassFiles}`; + } else { + cppImpl = `PURE VIRTUAL - no concrete subclasses found`; + } + } else { + cppImpl = `NOT FOUND - file does not exist: ${cppFile}`; + } + } catch (error) { + if (isAbstract) { + const subclasses = findConcreteSubclasses(className, allTypes); + if (subclasses.length > 0) { + const subclassFiles = subclasses.map(sc => `../../spine-cpp/src/spine/${sc}.cpp`).join(', '); + cppImpl = `PURE VIRTUAL - concrete implementations in: ${subclassFiles}`; + } else { + cppImpl = `PURE VIRTUAL - no concrete subclasses found`; + } + } else { + cppImpl = `NOT FOUND - searched for pattern "${grepPattern}" in ${cppFile}`; + } + } + + // Find Java implementation - search broadly across all packages + try { + // First try the standard location + const javaFile = `../../spine-libgdx/spine-libgdx/src/com/esotericsoftware/spine/${javaClassName}.java`; + if (fs.existsSync(javaFile)) { + const javaPattern = `\\b${methodName}\\s*\\(`; + const javaResult = execSync(`rg -n -A 9 "${javaPattern}" "${javaFile}"`, { encoding: 'utf8' }); + // Take only the first match and limit to 10 lines + const lines = javaResult.trim().split('\n').slice(0, 10); + const lineNum = lines[0].split(':')[0]; + javaImpl = `${javaFile}:${lineNum}\n${lines.join('\n')}`; + } else { + throw new Error('File not found in standard location'); + } + } catch (error) { + // Search broadly for the class across all packages + try { + const findResult = execSync(`rg -l "(class|interface) ${javaClassName}" ../../spine-libgdx/spine-libgdx/src/`, { encoding: 'utf8' }); + const foundJavaFile = findResult.trim().split('\n')[0]; + if (foundJavaFile) { + const javaPattern = `\\b${methodName}\\s*\\(`; + const javaResult = execSync(`rg -n -A 9 "${javaPattern}" "${foundJavaFile}"`, { encoding: 'utf8' }); + // Take only the first match and limit to 10 lines + const lines = javaResult.trim().split('\n').slice(0, 10); + const lineNum = lines[0].split(':')[0]; + javaImpl = `${foundJavaFile}:${lineNum}\n${lines.join('\n')}`; + } else { + // If class not found, search for the method name across all Java files (might be in a similarly named class) + try { + const methodSearchResult = execSync(`rg -l "\\b${methodName}\\s*\\(" ../../spine-libgdx/spine-libgdx/src/ --type java`, { encoding: 'utf8' }); + const candidateFiles = methodSearchResult.trim().split('\n').filter(f => + f.includes(javaClassName) || f.toLowerCase().includes(javaClassName.toLowerCase()) || + f.includes(className) || f.toLowerCase().includes(className.toLowerCase()) + ); + if (candidateFiles.length > 0) { + const javaPattern = `\\b${methodName}\\s*\\(`; + const javaResult = execSync(`rg -n -A 9 "${javaPattern}" "${candidateFiles[0]}"`, { encoding: 'utf8' }); + // Take only the first match and limit to 10 lines + const lines = javaResult.trim().split('\n').slice(0, 10); + const lineNum = lines[0].split(':')[0]; + javaImpl = `${candidateFiles[0]}:${lineNum}\n${lines.join('\n')}`; + } else { + javaImpl = `NOT FOUND - searched for class "${javaClassName}" (from C++ "${className}") and method "${methodName}" across all Java files`; + } + } catch (error) { + javaImpl = `NOT FOUND - searched for class "${javaClassName}" (from C++ "${className}") and method "${methodName}" across all Java files`; + } + } + } catch (error) { + javaImpl = `NOT FOUND - searched for class "${javaClassName}" (from C++ "${className}") and method "${methodName}" across all Java files`; + } + } + + return { cppImpl, javaImpl }; +} + // Main execution if (import.meta.url === `file://${process.argv[1]}`) { try {