Skip to content

Commit

Permalink
fix(config): Validate database config in all cases (#2856)
Browse files Browse the repository at this point in the history
A `continue` in `Validate` was preventing validation in cases where
no Go codegen is configured.

* Move some Go-specific config validation into codegen package
* Update config validation test
* Remove some dead code
  • Loading branch information
andrewmbenton authored Oct 16, 2023
1 parent f9bbfd9 commit 651cafd
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 65 deletions.
4 changes: 4 additions & 0 deletions internal/codegen/golang/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ func Generate(ctx context.Context, req *plugin.CodeGenRequest) (*plugin.CodeGenR
return nil, err
}

if err := validateOpts(options); err != nil {
return nil, err
}

enums := buildEnums(req, options)
structs := buildStructs(req, options)
queries, err := buildQueries(req, options, structs)
Expand Down
10 changes: 9 additions & 1 deletion internal/codegen/golang/opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type opts struct {
OmitUnusedStructs bool `json:"omit_unused_structs,omitempty"`
BuildTags string `json:"build_tags,omitempty"`

// Unused but left in for parsing convenience
// Unused but included in marshaled json we receive
Overrides json.RawMessage `json:"overrides,omitempty"`
Rename json.RawMessage `json:"rename,omitempty"`
}
Expand All @@ -59,3 +59,11 @@ func parseOpts(req *plugin.CodeGenRequest) (*opts, error) {

return options, nil
}

func validateOpts(opts *opts) error {
if opts.EmitMethodsWithDbArgument && opts.EmitPreparedQueries {
return fmt.Errorf("invalid options: emit_methods_with_db_argument and emit_prepared_queries options are mutually exclusive")
}

return nil
}
11 changes: 5 additions & 6 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,12 @@ func FuzzConfig(f *testing.F) {
func TestInvalidConfig(t *testing.T) {
err := Validate(&Config{
SQL: []SQL{{
Gen: SQLGen{
Go: &SQLGo{
EmitMethodsWithDBArgument: true,
EmitPreparedQueries: true,
},
Database: &Database{
URI: "",
Managed: false,
},
}}})
}},
})
if err == nil {
t.Errorf("expected err; got nil")
}
Expand Down
34 changes: 0 additions & 34 deletions internal/config/override.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"strings"

"github.com/sqlc-dev/sqlc/internal/pattern"
"github.com/sqlc-dev/sqlc/internal/sql/ast"
)

type Override struct {
Expand Down Expand Up @@ -49,39 +48,6 @@ type Override struct {
GoStructTags map[string]string
}

func (o *Override) Matches(n *ast.TableName, defaultSchema string) bool {
if n == nil {
return false
}

schema := n.Schema
if n.Schema == "" {
schema = defaultSchema
}

if o.TableCatalog != nil && !o.TableCatalog.MatchString(n.Catalog) {
return false
}

if o.TableSchema == nil && schema != "" {
return false
}

if o.TableSchema != nil && !o.TableSchema.MatchString(schema) {
return false
}

if o.TableRel == nil && n.Name != "" {
return false
}

if o.TableRel != nil && !o.TableRel.MatchString(n.Name) {
return false
}

return true
}

func (o *Override) Parse() (err error) {

// validate deprecated postgres_type field
Expand Down
17 changes: 0 additions & 17 deletions internal/config/python_type.go

This file was deleted.

7 changes: 0 additions & 7 deletions internal/config/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,6 @@ import "fmt"

func Validate(c *Config) error {
for _, sql := range c.SQL {
sqlGo := sql.Gen.Go
if sqlGo == nil {
continue
}
if sqlGo.EmitMethodsWithDBArgument && sqlGo.EmitPreparedQueries {
return fmt.Errorf("invalid config: emit_methods_with_db_argument and emit_prepared_queries settings are mutually exclusive")
}
if sql.Database != nil {
if sql.Database.URI == "" && !sql.Database.Managed {
return fmt.Errorf("invalid config: database must be managed or have a non-empty URI")
Expand Down

0 comments on commit 651cafd

Please sign in to comment.