Merge pull request #637 from tdakkota/refactor/location

fix: print pretty error for external references

cmd/ogen/main.go

@@ -92,15 +92,15 @@ func generate(data []byte, packageName, targetDir string, clean bool, opts gen.O
 	return nil
 }
 
-func handleGenerateError(w io.Writer, color bool, specPath string, data []byte, err error) (r bool) {
+func handleGenerateError(w io.Writer, color bool, err error) (r bool) {
 	defer func() {
 		// Add trailing newline to the error message if error is handled.
 		if r {
 			_, _ = fmt.Fprintln(w)
 		}
 	}()
 
-	if location.PrintPrettyError(w, color, specPath, data, err) {
+	if location.PrintPrettyError(w, color, err) {
 		return true
 	}
 
@@ -248,6 +248,11 @@ func run() error {
 	}
 
 	specDir, fileName := filepath.Split(specPath)
+	data, err := os.ReadFile(specPath)
+	if err != nil {
+		return err
+	}
+
 	opts := gen.Options{
 		NoClient:             *noClient,
 		NoServer:             *noServer,
@@ -267,7 +272,7 @@ func run() error {
 		},
 		IgnoreNotImplemented: strings.Split(*debugIgnoreNotImplemented, ","),
 		ContentTypeAliases:   ctAliases,
-		Filename:             fileName,
+		File:                 location.NewFile(fileName, specPath, data),
 		Logger:               logger,
 	}
 	if expr := *skipTestsRegex; expr != "" {
@@ -286,13 +291,8 @@ func run() error {
 		}
 	}
 
-	data, err := os.ReadFile(specPath)
-	if err != nil {
-		return err
-	}
-
 	if err := generate(data, *packageName, *targetDir, *clean, opts); err != nil {
-		if handleGenerateError(os.Stderr, logOptions.Color, fileName, data, err) {
+		if handleGenerateError(os.Stderr, logOptions.Color, err) {
 			return errors.New("generation failed")
 		}
 		return errors.Wrap(err, "generate")

gen/errors.go

@@ -58,7 +58,7 @@ func (e *ErrUnsupportedContentTypes) Error() string {
 	return fmt.Sprintf("unsupported content types: [%s]", strings.Join(e.ContentTypes, ", "))
 }
 
-func (g *Generator) trySkip(err error, msg string, l locatable) error {
+func (g *Generator) trySkip(err error, msg string, l pos) error {
 	if err == nil {
 		return nil
 	}
@@ -71,7 +71,7 @@ func (g *Generator) trySkip(err error, msg string, l locatable) error {
 		reason = uErr.Error()
 	}
 	g.log.Info(msg,
-		g.zapLocation(l),
+		g.zapPosition(l),
 		zap.String("reason_error", reason),
 	)
 	return nil

gen/gen_contents.go

@@ -374,7 +374,7 @@ func (g *Generator) generateContents(
 				}
 
 				g.log.Info(`Content type is unsupported, set "format" to "binary" to use io.Reader`,
-					g.zapLocation(media),
+					g.zapPosition(media),
 					zap.String("contentType", contentType),
 				)
 				unsupported = append(unsupported, contentType)

gen/gen_headers.go

@@ -22,7 +22,7 @@ func (g *Generator) generateHeaders(
 		if http.CanonicalHeaderKey(hname) == "Content-Type" {
 			g.log.Warn(
 				"Content-Type is described separately and will be ignored in this section.",
-				g.zapLocation(header),
+				g.zapPosition(header),
 			)
 			continue
 		}

gen/gen_schema.go

@@ -38,7 +38,7 @@ func saveSchemaTypes(ctx *genctx, gen *schemaGen) error {
 }
 
 func (g *Generator) generateSchema(ctx *genctx, name string, schema *jsonschema.Schema, optional bool) (*ir.Type, error) {
-	gen := newSchemaGen(g.opt.Filename, ctx.lookupRef)
+	gen := newSchemaGen(g.opt.File.Name, ctx.lookupRef)
 	gen.log = g.log.Named("schemagen")
 	gen.fail = g.fail
 

gen/generator.go

@@ -41,7 +41,7 @@ func NewGenerator(spec *ogen.Spec, opts Options) (*Generator, error) {
 	}
 	api, err := parser.Parse(spec, parser.Settings{
 		External:   external,
-		Filename:   opts.Filename,
+		File:       opts.File,
 		InferTypes: opts.InferSchemaType,
 	})
 	if err != nil {
@@ -108,7 +108,7 @@ func (g *Generator) makeOps(ops []*openapi.Operation) error {
 
 	for _, spec := range ops {
 		routePath := spec.Path.String()
-		log := g.log.With(g.zapLocation(spec))
+		log := g.log.With(g.zapPosition(spec))
 
 		if !g.opt.Filters.accept(spec) {
 			log.Info("Skipping filtered operation")
@@ -167,7 +167,7 @@ func (g *Generator) makeWebhooks(webhooks []openapi.Webhook) error {
 			Name: w.Name,
 		}
 		for _, spec := range w.Operations {
-			log := g.log.With(g.zapLocation(spec))
+			log := g.log.With(g.zapPosition(spec))
 
 			if !g.opt.Filters.accept(spec) {
 				log.Info("Skipping filtered operation")

gen/options.go

@@ -8,6 +8,7 @@ import (
 	"go.uber.org/zap"
 
 	"github.com/ogen-go/ogen/gen/ir"
+	"github.com/ogen-go/ogen/internal/location"
 	"github.com/ogen-go/ogen/internal/xslices"
 	"github.com/ogen-go/ogen/openapi"
 )
@@ -45,10 +46,10 @@ type Options struct {
 	// ContentTypeAliases contains content type aliases.
 	ContentTypeAliases ContentTypeAliases
 
-	// Filename is a name of the spec file.
+	// File is the file that is being parsed.
 	//
 	// Used for error messages.
-	Filename string
+	File location.File
 	// Logger to use.
 	Logger *zap.Logger
 }

gen/schema_gen.go

@@ -295,7 +295,7 @@ func (g *schemaGen) generate2(name string, schema *jsonschema.Schema) (ret *ir.T
 		return g.regtype(name, t), nil
 	case jsonschema.Empty:
 		g.log.Info("Type is not defined, using any",
-			g.zapLocation(schema),
+			g.zapPosition(schema),
 			zap.String("name", name),
 		)
 		return g.regtype(name, ir.Any(schema)), nil

gen/utils.go

@@ -75,25 +75,25 @@ func statusText(code int) string {
 	return fmt.Sprintf("Code%d", code)
 }
 
-type locatable interface {
-	Location() (location.Location, bool)
+type pos interface {
+	Position() (location.Position, bool)
 }
 
-func zapLocation(filename string, l locatable) zap.Field {
+func zapPosition(filename string, l pos) zap.Field {
 	if l == nil {
 		return zap.Skip()
 	}
-	loc, ok := l.Location()
+	loc, ok := l.Position()
 	if !ok {
 		return zap.Skip()
 	}
 	return zap.String("at", loc.WithFilename(filename))
 }
 
-func (g *Generator) zapLocation(l locatable) zap.Field {
-	return zapLocation(g.opt.Filename, l)
+func (g *Generator) zapPosition(l pos) zap.Field {
+	return zapPosition(g.opt.File.Name, l)
 }
 
-func (g *schemaGen) zapLocation(l locatable) zap.Field {
-	return zapLocation(g.filename, l)
+func (g *schemaGen) zapPosition(l pos) zap.Field {
+	return zapPosition(g.filename, l)
 }

gen_test.go

@@ -33,7 +33,7 @@ func testGenerate(t *testing.T, filename string, data []byte, aliases ctAliases,
 			notImplemented[name] = struct{}{}
 		},
 		ContentTypeAliases: aliases,
-		Filename:           filename,
+		File:               location.NewFile(filename, filename, data),
 		Logger:             log,
 	}
 	t.Run("Gen", func(t *testing.T) {
@@ -158,18 +158,19 @@ func TestNegative(t *testing.T) {
 		spec, err := ogen.Parse(data)
 		a.NoError(err)
 
+		f := location.NewFile(name, name, data)
 		_, err = parser.Parse(spec, parser.Settings{
-			Filename: name,
+			File: f,
 		})
 		a.NoError(err, "If the error is related to parser, move this test to parser package testdata")
 
 		_, err = gen.NewGenerator(spec, gen.Options{
-			Filename: name,
+			File: f,
 		})
 		a.Error(err)
 
 		var buf strings.Builder
-		if location.PrintPrettyError(&buf, true, name, data, err) {
+		if location.PrintPrettyError(&buf, true, err) {
 			t.Log(buf.String())
 		} else {
 			t.Logf("%+v", err)

internal/jsonpointer/resolve_ctx.go

@@ -5,6 +5,8 @@ import (
 	"strings"
 
 	"github.com/go-faster/errors"
+
+	"github.com/ogen-go/ogen/internal/location"
 )
 
 // RefKey is JSON reference key.
@@ -36,6 +38,7 @@ type ResolveCtx struct {
 	//
 	// "#/definitions/SchemaProperty" should be resolved against "https://example.com/schema".
 	locstack []string
+	files    []location.File
 	// Store references to detect infinite recursive references.
 	refs       map[RefKey]struct{}
 	depthLimit int
@@ -85,7 +88,7 @@ func (r *ResolveCtx) Key(ref string) (key RefKey, _ error) {
 }
 
 // AddKey adds reference key to context.
-func (r *ResolveCtx) AddKey(key RefKey) error {
+func (r *ResolveCtx) AddKey(key RefKey, file location.File) error {
 	if r.depthLimit <= 0 {
 		return errors.New("depth limit exceeded")
 	}
@@ -97,6 +100,7 @@ func (r *ResolveCtx) AddKey(key RefKey) error {
 
 	if key.Loc != "" {
 		r.locstack = append(r.locstack, key.Loc)
+		r.files = append(r.files, file)
 	}
 	return nil
 }
@@ -118,3 +122,12 @@ func (r *ResolveCtx) LastLoc() string {
 	}
 	return s[len(s)-1]
 }
+
+// File returns last file from stack.
+func (r *ResolveCtx) File() (f location.File) {
+	s := r.files
+	if len(s) == 0 {
+		return f
+	}
+	return s[len(s)-1]
+}