Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing package name on type from common API spec when referencing a path operation from another spec in generated code #1378

Closed
markuswustenberg opened this issue Dec 7, 2023 · 2 comments · Fixed by #1389

Comments

@markuswustenberg
Copy link

We're hitting a bug where generated code doesn't compile when using API specs referencing each other. This may or may not be related to #695, so I'm opening a separate issue just to be sure.

The setup is this (also available at https://github.com/maragudk/oapi-codegen-bug):

  • An api.yaml spec references a path in a common.yaml spec.
  • In common.yaml, a path /ping references a schema PingResponse in the same file.

When the spec is generated for api.yaml, PingResponse doesn't have the externalRef0 package name prefix, and so the generated code can't compile.

This is with v2 of oapi-codegen.

api.yaml

version: "3.0.0"

paths:
  /ping:
    $ref: "common.yaml#/paths/~1ping"

config.yaml

package: api
generate:
  gin-server: true
  embedded-spec: true
  strict-server: true
  models: true
import-mapping:
  common.yaml: github.com/maragudk/oapi-codegen-bug/oapi/common
output: api.gen.go

common.yaml

version: "3.0.0"

paths:
  /ping:
    get:
      responses:
        200:
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/PingResponse"

components:
  schemas:
    PingResponse:
      type: object
      properties:
        message:
          type: string

config.yaml

package: common
generate:
  gin-server: true
  embedded-spec: true
  strict-server: true
  models: true
output: common.gen.go
@markuswustenberg
Copy link
Author

For anyone in the same situation, I made a small script to parse the generated code, fix the type imports, and write the code back out:

package main

import (
	"encoding/json"
	"fmt"
	"go/ast"
	"go/parser"
	"go/printer"
	"go/token"
	"os"
)

func main() {
	if err := start(); err != nil {
		fmt.Println("Error: ", err)
		os.Exit(1)
	}
}

func start() error {
	// Check args
	if len(os.Args) < 2 {
		return fmt.Errorf("no files to fix provided")
	}

	// Read config from JSON file
	config, err := readConfig()
	if err != nil {
		return fmt.Errorf("cannot read config: %w", err)
	}

	for _, file := range os.Args[1:] {
		if err := fixFile(file, config); err != nil {
			return fmt.Errorf("cannot fix file: %w", err)
		}
	}

	return nil
}

// fixFile by loading the file into an AST, and replacing all type names if in the config.
func fixFile(file string, c config) error {
	// Load file into AST
	fset := token.NewFileSet()
	f, err := parser.ParseFile(fset, file, nil, parser.ParseComments)
	if err != nil {
		return fmt.Errorf("cannot parse file: %w", err)
	}

	// Replace all type names
	ast.Inspect(f, func(n ast.Node) bool {
		switch x := n.(type) {

		case *ast.Field: // Struct field
			switch fieldType := x.Type.(type) {
			case *ast.Ident:
				for _, r := range c.Replacements {
					if fieldType.Name == r.Search {
						fmt.Println("Replacing", r.Search, "with", r.Replace)
						fieldType.Name = r.Replace
					}
				}
			case *ast.StarExpr: // Struct field with pointer type
				if ident, ok := fieldType.X.(*ast.Ident); ok {
					for _, r := range c.Replacements {
						if ident.Name == r.Search {
							fmt.Println("Replacing", r.Search, "with", r.Replace)
							ident.Name = r.Replace
						}
					}
				}
			}

		case *ast.GenDecl:
			if x.Tok == token.TYPE {
				for _, spec := range x.Specs {
					if typeSpec, ok := spec.(*ast.TypeSpec); ok {
						if ident, ok := typeSpec.Type.(*ast.Ident); ok {
							for _, r := range c.Replacements {
								if ident.Name == r.Search {
									fmt.Println("Replacing", r.Search, "with", r.Replace)
									ident.Name = r.Replace
								}
							}
						}
					}
				}
			}
		}
		return true
	})

	// Write AST back to file
	outF, err := os.Create(file)
	if err != nil {
		return fmt.Errorf("cannot create file: %w", err)
	}
	defer func() {
		if err := outF.Close(); err != nil {
			fmt.Println("cannot close file:", err)
		}
	}()

	if err := printer.Fprint(outF, fset, f); err != nil {
		return fmt.Errorf("cannot write file: %w", err)
	}

	return nil
}

type config struct {
	Replacements []replacement
}

type replacement struct {
	Search  string
	Replace string
}

func readConfig() (config, error) {
	f, err := os.Open("replacements.json")
	if err != nil {
		return config{}, fmt.Errorf("cannot open file: %w", err)
	}
	defer func() {
		if err := f.Close(); err != nil {
			fmt.Println("cannot close file:", err)
		}
	}()

	var cfg config
	if err := json.NewDecoder(f).Decode(&cfg); err != nil {
		return config{}, fmt.Errorf("cannot decode JSON: %w", err)
	}

	return cfg, nil
}

replacements.json would look something like this:

{
  "Replacements": [
    {"Search": "YourType", "Replace": "externalRef0.YourType"}
  ]
}

@jamietanna jamietanna changed the title Missing package name on type from common API spec when referencing from another spec in generated code Missing package name on type from common API spec when referencing a path operation from another spec in generated code Dec 8, 2023
@markuswustenberg
Copy link
Author

I also saw this problem inside functions with statements such as var t YourType when using path parameters.

jamietanna added a commit that referenced this issue Jan 25, 2024
As noted in #1378, there are cases where a complex set of `$ref`s
between multiple files can lead to broken generated code, which does not
correctly import the package that has been prepared for the external
reference.

We can handle this by looking up any references, where there is a `.Ref`
passed into the type, and then iterate through relevant children.

This requires we handle the updating in-place for these by using a bit
of pointer + indexing fun.

This also adds a relevant test case to validate the fix.

Closes #1378.
jamietanna added a commit that referenced this issue Jan 25, 2024
As noted in #1378, there are cases where a complex set of `$ref`s
between multiple files can lead to broken generated code, which does not
correctly import the package that has been prepared for the external
reference.

We can handle this by looking up any references, where there is a `.Ref`
passed into the type, and then iterate through relevant children.

This requires we handle the updating in-place for these by using a bit
of pointer + indexing fun.

This also adds a relevant test case to validate the fix.

Closes #1378.
jamietanna added a commit that referenced this issue Jan 25, 2024
As noted in #1378, there are cases where a complex set of `$ref`s
between multiple files can lead to broken generated code, which does not
correctly import the package that has been prepared for the external
reference.

We can handle this by looking up any references, where there is a `.Ref`
passed into the type, and then iterate through relevant children.

This requires we handle the updating in-place for these by using a bit
of pointer + indexing fun.

This also adds a relevant test case to validate the fix.

Closes #1378.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant