Skip to content

Commit 7103de9

Browse files
alexandre-dauboisalexandre-daubois
committed
fix(extgen): extract Go function bodies via go/ast instead of brace counting
1 parent af07c17 commit 7103de9

4 files changed

Lines changed: 182 additions & 170 deletions

File tree

‎internal/extgen/astutil.go‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package extgen
2+
3+
import (
4+
"fmt"
5+
"go/ast"
6+
"go/token"
7+
"regexp"
8+
"strings"
9+
)
10+
11+
// findDirective searches a comment group for a line matching re and returns the
12+
// first capture group (typically the directive payload) along with the comment's
13+
// source line number. Returns "" when no comment matches.
14+
func findDirective(group *ast.CommentGroup, fset *token.FileSet, re *regexp.Regexp) (string, int) {
15+
if group == nil {
16+
return "", 0
17+
}
18+
for _, comment := range group.List {
19+
if matches := re.FindStringSubmatch(comment.Text); matches != nil {
20+
return strings.TrimSpace(matches[1]), fset.Position(comment.Pos()).Line
21+
}
22+
}
23+
return "", 0
24+
}
25+
26+
// extractNodeSource returns the verbatim source text covered by node in src.
27+
func extractNodeSource(src []byte, fset *token.FileSet, node ast.Node) string {
28+
start := fset.Position(node.Pos()).Offset
29+
end := fset.Position(node.End()).Offset
30+
if start < 0 || end > len(src) || start > end {
31+
return ""
32+
}
33+
return string(src[start:end])
34+
}
35+
36+
// checkOrphanDirectives returns an error for every comment that matches re but
37+
// whose source line was not consumed by a declaration.
38+
func checkOrphanDirectives(file *ast.File, fset *token.FileSet, re *regexp.Regexp, consumed map[int]bool, directiveLabel string) error {
39+
for _, group := range file.Comments {
40+
for _, comment := range group.List {
41+
if !re.MatchString(comment.Text) {
42+
continue
43+
}
44+
line := fset.Position(comment.Pos()).Line
45+
if !consumed[line] {
46+
return fmt.Errorf("%s directive at line %d is not followed by a function declaration", directiveLabel, line)
47+
}
48+
}
49+
}
50+
return nil
51+
}

‎internal/extgen/classparser.go‎

Lines changed: 64 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package extgen
22

33
import (
4-
"bufio"
54
"fmt"
65
"go/ast"
76
"go/parser"
@@ -204,91 +203,88 @@ func (cp *classParser) goTypeToPHPType(goType string) phpType {
204203
return phpMixed
205204
}
206205

207-
func (cp *classParser) parseMethods(filename string) (methods []phpClassMethod, err error) {
208-
file, err := os.Open(filename)
206+
func (cp *classParser) parseMethods(filename string) ([]phpClassMethod, error) {
207+
src, err := os.ReadFile(filename)
209208
if err != nil {
210209
return nil, err
211210
}
212211

213-
defer func() {
214-
e := file.Close()
215-
if err != nil {
216-
err = e
217-
}
218-
}()
219-
220-
scanner := bufio.NewScanner(file)
221-
var currentMethod *phpClassMethod
212+
fset := token.NewFileSet()
213+
file, err := parser.ParseFile(fset, filename, src, parser.ParseComments)
214+
if err != nil {
215+
return nil, fmt.Errorf("parsing file: %w", err)
216+
}
222217

223-
lineNumber := 0
224-
for scanner.Scan() {
225-
lineNumber++
226-
line := strings.TrimSpace(scanner.Text())
218+
validator := Validator{}
219+
var methods []phpClassMethod
220+
consumed := make(map[int]bool)
227221

228-
if matches := phpMethodRegex.FindStringSubmatch(line); matches != nil {
229-
className := strings.TrimSpace(matches[1])
230-
signature := strings.TrimSpace(matches[2])
222+
for _, decl := range file.Decls {
223+
funcDecl, ok := decl.(*ast.FuncDecl)
224+
if !ok {
225+
continue
226+
}
231227

232-
method, err := cp.parseMethodSignature(className, signature)
233-
if err != nil {
234-
fmt.Printf("Warning: Error parsing method signature %q: %v\n", signature, err)
228+
directive, directiveLine := findDirective(funcDecl.Doc, fset, phpMethodRegex)
229+
if directive == "" {
230+
continue
231+
}
232+
rawMatch := phpMethodRegex.FindStringSubmatch(findMatchingComment(funcDecl.Doc, phpMethodRegex))
233+
if len(rawMatch) != 3 {
234+
continue
235+
}
236+
className := strings.TrimSpace(rawMatch[1])
237+
signature := strings.TrimSpace(rawMatch[2])
238+
consumed[directiveLine] = true
235239

236-
continue
237-
}
240+
method, err := cp.parseMethodSignature(className, signature)
241+
if err != nil {
242+
fmt.Printf("Warning: Error parsing method signature %q: %v\n", signature, err)
243+
continue
244+
}
238245

239-
validator := Validator{}
240-
phpFunc := phpFunction{
241-
Name: method.Name,
242-
Signature: method.Signature,
243-
Params: method.Params,
244-
ReturnType: method.ReturnType,
245-
IsReturnNullable: method.isReturnNullable,
246-
}
246+
phpFunc := phpFunction{
247+
Name: method.Name,
248+
Signature: method.Signature,
249+
Params: method.Params,
250+
ReturnType: method.ReturnType,
251+
IsReturnNullable: method.isReturnNullable,
252+
}
253+
if err := validator.validateTypes(phpFunc); err != nil {
254+
fmt.Printf("Warning: Method \"%s::%s\" uses unsupported types: %v\n", className, method.Name, err)
255+
continue
256+
}
247257

248-
if err := validator.validateTypes(phpFunc); err != nil {
249-
fmt.Printf("Warning: Method \"%s::%s\" uses unsupported types: %v\n", className, method.Name, err)
258+
method.lineNumber = directiveLine
259+
method.GoFunction = extractNodeSource(src, fset, funcDecl)
250260

251-
continue
252-
}
253-
254-
method.lineNumber = lineNumber
255-
currentMethod = method
261+
phpFunc.GoFunction = method.GoFunction
262+
if err := validator.validateGoFunctionSignatureWithOptions(phpFunc, true); err != nil {
263+
fmt.Printf("Warning: Go method signature mismatch for '%s::%s': %v\n", method.ClassName, method.Name, err)
264+
continue
256265
}
257266

258-
if currentMethod != nil && strings.HasPrefix(line, "func ") {
259-
goFunc, err := cp.extractGoMethodFunction(scanner, line)
260-
if err != nil {
261-
return nil, fmt.Errorf("extracting Go method function: %w", err)
262-
}
263-
264-
currentMethod.GoFunction = goFunc
267+
methods = append(methods, *method)
268+
}
265269

266-
validator := Validator{}
267-
phpFunc := phpFunction{
268-
Name: currentMethod.Name,
269-
Signature: currentMethod.Signature,
270-
GoFunction: currentMethod.GoFunction,
271-
Params: currentMethod.Params,
272-
ReturnType: currentMethod.ReturnType,
273-
IsReturnNullable: currentMethod.isReturnNullable,
274-
}
270+
if err := checkOrphanDirectives(file, fset, phpMethodRegex, consumed, "//export_php:method"); err != nil {
271+
return nil, err
272+
}
275273

276-
if err := validator.validateGoFunctionSignatureWithOptions(phpFunc, true); err != nil {
277-
fmt.Printf("Warning: Go method signature mismatch for '%s::%s': %v\n", currentMethod.ClassName, currentMethod.Name, err)
278-
currentMethod = nil
279-
continue
280-
}
274+
return methods, nil
275+
}
281276

282-
methods = append(methods, *currentMethod)
283-
currentMethod = nil
284-
}
277+
// findMatchingComment returns the raw comment text whose line matches re.
278+
func findMatchingComment(group *ast.CommentGroup, re *regexp.Regexp) string {
279+
if group == nil {
280+
return ""
285281
}
286-
287-
if currentMethod != nil {
288-
return nil, fmt.Errorf("//export_php:method directive at line %d is not followed by a function declaration", currentMethod.lineNumber)
282+
for _, comment := range group.List {
283+
if re.MatchString(comment.Text) {
284+
return comment.Text
285+
}
289286
}
290-
291-
return methods, scanner.Err()
287+
return ""
292288
}
293289

294290
func (cp *classParser) parseMethodSignature(className, signature string) (*phpClassMethod, error) {
@@ -365,27 +361,3 @@ func (cp *classParser) sanitizeDefaultValue(value string) string {
365361
return strings.Trim(value, `'"`)
366362
}
367363

368-
func (cp *classParser) extractGoMethodFunction(scanner *bufio.Scanner, firstLine string) (string, error) {
369-
goFunc := firstLine + "\n"
370-
braceCount := 1
371-
372-
for scanner.Scan() {
373-
line := scanner.Text()
374-
goFunc += line + "\n"
375-
376-
for _, char := range line {
377-
switch char {
378-
case '{':
379-
braceCount++
380-
case '}':
381-
braceCount--
382-
}
383-
}
384-
385-
if braceCount == 0 {
386-
break
387-
}
388-
}
389-
390-
return goFunc, nil
391-
}

‎internal/extgen/funcparser.go‎

Lines changed: 44 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package extgen
22

33
import (
4-
"bufio"
54
"fmt"
5+
"go/ast"
6+
"go/parser"
7+
"go/token"
68
"os"
79
"regexp"
810
"strings"
@@ -14,102 +16,66 @@ var typeNameRegex = regexp.MustCompile(`(\??[\w|]+)\s+\$?(\w+)`)
1416

1517
type FuncParser struct{}
1618

17-
func (fp *FuncParser) parse(filename string) (functions []phpFunction, err error) {
18-
file, err := os.Open(filename)
19+
func (fp *FuncParser) parse(filename string) ([]phpFunction, error) {
20+
src, err := os.ReadFile(filename)
1921
if err != nil {
2022
return nil, err
2123
}
22-
defer func() {
23-
e := file.Close()
24-
if err == nil {
25-
err = e
26-
}
27-
}()
28-
29-
scanner := bufio.NewScanner(file)
30-
var currentPHPFunc *phpFunction
31-
validator := Validator{}
32-
33-
lineNumber := 0
34-
for scanner.Scan() {
35-
lineNumber++
36-
line := strings.TrimSpace(scanner.Text())
37-
38-
if matches := phpFuncRegex.FindStringSubmatch(line); matches != nil {
39-
signature := strings.TrimSpace(matches[1])
40-
phpFunc, err := fp.parseSignature(signature)
41-
if err != nil {
42-
fmt.Printf("Warning: Error parsing signature '%s': %v\n", signature, err)
43-
44-
continue
45-
}
4624

47-
if err := validator.validateFunction(*phpFunc); err != nil {
48-
fmt.Printf("Warning: Invalid function '%s': %v\n", phpFunc.Name, err)
49-
50-
continue
51-
}
25+
fset := token.NewFileSet()
26+
file, err := parser.ParseFile(fset, filename, src, parser.ParseComments)
27+
if err != nil {
28+
return nil, fmt.Errorf("parsing file: %w", err)
29+
}
5230

53-
if err := validator.validateTypes(*phpFunc); err != nil {
54-
fmt.Printf("Warning: Function '%s' uses unsupported types: %v\n", phpFunc.Name, err)
31+
validator := Validator{}
32+
var functions []phpFunction
33+
consumed := make(map[int]bool)
5534

56-
continue
57-
}
35+
for _, decl := range file.Decls {
36+
funcDecl, ok := decl.(*ast.FuncDecl)
37+
if !ok || funcDecl.Recv != nil {
38+
continue
39+
}
5840

59-
phpFunc.lineNumber = lineNumber
60-
currentPHPFunc = phpFunc
41+
directive, directiveLine := findDirective(funcDecl.Doc, fset, phpFuncRegex)
42+
if directive == "" {
43+
continue
6144
}
45+
consumed[directiveLine] = true
6246

63-
if currentPHPFunc != nil && strings.HasPrefix(line, "func ") {
64-
goFunc, err := fp.extractGoFunction(scanner, line)
65-
if err != nil {
66-
return nil, fmt.Errorf("extracting Go function: %w", err)
67-
}
47+
phpFunc, err := fp.parseSignature(directive)
48+
if err != nil {
49+
fmt.Printf("Warning: Error parsing signature '%s': %v\n", directive, err)
50+
continue
51+
}
6852

69-
currentPHPFunc.GoFunction = goFunc
53+
if err := validator.validateFunction(*phpFunc); err != nil {
54+
fmt.Printf("Warning: Invalid function '%s': %v\n", phpFunc.Name, err)
55+
continue
56+
}
7057

71-
if err := validator.validateGoFunctionSignatureWithOptions(*currentPHPFunc, false); err != nil {
72-
fmt.Printf("Warning: Go function signature mismatch for %q: %v\n", currentPHPFunc.Name, err)
73-
currentPHPFunc = nil
58+
if err := validator.validateTypes(*phpFunc); err != nil {
59+
fmt.Printf("Warning: Function '%s' uses unsupported types: %v\n", phpFunc.Name, err)
60+
continue
61+
}
7462

75-
continue
76-
}
63+
phpFunc.lineNumber = directiveLine
64+
phpFunc.GoFunction = extractNodeSource(src, fset, funcDecl)
7765

78-
functions = append(functions, *currentPHPFunc)
79-
currentPHPFunc = nil
66+
if err := validator.validateGoFunctionSignatureWithOptions(*phpFunc, false); err != nil {
67+
fmt.Printf("Warning: Go function signature mismatch for %q: %v\n", phpFunc.Name, err)
68+
continue
8069
}
81-
}
8270

83-
if currentPHPFunc != nil {
84-
return nil, fmt.Errorf("//export_php function directive at line %d is not followed by a function declaration", currentPHPFunc.lineNumber)
71+
functions = append(functions, *phpFunc)
8572
}
8673

87-
return functions, scanner.Err()
88-
}
89-
90-
func (fp *FuncParser) extractGoFunction(scanner *bufio.Scanner, firstLine string) (string, error) {
91-
goFunc := firstLine + "\n"
92-
braceCount := 1
93-
94-
for scanner.Scan() {
95-
line := scanner.Text()
96-
goFunc += line + "\n"
97-
98-
for _, char := range line {
99-
switch char {
100-
case '{':
101-
braceCount++
102-
case '}':
103-
braceCount--
104-
}
105-
}
106-
107-
if braceCount == 0 {
108-
break
109-
}
74+
if err := checkOrphanDirectives(file, fset, phpFuncRegex, consumed, "//export_php function"); err != nil {
75+
return nil, err
11076
}
11177

112-
return goFunc, nil
78+
return functions, nil
11379
}
11480

11581
func (fp *FuncParser) parseSignature(signature string) (*phpFunction, error) {

0 commit comments

Comments
 (0)