Эх сурвалжийг харах

[c] Improved null-analysis.ts, handle exclusions, resolve XXXGeneric to XXX in Java.

Mario Zechner 1 сар өмнө
parent
commit
c22644fb36

+ 226 - 54
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<string, {
+        filename: string;
+        line: number;
+        signature: string;
+        reasons: string[];
+    }>();
+    
     // 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<string, number>();
+    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 {