Skip to content

Commit

Permalink
refactor: use new location API
Browse files Browse the repository at this point in the history
  • Loading branch information
tdakkota committed Oct 20, 2022
1 parent c13de8e commit a848dac
Show file tree
Hide file tree
Showing 38 changed files with 360 additions and 225 deletions.
18 changes: 9 additions & 9 deletions cmd/ogen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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,
Expand All @@ -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 != "" {
Expand All @@ -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")
Expand Down
4 changes: 2 additions & 2 deletions gen/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion gen/gen_contents.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion gen/gen_headers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion gen/gen_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions gen/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down
5 changes: 3 additions & 2 deletions gen/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion gen/schema_gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions gen/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
9 changes: 5 additions & 4 deletions gen_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
15 changes: 14 additions & 1 deletion internal/jsonpointer/resolve_ctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"strings"

"github.com/go-faster/errors"

"github.com/ogen-go/ogen/internal/location"
)

// RefKey is JSON reference key.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")
}
Expand All @@ -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
}
Expand All @@ -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]
}
16 changes: 8 additions & 8 deletions jsonschema/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,30 @@ import (
// LocationError is a wrapper for an error that has a location.
type LocationError = location.Error

func (p *Parser) wrapField(field, filename string, l location.Locator, err error) error {
func (p *Parser) wrapField(field string, file location.File, l location.Locator, err error) error {
if err == nil || p == nil {
return err
}
return p.wrapLocation(filename, l.Field(field), err)
return p.wrapLocation(file, l.Field(field), err)
}

func (p *Parser) wrapLocation(filename string, l location.Locator, err error) error {
func (p *Parser) wrapLocation(file location.File, l location.Locator, err error) error {
var locErr *LocationError
// Do not wrap error if it is nil or is already a LocationError.
if err == nil || p == nil || errors.As(err, &locErr) {
return err
}

loc, ok := l.Location()
pos, ok := l.Position()
if !ok {
return err
}
if filename == "" {
filename = p.filename
if file.IsZero() {
file = p.file
}
return &LocationError{
File: filename,
Loc: loc,
File: file,
Pos: pos,
Err: err,
}
}
4 changes: 2 additions & 2 deletions jsonschema/jsonschema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ func TestNegative(t *testing.T) {
a.NoError(err)

p := jsonschema.NewParser(jsonschema.Settings{
Filename: name,
File: location.NewFile(name, file, data),
})
_, err = p.Parse(&schema)
a.Error(err)

var buf strings.Builder
ok := location.PrintPrettyError(&buf, true, name, data, err)
ok := location.PrintPrettyError(&buf, true, err)
// Ensure that the error message is pretty printed.
//
// There should be a good reason to remove this line.
Expand Down
Loading

0 comments on commit a848dac

Please sign in to comment.