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

WIP: capability to generate non Opt* struct for parameters with default value #1200

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions gen/ir/constructors.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func Array(item *Type, sem NilSemantic, schema *jsonschema.Schema) *Type {
func Alias(name string, to *Type) *Type {
return &Type{
Kind: KindAlias,
Name: name,
name: name,
AliasTo: to,
Validators: to.Validators,
Features: to.CloneFeatures(),
Expand All @@ -32,7 +32,7 @@ func Alias(name string, to *Type) *Type {

func Interface(name string) *Type {
return &Type{
Name: name,
name: name,
Kind: KindInterface,
InterfaceMethods: map[string]struct{}{},
Implementations: map[*Type]struct{}{},
Expand All @@ -49,9 +49,8 @@ func Pointer(to *Type, sem NilSemantic) *Type {
}

func Generic(name string, of *Type, v GenericVariant) *Type {
name = v.Name() + name
return &Type{
Name: name,
name: name,
Kind: KindGeneric,
GenericOf: of,
GenericVariant: v,
Expand All @@ -69,7 +68,7 @@ func Any(schema *jsonschema.Schema) *Type {
func Stream(name string, schema *jsonschema.Schema) *Type {
return &Type{
Kind: KindStream,
Name: name,
name: name,
Schema: schema,
}
}
45 changes: 40 additions & 5 deletions gen/ir/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
type Type struct {
Doc string // ogen documentation
Kind Kind // kind
Name string // only for struct, alias, interface, enum, stream, generic, map, sum
name string // only for struct, alias, interface, enum, stream, generic, map, sum
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make the field private and add an exported method Name instead

Primitive PrimitiveType // only for primitive, enum
AliasTo *Type // only for alias
PointerTo *Type // only for pointer
Expand Down Expand Up @@ -94,6 +94,23 @@
Features []string
}

type NameOpt struct {
CallerContext
}

func (t Type) Name(o NameOpt) string {
if t.Is(KindGeneric) {
isFeatureTurnedOn := true // generate non Opt* for default valued params, or not
if isFeatureTurnedOn && o.ServerClient == ServerClientServer && t.Default().Set {
return t.name
}
Comment on lines +103 to +106
Copy link
Contributor Author

@Liooo Liooo Mar 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where the main logic resides, if the feature is turned on and the call is made for the server side (and the param has default value), do not add Opt* prefix. Need to figure out a way to access the feature flags here.

The reason behind adding a flag rather than making it ogen default behavior is: https://t.me/ogen_dev/9482


return t.GenericVariant.Name() + t.name
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved the only anomaly that happens for Generic()

}

return t.name
}

// GoDoc returns type godoc.
func (t Type) GoDoc() []string {
s := t.Schema
Expand Down Expand Up @@ -129,7 +146,7 @@
var b strings.Builder
b.WriteString(string(t.Kind))
b.WriteRune('(')
b.WriteString(t.Go())

Check failure on line 149 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / test

not enough arguments in call to t.Go

Check failure on line 149 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / test (386, ubuntu-latest)

not enough arguments in call to t.Go

Check failure on line 149 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / test (amd64, ubuntu-latest)

not enough arguments in call to t.Go

Check failure on line 149 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / test (amd64, macos-latest)

not enough arguments in call to t.Go

Check failure on line 149 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / test (amd64, ubuntu-latest, -race)

not enough arguments in call to t.Go

Check failure on line 149 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / Analyze (go)

not enough arguments in call to t.Go

Check failure on line 149 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / golangci-lint

not enough arguments in call to t.Go

Check failure on line 149 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / test (amd64, windows-latest)

not enough arguments in call to t.Go
b.WriteRune(')')
if s := t.Schema; s != nil {
if ref := s.Ref.String(); ref != "" {
Expand Down Expand Up @@ -165,19 +182,37 @@
return false
}

type GoOpt struct {
CallerContext
}

// ServerClient tells if template method being called is for client or server side code generation
type ServerClient int

const (
ServerClientNone = iota
ServerClientClient
ServerClientServer
)
Comment on lines +189 to +196
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding ServerClientNone as a default zero value for the type, to handle any caller context that is not server nor client (not sure if there's such cases, I'll read the codebase and remove if this is not the case)


// CallerContext holds information about the caller side context of the template method being invoked
type CallerContext struct {
ServerClient
}
Comment on lines +198 to +201
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Defining a struct to hold the caller context, as the same mechanism might be necessary when implementing readonly/writeonly to store the information of if the method is called for write (POST, PUT etc) or read (GET etc) operation


// Go returns valid Go type for this Type.
func (t *Type) Go() string {
func (t *Type) Go(opt GoOpt) string {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This strategy will require us to basically rewrite many template methods to take an argument and pass that down the call stack.

switch t.Kind {
case KindPrimitive:
return t.Primitive.String()
case KindAny:
return "jx.Raw"
case KindArray:
return "[]" + t.Item.Go()
return "[]" + t.Item.Go(opt)
case KindPointer:
return "*" + t.PointerTo.Go()
return "*" + t.PointerTo.Go(opt)
case KindStruct, KindMap, KindAlias, KindInterface, KindGeneric, KindEnum, KindSum, KindStream:
return t.Name
return t.Name(NameOpt{CallerContext{opt.CallerContext.ServerClient}})
default:
panic(fmt.Sprintf("unexpected kind: %s", t.Kind))
}
Expand Down Expand Up @@ -243,7 +278,7 @@
case KindPointer:
return t.PointerTo.NamePostfix() + "Pointer"
case KindStruct, KindMap, KindAlias, KindInterface, KindGeneric, KindEnum, KindSum, KindStream:
return t.Name

Check failure on line 281 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / test

cannot use t.Name (value of type func(o NameOpt) string) as string value in return statement

Check failure on line 281 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / test (386, ubuntu-latest)

cannot use t.Name (value of type func(o NameOpt) string) as string value in return statement

Check failure on line 281 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / test (amd64, ubuntu-latest)

cannot use t.Name (value of type func(o NameOpt) string) as string value in return statement

Check failure on line 281 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / test (amd64, macos-latest)

cannot use t.Name (value of type func(o NameOpt) string) as string value in return statement

Check failure on line 281 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / test (amd64, ubuntu-latest, -race)

cannot use t.Name (value of type func(o NameOpt) string) as string value in return statement

Check failure on line 281 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / Analyze (go)

cannot use t.Name (value of type func(o NameOpt) string) as string value in return statement

Check failure on line 281 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / golangci-lint

cannot use t.Name (value of type func(o NameOpt) string) as string value in return statement

Check failure on line 281 in gen/ir/type.go

View workflow job for this annotation

GitHub Actions / test (amd64, windows-latest)

cannot use t.Name (value of type func(o NameOpt) string) as string value in return statement
default:
panic(fmt.Sprintf("unexpected kind: %s", t.Kind))
}
Expand Down
Loading