From b4821068423cd48dd1cc69716a2dce1f1b6a8334 Mon Sep 17 00:00:00 2001 From: Alexandre Daubois Date: Fri, 21 Nov 2025 16:57:06 +0100 Subject: [PATCH] feat(extgen): make the generator idempotent and avoid touching the original source --- docs/extensions.md | 13 +- internal/extgen/gofile.go | 171 ++++-- internal/extgen/gofile_test.go | 645 +++++++++++++++++---- internal/extgen/phpfunc.go | 11 +- internal/extgen/phpfunc_test.go | 6 +- internal/extgen/srcanalyzer.go | 19 +- internal/extgen/srcanalyzer_test.go | 110 +++- internal/extgen/templates/extension.c.tpl | 9 + internal/extgen/templates/extension.go.tpl | 45 +- internal/extgen/templates/extension.h.tpl | 9 + internal/extgen/templates/stub.php.tpl | 9 + 11 files changed, 832 insertions(+), 215 deletions(-) diff --git a/docs/extensions.md b/docs/extensions.md index 30f434eb58..f6806570af 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -587,7 +587,18 @@ GEN_STUB_SCRIPT=php-src/build/gen_stub.php frankenphp extension-init my_extensio > [!NOTE] > Don't forget to set the `GEN_STUB_SCRIPT` environment variable to the path of the `gen_stub.php` file in the PHP sources you downloaded earlier. This is the same `gen_stub.php` script mentioned in the manual implementation section. -If everything went well, a new directory named `build` should have been created. This directory contains the generated files for your extension, including the `my_extension.go` file with the generated PHP function stubs. +If everything went well, your project directory should contain the following files for your extension: + +- **`my_extension.go`** - Your original source file (remains unchanged) +- **`my_extension_generated.go`** - Generated file with CGO wrappers that call your functions +- **`my_extension.stub.php`** - PHP stub file for IDE autocompletion +- **`my_extension_arginfo.h`** - PHP argument information +- **`my_extension.h`** - C header file +- **`my_extension.c`** - C implementation file +- **`README.md`** - Documentation + +> [!IMPORTANT] +> **Your source file (`my_extension.go`) is never modified.** The generator creates a separate `_generated.go` file containing CGO wrappers that call your original functions. This means you can safely version control your source file without worrying about generated code polluting it. ### Integrating the Generated Extension into FrankenPHP diff --git a/internal/extgen/gofile.go b/internal/extgen/gofile.go index da015fe461..044696a47f 100644 --- a/internal/extgen/gofile.go +++ b/internal/extgen/gofile.go @@ -4,8 +4,9 @@ import ( "bytes" _ "embed" "fmt" - "os" + "go/format" "path/filepath" + "strings" "text/template" "github.com/Masterminds/sprig/v3" @@ -21,7 +22,7 @@ type GoFileGenerator struct { type goTemplateData struct { PackageName string BaseName string - Imports []string + SanitizedBaseName string Constants []phpConstant Variables []string InternalFunctions []string @@ -30,16 +31,7 @@ type goTemplateData struct { } func (gg *GoFileGenerator) generate() error { - filename := filepath.Join(gg.generator.BuildDir, gg.generator.BaseName+".go") - - if _, err := os.Stat(filename); err == nil { - backupFilename := filename + ".bak" - if err := os.Rename(filename, backupFilename); err != nil { - return fmt.Errorf("backing up existing Go file: %w", err) - } - - gg.generator.SourceFile = backupFilename - } + filename := filepath.Join(gg.generator.BuildDir, gg.generator.BaseName+"_generated.go") content, err := gg.buildContent() if err != nil { @@ -51,38 +43,18 @@ func (gg *GoFileGenerator) generate() error { func (gg *GoFileGenerator) buildContent() (string, error) { sourceAnalyzer := SourceAnalyzer{} - imports, variables, internalFunctions, err := sourceAnalyzer.analyze(gg.generator.SourceFile) + packageName, variables, internalFunctions, err := sourceAnalyzer.analyze(gg.generator.SourceFile) if err != nil { return "", fmt.Errorf("analyzing source file: %w", err) } - filteredImports := make([]string, 0, len(imports)) - for _, imp := range imports { - if imp != `"C"` && imp != `"unsafe"` && imp != `"github.com/dunglas/frankenphp"` && imp != `"runtime/cgo"` { - filteredImports = append(filteredImports, imp) - } - } - classes := make([]phpClass, len(gg.generator.Classes)) copy(classes, gg.generator.Classes) - if len(classes) > 0 { - hasCgo := false - for _, imp := range imports { - if imp == `"runtime/cgo"` { - hasCgo = true - break - } - } - if !hasCgo { - filteredImports = append(filteredImports, `"runtime/cgo"`) - } - } - templateContent, err := gg.getTemplateContent(goTemplateData{ - PackageName: SanitizePackageName(gg.generator.BaseName), + PackageName: packageName, BaseName: gg.generator.BaseName, - Imports: filteredImports, + SanitizedBaseName: SanitizePackageName(gg.generator.BaseName), Constants: gg.generator.Constants, Variables: variables, InternalFunctions: internalFunctions, @@ -94,7 +66,12 @@ func (gg *GoFileGenerator) buildContent() (string, error) { return "", fmt.Errorf("executing template: %w", err) } - return templateContent, nil + fc, err := format.Source([]byte(templateContent)) + if err != nil { + return "", fmt.Errorf("formatting source: %w", err) + } + + return string(fc), nil } func (gg *GoFileGenerator) getTemplateContent(data goTemplateData) (string, error) { @@ -106,6 +83,10 @@ func (gg *GoFileGenerator) getTemplateContent(data goTemplateData) (string, erro funcMap["isVoid"] = func(t phpType) bool { return t == phpVoid } + funcMap["extractGoFunctionName"] = extractGoFunctionName + funcMap["extractGoFunctionSignatureParams"] = extractGoFunctionSignatureParams + funcMap["extractGoFunctionSignatureReturn"] = extractGoFunctionSignatureReturn + funcMap["extractGoFunctionCallParams"] = extractGoFunctionCallParams tmpl := template.Must(template.New("gofile").Funcs(funcMap).Parse(goFileContent)) @@ -128,7 +109,7 @@ type GoParameter struct { Type string } -var phpToGoTypeMap= map[phpType]string{ +var phpToGoTypeMap = map[phpType]string{ phpString: "string", phpInt: "int64", phpFloat: "float64", @@ -146,3 +127,119 @@ func (gg *GoFileGenerator) phpTypeToGoType(phpT phpType) string { return "any" } + +// extractGoFunctionName extracts the Go function name from a Go function signature string. +func extractGoFunctionName(goFunction string) string { + idx := strings.Index(goFunction, "func ") + if idx == -1 { + return "" + } + + start := idx + len("func ") + + end := start + for end < len(goFunction) && goFunction[end] != '(' { + end++ + } + + if end >= len(goFunction) { + return "" + } + + return strings.TrimSpace(goFunction[start:end]) +} + +// extractGoFunctionSignatureParams extracts the parameters from a Go function signature. +func extractGoFunctionSignatureParams(goFunction string) string { + start := strings.IndexByte(goFunction, '(') + if start == -1 { + return "" + } + start++ + + depth := 1 + end := start + for end < len(goFunction) && depth > 0 { + switch goFunction[end] { + case '(': + depth++ + case ')': + depth-- + } + if depth > 0 { + end++ + } + } + + if end >= len(goFunction) { + return "" + } + + return strings.TrimSpace(goFunction[start:end]) +} + +// extractGoFunctionSignatureReturn extracts the return type from a Go function signature. +func extractGoFunctionSignatureReturn(goFunction string) string { + start := strings.IndexByte(goFunction, '(') + if start == -1 { + return "" + } + + depth := 1 + pos := start + 1 + for pos < len(goFunction) && depth > 0 { + switch goFunction[pos] { + case '(': + depth++ + case ')': + depth-- + } + pos++ + } + + if pos >= len(goFunction) { + return "" + } + + end := strings.IndexByte(goFunction[pos:], '{') + if end == -1 { + return "" + } + end += pos + + returnType := strings.TrimSpace(goFunction[pos:end]) + return returnType +} + +// extractGoFunctionCallParams extracts just the parameter names for calling a function. +func extractGoFunctionCallParams(goFunction string) string { + params := extractGoFunctionSignatureParams(goFunction) + if params == "" { + return "" + } + + var names []string + parts := strings.Split(params, ",") + for _, part := range parts { + part = strings.TrimSpace(part) + if len(part) == 0 { + continue + } + + words := strings.Fields(part) + if len(words) > 0 { + names = append(names, words[0]) + } + } + + var result strings.Builder + for i, name := range names { + if i > 0 { + result.WriteString(", ") + } + + result.WriteString(name) + } + + return result.String() +} diff --git a/internal/extgen/gofile_test.go b/internal/extgen/gofile_test.go index 754d95c0e1..fc323ba873 100644 --- a/internal/extgen/gofile_test.go +++ b/internal/extgen/gofile_test.go @@ -1,6 +1,9 @@ package extgen import ( + "bytes" + "crypto/sha256" + "encoding/hex" "os" "path/filepath" "strings" @@ -69,16 +72,20 @@ func anotherHelper() { goGen := GoFileGenerator{generator} require.NoError(t, goGen.generate()) - expectedFile := filepath.Join(tmpDir, "test.go") - require.FileExists(t, expectedFile) + sourceStillExists := filepath.Join(tmpDir, "test.go") + require.FileExists(t, sourceStillExists) + sourceStillContent, err := readFile(sourceStillExists) + require.NoError(t, err) + assert.Equal(t, sourceContent, sourceStillContent, "Source file should not be modified") + + generatedFile := filepath.Join(tmpDir, "test_generated.go") + require.FileExists(t, generatedFile) - content, err := readFile(expectedFile) + generatedContent, err := readFile(generatedFile) require.NoError(t, err) - testGoFileBasicStructure(t, content, "test") - testGoFileImports(t, content) - testGoFileExportedFunctions(t, content, generator.Functions) - testGoFileInternalFunctions(t, content) + testGeneratedFileBasicStructure(t, generatedContent, "main", "test") + testGeneratedFileWrappers(t, generatedContent, generator.Functions) } func TestGoFileGenerator_BuildContent(t *testing.T) { @@ -107,13 +114,14 @@ func test() { }, }, contains: []string{ - "package simple", + "package main", `#include "simple.h"`, `import "C"`, "func init()", "frankenphp.RegisterExtension(", - "//export test", - "func test()", + "//export go_test", + "func go_test()", + "test()", // wrapper calls original function }, }, { @@ -142,12 +150,10 @@ func process(data *go_string) *go_value { }, }, contains: []string{ - "package complex", - `"fmt"`, - `"strings"`, - `"encoding/json"`, - "//export process", + "package main", + "//export go_process", `"C"`, + "process(", // wrapper calls original function }, }, { @@ -173,9 +179,13 @@ func internalFunc2(data string) { }, }, contains: []string{ + "//export go_publicFunc", + "func go_publicFunc()", + "publicFunc()", // wrapper calls original function + }, + notContains: []string{ "func internalFunc1() string", "func internalFunc2(data string)", - "//export publicFunc", }, }, } @@ -195,6 +205,10 @@ func internalFunc2(data string) { for _, expected := range tt.contains { assert.Contains(t, content, expected, "Generated Go content should contain %q", expected) } + + for _, notExpected := range tt.notContains { + assert.NotContains(t, content, notExpected, "Generated Go content should NOT contain %q", notExpected) + } }) } } @@ -204,11 +218,11 @@ func TestGoFileGenerator_PackageNameSanitization(t *testing.T) { baseName string expectedPackage string }{ - {"simple", "simple"}, - {"my-extension", "my_extension"}, - {"ext.with.dots", "ext_with_dots"}, - {"123invalid", "_123invalid"}, - {"valid_name", "valid_name"}, + {"simple", "main"}, + {"my-extension", "main"}, + {"ext.with.dots", "main"}, + {"123invalid", "main"}, + {"valid_name", "main"}, } for _, tt := range tests { @@ -275,57 +289,6 @@ func TestGoFileGenerator_ErrorHandling(t *testing.T) { } } -func TestGoFileGenerator_ImportFiltering(t *testing.T) { - sourceContent := `package main - -import ( - "C" - "fmt" - "strings" - "github.com/dunglas/frankenphp/internal/extensions/types" - "github.com/other/package" - originalPkg "github.com/test/original" -) - -//export_php: test(): void -func test() {}` - - sourceFile := createTempSourceFile(t, sourceContent) - - generator := &Generator{ - BaseName: "importtest", - SourceFile: sourceFile, - Functions: []phpFunction{ - {Name: "test", ReturnType: phpVoid, GoFunction: "func test() {}"}, - }, - } - - goGen := GoFileGenerator{generator} - content, err := goGen.buildContent() - require.NoError(t, err) - - expectedImports := []string{ - `"fmt"`, - `"strings"`, - `"github.com/other/package"`, - } - - for _, imp := range expectedImports { - assert.Contains(t, content, imp, "Generated content should contain import: %s", imp) - } - - forbiddenImports := []string{ - `"C"`, - } - - cImportCount := strings.Count(content, `"C"`) - assert.Equal(t, 1, cImportCount, "Expected exactly 1 occurrence of 'import \"C\"'") - - for _, imp := range forbiddenImports[1:] { - assert.NotContains(t, content, imp, "Generated content should NOT contain import: %s", imp) - } -} - func TestGoFileGenerator_ComplexScenario(t *testing.T) { sourceContent := `package example @@ -398,26 +361,12 @@ func debugPrint(msg string) { goGen := GoFileGenerator{generator} content, err := goGen.buildContent() require.NoError(t, err) - assert.Contains(t, content, "package complex_example", "Package name should be sanitized") - - internalFuncs := []string{ - "func internalProcess(data string) string", - "func validateFormat(input string) bool", - "func jsonHelper(data any) ([]byte, error)", - "func debugPrint(msg string)", - } - - for _, fn := range internalFuncs { - assert.Contains(t, content, fn, "Generated content should contain internal function: %s", fn) - } + assert.Contains(t, content, "package example", "Package name should match source package") for _, fn := range functions { - exportDirective := "//export " + fn.Name + exportDirective := "//export go_" + fn.Name assert.Contains(t, content, exportDirective, "Generated content should contain export directive: %s", exportDirective) } - - assert.False(t, strings.Contains(content, "types.Array") || strings.Contains(content, "types.Bool"), "Types should be replaced (types.* should not appear)") - assert.True(t, strings.Contains(content, "return Array(") && strings.Contains(content, "return Bool("), "Replaced types should appear without types prefix") } func TestGoFileGenerator_MethodWrapperWithNullableParams(t *testing.T) { @@ -602,6 +551,434 @@ func (as *ArrayStruct) FilterData(data frankenphp.AssociativeArray, filter strin assert.Contains(t, content, "//export FilterData_wrapper", "Generated content should contain FilterData export directive") } +func TestGoFileGenerator_Idempotency(t *testing.T) { + tmpDir := t.TempDir() + + sourceContent := `package main + +import ( + "fmt" + "strings" +) + +//export_php: greet(name string): string +func greet(name *go_string) *go_value { + return String("Hello " + CStringToGoString(name)) +} + +func internalHelper(data string) string { + return strings.ToUpper(data) +}` + + sourceFile := filepath.Join(tmpDir, "test.go") + require.NoError(t, os.WriteFile(sourceFile, []byte(sourceContent), 0644)) + + generator := &Generator{ + BaseName: "test", + SourceFile: sourceFile, + BuildDir: tmpDir, + Functions: []phpFunction{ + { + Name: "greet", + ReturnType: phpString, + GoFunction: `func greet(name *go_string) *go_value { + return String("Hello " + CStringToGoString(name)) +}`, + }, + }, + } + + goGen := GoFileGenerator{generator} + require.NoError(t, goGen.generate(), "First generation should succeed") + + generatedFile := filepath.Join(tmpDir, "test_generated.go") + require.FileExists(t, generatedFile, "Generated file should exist after first run") + + firstRunContent, err := os.ReadFile(generatedFile) + require.NoError(t, err) + firstRunSourceContent, err := os.ReadFile(sourceFile) + require.NoError(t, err) + + require.NoError(t, goGen.generate(), "Second generation should succeed") + + secondRunContent, err := os.ReadFile(generatedFile) + require.NoError(t, err) + secondRunSourceContent, err := os.ReadFile(sourceFile) + require.NoError(t, err) + + assert.True(t, bytes.Equal(firstRunContent, secondRunContent), "Generated file content should be identical between runs") + assert.True(t, bytes.Equal(firstRunSourceContent, secondRunSourceContent), "Source file should remain unchanged after both runs") + assert.Equal(t, sourceContent, string(secondRunSourceContent), "Source file content should match original") +} + +func TestGoFileGenerator_HeaderComments(t *testing.T) { + tmpDir := t.TempDir() + + sourceContent := `package main + +//export_php: test(): void +func test() { + // simple function +}` + + sourceFile := filepath.Join(tmpDir, "test.go") + require.NoError(t, os.WriteFile(sourceFile, []byte(sourceContent), 0644)) + + generator := &Generator{ + BaseName: "test", + SourceFile: sourceFile, + BuildDir: tmpDir, + Functions: []phpFunction{ + { + Name: "test", + ReturnType: phpVoid, + GoFunction: "func test() {\n\t// simple function\n}", + }, + }, + } + + goGen := GoFileGenerator{generator} + require.NoError(t, goGen.generate()) + + generatedFile := filepath.Join(tmpDir, "test_generated.go") + require.FileExists(t, generatedFile) + + assertContainsHeaderComment(t, generatedFile) +} + +func TestExtractGoFunctionName(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "simple function", + input: "func test() {}", + expected: "test", + }, + { + name: "function with params", + input: "func calculate(a int, b int) int {}", + expected: "calculate", + }, + { + name: "function with complex params", + input: "func process(data *go_string, opts *go_nullable) *go_value {}", + expected: "process", + }, + { + name: "function with whitespace", + input: "func spacedName () {}", + expected: "spacedName", + }, + { + name: "no func keyword", + input: "test() {}", + expected: "", + }, + { + name: "empty string", + input: "", + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractGoFunctionName(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestExtractGoFunctionSignatureParams(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "no parameters", + input: "func test() {}", + expected: "", + }, + { + name: "single parameter", + input: "func test(name string) {}", + expected: "name string", + }, + { + name: "multiple parameters", + input: "func test(a int, b string, c bool) {}", + expected: "a int, b string, c bool", + }, + { + name: "pointer parameters", + input: "func test(data *go_string) {}", + expected: "data *go_string", + }, + { + name: "nested parentheses", + input: "func test(fn func(int) string) {}", + expected: "fn func(int) string", + }, + { + name: "variadic parameters", + input: "func test(args ...string) {}", + expected: "args ...string", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractGoFunctionSignatureParams(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestExtractGoFunctionSignatureReturn(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "no return type", + input: "func test() {}", + expected: "", + }, + { + name: "single return type", + input: "func test() string {}", + expected: "string", + }, + { + name: "pointer return type", + input: "func test() *go_value {}", + expected: "*go_value", + }, + { + name: "multiple return types", + input: "func test() (string, error) {}", + expected: "(string, error)", + }, + { + name: "named return values", + input: "func test() (result string, err error) {}", + expected: "(result string, err error)", + }, + { + name: "complex return type", + input: "func test() unsafe.Pointer {}", + expected: "unsafe.Pointer", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractGoFunctionSignatureReturn(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestExtractGoFunctionCallParams(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "no parameters", + input: "func test() {}", + expected: "", + }, + { + name: "single parameter", + input: "func test(name string) {}", + expected: "name", + }, + { + name: "multiple parameters", + input: "func test(a int, b string, c bool) {}", + expected: "a, b, c", + }, + { + name: "pointer parameters", + input: "func test(data *go_string, opts *go_nullable) {}", + expected: "data, opts", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractGoFunctionCallParams(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGoFileGenerator_SourceFilePreservation(t *testing.T) { + tmpDir := t.TempDir() + + sourceContent := `package main + +import "fmt" + +//export_php: greet(name string): string +func greet(name *go_string) *go_value { + return String(fmt.Sprintf("Hello, %s!", CStringToGoString(name))) +} + +func internalHelper() { + fmt.Println("internal") +}` + + sourceFile := filepath.Join(tmpDir, "test.go") + require.NoError(t, os.WriteFile(sourceFile, []byte(sourceContent), 0644)) + + hashBefore := computeFileHash(t, sourceFile) + + generator := &Generator{ + BaseName: "test", + SourceFile: sourceFile, + BuildDir: tmpDir, + Functions: []phpFunction{ + { + Name: "greet", + ReturnType: phpString, + GoFunction: `func greet(name *go_string) *go_value { + return String(fmt.Sprintf("Hello, %s!", CStringToGoString(name))) +}`, + }, + }, + } + + goGen := GoFileGenerator{generator} + require.NoError(t, goGen.generate()) + + hashAfter := computeFileHash(t, sourceFile) + + assert.Equal(t, hashBefore, hashAfter, "Source file hash should remain unchanged after generation") + + contentAfter, err := os.ReadFile(sourceFile) + require.NoError(t, err) + assert.Equal(t, sourceContent, string(contentAfter), "Source file content should be byte-for-byte identical") +} + +func TestGoFileGenerator_WrapperParameterForwarding(t *testing.T) { + tmpDir := t.TempDir() + + sourceContent := `package main + +import "fmt" + +//export_php: process(name string, count int): string +func process(name *go_string, count long) *go_value { + n := CStringToGoString(name) + return String(fmt.Sprintf("%s: %d", n, count)) +} + +//export_php: simple(): void +func simple() {}` + + sourceFile := filepath.Join(tmpDir, "test.go") + require.NoError(t, os.WriteFile(sourceFile, []byte(sourceContent), 0644)) + + functions := []phpFunction{ + { + Name: "process", + ReturnType: phpString, + GoFunction: `func process(name *go_string, count long) *go_value { + n := CStringToGoString(name) + return String(fmt.Sprintf("%s: %d", n, count)) +}`, + }, + { + Name: "simple", + ReturnType: phpVoid, + GoFunction: "func simple() {}", + }, + } + + generator := &Generator{ + BaseName: "wrapper_test", + SourceFile: sourceFile, + BuildDir: tmpDir, + Functions: functions, + } + + goGen := GoFileGenerator{generator} + content, err := goGen.buildContent() + require.NoError(t, err) + + assert.Contains(t, content, "//export go_process", "Should have wrapper export directive") + assert.Contains(t, content, "func go_process(", "Should have wrapper function") + assert.Contains(t, content, "process(", "Wrapper should call original function") + + assert.Contains(t, content, "//export go_simple", "Should have simple wrapper export directive") + assert.Contains(t, content, "func go_simple()", "Should have simple wrapper function") + assert.Contains(t, content, "simple()", "Simple wrapper should call original function") +} + +func TestGoFileGenerator_MalformedSource(t *testing.T) { + tests := []struct { + name string + sourceContent string + expectError bool + }{ + { + name: "missing package declaration", + sourceContent: "func test() {}", + expectError: true, + }, + { + name: "syntax error", + sourceContent: "package main\nfunc test( {}", + expectError: true, + }, + { + name: "incomplete function", + sourceContent: "package main\nfunc test() {", + expectError: true, + }, + { + name: "valid minimal source", + sourceContent: "package main\nfunc test() {}", + expectError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := t.TempDir() + sourceFile := filepath.Join(tmpDir, "test.go") + require.NoError(t, os.WriteFile(sourceFile, []byte(tt.sourceContent), 0644)) + + generator := &Generator{ + BaseName: "test", + SourceFile: sourceFile, + BuildDir: tmpDir, + } + + goGen := GoFileGenerator{generator} + _, err := goGen.buildContent() + + if tt.expectError { + assert.Error(t, err, "Expected error for malformed source") + } else { + assert.NoError(t, err, "Should not error for valid source") + } + + contentAfter, readErr := os.ReadFile(sourceFile) + require.NoError(t, readErr) + assert.Equal(t, tt.sourceContent, string(contentAfter), "Source file should remain unchanged even on error") + }) + } +} + func TestGoFileGenerator_MethodWrapperWithNullableArrayParams(t *testing.T) { tmpDir := t.TempDir() @@ -672,37 +1049,6 @@ func createTempSourceFile(t *testing.T, content string) string { return tmpFile } -func testGoFileBasicStructure(t *testing.T, content, baseName string) { - requiredElements := []string{ - "package " + SanitizePackageName(baseName), - "// #include ", - `// #include "` + baseName + `.h"`, - `import "C"`, - "func init() {", - "frankenphp.RegisterExtension(", - "}", - } - - for _, element := range requiredElements { - assert.Contains(t, content, element, "Go file should contain: %s", element) - } -} - -func testGoFileImports(t *testing.T, content string) { - cImportCount := strings.Count(content, `"C"`) - assert.Equal(t, 1, cImportCount, "Expected exactly 1 C import") -} - -func testGoFileExportedFunctions(t *testing.T, content string, functions []phpFunction) { - for _, fn := range functions { - exportDirective := "//export " + fn.Name - assert.Contains(t, content, exportDirective, "Go file should contain export directive: %s", exportDirective) - - funcStart := "func " + fn.Name + "(" - assert.Contains(t, content, funcStart, "Go file should contain function definition: %s", funcStart) - } -} - func TestGoFileGenerator_MethodWrapperWithCallableParams(t *testing.T) { tmpDir := t.TempDir() @@ -841,3 +1187,64 @@ func testGoFileInternalFunctions(t *testing.T, content string) { t.Log("No internal functions found (this may be expected)") } } + +func testGeneratedFileBasicStructure(t *testing.T, content, expectedPackage, baseName string) { + requiredElements := []string{ + "package " + expectedPackage, + "// #include ", + `// #include "` + baseName + `.h"`, + `import "C"`, + "func init() {", + "frankenphp.RegisterExtension(", + "}", + } + + for _, element := range requiredElements { + assert.Contains(t, content, element, "Generated file should contain: %s", element) + } + + assert.NotContains(t, content, "func internalHelper", "Generated file should not contain internal functions from source") + assert.NotContains(t, content, "func anotherHelper", "Generated file should not contain internal functions from source") +} + +func testGeneratedFileWrappers(t *testing.T, content string, functions []phpFunction) { + for _, fn := range functions { + exportDirective := "//export go_" + fn.Name + assert.Contains(t, content, exportDirective, "Generated file should contain export directive: %s", exportDirective) + + wrapperFunc := "func go_" + fn.Name + "(" + assert.Contains(t, content, wrapperFunc, "Generated file should contain wrapper function: %s", wrapperFunc) + + funcName := extractGoFunctionName(fn.GoFunction) + if funcName != "" { + assert.Contains(t, content, funcName+"(", "Generated wrapper should call original function: %s", funcName) + } + } +} + +// compareFileContents compares two files byte-by-byte +func compareFileContents(t *testing.T, file1, file2 string) bool { + content1, err := os.ReadFile(file1) + require.NoError(t, err) + content2, err := os.ReadFile(file2) + require.NoError(t, err) + return bytes.Equal(content1, content2) +} + +// computeFileHash returns SHA256 hash of file +func computeFileHash(t *testing.T, filename string) string { + content, err := os.ReadFile(filename) + require.NoError(t, err) + hash := sha256.Sum256(content) + return hex.EncodeToString(hash[:]) +} + +// assertContainsHeaderComment verifies file has autogenerated header +func assertContainsHeaderComment(t *testing.T, filename string) { + content, err := os.ReadFile(filename) + require.NoError(t, err) + + headerSection := string(content[:min(len(content), 500)]) + assert.Contains(t, headerSection, "AUTOGENERATED FILE - DO NOT EDIT", "File should contain autogenerated header comment") + assert.Contains(t, headerSection, "FrankenPHP extension generator", "File should mention FrankenPHP extension generator") +} diff --git a/internal/extgen/phpfunc.go b/internal/extgen/phpfunc.go index 13cad8206b..1e11172a20 100644 --- a/internal/extgen/phpfunc.go +++ b/internal/extgen/phpfunc.go @@ -37,24 +37,25 @@ func (pfg *PHPFuncGenerator) generate(fn phpFunction) string { func (pfg *PHPFuncGenerator) generateGoCall(fn phpFunction) string { callParams := pfg.paramParser.generateGoCallParams(fn.Params) + goFuncName := "go_" + fn.Name if fn.ReturnType == phpVoid { - return fmt.Sprintf(" %s(%s);", fn.Name, callParams) + return fmt.Sprintf(" %s(%s);", goFuncName, callParams) } if fn.ReturnType == phpString { - return fmt.Sprintf(" zend_string *result = %s(%s);", fn.Name, callParams) + return fmt.Sprintf(" zend_string *result = %s(%s);", goFuncName, callParams) } if fn.ReturnType == phpArray { - return fmt.Sprintf(" zend_array *result = %s(%s);", fn.Name, callParams) + return fmt.Sprintf(" zend_array *result = %s(%s);", goFuncName, callParams) } if fn.ReturnType == phpMixed { - return fmt.Sprintf(" zval *result = %s(%s);", fn.Name, callParams) + return fmt.Sprintf(" zval *result = %s(%s);", goFuncName, callParams) } - return fmt.Sprintf(" %s result = %s(%s);", pfg.getCReturnType(fn.ReturnType), fn.Name, callParams) + return fmt.Sprintf(" %s result = %s(%s);", pfg.getCReturnType(fn.ReturnType), goFuncName, callParams) } func (pfg *PHPFuncGenerator) getCReturnType(returnType phpType) string { diff --git a/internal/extgen/phpfunc_test.go b/internal/extgen/phpfunc_test.go index 9725dc6bdc..3a0365ccd5 100644 --- a/internal/extgen/phpfunc_test.go +++ b/internal/extgen/phpfunc_test.go @@ -26,7 +26,7 @@ func TestPHPFunctionGenerator_Generate(t *testing.T) { "PHP_FUNCTION(greet)", "zend_string *name = NULL;", "Z_PARAM_STR(name)", - "zend_string *result = greet(name);", + "zend_string *result = go_greet(name);", "RETURN_STR(result)", }, }, @@ -61,7 +61,7 @@ func TestPHPFunctionGenerator_Generate(t *testing.T) { }, contains: []string{ "PHP_FUNCTION(doSomething)", - "doSomething(action);", + "go_doSomething(action);", }, }, { @@ -109,7 +109,7 @@ func TestPHPFunctionGenerator_Generate(t *testing.T) { "PHP_FUNCTION(process_array)", "zend_array *input = NULL;", "Z_PARAM_ARRAY_HT(input)", - "zend_array *result = process_array(input);", + "zend_array *result = go_process_array(input);", "RETURN_ARR(result)", }, }, diff --git a/internal/extgen/srcanalyzer.go b/internal/extgen/srcanalyzer.go index a7f4b1cf4e..32ebff4040 100644 --- a/internal/extgen/srcanalyzer.go +++ b/internal/extgen/srcanalyzer.go @@ -10,33 +10,24 @@ import ( type SourceAnalyzer struct{} -func (sa *SourceAnalyzer) analyze(filename string) (imports []string, variables []string, internalFunctions []string, err error) { +func (sa *SourceAnalyzer) analyze(filename string) (packageName string, variables []string, internalFunctions []string, err error) { fset := token.NewFileSet() node, err := parser.ParseFile(fset, filename, nil, parser.ParseComments) if err != nil { - return nil, nil, nil, fmt.Errorf("parsing file: %w", err) + return "", nil, nil, fmt.Errorf("parsing file: %w", err) } - for _, imp := range node.Imports { - if imp.Path != nil { - importPath := imp.Path.Value - if imp.Name != nil { - imports = append(imports, fmt.Sprintf("%s %s", imp.Name.Name, importPath)) - } else { - imports = append(imports, importPath) - } - } - } + packageName = node.Name.Name sourceContent, err := os.ReadFile(filename) if err != nil { - return nil, nil, nil, fmt.Errorf("reading source file: %w", err) + return "", nil, nil, fmt.Errorf("reading source file: %w", err) } variables = sa.extractVariables(string(sourceContent)) internalFunctions = sa.extractInternalFunctions(string(sourceContent)) - return imports, variables, internalFunctions, nil + return packageName, variables, internalFunctions, nil } func (sa *SourceAnalyzer) extractVariables(content string) []string { diff --git a/internal/extgen/srcanalyzer_test.go b/internal/extgen/srcanalyzer_test.go index 717f99b5f9..74207b1b7c 100644 --- a/internal/extgen/srcanalyzer_test.go +++ b/internal/extgen/srcanalyzer_test.go @@ -227,7 +227,7 @@ func testFunction() { require.NoError(t, os.WriteFile(filename, []byte(tt.sourceContent), 0644)) analyzer := &SourceAnalyzer{} - imports, variables, functions, err := analyzer.analyze(filename) + _, variables, functions, err := analyzer.analyze(filename) if tt.expectError { assert.Error(t, err, "expected error") @@ -236,10 +236,6 @@ func testFunction() { assert.NoError(t, err, "unexpected error") - if len(imports) != 0 && len(tt.expectedImports) != 0 { - assert.Equal(t, tt.expectedImports, imports, "imports mismatch") - } - assert.Equal(t, tt.expectedVariables, variables, "variables mismatch") assert.Len(t, functions, len(tt.expectedFunctions), "function count mismatch") @@ -385,6 +381,110 @@ var x = 10`, } } +func TestSourceAnalyzer_InternalFunctionPreservation(t *testing.T) { + tmpDir := t.TempDir() + + sourceContent := `package main + +import ( + "fmt" + "strings" +) + +//export_php: exported1(): string +func exported1() *go_value { + return String(internal1()) +} + +func internal1() string { + return "helper1" +} + +//export_php: exported2(): void +func exported2() { + internal2() +} + +func internal2() { + fmt.Println("helper2") +} + +func internal3(data string) string { + return strings.ToUpper(data) +}` + + sourceFile := filepath.Join(tmpDir, "test.go") + require.NoError(t, os.WriteFile(sourceFile, []byte(sourceContent), 0644)) + + analyzer := &SourceAnalyzer{} + packageName, variables, internalFuncs, err := analyzer.analyze(sourceFile) + require.NoError(t, err) + + assert.Equal(t, "main", packageName) + + assert.Len(t, internalFuncs, 3, "Should extract exactly 3 internal functions") + + expectedInternalFuncs := []string{ + `func internal1() string { + return "helper1" +}`, + `func internal2() { + fmt.Println("helper2") +}`, + `func internal3(data string) string { + return strings.ToUpper(data) +}`, + } + + for i, expected := range expectedInternalFuncs { + assert.Equal(t, expected, internalFuncs[i], "Internal function %d should match", i) + } + + assert.Empty(t, variables, "Should not have variables") +} + +func TestSourceAnalyzer_VariableBlockPreservation(t *testing.T) { + tmpDir := t.TempDir() + + sourceContent := `package main + +import ( + "sync" +) + +var ( + mu sync.RWMutex + cache = make(map[string]string) +) + +var globalCounter int = 0 + +//export_php: test(): void +func test() {}` + + sourceFile := filepath.Join(tmpDir, "test.go") + require.NoError(t, os.WriteFile(sourceFile, []byte(sourceContent), 0644)) + + analyzer := &SourceAnalyzer{} + packageName, variables, internalFuncs, err := analyzer.analyze(sourceFile) + require.NoError(t, err) + + assert.Equal(t, "main", packageName) + + assert.Len(t, variables, 2, "Should extract exactly 2 variable declarations") + + expectedVar1 := `var ( + mu sync.RWMutex + cache = make(map[string]string) +)` + expectedVar2 := `var globalCounter int = 0` + + assert.Equal(t, expectedVar1, variables[0], "First variable block should match") + assert.Equal(t, expectedVar2, variables[1], "Second variable declaration should match") + + assert.Empty(t, internalFuncs, "Should not have internal functions (only exported function)") +} + func BenchmarkSourceAnalyzer_Analyze(b *testing.B) { content := `package main diff --git a/internal/extgen/templates/extension.c.tpl b/internal/extgen/templates/extension.c.tpl index f511a1baf0..87b67475bb 100644 --- a/internal/extgen/templates/extension.c.tpl +++ b/internal/extgen/templates/extension.c.tpl @@ -1,3 +1,12 @@ +// AUTOGENERATED FILE - DO NOT EDIT. +// +// This file has been automatically generated by FrankenPHP extension generator +// and should not be edited as it will be overwritten when running the +// extension generator again. +// +// You may edit the file and remove this comment if you plan to manually maintain +// this file going forward. + #include #include #include diff --git a/internal/extgen/templates/extension.go.tpl b/internal/extgen/templates/extension.go.tpl index 24b665700e..7576bf4c46 100644 --- a/internal/extgen/templates/extension.go.tpl +++ b/internal/extgen/templates/extension.go.tpl @@ -1,43 +1,32 @@ package {{.PackageName}} +// AUTOGENERATED FILE - DO NOT EDIT. +// +// This file has been automatically generated by FrankenPHP extension generator +// and should not be edited as it will be overwritten when running the +// extension generator again. +// +// You may edit the file and remove this comment if you plan to manually maintain +// this file going forward. + // #include // #include "{{.BaseName}}.h" import "C" import ( "unsafe" + "runtime/cgo" "github.com/dunglas/frankenphp" -{{- range .Imports}} - {{.}} -{{- end}} ) func init() { - frankenphp.RegisterExtension(unsafe.Pointer(&C.{{.BaseName}}_module_entry)) + frankenphp.RegisterExtension(unsafe.Pointer(&C.{{.SanitizedBaseName}}_module_entry)) } -{{ range .Constants}} -const {{.Name}} = {{.Value}} - -{{- end}} -{{- range .Variables}} - -{{.}} -{{- end}} -{{- range .InternalFunctions}} -{{.}} - -{{- end}} {{- range .Functions}} -//export {{.Name}} -{{.GoFunction}} - -{{- end}} -{{- range .Classes}} -type {{.GoStruct}} struct { -{{- range .Properties}} - {{.Name}} {{.GoType}} -{{- end}} +//export go_{{.Name}} +func go_{{.Name}}({{extractGoFunctionSignatureParams .GoFunction}}) {{extractGoFunctionSignatureReturn .GoFunction}} { + {{if not (isVoid .ReturnType)}}return {{end}}{{extractGoFunctionName .GoFunction}}({{extractGoFunctionCallParams .GoFunction}}) } {{- end}} @@ -68,12 +57,6 @@ func create_{{.GoStruct}}_object() C.uintptr_t { return registerGoObject(obj) } -{{- range .Methods}} -{{- if .GoFunction}} -{{.GoFunction}} -{{- end}} - -{{- end}} {{- range .Methods}} //export {{.Name}}_wrapper func {{.Name}}_wrapper(handle C.uintptr_t{{range .Params}}{{if eq .PhpType "string"}}, {{.Name}} *C.zend_string{{else if eq .PhpType "array"}}, {{.Name}} *C.zval{{else if eq .PhpType "callable"}}, {{.Name}} *C.zval{{else}}, {{.Name}} {{if .IsNullable}}*{{end}}{{phpTypeToGoType .PhpType}}{{end}}{{end}}){{if not (isVoid .ReturnType)}}{{if isStringOrArray .ReturnType}} unsafe.Pointer{{else}} {{phpTypeToGoType .ReturnType}}{{end}}{{end}} { diff --git a/internal/extgen/templates/extension.h.tpl b/internal/extgen/templates/extension.h.tpl index 2607185222..eca5b92508 100644 --- a/internal/extgen/templates/extension.h.tpl +++ b/internal/extgen/templates/extension.h.tpl @@ -1,3 +1,12 @@ +// AUTOGENERATED FILE - DO NOT EDIT. +// +// This file has been automatically generated by FrankenPHP extension generator +// and should not be edited as it will be overwritten when running the +// extension generator again. +// +// You may edit the file and remove this comment if you plan to manually maintain +// this file going forward. + #ifndef _{{.HeaderGuard}} #define _{{.HeaderGuard}} diff --git a/internal/extgen/templates/stub.php.tpl b/internal/extgen/templates/stub.php.tpl index cd16115d99..32eb63aa66 100644 --- a/internal/extgen/templates/stub.php.tpl +++ b/internal/extgen/templates/stub.php.tpl @@ -1,5 +1,14 @@