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

Fix eval.IncompatibleDSL() to not show internal DSL #3502

Merged
merged 3 commits into from Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 17 additions & 11 deletions eval/eval.go
Expand Up @@ -2,10 +2,10 @@ package eval

import (
"fmt"
"path/filepath"
"reflect"
"runtime"
"strings"
"unicode"
)

// RunDSL iterates through the root expressions and calls WalkSets on each to
Expand Down Expand Up @@ -116,8 +116,7 @@ func ReportError(fm string, vals ...any) {
// IncompatibleDSL should be called by DSL functions when they are invoked in an
// incorrect context (e.g. "Params" in "Service").
func IncompatibleDSL() {
elems := strings.Split(caller(), ".")
ReportError("invalid use of %s", elems[len(elems)-1])
ReportError("invalid use of %s", caller())
}

// InvalidArgError records an invalid argument error. It is used by DSL
Expand Down Expand Up @@ -235,13 +234,20 @@ func finalizeSet(set ExpressionSet) {

// caller returns the name of calling function.
func caller() string {
pc, file, _, ok := runtime.Caller(2)
if ok && filepath.Base(file) == "current.go" {
pc, _, _, ok = runtime.Caller(3)
}
if !ok {
return "<unknown>"
for skip := 2; skip <= 3; skip++ {
pc, _, _, ok := runtime.Caller(skip)
if !ok {
break
}
name := runtime.FuncForPC(pc).Name()
elems := strings.Split(name, ".")
caller := elems[len(elems)-1]
for _, first := range caller {
if unicode.IsUpper(first) {
return caller
}
break
}
}

return runtime.FuncForPC(pc).Name()
return "<unknown>"
}
6 changes: 4 additions & 2 deletions expr/http_endpoint_test.go
@@ -1,6 +1,7 @@
package expr_test

import (
"strings"
"testing"

"goa.design/goa/v3/eval"
Expand All @@ -15,18 +16,19 @@ func TestHTTPRouteValidation(t *testing.T) {
Error string
}{
{"valid", testdata.ValidRouteDSL, ""},
{"invalid", testdata.DuplicateWCRouteDSL, `route POST "/{id}" of service "InvalidRoute" HTTP endpoint "Method": Wildcard "id" appears multiple times in full path "/{id}/{id}"`},
{"duplicate-wc-route", testdata.DuplicateWCRouteDSL, `route POST "/{id}" of service "DuplicateWCRoute" HTTP endpoint "Method": Wildcard "id" appears multiple times in full path "/{id}/{id}"`},
{"disallow-response-body", testdata.DisallowResponseBodyHeadDSL, `route HEAD "/" of service "DisallowResponseBody" HTTP endpoint "Method": HTTP status 200: Response body defined for HEAD method which does not allow response body.
route HEAD "/" of service "DisallowResponseBody" HTTP endpoint "Method": HTTP status 404: Response body defined for HEAD method which does not allow response body.`,
},
{"invalid", testdata.InvalidRouteDSL, "invalid use of POST in service \"InvalidRoute\""},
}
for _, c := range cases {
t.Run(c.Name, func(t *testing.T) {
if c.Error == "" {
expr.RunDSL(t, c.DSL)
} else {
err := expr.RunInvalidDSL(t, c.DSL)
if err.Error() != c.Error {
if !strings.HasSuffix(err.Error(), c.Error) {
t.Errorf("got error %q\nexpected %q", err.Error(), c.Error)
}
}
Expand Down
10 changes: 9 additions & 1 deletion expr/testdata/endpoint_dsls.go
Expand Up @@ -22,7 +22,7 @@ var ValidRouteDSL = func() {
}

var DuplicateWCRouteDSL = func() {
Service("InvalidRoute", func() {
Service("DuplicateWCRoute", func() {
HTTP(func() {
Path("/{id}")
})
Expand Down Expand Up @@ -52,6 +52,14 @@ var DisallowResponseBodyHeadDSL = func() {
})
}

var InvalidRouteDSL = func() {
Service("InvalidRoute", func() {
HTTP(func() {
POST("/{id}")
})
})
}

var EndpointWithParentDSL = func() {
Service("Parent", func() {
Method("show", func() {
Expand Down