Skip to content

Commit

Permalink
Fix issues reported by DeepSource (#3349)
Browse files Browse the repository at this point in the history
  • Loading branch information
raphael authored Sep 10, 2023
1 parent 47c4d8c commit 286aa67
Show file tree
Hide file tree
Showing 14 changed files with 72 additions and 60 deletions.
7 changes: 6 additions & 1 deletion cmd/goa/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,12 @@ func cleanupDirs(cmd, output string) []string {
if err != nil {
return nil
}
defer gendir.Close()
defer func() {
err := gendir.Close()
if err != nil {
fmt.Fprintf(os.Stderr, "failed to close gendir: %s", err)
}
}()
finfos, err := gendir.Readdir(-1)
if err != nil {
return []string{gendirPath}
Expand Down
19 changes: 10 additions & 9 deletions cmd/goa/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,10 @@ func main() {
}
}

gen(cmd, path, output, debug)
if err := gen(cmd, path, output, debug); err != nil {
fmt.Fprintln(os.Stderr, err.Error())
os.Exit(1)
}
}

// help with tests
Expand All @@ -71,7 +74,7 @@ var (
gen = generate
)

func generate(cmd, path, output string, debug bool) {
func generate(cmd, path, output string, debug bool) error {
var (
files []string
err error
Expand All @@ -83,9 +86,6 @@ func generate(cmd, path, output string, debug bool) {
}

tmp = NewGenerator(cmd, path, output)
if !debug {
defer tmp.Remove()
}

if err = tmp.Write(debug); err != nil {
goto fail
Expand All @@ -100,13 +100,15 @@ func generate(cmd, path, output string, debug bool) {
}

fmt.Println(strings.Join(files, "\n"))
return
if !debug {
tmp.Remove()
}
return nil
fail:
fmt.Fprintln(os.Stderr, err.Error())
if !debug && tmp != nil {
tmp.Remove()
}
os.Exit(1)
return err
}

func help() {
Expand Down Expand Up @@ -142,5 +144,4 @@ Example:
goa gen goa.design/examples/cellar/design -o gendir
`)
os.Exit(1)
}
2 changes: 1 addition & 1 deletion cmd/goa/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestCmdLine(t *testing.T) {
)

usage = func() { usageCalled = true }
gen = func(c string, p, o string, d bool) { cmd, path, output, debug = c, p, o, d }
gen = func(c string, p, o string, d bool) error { cmd, path, output, debug = c, p, o, d; return nil }
defer func() {
usage = help
gen = generate
Expand Down
4 changes: 2 additions & 2 deletions codegen/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,14 @@ func (f *File) Render(dir string) (string, error) {
}
}

if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
if err := os.MkdirAll(filepath.Dir(path), 0750); err != nil {
return "", err
}

file, err := os.OpenFile(
path,
os.O_CREATE|os.O_APPEND|os.O_WRONLY,
0644,
0600,
)
if err != nil {
return "", err
Expand Down
2 changes: 1 addition & 1 deletion codegen/generator/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func Generate(dir, cmd string) (outputs []string, err1 error) {
return nil, err
}
path := filepath.Join(base, codegen.Gendir)
if err := os.MkdirAll(path, 0777); err != nil {
if err := os.MkdirAll(path, 0750); err != nil {
return nil, err
}

Expand Down
45 changes: 23 additions & 22 deletions codegen/generator/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,30 +14,31 @@ func Service(genpkg string, roots []eval.Root) ([]*codegen.File, error) {
var files []*codegen.File
var userTypePkgs = make(map[string][]string)
for _, root := range roots {
switch r := root.(type) {
case *expr.RootExpr:
for _, s := range r.Services {
// Make sure service is first so name scope is
// properly initialized.
files = append(files, service.Files(genpkg, s, userTypePkgs)...)
files = append(files, service.EndpointFile(genpkg, s))
files = append(files, service.ClientFile(genpkg, s))
if f := service.ViewsFile(genpkg, s); f != nil {
files = append(files, f)
}
for _, f := range files {
if len(f.SectionTemplates) > 0 {
service.AddServiceDataMetaTypeImports(f.SectionTemplates[0], s)
}
}
f, err := service.ConvertFile(r, s)
if err != nil {
return nil, err
}
if f != nil {
files = append(files, f)
r, ok := root.(*expr.RootExpr)
if !ok {
continue
}
for _, s := range r.Services {
// Make sure service is first so name scope is
// properly initialized.
files = append(files, service.Files(genpkg, s, userTypePkgs)...)
files = append(files, service.EndpointFile(genpkg, s))
files = append(files, service.ClientFile(genpkg, s))
if f := service.ViewsFile(genpkg, s); f != nil {
files = append(files, f)
}
for _, f := range files {
if len(f.SectionTemplates) > 0 {
service.AddServiceDataMetaTypeImports(f.SectionTemplates[0], s)
}
}
f, err := service.ConvertFile(r, s)
if err != nil {
return nil, err
}
if f != nil {
files = append(files, f)
}
}
}
return files, nil
Expand Down
4 changes: 2 additions & 2 deletions codegen/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ func safelyGetMetaTypeImports(att *expr.AttributeExpr, seen map[string]struct{})
}
for imp := range uniqueImports {
// Copy loop variable into body so next iteration doesn't overwrite its address https://stackoverflow.com/questions/27610039/golang-appending-leaves-only-last-element
copy := imp
imports = append(imports, &copy)
cp := imp
imports = append(imports, &cp)
}
return imports
}
Expand Down
22 changes: 11 additions & 11 deletions codegen/service/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ type dtRec struct {
seen map[string]expr.DataType
}

func (r dtRec) append(p string) dtRec {
func appendPath(r dtRec, p string) dtRec {
r.path += p
return r
}
Expand Down Expand Up @@ -414,7 +414,7 @@ func buildDesignType(dt *expr.DataType, t reflect.Type, ref expr.DataType, recs
eref = expr.AsArray(ref).ElemType.Type
}
var elem expr.DataType
if err := buildDesignType(&elem, e, eref, rec.append("[0]")); err != nil {
if err := buildDesignType(&elem, e, eref, appendPath(rec, "[0]")); err != nil {
return fmt.Errorf("%s", err)
}
*dt = &expr.Array{ElemType: &expr.AttributeExpr{Type: elem}}
Expand All @@ -427,11 +427,11 @@ func buildDesignType(dt *expr.DataType, t reflect.Type, ref expr.DataType, recs
vref = m.ElemType.Type
}
var kt expr.DataType
if err := buildDesignType(&kt, t.Key(), kref, rec.append(".key")); err != nil {
if err := buildDesignType(&kt, t.Key(), kref, appendPath(rec, ".key")); err != nil {
return fmt.Errorf("%s", err)
}
var vt expr.DataType
if err := buildDesignType(&vt, t.Elem(), vref, rec.append(".value")); err != nil {
if err := buildDesignType(&vt, t.Elem(), vref, appendPath(rec, ".value")); err != nil {
return fmt.Errorf("%s", err)
}
*dt = &expr.Map{KeyType: &expr.AttributeExpr{Type: kt}, ElemType: &expr.AttributeExpr{Type: vt}}
Expand Down Expand Up @@ -475,7 +475,7 @@ func buildDesignType(dt *expr.DataType, t reflect.Type, ref expr.DataType, recs
rec.seen[t.Name()] = ut
var required []string
for i, f := range fields {
recf := rec.append("." + f.Name)
recf := appendPath(rec, "."+f.Name)
atn, fn := attributeName(oref, f.Name)
var aref expr.DataType
if oref != nil {
Expand All @@ -500,7 +500,7 @@ func buildDesignType(dt *expr.DataType, t reflect.Type, ref expr.DataType, recs
if isPrimitive(f.Type) {
required = append(required, atn)
}
if err := buildDesignType(&fdt, f.Type, aref, rec.append("."+f.Name)); err != nil {
if err := buildDesignType(&fdt, f.Type, aref, appendPath(rec, "."+f.Name)); err != nil {
return fmt.Errorf("%q.%s: %s", t.Name(), f.Name, err)
}
}
Expand Down Expand Up @@ -625,7 +625,7 @@ type compRec struct {
seen map[string]struct{}
}

func (r compRec) append(p string) compRec {
func appendCompPath(r compRec, p string) compRec {
r.path += p
return r
}
Expand Down Expand Up @@ -662,7 +662,7 @@ func compatible(from expr.DataType, to reflect.Type, recs ...compRec) error {
return compatible(
expr.AsArray(from).ElemType.Type,
to.Elem(),
rec.append("[0]"),
appendCompPath(rec, "[0]"),
)
}

Expand All @@ -673,14 +673,14 @@ func compatible(from expr.DataType, to reflect.Type, recs ...compRec) error {
if err := compatible(
expr.AsMap(from).ElemType.Type,
to.Elem(),
rec.append(".value"),
appendCompPath(rec, ".value"),
); err != nil {
return err
}
return compatible(
expr.AsMap(from).KeyType.Type,
to.Key(),
rec.append(".key"),
appendCompPath(rec, ".key"),
)
}

Expand Down Expand Up @@ -722,7 +722,7 @@ func compatible(from expr.DataType, to reflect.Type, recs ...compRec) error {
err := compatible(
nat.Attribute.Type,
field.Type,
rec.append("."+fname),
appendCompPath(rec, "."+fname),
)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion http/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (dd *debugDoer) Fprint(w io.Writer) {
}

// Error builds an error message.
func (c *ClientError) Error() string {
func (c ClientError) Error() string {
return fmt.Sprintf("[%s %s]: %s", c.Service, c.Method, c.Message)
}

Expand Down
7 changes: 6 additions & 1 deletion http/codegen/client_encode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,12 @@ func TestClientEncode(t *testing.T) {
if _, err := golden.WriteString("package testdata\n"); err != nil {
t.Fatal(err)
}
defer golden.Close()
defer func() {
err := golden.Close()
if err != nil {
t.Fatal(err)
}
}()
}
for _, c := range cases {
t.Run(c.Name, func(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions http/codegen/openapi/v2/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,9 +374,9 @@ func paramFor(at *expr.AttributeExpr, name, in string, required bool) *Parameter

func itemsFromExpr(at *expr.AttributeExpr) *Items {
items := &Items{Type: at.Type.Name()}
switch actual := at.Type.(type) {
case expr.Primitive:
switch actual.Kind() {
p, ok := at.Type.(expr.Primitive)
if ok {
switch p.Kind() {
case expr.IntKind, expr.Int64Kind, expr.UIntKind, expr.UInt64Kind, expr.Int32Kind, expr.UInt32Kind:
items.Type = "integer"
case expr.Float32Kind, expr.Float64Kind:
Expand Down
8 changes: 4 additions & 4 deletions http/codegen/openapi/v3/openapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,8 @@ func (o Operation) MarshalJSON() ([]byte, error) {
}

// MarshalJSON returns the JSON encoding of p.
func (p Parameter) MarshalJSON() ([]byte, error) {
return openapi.MarshalJSON(_Parameter(p), p.Extensions)
func (p *Parameter) MarshalJSON() ([]byte, error) {
return openapi.MarshalJSON((*_Parameter)(p), p.Extensions)
}

// MarshalJSON returns the JSON encoding of r.
Expand Down Expand Up @@ -314,8 +314,8 @@ func (o Operation) MarshalYAML() (any, error) {
}

// MarshalYAML returns value which marshaled in place of the original value
func (p Parameter) MarshalYAML() (any, error) {
return openapi.MarshalYAML(_Parameter(p), p.Extensions)
func (p *Parameter) MarshalYAML() (any, error) {
return openapi.MarshalYAML((*_Parameter)(p), p.Extensions)
}

// MarshalYAML returns value which marshaled in place of the original value
Expand Down
1 change: 0 additions & 1 deletion http/codegen/server_decode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ func TestDecode(t *testing.T) {
{"decode-multipart-body-map-type", testdata.PayloadMultipartMapTypeDSL, testdata.PayloadMultipartMapTypeDecodeCode},
{"decode-with-params-and-headers-dsl", testdata.WithParamsAndHeadersBlockDSL, testdata.WithParamsAndHeadersBlockDecodeCode},

//decode- aliases
{"decode-query-int-alias", testdata.QueryIntAliasDSL, testdata.QueryIntAliasDecodeCode},
{"decode-query-int-alias-validate", testdata.QueryIntAliasValidateDSL, testdata.QueryIntAliasValidateDecodeCode},
{"decode-query-array-alias", testdata.QueryArrayAliasDSL, testdata.QueryArrayAliasDecodeCode},
Expand Down
3 changes: 2 additions & 1 deletion http/codegen/service_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -2390,7 +2390,8 @@ func extractQueryParams(a *expr.MappedAttributeExpr, service *expr.AttributeExpr
pointer bool
fptr bool
)
if pointer = a.IsPrimitivePointer(name, true); pointer {
pointer = a.IsPrimitivePointer(name, true)
if pointer {
typeRef = "*" + typeRef
}
fieldName := codegen.Goify(name, true)
Expand Down

0 comments on commit 286aa67

Please sign in to comment.