diff --git a/docs/cn/extensions.md b/docs/cn/extensions.md index cb5522f479..835b2e5c00 100644 --- a/docs/cn/extensions.md +++ b/docs/cn/extensions.md @@ -156,15 +156,15 @@ echo $processor->process('Hello World', StringProcessor::MODE_UPPERCASE); // "H | `array` | `frankenphp.AssociativeArray` | ❌ | `frankenphp.GoAssociativeArray()` | `frankenphp.PHPAssociativeArray()` | ✅ | | `array` | `map[string]any` | ❌ | `frankenphp.GoMap()` | `frankenphp.PHPMap()` | ✅ | | `array` | `[]any` | ❌ | `frankenphp.GoPackedArray()` | `frankenphp.PHPPackedArray()` | ✅ | -| `mixed` | `any` | ❌ | `GoValue()` | `PHPValue()` | ❌ | -| `callable` | `*C.zval` | ❌ | - | `frankenphp.CallPHPCallable()` | ❌ | +| `mixed` | `*C.zval` | ❌ | `frankenphp.GoValue()` | `frankenphp.PHPValue()` | ✅ | +| `callable` | `*C.zval` | ❌ | - | `frankenphp.CallPHPCallable()` | ✅ | | `object` | `struct` | ❌ | _尚未实现_ | _尚未实现_ | ❌ | > [!NOTE] > > 此表尚不详尽，将随着 FrankenPHP 类型 API 变得更加完整而完善。 > -> 特别是对于类方法，目前支持原始类型和数组。对象尚不能用作方法参数或返回类型。 +> 特别是对于类方法，目前支持原始类型、数组、`mixed` 和 `callable`。对象尚不能用作方法参数或返回类型。 如果你参考上一节的代码片段，你可以看到助手用于转换第一个参数和返回值。我们的 `repeat_this()` 函数的第二和第三个参数不需要转换，因为底层类型的内存表示对于 C 和 Go 都是相同的。 diff --git a/docs/es/extensions.md b/docs/es/extensions.md index c4ab714800..3d87f3b0ee 100644 --- a/docs/es/extensions.md +++ b/docs/es/extensions.md @@ -99,15 +99,15 @@ Esta tabla resume lo que necesitas saber: | `array` | `frankenphp.AssociativeArray` | ❌ | `frankenphp.GoAssociativeArray()` | `frankenphp.PHPAssociativeArray()` | ✅ | | `array` | `map[string]any` | ❌ | `frankenphp.GoMap()` | `frankenphp.PHPMap()` | ✅ | | `array` | `[]any` | ❌ | `frankenphp.GoPackedArray()` | `frankenphp.PHPPackedArray()` | ✅ | -| `mixed` | `any` | ❌ | `GoValue()` | `PHPValue()` | ❌ | -| `callable` | `*C.zval` | ❌ | - | frankenphp.CallPHPCallable() | ❌ | +| `mixed` | `*C.zval` | ❌ | `frankenphp.GoValue()` | `frankenphp.PHPValue()` | ✅ | +| `callable` | `*C.zval` | ❌ | - | frankenphp.CallPHPCallable() | ✅ | | `object` | `struct` | ❌ | _Aún no implementado_ | _Aún no implementado_ | ❌ | > [!NOTE] > > Esta tabla aún no es exhaustiva y se completará a medida que la API de tipos de FrankenPHP se vuelva más completa. > -> Para métodos de clase específicamente, los tipos primitivos y los arrays están actualmente soportados. Los objetos aún no pueden usarse como parámetros de métodos o tipos de retorno. +> Para métodos de clase específicamente, los tipos primitivos, los arrays, `mixed` y `callable` están actualmente soportados. Los objetos aún no pueden usarse como parámetros de métodos o tipos de retorno. Si te refieres al fragmento de código de la sección anterior, puedes ver que se usan helpers para convertir el primer parámetro y el valor de retorno. El segundo y tercer parámetro de nuestra función `repeat_this()` no necesitan ser convertidos ya que la representación en memoria de los tipos subyacentes es la misma para C y Go. diff --git a/docs/extensions.md b/docs/extensions.md index b08f730520..c495fec713 100644 --- a/docs/extensions.md +++ b/docs/extensions.md @@ -163,15 +163,15 @@ This table summarizes what you need to know: | `array` | `frankenphp.AssociativeArray` | ❌ | `frankenphp.GoAssociativeArray()` | `frankenphp.PHPAssociativeArray()` | ✅ | | `array` | `map[string]any` | ❌ | `frankenphp.GoMap()` | `frankenphp.PHPMap()` | ✅ | | `array` | `[]any` | ❌ | `frankenphp.GoPackedArray()` | `frankenphp.PHPPackedArray()` | ✅ | -| `mixed` | `any` | ❌ | `GoValue()` | `PHPValue()` | ❌ | -| `callable` | `*C.zval` | ❌ | - | frankenphp.CallPHPCallable() | ❌ | +| `mixed` | `*C.zval` | ❌ | `frankenphp.GoValue()` | `frankenphp.PHPValue()` | ✅ | +| `callable` | `*C.zval` | ❌ | - | frankenphp.CallPHPCallable() | ✅ | | `object` | `struct` | ❌ | _Not yet implemented_ | _Not yet implemented_ | ❌ | > [!NOTE] > > This table is not exhaustive yet and will be completed as the FrankenPHP types API gets more complete. > -> For class methods specifically, primitive types and arrays are currently supported. Objects cannot be used as method parameters or return types yet. +> For class methods specifically, primitive types, arrays, `mixed` and `callable` are currently supported. Objects cannot be used as method parameters or return types yet. If you refer to the code snippet of the previous section, you can see that helpers are used to convert the first parameter and the return value. The second and third parameters of our `repeat_this()` function don't need to be converted, as the memory representation of the underlying types is the same for both C and Go. diff --git a/docs/fr/extensions.md b/docs/fr/extensions.md index 6aba81fd0d..c40fead436 100644 --- a/docs/fr/extensions.md +++ b/docs/fr/extensions.md @@ -98,13 +98,14 @@ Bien que certains types de variables aient la même représentation mémoire ent | `array` | `frankenphp.AssociativeArray` | ❌ | `frankenphp.GoAssociativeArray()` | `frankenphp.PHPAssociativeArray()` | ✅ | | `array` | `map[string]any` | ❌ | `frankenphp.GoMap()` | `frankenphp.PHPMap()` | ✅ | | `array` | `[]any` | ❌ | `frankenphp.GoPackedArray()` | `frankenphp.PHPPackedArray()` | ✅ | -| `mixed` | `any` | ❌ | `GoValue()` | `PHPValue()` | ❌ | +| `mixed` | `*C.zval` | ❌ | `frankenphp.GoValue()` | `frankenphp.PHPValue()` | ✅ | +| `callable` | `*C.zval` | ❌ | - | frankenphp.CallPHPCallable() | ✅ | | `object` | `struct` | ❌ | _Pas encore implémenté_ | _Pas encore implémenté_ | ❌ | > [!NOTE] > Ce tableau n'est pas encore exhaustif et sera complété au fur et à mesure que l'API de types FrankenPHP deviendra plus complète. > -> Pour les méthodes de classe spécifiquement, les types primitifs et les tableaux sont supportés. Les objets ne peuvent pas encore être utilisés comme paramètres de méthode ou types de retour. +> Pour les méthodes de classe spécifiquement, les types primitifs, les tableaux, `mixed` et `callable` sont supportés. Les objets ne peuvent pas encore être utilisés comme paramètres de méthode ou types de retour. Si vous vous référez à l'extrait de code de la section précédente, vous pouvez voir que des assistants sont utilisés pour convertir le premier paramètre et la valeur de retour. Les deuxième et troisième paramètres de notre fonction `repeat_this()` n'ont pas besoin d'être convertis car la représentation mémoire des types sous-jacents est la même pour C et Go. diff --git a/docs/it/extensions.md b/docs/it/extensions.md index 76e0414595..f4f8ce0552 100644 --- a/docs/it/extensions.md +++ b/docs/it/extensions.md @@ -158,15 +158,15 @@ Questa tabella riassume il necessario: | `array` | `frankenphp.AssociativeArray` | ❌| `frankenphp.GoAssociativeArray()` | `frankenphp.PHPAssociativeArray()` | ✅ | | `array` | `map[string]any` | ❌| `frankenphp.GoMap()` | `frankenphp.PHPMap()` | ✅ | | `array` | `[]any` | ❌| `frankenphp.GoPackedArray()` | `frankenphp.PHPPackedArray()` | ✅ | -| `mixed` | `any` | ❌| `GoValue()` | `PHPValue()` | ❌| -| `callable` | `*C.zval` | ❌| - | frankenphp.CallPHPCallable() | ❌| +| `mixed` | `*C.zval` | ❌| `frankenphp.GoValue()` | `frankenphp.PHPValue()` | ✅| +| `callable` | `*C.zval` | ❌| - | frankenphp.CallPHPCallable() | ✅| | `object` | `struct` | ❌| _Non ancora implementato_ | _Non ancora implementato_ | ❌| > [!NOTE] > > Questa tabella non è ancora esaustiva e verrà completata man mano che l'API dei tipi FrankenPHP diventerà più completa. > -> Per i metodi di classe, in particolare, sono attualmente supportati i tipi primitivi e gli array. Gli oggetti non possono ancora essere utilizzati come parametri di metodo o tipi di ritorno. +> Per i metodi di classe, in particolare, sono attualmente supportati i tipi primitivi, gli array, `mixed` e `callable`. Gli oggetti non possono ancora essere utilizzati come parametri di metodo o tipi di ritorno. Facendo riferimento allo snippet di codice della sezione precedente, si può vedere che gli helper vengono utilizzati per convertire il primo parametro e il valore restituito. Non è necessario convertire il secondo e il terzo parametro della nostra funzione `repeat_this()`, poiché la rappresentazione in memoria dei tipi sottostanti è la stessa sia per C sia per Go. diff --git a/docs/ja/extensions.md b/docs/ja/extensions.md index 1dd221ac4e..91e4973be5 100644 --- a/docs/ja/extensions.md +++ b/docs/ja/extensions.md @@ -98,15 +98,15 @@ C/PHPとGoの間でメモリ表現が同じ変数型もありますが、直接 | `array` | `frankenphp.AssociativeArray` | ❌ | `frankenphp.GoAssociativeArray()` | `frankenphp.PHPAssociativeArray()` | ✅ | | `array` | `map[string]any` | ❌ | `frankenphp.GoMap()` | `frankenphp.PHPMap()` | ✅ | | `array` | `[]any` | ❌ | `frankenphp.GoPackedArray()` | `frankenphp.PHPPackedArray()` | ✅ | -| `mixed` | `any` | ❌ | `GoValue()` | `PHPValue()` | ❌ | -| `callable` | `*C.zval` | ❌ | - | `frankenphp.CallPHPCallable()` | ❌ | +| `mixed` | `*C.zval` | ❌ | `frankenphp.GoValue()` | `frankenphp.PHPValue()` | ✅ | +| `callable` | `*C.zval` | ❌ | - | `frankenphp.CallPHPCallable()` | ✅ | | `object` | `struct` | ❌ | _未実装_ | _未実装_ | ❌ | > [!NOTE] > > この表はまだ完全ではなく、FrankenPHPの型APIがより完全になるにつれて完成されます。 > -> クラスメソッドについては、現在プリミティブ型と配列がサポートされています。オブジェクトはまだメソッドパラメータや戻り値の型として使用できません。 +> クラスメソッドについては、現在プリミティブ型、配列、`mixed`、`callable` がサポートされています。オブジェクトはまだメソッドパラメータや戻り値の型として使用できません。 前のセクションのコードスニペットを参照すると、最初のパラメータと戻り値の変換にヘルパーが使用されていることがわかります。 `repeat_this()`関数の2番目と3番目の引数は、基礎となる型のメモリ表現がCとGoで同じであるため、変換する必要がありません。 diff --git a/docs/pt-br/extensions.md b/docs/pt-br/extensions.md index 880c7833cb..a874c4ebe9 100644 --- a/docs/pt-br/extensions.md +++ b/docs/pt-br/extensions.md @@ -139,20 +139,20 @@ Esta tabela resume o que você precisa saber: | `?float` | `*float64` | ✅ | - | - | ✅ | | `bool` | `bool` | ✅ | - | - | ✅ | | `?bool` | `*bool` | ✅ | - | - | ✅ | -| `?bool` | `*bool` | ✅ | - | - | ✅ | | `string`/`?string` | `*C.zend_string` | ❌ | `frankenphp.GoString()` | `frankenphp.PHPString()` | ✅ | | `array` | `frankenphp.AssociativeArray` | ❌ | `frankenphp.GoAssociativeArray()` | `frankenphp.PHPAssociativeArray()` | ✅ | | `array` | `map[string]any` | ❌ | `frankenphp.GoMap()` | `frankenphp.PHPMap()` | ✅ | | `array` | `[]any` | ❌ | `frankenphp.GoPackedArray()` | `frankenphp.PHPPackedArray()` | ✅ | -| `mixed` | `any` | ❌ | `GoValue()` | `PHPValue()` | ❌ | +| `mixed` | `*C.zval` | ❌ | `frankenphp.GoValue()` | `frankenphp.PHPValue()` | ✅ | +| `callable` | `*C.zval` | ❌ | - | frankenphp.CallPHPCallable() | ✅ | | `object` | `struct` | ❌ | _Ainda não implementado_ | _Ainda não implementado_ | ❌ | > [!NOTE] > Esta tabela ainda não é exaustiva e será completada à medida que a API de > tipos do FrankenPHP se tornar mais completa. > -> Tipos primitivos e arrays são suportados atualmente, especificamente para -> métodos de classe. +> Tipos primitivos, arrays, `mixed` e `callable` são suportados atualmente, +> especificamente para métodos de classe. > Objetos ainda não podem ser usados como parâmetros de métodos ou tipos de > retorno. diff --git a/docs/ru/extensions.md b/docs/ru/extensions.md index 5921c327d4..f6ce497e49 100644 --- a/docs/ru/extensions.md +++ b/docs/ru/extensions.md @@ -157,15 +157,15 @@ echo $processor->process('Hello World', StringProcessor::MODE_UPPERCASE); // "H | `array` | `frankenphp.AssociativeArray` | ❌ | `frankenphp.GoAssociativeArray()` | `frankenphp.PHPAssociativeArray()` | ✅ | | `array` | `map[string]any` | ❌ | `frankenphp.GoMap()` | `frankenphp.PHPMap()` | ✅ | | `array` | `[]any` | ❌ | `frankenphp.GoPackedArray()` | `frankenphp.PHPPackedArray()` | ✅ | -| `mixed` | `any` | ❌ | `GoValue()` | `PHPValue()` | ❌ | -| `callable` | `*C.zval` | ❌ | - | `frankenphp.CallPHPCallable()` | ❌ | +| `mixed` | `*C.zval` | ❌ | `frankenphp.GoValue()` | `frankenphp.PHPValue()` | ✅ | +| `callable` | `*C.zval` | ❌ | - | `frankenphp.CallPHPCallable()` | ✅ | | `object` | `struct` | ❌ | _Пока не реализовано_ | _Пока не реализовано_ | ❌ | > [!NOTE] > > Эта таблица еще не исчерпывающая и будет пополняться по мере доработки API типов FrankenPHP. > -> Для методов классов, в частности, в настоящее время поддерживаются примитивные типы и массивы. Объекты пока не могут использоваться в качестве параметров методов или возвращаемых типов. +> Для методов классов, в частности, в настоящее время поддерживаются примитивные типы, массивы, `mixed` и `callable`. Объекты пока не могут использоваться в качестве параметров методов или возвращаемых типов. Если вы обратитесь к фрагменту кода из предыдущего раздела, вы увидите, что для преобразования первого параметра и возвращаемого значения используются вспомогательные функции. Второй и третий параметры нашей функции `repeat_this()` не требуют преобразования, так как представление в памяти базовых типов одинаково как для C, так и для Go. diff --git a/docs/tr/extensions.md b/docs/tr/extensions.md index de4f5057b0..7c01282900 100644 --- a/docs/tr/extensions.md +++ b/docs/tr/extensions.md @@ -157,15 +157,15 @@ Bu tablo, bilmeniz gerekenleri özetler: | `array` | `frankenphp.AssociativeArray` | ❌ | `frankenphp.GoAssociativeArray()` | `frankenphp.PHPAssociativeArray()` | ✅ | | `array` | `map[string]any` | ❌ | `frankenphp.GoMap()` | `frankenphp.PHPMap()` | ✅ | | `array` | `[]any` | ❌ | `frankenphp.GoPackedArray()` | `frankenphp.PHPPackedArray()` | ✅ | -| `mixed` | `any` | ❌ | `GoValue()` | `PHPValue()` | ❌ | -| `callable` | `*C.zval` | ❌ | - | frankenphp.CallPHPCallable() | ❌ | +| `mixed` | `*C.zval` | ❌ | `frankenphp.GoValue()` | `frankenphp.PHPValue()` | ✅ | +| `callable` | `*C.zval` | ❌ | - | frankenphp.CallPHPCallable() | ✅ | | `object` | `struct` | ❌ | _Henüz uygulanmadı_ | _Henüz uygulanmadı_ | ❌ | > [!NOTE] > > Bu tablo henüz kapsamlı değildir ve FrankenPHP tür API'si daha eksiksiz hale geldikçe tamamlanacaktır. > -> Özellikle sınıf metotları için, ilkel türler ve diziler şu anda desteklenmektedir. Nesneler henüz metot parametresi veya dönüş türü olarak kullanılamaz. +> Özellikle sınıf metotları için, ilkel türler, diziler, `mixed` ve `callable` şu anda desteklenmektedir. Nesneler henüz metot parametresi veya dönüş türü olarak kullanılamaz. Önceki bölümdeki kod parçacığına bakarsanız, ilk parametreyi ve dönüş değerini dönüştürmek için yardımcıların kullanıldığını görebilirsiniz. `repeat_this()` işlevimizin ikinci ve üçüncü parametrelerinin dönüştürülmesi gerekmez, çünkü temel türlerin bellek gösterimi hem C hem de Go için aynıdır. diff --git a/internal/extgen/astutil.go b/internal/extgen/astutil.go index ce96da93c7..d630be6e16 100644 --- a/internal/extgen/astutil.go +++ b/internal/extgen/astutil.go @@ -33,6 +33,24 @@ func extractNodeSource(src []byte, fset *token.FileSet, node ast.Node) string { return string(src[start:end]) } +// flattenParamTypes expands a parameter list into one entry per parameter. +// A single ast.Field can declare several parameters at once ("func f(a, b int)"), +// so the field list length is not the parameter count. +func flattenParamTypes(params *ast.FieldList) []ast.Expr { + if params == nil { + return nil + } + + var types []ast.Expr + for _, field := range params.List { + for range max(len(field.Names), 1) { + types = append(types, field.Type) + } + } + + return types +} + // checkOrphanDirectives returns an error for the first comment that matches re // but whose source line was not consumed by a declaration. func checkOrphanDirectives(file *ast.File, fset *token.FileSet, re *regexp.Regexp, consumed map[int]bool, directiveLabel string) error { diff --git a/internal/extgen/cfile_phpmethod_test.go b/internal/extgen/cfile_phpmethod_test.go index 7e666dcd8e..98e98b3812 100644 --- a/internal/extgen/cfile_phpmethod_test.go +++ b/internal/extgen/cfile_phpmethod_test.go @@ -3,6 +3,7 @@ package extgen import ( "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -338,3 +339,31 @@ func TestCFile_ClassMethodParamCastsByParamType(t *testing.T) { }) } } + +func TestCFileGenerator_MethodMixedParamAndReturn(t *testing.T) { + tmpDir := t.TempDir() + generator := &Generator{ + BaseName: "mixed_ext", + BuildDir: tmpDir, + Classes: []phpClass{{ + Name: "Box", + GoStruct: "Box", + Methods: []phpClassMethod{{ + Name: "swap", + PhpName: "swap", + ClassName: "Box", + ReturnType: phpMixed, + Params: []phpParameter{{Name: "value", PhpType: phpMixed}}, + }}, + }}, + } + + cGen := cFileGenerator{generator} + content, err := cGen.buildContent() + require.NoError(t, err) + + assert.Contains(t, content, "zval *value = NULL;", "mixed parameter must be declared as a zval") + assert.Contains(t, content, "Z_PARAM_ZVAL(value)", "mixed parameter must be parsed as a zval") + assert.Contains(t, content, "Box_swap_wrapper(intern->go_handle, value)", "mixed parameter must be forwarded to the Go wrapper") + assert.Contains(t, content, "RETURN_COPY_VALUE(result);", "mixed return value must reach PHP") +} diff --git a/internal/extgen/classparser.go b/internal/extgen/classparser.go index 52894daa2a..cc8778f576 100644 --- a/internal/extgen/classparser.go +++ b/internal/extgen/classparser.go @@ -7,6 +7,7 @@ import ( "go/token" "os" "regexp" + "slices" "strings" ) @@ -20,13 +21,14 @@ type exportDirective struct { type classParser struct{} -func (cp *classParser) Parse(filename string) ([]phpClass, error) { - return cp.parse(filename) -} - func (cp *classParser) parse(filename string) (classes []phpClass, err error) { + src, err := os.ReadFile(filename) + if err != nil { + return nil, err + } + fset := token.NewFileSet() - node, err := parser.ParseFile(fset, filename, nil, parser.ParseComments) + node, err := parser.ParseFile(fset, filename, src, parser.ParseComments) if err != nil { return nil, fmt.Errorf("parsing file: %w", err) } @@ -34,7 +36,7 @@ func (cp *classParser) parse(filename string) (classes []phpClass, err error) { validator := Validator{} exportDirectives := cp.collectExportDirectives(node, fset) - methods, err := cp.parseMethods(filename) + methods, err := cp.parseMethods(src, node, fset) if err != nil { return nil, fmt.Errorf("parsing methods: %w", err) } @@ -83,7 +85,7 @@ func (cp *classParser) parse(filename string) (classes []phpClass, err error) { } if err := validator.validateClass(class); err != nil { - fmt.Fprintf(os.Stderr, "Warning: Invalid class '%s': %v\n", class.Name, err) + warnf("Warning: Invalid class %q: %v\n", class.Name, err) continue } @@ -97,6 +99,15 @@ func (cp *classParser) parse(filename string) (classes []phpClass, err error) { } } + // Match against the declared directives rather than the classes that survived + // validation, so an invalid class does not turn its methods into a misleading + // "never exported" error. + for _, method := range methods { + if !slices.ContainsFunc(exportDirectives, func(d exportDirective) bool { return d.className == method.ClassName }) { + return nil, fmt.Errorf("//export_php:method directive at line %d targets class %q, which is not exported by any //export_php:class directive", method.lineNumber, method.ClassName) + } + } + return classes, nil } @@ -201,18 +212,7 @@ func (cp *classParser) goTypeToPHPType(goType string) phpType { return phpMixed } -func (cp *classParser) parseMethods(filename string) ([]phpClassMethod, error) { - src, err := os.ReadFile(filename) - if err != nil { - return nil, err - } - - fset := token.NewFileSet() - file, err := parser.ParseFile(fset, filename, src, parser.ParseComments) - if err != nil { - return nil, fmt.Errorf("parsing file: %w", err) - } - +func (cp *classParser) parseMethods(src []byte, file *ast.File, fset *token.FileSet) ([]phpClassMethod, error) { validator := Validator{} var methods []phpClassMethod consumed := make(map[int]bool) @@ -223,21 +223,23 @@ func (cp *classParser) parseMethods(filename string) ([]phpClassMethod, error) { continue } - directive, directiveLine := findDirective(funcDecl.Doc, fset, phpMethodRegex) - if directive == "" { + comment := findMatchingComment(funcDecl.Doc, phpMethodRegex) + if comment == nil { continue } - rawMatch := phpMethodRegex.FindStringSubmatch(findMatchingComment(funcDecl.Doc, phpMethodRegex)) + + rawMatch := phpMethodRegex.FindStringSubmatch(comment.Text) if len(rawMatch) != 3 { continue } className := strings.TrimSpace(rawMatch[1]) signature := strings.TrimSpace(rawMatch[2]) + directiveLine := fset.Position(comment.Pos()).Line consumed[directiveLine] = true method, err := cp.parseMethodSignature(className, signature) if err != nil { - fmt.Fprintf(os.Stderr, "Warning: Error parsing method signature %q: %v\n", signature, err) + warnf("Warning: Error parsing method signature %q: %v\n", signature, err) continue } @@ -249,7 +251,7 @@ func (cp *classParser) parseMethods(filename string) ([]phpClassMethod, error) { IsReturnNullable: method.isReturnNullable, } if err := validator.validateTypes(phpFunc); err != nil { - fmt.Fprintf(os.Stderr, "Warning: Method \"%s::%s\" uses unsupported types: %v\n", className, method.Name, err) + warnf("Warning: Method \"%s::%s\" uses unsupported types: %v\n", className, method.Name, err) continue } @@ -258,7 +260,7 @@ func (cp *classParser) parseMethods(filename string) ([]phpClassMethod, error) { phpFunc.GoFunction = method.GoFunction if err := validator.validateGoFunctionSignatureWithOptions(phpFunc, true); err != nil { - fmt.Fprintf(os.Stderr, "Warning: Go method signature mismatch for '%s::%s': %v\n", method.ClassName, method.Name, err) + warnf("Warning: Go method signature mismatch for %q: %v\n", method.ClassName+"::"+method.Name, err) continue } @@ -272,17 +274,17 @@ func (cp *classParser) parseMethods(filename string) ([]phpClassMethod, error) { return methods, nil } -// findMatchingComment returns the raw comment text whose line matches re. -func findMatchingComment(group *ast.CommentGroup, re *regexp.Regexp) string { +// findMatchingComment returns the first comment of the group matching re. +func findMatchingComment(group *ast.CommentGroup, re *regexp.Regexp) *ast.Comment { if group == nil { - return "" + return nil } for _, comment := range group.List { if re.MatchString(comment.Text) { - return comment.Text + return comment } } - return "" + return nil } func (cp *classParser) parseMethodSignature(className, signature string) (*phpClassMethod, error) { diff --git a/internal/extgen/classparser_test.go b/internal/extgen/classparser_test.go index 35df47d96e..743252ffbc 100644 --- a/internal/extgen/classparser_test.go +++ b/internal/extgen/classparser_test.go @@ -639,3 +639,54 @@ func validFloat(tc *TestClass, value float64) float64 { }) } } + +func TestClassParserMethodOnUnknownClass(t *testing.T) { + tests := []struct { + name string + input string + wantErr bool + }{ + { + name: "method targeting an exported class is accepted", + input: `package main + +//export_php:class Known +type Known struct{} + +//export_php:method Known::getName(): string +func (k *Known) getName() unsafe.Pointer { return nil }`, + }, + { + name: "method targeting a class nobody exports is rejected", + input: `package main + +//export_php:class Known +type Known struct{} + +//export_php:method Unknown::getName(): string +func (u *Unknown) getName() unsafe.Pointer { return nil }`, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpFile := filepath.Join(t.TempDir(), "class.go") + require.NoError(t, os.WriteFile(tmpFile, []byte(tt.input), 0644)) + + parser := classParser{} + classes, err := parser.parse(tmpFile) + + if tt.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), `targets class "Unknown"`) + + return + } + + require.NoError(t, err) + require.Len(t, classes, 1) + assert.Len(t, classes[0].Methods, 1) + }) + } +} diff --git a/internal/extgen/constparser.go b/internal/extgen/constparser.go index 00a562519c..744683164b 100644 --- a/internal/extgen/constparser.go +++ b/internal/extgen/constparser.go @@ -12,8 +12,11 @@ import ( var constRegex = regexp.MustCompile(`//\s*export_php:const$`) var classConstRegex = regexp.MustCompile(`//\s*export_php:classconst\s+(\w+)$`) -var constDeclRegex = regexp.MustCompile(`const\s+(\w+)\s*=\s*(.+)`) -var constBlockDeclRegex = regexp.MustCompile(`^(\w+)\s*=\s*(.+)$`) + +// The optional group before "=" is the explicit Go type of a typed constant +// declaration such as "const Perm os.FileMode = 0o755". +var constDeclRegex = regexp.MustCompile(`const\s+(\w+)(?:\s+[\w.\[\]*]+)?\s*=\s*(.+)`) +var constBlockDeclRegex = regexp.MustCompile(`^(\w+)(?:\s+[\w.\[\]*]+)?\s*=\s*(.+)$`) var constNameRegex = regexp.MustCompile(`^(\w+)$`) type ConstantParser struct{} @@ -84,7 +87,7 @@ func (cp *ConstantParser) parse(filename string) (constants []phpConstant, err e matches := constDeclRegex.FindStringSubmatch(line) if len(matches) == 3 { name := matches[1] - value := strings.TrimSpace(matches[2]) + value := normalizeConstValue(strings.TrimSpace(matches[2])) constant := phpConstant{ Name: name, @@ -113,7 +116,7 @@ func (cp *ConstantParser) parse(filename string) (constants []phpConstant, err e } else if inConstBlock && (expectConstDecl || expectClassConstDecl || exportAllInBlock) { if matches := constBlockDeclRegex.FindStringSubmatch(line); len(matches) == 3 { name := matches[1] - value := strings.TrimSpace(matches[2]) + value := normalizeConstValue(strings.TrimSpace(matches[2])) constant := phpConstant{ Name: name, @@ -177,6 +180,22 @@ func (cp *ConstantParser) parse(filename string) (constants []phpConstant, err e return constants, scanner.Err() } +// normalizeConstValue rewrites Go literals that neither C nor PHP understand. +// Only raw string literals qualify: they are re-quoted as regular double-quoted +// strings, which both languages accept. +func normalizeConstValue(value string) string { + if !strings.HasPrefix(value, "`") || !strings.HasSuffix(value, "`") || len(value) < 2 { + return value + } + + unquoted, err := strconv.Unquote(value) + if err != nil { + return value + } + + return strconv.Quote(unquoted) +} + // determineConstantType analyzes the value and determines its type func determineConstantType(value string) phpType { value = strings.TrimSpace(value) diff --git a/internal/extgen/constparser_test.go b/internal/extgen/constparser_test.go index bf3ba2d9bb..6019a8f3ef 100644 --- a/internal/extgen/constparser_test.go +++ b/internal/extgen/constparser_test.go @@ -744,3 +744,83 @@ func TestPHPConstantCValue(t *testing.T) { }) } } + +func TestConstantParserGoOnlyIntLiterals(t *testing.T) { + input := `package main + +//export_php:const +const UNDERSCORED = 1_000_000 + +//export_php:const +const BINARY = 0b1010 + +//export_php:const +const HEXADECIMAL = 0xFF` + + tmpFile := filepath.Join(t.TempDir(), "literals.go") + require.NoError(t, os.WriteFile(tmpFile, []byte(input), 0644)) + + parser := &ConstantParser{} + constants, err := parser.parse(tmpFile) + require.NoError(t, err) + require.Len(t, constants, 3) + + byName := make(map[string]phpConstant, len(constants)) + for _, c := range constants { + byName[c.Name] = c + } + + // PHP understands Go's digit separators and base prefixes, C does not. + assert.Equal(t, "1_000_000", byName["UNDERSCORED"].Value) + assert.Equal(t, "1000000", byName["UNDERSCORED"].CValue()) + assert.Equal(t, "0b1010", byName["BINARY"].Value) + assert.Equal(t, "10", byName["BINARY"].CValue()) + + // hexadecimal is valid C and is left alone + assert.Equal(t, "0xFF", byName["HEXADECIMAL"].CValue()) +} + +func TestConstantParserRawStringConstant(t *testing.T) { + input := "package main\n\n//export_php:const\nconst RAW = `raw \"quoted\" value`" + + tmpFile := filepath.Join(t.TempDir(), "raw.go") + require.NoError(t, os.WriteFile(tmpFile, []byte(input), 0644)) + + parser := &ConstantParser{} + constants, err := parser.parse(tmpFile) + require.NoError(t, err) + require.Len(t, constants, 1) + + // backticks delimit a string in Go only: both C and PHP need double quotes + assert.Equal(t, phpString, constants[0].PhpType) + assert.Equal(t, `"raw \"quoted\" value"`, constants[0].Value) + assert.Equal(t, `"raw \"quoted\" value"`, constants[0].CValue()) +} + +func TestConstantParserTypedConstants(t *testing.T) { + input := `package main + +//export_php:const +const PERM os.FileMode = 0o755 + +//export_php:const +const ( + SMALL int64 = 1 + LARGE = 2 +)` + + tmpFile := filepath.Join(t.TempDir(), "typed.go") + require.NoError(t, os.WriteFile(tmpFile, []byte(input), 0644)) + + parser := &ConstantParser{} + constants, err := parser.parse(tmpFile) + require.NoError(t, err) + require.Len(t, constants, 3) + + assert.Equal(t, "PERM", constants[0].Name) + assert.Equal(t, "493", constants[0].CValue()) + assert.Equal(t, "SMALL", constants[1].Name) + assert.Equal(t, "1", constants[1].Value) + assert.Equal(t, "LARGE", constants[2].Name) + assert.Equal(t, "2", constants[2].Value) +} diff --git a/internal/extgen/gofile.go b/internal/extgen/gofile.go index 044696a47f..81ce18958b 100644 --- a/internal/extgen/gofile.go +++ b/internal/extgen/gofile.go @@ -23,9 +23,6 @@ type goTemplateData struct { PackageName string BaseName string SanitizedBaseName string - Constants []phpConstant - Variables []string - InternalFunctions []string Functions []phpFunction Classes []phpClass } @@ -43,23 +40,17 @@ func (gg *GoFileGenerator) generate() error { func (gg *GoFileGenerator) buildContent() (string, error) { sourceAnalyzer := SourceAnalyzer{} - packageName, variables, internalFunctions, err := sourceAnalyzer.analyze(gg.generator.SourceFile) + packageName, err := sourceAnalyzer.analyze(gg.generator.SourceFile) if err != nil { return "", fmt.Errorf("analyzing source file: %w", err) } - classes := make([]phpClass, len(gg.generator.Classes)) - copy(classes, gg.generator.Classes) - templateContent, err := gg.getTemplateContent(goTemplateData{ PackageName: packageName, BaseName: gg.generator.BaseName, SanitizedBaseName: SanitizePackageName(gg.generator.BaseName), - Constants: gg.generator.Constants, - Variables: variables, - InternalFunctions: internalFunctions, Functions: gg.generator.Functions, - Classes: classes, + Classes: gg.generator.Classes, }) if err != nil { @@ -76,10 +67,11 @@ func (gg *GoFileGenerator) buildContent() (string, error) { func (gg *GoFileGenerator) getTemplateContent(data goTemplateData) (string, error) { funcMap := sprig.FuncMap() - funcMap["phpTypeToGoType"] = gg.phpTypeToGoType - funcMap["isStringOrArray"] = func(t phpType) bool { - return t == phpString || t == phpArray - } + // Reuse the validator's mapping so the signatures the generator emits cannot + // drift from the ones it accepts. + validator := &Validator{} + funcMap["goParamType"] = validator.phpTypeToGoType + funcMap["goReturnType"] = validator.phpReturnTypeToGoType funcMap["isVoid"] = func(t phpType) bool { return t == phpVoid } @@ -98,55 +90,30 @@ func (gg *GoFileGenerator) getTemplateContent(data goTemplateData) (string, erro return buf.String(), nil } -type GoMethodSignature struct { - MethodName string - Params []GoParameter - ReturnType string -} - -type GoParameter struct { - Name string - Type string -} - -var phpToGoTypeMap = map[phpType]string{ - phpString: "string", - phpInt: "int64", - phpFloat: "float64", - phpBool: "bool", - phpArray: "*frankenphp.Array", - phpMixed: "any", - phpVoid: "", - phpCallable: "*C.zval", -} - -func (gg *GoFileGenerator) phpTypeToGoType(phpT phpType) string { - if goType, exists := phpToGoTypeMap[phpT]; exists { - return goType - } - - return "any" -} - -// extractGoFunctionName extracts the Go function name from a Go function signature string. +// extractGoFunctionName extracts the Go function or method 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++ + rest := strings.TrimLeft(goFunction[idx+len("func "):], " \t") + if strings.HasPrefix(rest, "(") { + // method: skip the receiver so the name, not the receiver, is returned + closing := strings.IndexByte(rest, ')') + if closing == -1 { + return "" + } + rest = rest[closing+1:] } - if end >= len(goFunction) { + end := strings.IndexByte(rest, '(') + if end == -1 { return "" } - return strings.TrimSpace(goFunction[start:end]) + return strings.TrimSpace(rest[:end]) } // extractGoFunctionSignatureParams extracts the parameters from a Go function signature. diff --git a/internal/extgen/gofile_test.go b/internal/extgen/gofile_test.go index a64504a422..196ca4214f 100644 --- a/internal/extgen/gofile_test.go +++ b/internal/extgen/gofile_test.go @@ -242,6 +242,7 @@ func (ts *TestStruct) GetValue() string { { Name: "GetValue", ReturnType: phpString, + GoFunction: "func (ts *TestStruct) GetValue() unsafe.Pointer {\n\treturn nil\n}", }, }, }, @@ -509,13 +510,13 @@ func (ts *TestStruct) ProcessData(name string, count *int64, enabled *bool) stri content, err := goGen.buildContent() require.NoError(t, err) - expectedWrapperSignature := "func ProcessData_wrapper(handle C.uintptr_t, name *C.zend_string, count *int64, enabled *bool)" + expectedWrapperSignature := "func TestClass_ProcessData_wrapper(handle C.uintptr_t, name *C.zend_string, count *int64, enabled *bool)" assert.Contains(t, content, expectedWrapperSignature, "Generated content should contain wrapper with nullable pointer types: %s", expectedWrapperSignature) expectedCall := "structObj.ProcessData(name, count, enabled)" assert.Contains(t, content, expectedCall, "Generated content should contain correct method call: %s", expectedCall) - exportDirective := "//export ProcessData_wrapper" + exportDirective := "//export TestClass_ProcessData_wrapper" assert.Contains(t, content, exportDirective, "Generated content should contain export directive: %s", exportDirective) } @@ -604,10 +605,10 @@ func (as *ArrayStruct) FilterData(data frankenphp.AssociativeArray, filter strin content, err := goGen.buildContent() require.NoError(t, err) - expectedArrayWrapperSignature := "func ProcessArray_wrapper(handle C.uintptr_t, items *C.zval) unsafe.Pointer" + expectedArrayWrapperSignature := "func ArrayClass_ProcessArray_wrapper(handle C.uintptr_t, items *C.zend_array) unsafe.Pointer" assert.Contains(t, content, expectedArrayWrapperSignature, "Generated content should contain array wrapper signature: %s", expectedArrayWrapperSignature) - expectedMixedWrapperSignature := "func FilterData_wrapper(handle C.uintptr_t, data *C.zval, filter *C.zend_string) unsafe.Pointer" + expectedMixedWrapperSignature := "func ArrayClass_FilterData_wrapper(handle C.uintptr_t, data *C.zend_array, filter *C.zend_string) unsafe.Pointer" assert.Contains(t, content, expectedMixedWrapperSignature, "Generated content should contain mixed wrapper signature: %s", expectedMixedWrapperSignature) expectedArrayCall := "structObj.ProcessArray(items)" @@ -616,8 +617,8 @@ func (as *ArrayStruct) FilterData(data frankenphp.AssociativeArray, filter strin expectedMixedCall := "structObj.FilterData(data, filter)" assert.Contains(t, content, expectedMixedCall, "Generated content should contain mixed method call: %s", expectedMixedCall) - assert.Contains(t, content, "//export ProcessArray_wrapper", "Generated content should contain ProcessArray export directive") - assert.Contains(t, content, "//export FilterData_wrapper", "Generated content should contain FilterData export directive") + assert.Contains(t, content, "//export ArrayClass_ProcessArray_wrapper", "Generated content should contain ProcessArray export directive") + assert.Contains(t, content, "//export ArrayClass_FilterData_wrapper", "Generated content should contain FilterData export directive") } func TestGoFileGenerator_Idempotency(t *testing.T) { @@ -741,6 +742,16 @@ func TestExtractGoFunctionName(t *testing.T) { input: "func spacedName () {}", expected: "spacedName", }, + { + name: "method with a pointer receiver", + input: "func (s *MyStruct) getValue() string {}", + expected: "getValue", + }, + { + name: "method with a value receiver", + input: "func (s MyStruct) get_value() string {}", + expected: "get_value", + }, { name: "no func keyword", input: "test() {}", @@ -1100,13 +1111,13 @@ func (nas *NullableArrayStruct) ProcessOptionalArray(items frankenphp.Associativ content, err := goGen.buildContent() require.NoError(t, err) - expectedWrapperSignature := "func ProcessOptionalArray_wrapper(handle C.uintptr_t, items *C.zval, name *C.zend_string) unsafe.Pointer" + expectedWrapperSignature := "func NullableArrayClass_ProcessOptionalArray_wrapper(handle C.uintptr_t, items *C.zend_array, name *C.zend_string) unsafe.Pointer" assert.Contains(t, content, expectedWrapperSignature, "Generated content should contain nullable array wrapper signature: %s", expectedWrapperSignature) expectedCall := "structObj.ProcessOptionalArray(items, name)" assert.Contains(t, content, expectedCall, "Generated content should contain method call: %s", expectedCall) - assert.Contains(t, content, "//export ProcessOptionalArray_wrapper", "Generated content should contain export directive") + assert.Contains(t, content, "//export NullableArrayClass_ProcessOptionalArray_wrapper", "Generated content should contain export directive") } func createTempSourceFile(t *testing.T, content string) string { @@ -1189,10 +1200,10 @@ func (cs *CallableStruct) ProcessOptionalCallback(callback *C.zval) string { content, err := goGen.buildContent() require.NoError(t, err) - expectedCallableWrapperSignature := "func ProcessCallback_wrapper(handle C.uintptr_t, callback *C.zval) unsafe.Pointer" + expectedCallableWrapperSignature := "func CallableClass_ProcessCallback_wrapper(handle C.uintptr_t, callback *C.zval) unsafe.Pointer" assert.Contains(t, content, expectedCallableWrapperSignature, "Generated content should contain callable wrapper signature: %s", expectedCallableWrapperSignature) - expectedOptionalCallableWrapperSignature := "func ProcessOptionalCallback_wrapper(handle C.uintptr_t, callback *C.zval) unsafe.Pointer" + expectedOptionalCallableWrapperSignature := "func CallableClass_ProcessOptionalCallback_wrapper(handle C.uintptr_t, callback *C.zval) unsafe.Pointer" assert.Contains(t, content, expectedOptionalCallableWrapperSignature, "Generated content should contain optional callable wrapper signature: %s", expectedOptionalCallableWrapperSignature) expectedCallableCall := "structObj.ProcessCallback(callback)" @@ -1201,40 +1212,8 @@ func (cs *CallableStruct) ProcessOptionalCallback(callback *C.zval) string { expectedOptionalCallableCall := "structObj.ProcessOptionalCallback(callback)" assert.Contains(t, content, expectedOptionalCallableCall, "Generated content should contain optional callable method call: %s", expectedOptionalCallableCall) - assert.Contains(t, content, "//export ProcessCallback_wrapper", "Generated content should contain ProcessCallback export directive") - assert.Contains(t, content, "//export ProcessOptionalCallback_wrapper", "Generated content should contain ProcessOptionalCallback export directive") -} - -func TestGoFileGenerator_phpTypeToGoType(t *testing.T) { - generator := &Generator{} - goGen := GoFileGenerator{generator} - - tests := []struct { - phpType phpType - expected string - }{ - {phpString, "string"}, - {phpInt, "int64"}, - {phpFloat, "float64"}, - {phpBool, "bool"}, - {phpArray, "*frankenphp.Array"}, - {phpMixed, "any"}, - {phpVoid, ""}, - {phpCallable, "*C.zval"}, - } - - for _, tt := range tests { - t.Run(string(tt.phpType), func(t *testing.T) { - result := goGen.phpTypeToGoType(tt.phpType) - assert.Equal(t, tt.expected, result, "phpTypeToGoType(%s) should return %s", tt.phpType, tt.expected) - }) - } - - t.Run("unknown_type", func(t *testing.T) { - unknownType := phpType("unknown") - result := goGen.phpTypeToGoType(unknownType) - assert.Equal(t, "any", result, "phpTypeToGoType should fallback to interface{} for unknown types") - }) + assert.Contains(t, content, "//export CallableClass_ProcessCallback_wrapper", "Generated content should contain ProcessCallback export directive") + assert.Contains(t, content, "//export CallableClass_ProcessOptionalCallback_wrapper", "Generated content should contain ProcessOptionalCallback export directive") } func testGeneratedFileBasicStructure(t *testing.T, content, expectedPackage, baseName string) { @@ -1288,3 +1267,72 @@ func assertContainsHeaderComment(t *testing.T, filename string) { 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") } + +func TestGoFileGenerator_MethodWrappersAreClassQualified(t *testing.T) { + tmpDir := t.TempDir() + sourceFile := filepath.Join(tmpDir, "source.go") + require.NoError(t, os.WriteFile(sourceFile, []byte("package main\n"), 0644)) + + // Both classes expose a method with the same PHP name: the exported cgo + // symbols must not collide. + newClass := func(phpName, goStruct string) phpClass { + return phpClass{ + Name: phpName, + GoStruct: goStruct, + Methods: []phpClassMethod{{ + Name: "getName", + PhpName: "getName", + ClassName: phpName, + ReturnType: phpString, + GoFunction: "func (s *" + goStruct + ") GetName() unsafe.Pointer {\n\treturn nil\n}", + }}, + } + } + + generator := &Generator{ + BaseName: "collide", + SourceFile: sourceFile, + BuildDir: tmpDir, + Classes: []phpClass{newClass("User", "UserStruct"), newClass("Group", "GroupStruct")}, + } + + goGen := GoFileGenerator{generator} + content, err := goGen.buildContent() + require.NoError(t, err) + + assert.Contains(t, content, "//export User_getName_wrapper") + assert.Contains(t, content, "//export Group_getName_wrapper") + assert.NotContains(t, content, "//export getName_wrapper") +} + +func TestGoFileGenerator_MethodWrapperUsesActualGoName(t *testing.T) { + tmpDir := t.TempDir() + sourceFile := filepath.Join(tmpDir, "source.go") + require.NoError(t, os.WriteFile(sourceFile, []byte("package main\n"), 0644)) + + generator := &Generator{ + BaseName: "snake_test", + SourceFile: sourceFile, + BuildDir: tmpDir, + Classes: []phpClass{{ + Name: "SnakeClass", + GoStruct: "SnakeStruct", + Methods: []phpClassMethod{ + { + Name: "get_value", + PhpName: "get_value", + ClassName: "SnakeClass", + ReturnType: phpInt, + GoFunction: "func (s *SnakeStruct) GetValue() int64 {\n\treturn 42\n}", + }, + }, + }}, + } + + goGen := GoFileGenerator{generator} + content, err := goGen.buildContent() + require.NoError(t, err) + + assert.Contains(t, content, "return structObj.GetValue()") + assert.NotContains(t, content, "structObj.Get_Value(") +} diff --git a/internal/extgen/nodes.go b/internal/extgen/nodes.go index 5afd1e38a0..3017bc51f1 100644 --- a/internal/extgen/nodes.go +++ b/internal/extgen/nodes.go @@ -1,8 +1,8 @@ package extgen import ( + "regexp" "strconv" - "strings" ) // phpType represents a PHP type @@ -77,17 +77,20 @@ type phpConstant struct { ClassName string // empty for global constants, set for class constants } +// goOnlyIntLiteral matches the integer spellings Go accepts but C does not: +// the "0o"/"0b" base prefixes and digit separators. +var goOnlyIntLiteral = regexp.MustCompile(`^[+-]?0[oObB]|_`) + // CValue returns the constant value in C-compatible format func (c phpConstant) CValue() string { - if c.PhpType != phpInt { + if c.PhpType != phpInt || !goOnlyIntLiteral.MatchString(c.Value) { return c.Value } - if strings.HasPrefix(c.Value, "0o") { - if val, err := strconv.ParseInt(c.Value, 0, 64); err == nil { - return strconv.FormatInt(val, 10) - } + val, err := strconv.ParseInt(c.Value, 0, 64) + if err != nil { + return c.Value } - return c.Value + return strconv.FormatInt(val, 10) } diff --git a/internal/extgen/paramparser.go b/internal/extgen/paramparser.go index 6dcc365e3a..a9f40c3054 100644 --- a/internal/extgen/paramparser.go +++ b/internal/extgen/paramparser.go @@ -72,8 +72,8 @@ func (pp *ParameterParser) generateSingleParamDeclaration(param phpParameter) [] decls = append(decls, fmt.Sprintf("zend_array *%s = NULL;", param.Name)) case phpMixed: decls = append(decls, fmt.Sprintf("zval *%s = NULL;", param.Name)) - case "callable": - decls = append(decls, fmt.Sprintf("zval *%s_callback;", param.Name)) + case phpCallable: + decls = append(decls, fmt.Sprintf("zval *%s_callback = NULL;", param.Name)) } return decls diff --git a/internal/extgen/paramparser_test.go b/internal/extgen/paramparser_test.go index 8d19e53f00..c53a55874d 100644 --- a/internal/extgen/paramparser_test.go +++ b/internal/extgen/paramparser_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestParameterParser_AnalyzeParameters(t *testing.T) { @@ -182,14 +183,14 @@ func TestParameterParser_GenerateParamDeclarations(t *testing.T) { params: []phpParameter{ {Name: "callback", PhpType: phpCallable, HasDefault: false}, }, - expected: " zval *callback_callback;", + expected: " zval *callback_callback = NULL;", }, { name: "nullable callable parameter", params: []phpParameter{ {Name: "callback", PhpType: phpCallable, HasDefault: false, IsNullable: true}, }, - expected: " zval *callback_callback;", + expected: " zval *callback_callback = NULL;", }, { name: "mixed types with callable", @@ -198,7 +199,7 @@ func TestParameterParser_GenerateParamDeclarations(t *testing.T) { {Name: "callback", PhpType: phpCallable, HasDefault: false}, {Name: "options", PhpType: phpInt, HasDefault: true, DefaultValue: "0"}, }, - expected: " zend_array *data = NULL;\n zval *callback_callback;\n zend_long options = 0;", + expected: " zend_array *data = NULL;\n zval *callback_callback = NULL;\n zend_long options = 0;", }, } @@ -625,12 +626,12 @@ func TestParameterParser_GenerateSingleParamDeclaration(t *testing.T) { { name: "callable parameter", param: phpParameter{Name: "callback", PhpType: "callable", HasDefault: false}, - expected: []string{"zval *callback_callback;"}, + expected: []string{"zval *callback_callback = NULL;"}, }, { name: "nullable callable parameter", param: phpParameter{Name: "callback", PhpType: "callable", HasDefault: false, IsNullable: true}, - expected: []string{"zval *callback_callback;"}, + expected: []string{"zval *callback_callback = NULL;"}, }, } @@ -672,3 +673,18 @@ func TestParameterParser_Integration(t *testing.T) { goCallParams := pp.generateGoCallParams(params) assert.Equal(t, "name, (long) count, (int) enabled", goCallParams) } + +func TestParameterParser_OptionalCallableIsInitialized(t *testing.T) { + pp := &ParameterParser{} + + // An omitted optional argument leaves the local untouched, so it must start + // out NULL rather than holding whatever was on the stack. + decls := pp.generateSingleParamDeclaration(phpParameter{ + Name: "cb", + PhpType: phpCallable, + IsNullable: true, + HasDefault: true, + }) + + require.Equal(t, []string{"zval *cb_callback = NULL;"}, decls) +} diff --git a/internal/extgen/phpfunc.go b/internal/extgen/phpfunc.go index 66a0123eeb..2e2ae9a760 100644 --- a/internal/extgen/phpfunc.go +++ b/internal/extgen/phpfunc.go @@ -78,7 +78,7 @@ func (pfg *PHPFuncGenerator) generateReturnCode(returnType phpType) string { RETURN_STR(result); } - RETURN_EMPTY_STRING();` + RETURN_EMPTY_STRING();` case phpInt: return ` RETURN_LONG(result);` case phpFloat: @@ -90,7 +90,13 @@ func (pfg *PHPFuncGenerator) generateReturnCode(returnType phpType) string { RETURN_ARR(result); } - RETURN_EMPTY_ARRAY();` + RETURN_EMPTY_ARRAY();` + case phpMixed: + return ` if (result) { + RETURN_COPY_VALUE(result); + } + + RETURN_NULL();` default: return "" } diff --git a/internal/extgen/phpfunc_test.go b/internal/extgen/phpfunc_test.go index 3a0365ccd5..354447b004 100644 --- a/internal/extgen/phpfunc_test.go +++ b/internal/extgen/phpfunc_test.go @@ -419,3 +419,22 @@ func TestPHPFunctionGenerator_AnalyzeParameters(t *testing.T) { }) } } + +func TestPHPFunctionGenerator_MixedReturn(t *testing.T) { + generator := PHPFuncGenerator{paramParser: &ParameterParser{}} + + t.Run("mixed return value is forwarded to PHP", func(t *testing.T) { + result := generator.generate(phpFunction{Name: "pick", ReturnType: phpMixed}) + + assert.Contains(t, result, "zval *result = go_pick();") + assert.Contains(t, result, "RETURN_COPY_VALUE(result);") + assert.Contains(t, result, "RETURN_NULL();") + }) + + t.Run("void return declares no result", func(t *testing.T) { + result := generator.generate(phpFunction{Name: "run", ReturnType: phpVoid}) + + assert.Contains(t, result, "go_run();") + assert.NotContains(t, result, "result") + }) +} diff --git a/internal/extgen/srcanalyzer.go b/internal/extgen/srcanalyzer.go index 32ebff4040..299a2f8f76 100644 --- a/internal/extgen/srcanalyzer.go +++ b/internal/extgen/srcanalyzer.go @@ -4,138 +4,15 @@ import ( "fmt" "go/parser" "go/token" - "os" - "strings" ) type SourceAnalyzer struct{} -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) +func (sa *SourceAnalyzer) analyze(filename string) (packageName string, err error) { + node, err := parser.ParseFile(token.NewFileSet(), filename, nil, parser.SkipObjectResolution) if err != nil { - return "", nil, nil, fmt.Errorf("parsing file: %w", err) + return "", fmt.Errorf("parsing file: %w", err) } - packageName = node.Name.Name - - sourceContent, err := os.ReadFile(filename) - if err != nil { - return "", nil, nil, fmt.Errorf("reading source file: %w", err) - } - - variables = sa.extractVariables(string(sourceContent)) - internalFunctions = sa.extractInternalFunctions(string(sourceContent)) - - return packageName, variables, internalFunctions, nil -} - -func (sa *SourceAnalyzer) extractVariables(content string) []string { - lines := strings.Split(content, "\n") - var ( - variables []string - currentVar strings.Builder - inVarBlock bool - parenCount int - ) - - for _, line := range lines { - trimmedLine := strings.TrimSpace(line) - - if strings.HasPrefix(trimmedLine, "var ") && !inVarBlock { - if strings.Contains(trimmedLine, "(") { - inVarBlock = true - parenCount = 1 - currentVar.Reset() - currentVar.WriteString(line + "\n") - } else { - variables = append(variables, strings.TrimSpace(line)) - } - } else if inVarBlock { - currentVar.WriteString(line + "\n") - - for _, char := range line { - switch char { - case '(': - parenCount++ - case ')': - parenCount-- - } - } - - if parenCount == 0 { - varContent := currentVar.String() - variables = append(variables, strings.TrimSpace(varContent)) - inVarBlock = false - currentVar.Reset() - } - } - } - - return variables -} - -func (sa *SourceAnalyzer) extractInternalFunctions(content string) []string { - lines := strings.Split(content, "\n") - var ( - functions []string - currentFunc strings.Builder - inFunction, hasPHPFunc bool - braceCount int - ) - - for i, line := range lines { - trimmedLine := strings.TrimSpace(line) - - if strings.HasPrefix(trimmedLine, "func ") && !inFunction { - inFunction = true - braceCount = 0 - hasPHPFunc = false - currentFunc.Reset() - - // look backwards for export_php comment - for j := i - 1; j >= 0 && j >= i-5; j-- { - prevLine := strings.TrimSpace(lines[j]) - if prevLine == "" { - continue - } - - if strings.Contains(prevLine, "export_php:") { - hasPHPFunc = true - - break - } - - if !strings.HasPrefix(prevLine, "//") { - break - } - } - } - - if inFunction { - currentFunc.WriteString(line + "\n") - - for _, char := range line { - switch char { - case '{': - braceCount++ - case '}': - braceCount-- - } - } - - if braceCount == 0 && strings.Contains(line, "}") { - funcContent := currentFunc.String() - - if !hasPHPFunc { - functions = append(functions, strings.TrimSpace(funcContent)) - } - - inFunction = false - currentFunc.Reset() - } - } - } - - return functions + return node.Name.Name, nil } diff --git a/internal/extgen/srcanalyzer_test.go b/internal/extgen/srcanalyzer_test.go index 74207b1b7c..e9f16b1ff5 100644 --- a/internal/extgen/srcanalyzer_test.go +++ b/internal/extgen/srcanalyzer_test.go @@ -5,243 +5,46 @@ import ( "path/filepath" "testing" - "github.com/stretchr/testify/require" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestSourceAnalyzer_Analyze(t *testing.T) { tests := []struct { - name string - sourceContent string - expectedImports []string - expectedVariables []string - expectedFunctions []string - expectError bool + name string + sourceContent string + expectedPackage string }{ { - name: "simple file with imports and functions", + name: "package main", sourceContent: `package main -import ( - "fmt" - "strings" -) +import "fmt" -func regularFunction() { - fmt.Println("hello") -} +var globalVar = "test" -//export_php:function -func exportedFunction() string { - return "exported" -}`, - expectedImports: []string{`"fmt"`, `"strings"`}, - expectedVariables: nil, - expectedFunctions: []string{ - `func regularFunction() { - fmt.Println("hello") +func helper() { + fmt.Println("helper") }`, - }, - expectError: false, + expectedPackage: "main", }, { - name: "file with named imports", - sourceContent: `package main - -import ( - custom "fmt" - . "strings" - _ "os" -) - -func test() {}`, - expectedImports: []string{`custom "fmt"`, `. "strings"`, `_ "os"`}, - expectedVariables: nil, - expectedFunctions: []string{ - `func test() {}`, - }, - expectError: false, + name: "custom package name", + sourceContent: "package myextension\n", + expectedPackage: "myextension", }, - { - name: "file with multiple functions and export comments", - sourceContent: `package main - -func internalOne() { - // some code -} - -// This function is exported to PHP -//export_php:function -func exportedOne() int { - return 42 -} - -func internalTwo() string { - return "internal" -} - -// Another exported function -//export_php:function -func exportedTwo() bool { - return true -}`, - expectedImports: []string{}, - expectedVariables: nil, - expectedFunctions: []string{ - `func internalOne() { - // some code -}`, - `func internalTwo() string { - return "internal" -}`, - }, - expectError: false, - }, - { - name: "file with nested braces", - sourceContent: `package main - -func complexFunction() { - if true { - for i := 0; i < 10; i++ { - if i%2 == 0 { - fmt.Println(i) - } - } - } -} - -//export_php:function -func exportedComplex() { - obj := struct{ - field string - }{ - field: "value", - } - fmt.Println(obj) -}`, - expectedImports: []string{}, - expectedVariables: nil, - expectedFunctions: []string{ - `func complexFunction() { - if true { - for i := 0; i < 10; i++ { - if i%2 == 0 { - fmt.Println(i) - } - } } -}`, - }, - expectError: false, - }, - { - name: "empty file", - sourceContent: `package main`, - expectedImports: []string{}, - expectedFunctions: []string{}, - expectError: false, - }, - { - name: "file with only exported functions", - sourceContent: `package main - -//export_php:function -func onlyExported() {} -//export_php:function -func anotherExported() string { - return "test" -}`, - expectedImports: []string{}, - expectedFunctions: []string{}, - expectError: false, - }, - { - name: "file with export comment not immediately before function", - sourceContent: `package main - -//export_php:function -// Some other comment -func shouldNotBeExported() {} - -func normalFunction() { - //export_php:function inside function should not count -}`, - expectedImports: []string{}, - expectedVariables: nil, - expectedFunctions: []string{ - `func normalFunction() { - //export_php:function inside function should not count -}`, - }, - expectError: false, - }, - { - name: "file with variable blocks", - sourceContent: `package main - -import ( - "sync" -) - -var ( - mu sync.RWMutex - store = map[string]struct { - val string - expires int64 - }{} -) - -var singleVar = "test" - -func testFunction() { - // test function -}`, - expectedImports: []string{`"sync"`}, - expectedVariables: []string{ - `var ( - mu sync.RWMutex - store = map[string]struct { - val string - expires int64 - }{} -)`, - `var singleVar = "test"`, - }, - expectedFunctions: []string{ - `func testFunction() { - // test function -}`, - }, - expectError: false, - }, - } + analyzer := &SourceAnalyzer{} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - tempDir := t.TempDir() - filename := filepath.Join(tempDir, "test.go") - + filename := filepath.Join(t.TempDir(), "source.go") require.NoError(t, os.WriteFile(filename, []byte(tt.sourceContent), 0644)) - analyzer := &SourceAnalyzer{} - _, variables, functions, err := analyzer.analyze(filename) - - if tt.expectError { - assert.Error(t, err, "expected error") - return - } - - assert.NoError(t, err, "unexpected error") - - assert.Equal(t, tt.expectedVariables, variables, "variables mismatch") - assert.Len(t, functions, len(tt.expectedFunctions), "function count mismatch") - - for i, expected := range tt.expectedFunctions { - assert.Equal(t, expected, functions[i], "function %d mismatch", i) - } + packageName, err := analyzer.analyze(filename) + require.NoError(t, err) + assert.Equal(t, tt.expectedPackage, packageName) }) } } @@ -250,13 +53,12 @@ func TestSourceAnalyzer_Analyze_InvalidFile(t *testing.T) { analyzer := &SourceAnalyzer{} t.Run("nonexistent file", func(t *testing.T) { - _, _, _, err := analyzer.analyze("/nonexistent/file.go") + _, err := analyzer.analyze("/nonexistent/file.go") assert.Error(t, err, "expected error for nonexistent file") }) t.Run("invalid Go syntax", func(t *testing.T) { - tempDir := t.TempDir() - filename := filepath.Join(tempDir, "invalid.go") + filename := filepath.Join(t.TempDir(), "invalid.go") invalidContent := `package main func incomplete( { @@ -265,242 +67,28 @@ func TestSourceAnalyzer_Analyze_InvalidFile(t *testing.T) { require.NoError(t, os.WriteFile(filename, []byte(invalidContent), 0644)) - _, _, _, err := analyzer.analyze(filename) + _, err := analyzer.analyze(filename) assert.Error(t, err, "expected error for invalid syntax") }) -} - -func TestSourceAnalyzer_ExtractInternalFunctions(t *testing.T) { - tests := []struct { - name string - content string - expected []string - }{ - { - name: "single function without export", - content: `func test() { - fmt.Println("test") -}`, - expected: []string{ - `func test() { - fmt.Println("test") -}`, - }, - }, - { - name: "function with export comment", - content: `//export_php:function -func exported() {}`, - expected: []string{}, - }, - { - name: "mixed functions", - content: `func internal() {} - -//export_php:function -func exported() {} - -func anotherInternal() { - return "test" -}`, - expected: []string{ - "func internal() {}", - `func anotherInternal() { - return "test" -}`, - }, - }, - { - name: "export comment with spacing", - content: `//export_php:function -func exported1() {} -//export_php:function -func exported2() {} + t.Run("missing package clause", func(t *testing.T) { + filename := filepath.Join(t.TempDir(), "orphan.go") + require.NoError(t, os.WriteFile(filename, []byte("func orphan() {}\n"), 0644)) -// export_php:function -func exported3() {}`, - expected: []string{}, - }, - { - name: "complex function with nested braces", - content: `func complex() { - if true { - for { - switch x { - case 1: - { - // nested block - } - } - } - } -}`, - expected: []string{ - `func complex() { - if true { - for { - switch x { - case 1: - { - // nested block - } - } - } - } -}`, - }, - }, - { - name: "empty content", - content: "", - expected: []string{}, - }, - { - name: "no functions", - content: `package main - -import "fmt" - -var x = 10`, - expected: []string{}, - }, - } - - analyzer := &SourceAnalyzer{} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := analyzer.extractInternalFunctions(tt.content) - - assert.Len(t, result, len(tt.expected), "function count mismatch") - - for i, expected := range tt.expected { - assert.Equal(t, expected, result[i], "function %d mismatch", i) - } - }) - } -} - -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)") + _, err := analyzer.analyze(filename) + assert.Error(t, err, "expected error for a file without a package clause") + }) } func BenchmarkSourceAnalyzer_Analyze(b *testing.B) { content := `package main -import ( - "fmt" - "strings" - "os" -) +import "fmt" -func internalOne() { - fmt.Println("test") -} +var globalVar = "test" -//export_php:function -func exported() string { - return "exported" +func internalOne() { + fmt.Println("one") } func internalTwo() { @@ -511,33 +99,13 @@ func internalTwo() { } }` - tempDir := b.TempDir() - filename := filepath.Join(tempDir, "bench.go") - + filename := filepath.Join(b.TempDir(), "bench.go") require.NoError(b, os.WriteFile(filename, []byte(content), 0644)) analyzer := &SourceAnalyzer{} for b.Loop() { - _, _, _, err := analyzer.analyze(filename) + _, err := analyzer.analyze(filename) require.NoError(b, err) } } - -func BenchmarkSourceAnalyzer_ExtractInternalFunctions(b *testing.B) { - content := `func test1() { fmt.Println("1") } -func test2() { fmt.Println("2") } -//export_php:function -func exported() {} -func test3() { - for i := 0; i < 10; i++ { - fmt.Println(i) - } -}` - - analyzer := &SourceAnalyzer{} - - for b.Loop() { - analyzer.extractInternalFunctions(content) - } -} diff --git a/internal/extgen/templates/extension.c.tpl b/internal/extgen/templates/extension.c.tpl index fd7911d812..9a03bace2e 100644 --- a/internal/extgen/templates/extension.c.tpl +++ b/internal/extgen/templates/extension.c.tpl @@ -9,9 +9,9 @@ {{define "methodCallArg" -}} {{- if .IsNullable -}} -{{if eq .PhpType "string"}}{{.Name}}_is_null ? NULL : {{.Name}}{{else if eq .PhpType "int"}}{{.Name}}_is_null ? NULL : &{{.Name}}{{else if eq .PhpType "float"}}{{.Name}}_is_null ? NULL : &{{.Name}}{{else if eq .PhpType "bool"}}{{.Name}}_is_null ? NULL : &{{.Name}}{{else if eq .PhpType "array"}}{{.Name}}{{else if eq .PhpType "callable"}}{{.Name}}_callback{{end}} +{{if eq .PhpType "string"}}{{.Name}}_is_null ? NULL : {{.Name}}{{else if eq .PhpType "int"}}{{.Name}}_is_null ? NULL : &{{.Name}}{{else if eq .PhpType "float"}}{{.Name}}_is_null ? NULL : &{{.Name}}{{else if eq .PhpType "bool"}}{{.Name}}_is_null ? NULL : &{{.Name}}{{else if eq .PhpType "array"}}{{.Name}}{{else if eq .PhpType "mixed"}}{{.Name}}{{else if eq .PhpType "callable"}}{{.Name}}_callback{{end}} {{- else -}} -{{if eq .PhpType "string"}}{{.Name}}{{else if eq .PhpType "int"}}(long){{.Name}}{{else if eq .PhpType "float"}}(double){{.Name}}{{else if eq .PhpType "bool"}}(int){{.Name}}{{else if eq .PhpType "array"}}{{.Name}}{{else if eq .PhpType "callable"}}{{.Name}}_callback{{end}} +{{if eq .PhpType "string"}}{{.Name}}{{else if eq .PhpType "int"}}(long){{.Name}}{{else if eq .PhpType "float"}}(double){{.Name}}{{else if eq .PhpType "bool"}}(int){{.Name}}{{else if eq .PhpType "array"}}{{.Name}}{{else if eq .PhpType "mixed"}}{{.Name}}{{else if eq .PhpType "callable"}}{{.Name}}_callback{{end}} {{- end -}} {{- end}} @@ -113,8 +113,10 @@ PHP_METHOD({{namespacedClassName $.Namespace .ClassName}}, {{.PhpName}}) { zend_bool {{$param.Name}}_is_null = 0;{{end}} {{- else if eq $param.PhpType "array"}} zend_array *{{$param.Name}} = NULL; + {{- else if eq $param.PhpType "mixed"}} + zval *{{$param.Name}} = NULL; {{- else if eq $param.PhpType "callable"}} - zval *{{$param.Name}}_callback; + zval *{{$param.Name}}_callback = NULL; {{- end}} {{- end}} @@ -123,7 +125,7 @@ PHP_METHOD({{namespacedClassName $.Namespace .ClassName}}, {{.PhpName}}) { {{$optionalStarted := false}}{{range .Params}}{{if .HasDefault}}{{if not $optionalStarted -}} Z_PARAM_OPTIONAL {{$optionalStarted = true}}{{end}}{{end -}} - {{if .IsNullable}}{{if eq .PhpType "string"}}Z_PARAM_STR_OR_NULL({{.Name}}, {{.Name}}_is_null){{else if eq .PhpType "int"}}Z_PARAM_LONG_OR_NULL({{.Name}}, {{.Name}}_is_null){{else if eq .PhpType "float"}}Z_PARAM_DOUBLE_OR_NULL({{.Name}}, {{.Name}}_is_null){{else if eq .PhpType "bool"}}Z_PARAM_BOOL_OR_NULL({{.Name}}, {{.Name}}_is_null){{else if eq .PhpType "array"}}Z_PARAM_ARRAY_HT_OR_NULL({{.Name}}){{else if eq .PhpType "callable"}}Z_PARAM_ZVAL_OR_NULL({{.Name}}_callback){{end}}{{else}}{{if eq .PhpType "string"}}Z_PARAM_STR({{.Name}}){{else if eq .PhpType "int"}}Z_PARAM_LONG({{.Name}}){{else if eq .PhpType "float"}}Z_PARAM_DOUBLE({{.Name}}){{else if eq .PhpType "bool"}}Z_PARAM_BOOL({{.Name}}){{else if eq .PhpType "array"}}Z_PARAM_ARRAY_HT({{.Name}}){{else if eq .PhpType "callable"}}Z_PARAM_ZVAL({{.Name}}_callback){{end}}{{end}} + {{if .IsNullable}}{{if eq .PhpType "string"}}Z_PARAM_STR_OR_NULL({{.Name}}, {{.Name}}_is_null){{else if eq .PhpType "int"}}Z_PARAM_LONG_OR_NULL({{.Name}}, {{.Name}}_is_null){{else if eq .PhpType "float"}}Z_PARAM_DOUBLE_OR_NULL({{.Name}}, {{.Name}}_is_null){{else if eq .PhpType "bool"}}Z_PARAM_BOOL_OR_NULL({{.Name}}, {{.Name}}_is_null){{else if eq .PhpType "array"}}Z_PARAM_ARRAY_HT_OR_NULL({{.Name}}){{else if eq .PhpType "mixed"}}Z_PARAM_ZVAL_OR_NULL({{.Name}}){{else if eq .PhpType "callable"}}Z_PARAM_ZVAL_OR_NULL({{.Name}}_callback){{end}}{{else}}{{if eq .PhpType "string"}}Z_PARAM_STR({{.Name}}){{else if eq .PhpType "int"}}Z_PARAM_LONG({{.Name}}){{else if eq .PhpType "float"}}Z_PARAM_DOUBLE({{.Name}}){{else if eq .PhpType "bool"}}Z_PARAM_BOOL({{.Name}}){{else if eq .PhpType "array"}}Z_PARAM_ARRAY_HT({{.Name}}){{else if eq .PhpType "mixed"}}Z_PARAM_ZVAL({{.Name}}){{else if eq .PhpType "callable"}}Z_PARAM_ZVAL({{.Name}}_callback){{end}}{{end}} {{end -}} ZEND_PARSE_PARAMETERS_END(); {{else}} @@ -132,31 +134,38 @@ PHP_METHOD({{namespacedClassName $.Namespace .ClassName}}, {{.PhpName}}) { {{- if ne .ReturnType "void"}} {{- if eq .ReturnType "string"}} - zend_string* result = {{.Name}}_wrapper(intern->go_handle{{range .Params}}, {{template "methodCallArg" .}}{{end}}); + zend_string* result = {{.ClassName}}_{{.Name}}_wrapper(intern->go_handle{{range .Params}}, {{template "methodCallArg" .}}{{end}}); if (result) { RETURN_STR(result); } RETURN_EMPTY_STRING(); {{- else if eq .ReturnType "int"}} - zend_long result = {{.Name}}_wrapper(intern->go_handle{{range .Params}}, {{template "methodCallArg" .}}{{end}}); + zend_long result = {{.ClassName}}_{{.Name}}_wrapper(intern->go_handle{{range .Params}}, {{template "methodCallArg" .}}{{end}}); RETURN_LONG(result); {{- else if eq .ReturnType "float"}} - double result = {{.Name}}_wrapper(intern->go_handle{{range .Params}}, {{template "methodCallArg" .}}{{end}}); + double result = {{.ClassName}}_{{.Name}}_wrapper(intern->go_handle{{range .Params}}, {{template "methodCallArg" .}}{{end}}); RETURN_DOUBLE(result); {{- else if eq .ReturnType "bool"}} - int result = {{.Name}}_wrapper(intern->go_handle{{range .Params}}, {{template "methodCallArg" .}}{{end}}); + int result = {{.ClassName}}_{{.Name}}_wrapper(intern->go_handle{{range .Params}}, {{template "methodCallArg" .}}{{end}}); RETURN_BOOL(result); {{- else if eq .ReturnType "array"}} - void* result = {{.Name}}_wrapper(intern->go_handle{{range .Params}}, {{template "methodCallArg" .}}{{end}}); + void* result = {{.ClassName}}_{{.Name}}_wrapper(intern->go_handle{{range .Params}}, {{template "methodCallArg" .}}{{end}}); if (result != NULL) { HashTable *ht = (HashTable*)result; RETURN_ARR(ht); } else { RETURN_NULL(); } + {{- else if eq .ReturnType "mixed"}} + zval* result = {{.ClassName}}_{{.Name}}_wrapper(intern->go_handle{{range .Params}}, {{template "methodCallArg" .}}{{end}}); + if (result != NULL) { + RETURN_COPY_VALUE(result); + } else { + RETURN_NULL(); + } {{- end}} {{- else}} - {{.Name}}_wrapper(intern->go_handle{{range .Params}}, {{template "methodCallArg" .}}{{end}}); + {{.ClassName}}_{{.Name}}_wrapper(intern->go_handle{{range .Params}}, {{template "methodCallArg" .}}{{end}}); {{- end}} } {{end}}{{end}} diff --git a/internal/extgen/templates/extension.go.tpl b/internal/extgen/templates/extension.go.tpl index 313ec46f3e..d5d8889fc8 100644 --- a/internal/extgen/templates/extension.go.tpl +++ b/internal/extgen/templates/extension.go.tpl @@ -58,23 +58,19 @@ func create_{{.GoStruct}}_object() C.uintptr_t { } {{- 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}} { +//export {{.ClassName}}_{{.Name}}_wrapper +func {{.ClassName}}_{{.Name}}_wrapper(handle C.uintptr_t{{range .Params}}, {{.Name}} {{goParamType .PhpType .IsNullable}}{{end}}){{if not (isVoid .ReturnType)}} {{goReturnType .ReturnType}}{{end}} { obj := getGoObject(handle) if obj == nil { -{{- if not (isVoid .ReturnType)}} -{{- if isStringOrArray .ReturnType}} - return nil +{{- if isVoid .ReturnType}} + return {{- else}} - var zero {{phpTypeToGoType .ReturnType}} + var zero {{goReturnType .ReturnType}} return zero -{{- end}} -{{- else}} - return {{- end}} } structObj := obj.(*{{$class.GoStruct}}) - {{if not (isVoid .ReturnType)}}return {{end}}structObj.{{.Name | title}}({{range $i, $param := .Params}}{{if $i}}, {{end}}{{$param.Name}}{{end}}) + {{if not (isVoid .ReturnType)}}return {{end}}structObj.{{extractGoFunctionName .GoFunction}}({{range $i, $param := .Params}}{{if $i}}, {{end}}{{$param.Name}}{{end}}) } {{end}} {{- end}} diff --git a/internal/extgen/validator.go b/internal/extgen/validator.go index f9b59763de..8c496a78d1 100644 --- a/internal/extgen/validator.go +++ b/internal/extgen/validator.go @@ -142,45 +142,26 @@ func (v *Validator) validateGoFunctionSignatureWithOptions(phpFunc phpFunction, return fmt.Errorf("no function declaration found in Go function") } - goParamCount := 0 - if goFunc.Type.Params != nil { - goParamCount = len(goFunc.Type.Params.List) - } + goParamTypes := flattenParamTypes(goFunc.Type.Params) - hasReceiver := goFunc.Recv != nil && len(goFunc.Recv.List) > 0 paramOffset := 0 - effectiveGoParamCount := goParamCount - - if hasReceiver { - paramOffset = 0 - effectiveGoParamCount = goParamCount - } else if isMethod && goParamCount > 0 { + hasReceiver := goFunc.Recv != nil && len(goFunc.Recv.List) > 0 + if !hasReceiver && isMethod && len(goParamTypes) > 0 { // this is a method-like function, first parameter should be the struct paramOffset = 1 - effectiveGoParamCount = goParamCount - 1 } - expectedGoParams := len(phpFunc.Params) - - if expectedGoParams != effectiveGoParamCount { - return fmt.Errorf("parameter count mismatch: PHP function has %d parameters (expecting %d Go parameters) but Go function has %d", len(phpFunc.Params), expectedGoParams, effectiveGoParamCount) + effectiveGoParamCount := len(goParamTypes) - paramOffset + if len(phpFunc.Params) != effectiveGoParamCount { + return fmt.Errorf("parameter count mismatch: PHP function has %d parameters but Go function has %d", len(phpFunc.Params), effectiveGoParamCount) } - if goFunc.Type.Params != nil && len(phpFunc.Params) > 0 { - for i, phpParam := range phpFunc.Params { - goParamIndex := i + paramOffset - - if goParamIndex >= len(goFunc.Type.Params.List) { - break - } + for i, phpParam := range phpFunc.Params { + expectedGoType := v.phpTypeToGoType(phpParam.PhpType, phpParam.IsNullable) + actualGoType := v.goTypeToString(goParamTypes[i+paramOffset]) - goParam := goFunc.Type.Params.List[goParamIndex] - expectedGoType := v.phpTypeToGoType(phpParam.PhpType, phpParam.IsNullable) - actualGoType := v.goTypeToString(goParam.Type) - - if !v.isCompatibleGoType(expectedGoType, actualGoType) { - return fmt.Errorf("parameter %d type mismatch: PHP %q requires Go type %q but found %q", i+1, phpParam.PhpType, expectedGoType, actualGoType) - } + if !v.isCompatibleGoType(expectedGoType, actualGoType) { + return fmt.Errorf("parameter %d type mismatch: PHP %q requires Go type %q but found %q", i+1, phpParam.PhpType, expectedGoType, actualGoType) } } @@ -215,7 +196,9 @@ func (v *Validator) phpTypeToGoType(t phpType, isNullable bool) string { baseType = "any" } - if isNullable && t != phpString && t != phpArray && t != phpCallable { + // Types already passed as a pointer carry PHP null in the pointer itself, + // so they never gain an extra level of indirection when nullable. + if isNullable && t != phpString && t != phpArray && t != phpMixed && t != phpCallable { return "*" + baseType } @@ -250,7 +233,7 @@ func (v *Validator) phpReturnTypeToGoType(phpReturnType phpType) string { return "float64" case phpBool: return "bool" - case phpArray: + case phpArray, phpMixed: return "unsafe.Pointer" default: return "any" diff --git a/internal/extgen/validator_test.go b/internal/extgen/validator_test.go index 7be0f953e4..8bea76821a 100644 --- a/internal/extgen/validator_test.go +++ b/internal/extgen/validator_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestValidateFunction(t *testing.T) { @@ -686,7 +687,7 @@ func TestValidateGoFunctionSignature(t *testing.T) { }`, }, expectError: true, - errorMsg: "parameter count mismatch: PHP function has 2 parameters (expecting 2 Go parameters) but Go function has 1", + errorMsg: "parameter count mismatch: PHP function has 2 parameters but Go function has 1", }, { name: "parameter type mismatch", @@ -900,6 +901,7 @@ func TestPhpReturnTypeToGoType(t *testing.T) { {"bool", "bool"}, {"array", "unsafe.Pointer"}, {"array", "unsafe.Pointer"}, + {"mixed", "unsafe.Pointer"}, {"unknown", "any"}, } @@ -937,3 +939,89 @@ func TestIsCompatibleGoType(t *testing.T) { }) } } + +func TestValidateGoFunctionSignature_GroupedParameters(t *testing.T) { + validator := Validator{} + + tests := []struct { + name string + phpFunc phpFunction + wantErrMsg string + }{ + { + name: "grouped parameters sharing a type are counted individually", + phpFunc: phpFunction{ + Name: "add", + ReturnType: phpInt, + Params: []phpParameter{ + {Name: "a", PhpType: phpInt}, + {Name: "b", PhpType: phpInt}, + }, + GoFunction: "func add(a, b int64) int64 { return a + b }", + }, + }, + { + name: "grouped parameters of the wrong type are still rejected", + phpFunc: phpFunction{ + Name: "concat", + ReturnType: phpInt, + Params: []phpParameter{ + {Name: "a", PhpType: phpInt}, + {Name: "b", PhpType: phpInt}, + }, + GoFunction: "func concat(a, b string) int64 { return 0 }", + }, + wantErrMsg: `parameter 1 type mismatch`, + }, + { + name: "grouped parameters declaring too many arguments are rejected", + phpFunc: phpFunction{ + Name: "add", + ReturnType: phpInt, + Params: []phpParameter{ + {Name: "a", PhpType: phpInt}, + }, + GoFunction: "func add(a, b int64) int64 { return a + b }", + }, + wantErrMsg: "parameter count mismatch: PHP function has 1 parameters but Go function has 2", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validator.validateGoFunctionSignatureWithOptions(tt.phpFunc, false) + if tt.wantErrMsg == "" { + require.NoError(t, err) + + return + } + + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErrMsg) + }) + } +} + +func TestPhpTypeToGoType_NullablePointerTypes(t *testing.T) { + // Types PHP already hands over as a pointer carry null in the pointer itself, + // so they must not gain a second level of indirection when nullable. + tests := []struct { + phpType phpType + expected string + }{ + {phpString, "*C.zend_string"}, + {phpArray, "*C.zend_array"}, + {phpMixed, "*C.zval"}, + {phpCallable, "*C.zval"}, + {phpInt, "*int64"}, + {phpFloat, "*float64"}, + {phpBool, "*bool"}, + } + + validator := Validator{} + for _, tt := range tests { + t.Run(string(tt.phpType), func(t *testing.T) { + assert.Equal(t, tt.expected, validator.phpTypeToGoType(tt.phpType, true)) + }) + } +}