pandalee99 commented on code in PR #2553:
URL: https://github.com/apache/fory/pull/2553#discussion_r2318191130
##########
go/fory/go.mod:
##########
@@ -22,5 +22,6 @@ go 1.13
require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/spaolacci/murmur3 v1.1.0
- github.com/stretchr/testify v1.7.0
+ github.com/stretchr/testify v1.6.1
Review Comment:
It seems that lowering the version makes no sense
##########
go/fory/codegen/forygen.go:
##########
@@ -0,0 +1,825 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package main
+
+import (
+ "bytes"
+ "crypto/md5"
+ "encoding/binary"
+ "flag"
+ "fmt"
+ "go/format"
+ "go/types"
+ "io/ioutil"
+ "log"
+ "path/filepath"
+ "sort"
+ "strings"
+ "time"
+ "unicode"
+
+ "golang.org/x/tools/go/packages"
+)
+
+var (
+ typeFlag = flag.String("type", "", "comma-separated list of types to
generate code for")
+ pkgFlag = flag.String("pkg", ".", "package directory to search for
types")
+)
+
+// StructInfo contains metadata about a struct to generate code for
+type StructInfo struct {
+ Name string
+ Fields []*FieldInfo
+}
+
+// FieldInfo contains metadata about a struct field
+type FieldInfo struct {
+ GoName string // Original Go field name
+ SnakeName string // snake_case field name for sorting
+ Type types.Type // Go type information
+ Index int // Original field index in struct
+ IsPrimitive bool // Whether it's a Fory primitive type
+ IsPointer bool // Whether it's a pointer type
+ TypeID string // Fory TypeID for sorting
+ PrimitiveSize int // Size for primitive type sorting
+}
+
+func main() {
+ flag.Parse()
+
+ if err := run(); err != nil {
+ log.Fatalf("forygen failed: %v", err)
+ }
+}
+
+func run() error {
+ // Load packages
+ cfg := &packages.Config{
+ Mode: packages.NeedTypes | packages.NeedSyntax |
packages.NeedName | packages.NeedFiles | packages.NeedTypesInfo,
+ }
+
+ pkgs, err := packages.Load(cfg, *pkgFlag)
+ if err != nil {
+ return fmt.Errorf("loading packages: %w", err)
+ }
+
+ if len(pkgs) == 0 {
+ return fmt.Errorf("no packages found")
+ }
+
+ if packages.PrintErrors(pkgs) > 0 {
+ return fmt.Errorf("errors in packages")
+ }
+
+ // Process each package
+ for _, pkg := range pkgs {
+ if err := processPackage(pkg); err != nil {
+ return fmt.Errorf("processing package %s: %w",
pkg.PkgPath, err)
+ }
+ }
+
+ return nil
+}
+
+func processPackage(pkg *packages.Package) error {
+ // Find structs to generate code for
+ var targetTypes []string
+ if *typeFlag != "" {
+ targetTypes = strings.Split(*typeFlag, ",")
+ }
+
+ var structs []*StructInfo
+
+ // Check if package has types
+ if pkg.Types == nil {
+ return fmt.Errorf("package %s has no type information",
pkg.PkgPath)
+ }
+
+ // Iterate through all types in the package
+ scope := pkg.Types.Scope()
+ allNames := scope.Names()
+
+ // Also check if there are any compilation errors
+ if len(pkg.Errors) > 0 {
+ for _, err := range pkg.Errors {
+ log.Printf("package error: %s", err)
+ }
+ }
+
+ for _, name := range allNames {
+ obj := scope.Lookup(name)
+ if obj == nil {
+ continue
+ }
+
+ // Check if it's a named type
+ named, ok := obj.Type().(*types.Named)
+ if !ok {
+ continue
+ }
+
+ // Check if underlying type is struct
+ structType, ok := named.Underlying().(*types.Struct)
+ if !ok {
+ continue
+ }
+
+ // Check if we should generate code for this type
+ shouldGenerate := false
+ if len(targetTypes) > 0 {
+ for _, t := range targetTypes {
+ if strings.TrimSpace(t) == name {
+ shouldGenerate = true
+ break
+ }
+ }
+ } else {
+ // TODO: Check for fory:gen comment
+ shouldGenerate = false
+ }
+
+ if !shouldGenerate {
+ continue
+ }
+
+ // Extract struct information
+ structInfo, err := extractStructInfo(name, structType)
+ if err != nil {
+ return fmt.Errorf("extracting struct info for %s: %w",
name, err)
+ }
+
+ structs = append(structs, structInfo)
+ }
+
+ if len(structs) == 0 {
+ if len(targetTypes) > 0 {
+ log.Printf("Warning: No matching structs found for
target types: %v", targetTypes)
+ log.Printf("Available types in package: %v", allNames)
Review Comment:
Why was "log" only used in this file, and why is it not a good habit to mix
"log" and "fmt".
##########
go/fory/codegen/forygen.go:
##########
@@ -0,0 +1,825 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package main
Review Comment:
I still think this name is inappropriate
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
