Skip to content

Commit 7cd7983

Browse files
committed
only import runtime/cgo when it needs to
Signed-off-by: Robert Landers <landers.robert@gmail.com>
1 parent 94b01dc commit 7cd7983

File tree

2 files changed

+161
-0
lines changed

2 files changed

+161
-0
lines changed

internal/extgen/gofile_test.go

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -732,3 +732,162 @@ func testGoFileInternalFunctions(t *testing.T, content string) {
732732
t.Log("No internal functions found (this may be expected)")
733733
}
734734
}
735+
736+
func TestGoFileGenerator_RuntimeCgoImportGating(t *testing.T) {
737+
tests := []struct {
738+
name string
739+
baseName string
740+
sourceFile string
741+
functions []phpFunction
742+
classes []phpClass
743+
expectCgoImport bool
744+
description string
745+
}{
746+
{
747+
name: "extension without classes should not include runtime/cgo import",
748+
baseName: "no_classes",
749+
sourceFile: createTempSourceFile(t, `package main
750+
751+
//export_php: simpleFunc(): void
752+
func simpleFunc() {
753+
// simple function
754+
}`),
755+
functions: []phpFunction{
756+
{
757+
Name: "simpleFunc",
758+
ReturnType: phpVoid,
759+
GoFunction: "func simpleFunc() {\n\t// simple function\n}",
760+
},
761+
},
762+
classes: nil,
763+
expectCgoImport: false,
764+
description: "Extensions with only functions should not import runtime/cgo",
765+
},
766+
{
767+
name: "extension with classes should include runtime/cgo import",
768+
baseName: "with_classes",
769+
sourceFile: createTempSourceFile(t, `package main
770+
771+
//export_php:class TestClass
772+
type TestStruct struct {
773+
name string
774+
}
775+
776+
//export_php:method TestClass::getName(): string
777+
func (ts *TestStruct) GetName() string {
778+
return ts.name
779+
}`),
780+
functions: nil,
781+
classes: []phpClass{
782+
{
783+
Name: "TestClass",
784+
GoStruct: "TestStruct",
785+
Methods: []phpClassMethod{
786+
{
787+
Name: "GetName",
788+
PhpName: "getName",
789+
ClassName: "TestClass",
790+
Signature: "getName(): string",
791+
ReturnType: phpString,
792+
Params: []phpParameter{},
793+
GoFunction: "func (ts *TestStruct) GetName() string {\n\treturn ts.name\n}",
794+
},
795+
},
796+
},
797+
},
798+
expectCgoImport: true,
799+
description: "Extensions with classes should import runtime/cgo for handle management",
800+
},
801+
{
802+
name: "extension with functions and classes should include runtime/cgo import",
803+
baseName: "mixed",
804+
sourceFile: createTempSourceFile(t, `package main
805+
806+
//export_php: utilFunc(): string
807+
func utilFunc() string {
808+
return "utility"
809+
}
810+
811+
//export_php:class MixedClass
812+
type MixedStruct struct {
813+
value int
814+
}
815+
816+
//export_php:method MixedClass::getValue(): int
817+
func (ms *MixedStruct) GetValue() int {
818+
return ms.value
819+
}`),
820+
functions: []phpFunction{
821+
{
822+
Name: "utilFunc",
823+
ReturnType: phpString,
824+
GoFunction: "func utilFunc() string {\n\treturn \"utility\"\n}",
825+
},
826+
},
827+
classes: []phpClass{
828+
{
829+
Name: "MixedClass",
830+
GoStruct: "MixedStruct",
831+
Methods: []phpClassMethod{
832+
{
833+
Name: "GetValue",
834+
PhpName: "getValue",
835+
ClassName: "MixedClass",
836+
Signature: "getValue(): int",
837+
ReturnType: phpInt,
838+
Params: []phpParameter{},
839+
GoFunction: "func (ms *MixedStruct) GetValue() int {\n\treturn ms.value\n}",
840+
},
841+
},
842+
},
843+
},
844+
expectCgoImport: true,
845+
description: "Extensions with both functions and classes should import runtime/cgo",
846+
},
847+
{
848+
name: "empty extension should not include runtime/cgo import",
849+
baseName: "empty",
850+
sourceFile: createTempSourceFile(t, `package main
851+
852+
// Empty extension for testing
853+
`),
854+
functions: nil,
855+
classes: nil,
856+
expectCgoImport: false,
857+
description: "Empty extensions should not import runtime/cgo",
858+
},
859+
}
860+
861+
for _, tt := range tests {
862+
t.Run(tt.name, func(t *testing.T) {
863+
generator := &Generator{
864+
BaseName: tt.baseName,
865+
SourceFile: tt.sourceFile,
866+
Functions: tt.functions,
867+
Classes: tt.classes,
868+
}
869+
870+
goGen := GoFileGenerator{generator}
871+
content, err := goGen.buildContent()
872+
require.NoError(t, err)
873+
874+
cgoImportPresent := strings.Contains(content, `import "runtime/cgo"`)
875+
876+
if tt.expectCgoImport {
877+
assert.True(t, cgoImportPresent, "Extension should import runtime/cgo: %s", tt.description)
878+
// Verify that cgo functions are also present when import is included
879+
assert.Contains(t, content, "cgo.NewHandle", "Should contain cgo.NewHandle usage when runtime/cgo is imported")
880+
assert.Contains(t, content, "cgo.Handle", "Should contain cgo.Handle usage when runtime/cgo is imported")
881+
} else {
882+
assert.False(t, cgoImportPresent, "Extension should not import runtime/cgo: %s", tt.description)
883+
// Verify that cgo functions are not present when import is not included
884+
assert.NotContains(t, content, "cgo.NewHandle", "Should not contain cgo.NewHandle usage when runtime/cgo is not imported")
885+
assert.NotContains(t, content, "cgo.Handle", "Should not contain cgo.Handle usage when runtime/cgo is not imported")
886+
}
887+
888+
// Ensure exactly one C import is always present
889+
cImportCount := strings.Count(content, `import "C"`)
890+
assert.Equal(t, 1, cImportCount, "Should have exactly one C import")
891+
})
892+
}
893+
}

internal/extgen/templates/extension.go.tpl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ package {{.PackageName}}
55
#include "{{.BaseName}}.h"
66
*/
77
import "C"
8+
{{- if .Classes}}
89
import "runtime/cgo"
10+
{{- end}}
911
{{- range .Imports}}
1012
import {{.}}
1113
{{- end}}

0 commit comments

Comments
 (0)