Repository: goadesign/goa

Issue:
Poor diagnostic for use of View in Type
I was trying to move an old project to newer goa. Jumping directly to v3.23.2 from v3.10 or so I received diagnostics like this:
> [../../../go/pkg/mod/goa.design/goa/v3@v3.23.2/dsl/result_type.go:215] invalid use of View in attribute

If I downgrade to v3.23.1 I instead receive a more helpful path, so that aspect is new:
> [.$redacted_path/$file.go:131] invalid use of View in attribute

Even that could presumably be improved to mention "Type" instead of "attribute". This is true pretty far back, probably to the introduction of this 

For reference, all instances of this I saw were caused by DSL of the following general shape, though with more attributes and other noise. The fix to the design appears to be to change from Type to ResultType, or to remove the View; apparently older goa allowed this through.

```go
var SomeType = Type("SomeType", func() {
	Attribute("attr")
	View("default", func() {
		Attribute("attr")
	})
})
```

Discussion:
@MichaelUrman: Presumably a separate problem that I've yet to minimize, I am seeing duplicate funcs new${type}, new${type}View generated in gen/${service}/service.go and duplicate types ${type}View and funcs Validate${type}View in gen/${service}/view.go; any hints on tracking that down in existing non-trivial designs would be appreciated. Commenting out the View was insufficient to make it go away.@raphael: Goa has code that tries to get the right callstack entry for DSL errors but it looks like that can be improved... https://github.com/goadesign/goa/blob/v3/eval/error.go#L54

Would you be able to provide a repro for the duplicate codegen issue? This is where the code that generates these live: https://github.com/goadesign/goa/blob/v3/codegen/service/views.go@raphael: This has been fixed in #3861.

**Root cause**: When Goa is consumed from the Go module cache, file paths contain `@version` segments (e.g., `goa/v3@v3.23.2/dsl/...`). The error location heuristic was string-matching against DSL package paths (`goa.design/goa/v3/dsl`) but those don't include the version segment, so the match failed and the error pointed at internal DSL files.

**Fix**:
1. Normalize paths by stripping `@version` segments before matching
2. Also check function names (which don't include versions) as a fallback
3. Report `type "X"` instead of `attribute` when the expression is a top-level `Type()`

After the fix, your example produces:
```
[design.go:5] invalid use of View in type "SomeType"
```

Thanks for reporting!@MichaelUrman: Thanks for fixing the main report!

> Would you be able to provide a repro for the duplicate codegen issue? This is where the code that generates these live: https://github.com/goadesign/goa/blob/v3/codegen/service/views.go

I'll see what I can figure out when I have the chance. (I've gotten busy. 😅) Presumably something in there is getting populated multiple times, but I don't know yet what the triggers are. If I get somewhere useful I'll open a new issue for it.


Solution patch:
diff --git a/eval/error.go b/eval/error.go
index 64da72ec..3d291d53 100644
--- a/eval/error.go
+++ b/eval/error.go
@@ -46,29 +46,49 @@ func (e *Error) Error() string {
 	return ""
 }
 
+// normalizeFileForPackageMatch strips @version segments from module cache paths
+// so that package matching works regardless of where the module is cached.
+// For example: ".../goa/v3@v3.23.2/dsl/..." becomes ".../goa/v3/dsl/...".
+func normalizeFileForPackageMatch(file string) string {
+	file = filepath.ToSlash(file)
+	parts := strings.Split(file, "/")
+	for i, p := range parts {
+		if at := strings.IndexByte(p, '@'); at >= 0 {
+			parts[i] = p[:at]
+		}
+	}
+	return strings.Join(parts, "/")
+}
+
 // computeErrorLocation implements a heuristic to find the location in the user
 // code where the error occurred. It walks back the callstack until the file
 // doesn't match "/goa/design/*.go" or one of the DSL package paths.
 // When successful it returns the file name and line number, empty string and
 // 0 otherwise.
 func computeErrorLocation() (file string, line int) {
-	skipFunc := func(file string) bool {
+	skipFunc := func(pc uintptr, file string) bool {
 		if strings.HasSuffix(file, "_test.go") { // Be nice with tests
 			return false
 		}
 		file = filepath.ToSlash(file)
+		fn := runtime.FuncForPC(pc)
+		var name string
+		if fn != nil {
+			name = fn.Name()
+		}
+		normalized := normalizeFileForPackageMatch(file)
 		for _, pkg := range Context.dslPackages {
-			if strings.Contains(file, pkg) {
+			if strings.Contains(file, pkg) || strings.Contains(normalized, pkg) || strings.Contains(name, pkg) {
 				return true
 			}
 		}
 		return false
 	}
 	depth := 3
-	_, file, line, _ = runtime.Caller(depth)
-	for skipFunc(file) {
+	pc, file, line, _ := runtime.Caller(depth)
+	for skipFunc(pc, file) {
 		depth++
-		_, file, line, _ = runtime.Caller(depth)
+		pc, file, line, _ = runtime.Caller(depth)
 	}
 	wd, err := os.Getwd()
 	if err != nil {
diff --git a/expr/attribute.go b/expr/attribute.go
index 71a9d9d7..17e1e4a0 100644
--- a/expr/attribute.go
+++ b/expr/attribute.go
@@ -223,7 +223,12 @@ func walkAttribute(att *AttributeExpr, it func(name string, a *AttributeExpr) er
 }
 
 // EvalName returns the name used by the DSL evaluation.
-func (*AttributeExpr) EvalName() string {
+func (a *AttributeExpr) EvalName() string {
+	if a != nil {
+		if n, ok := a.Meta["openapi:typename"]; ok && len(n) > 0 {
+			return fmt.Sprintf("type %#v", n[0])
+		}
+	}
 	return "attribute"
 }
 



Test patch:
diff --git a/eval/error_internal_test.go b/eval/error_internal_test.go
new file mode 100644
index 00000000..4d215efd
--- /dev/null
+++ b/eval/error_internal_test.go
@@ -0,0 +1,33 @@
+package eval
+
+import "testing"
+
+func TestNormalizeFileForPackageMatch(t *testing.T) {
+	cases := map[string]struct {
+		in   string
+		want string
+	}{
+		"no version": {
+			in:   "/home/me/src/goa/eval/error.go",
+			want: "/home/me/src/goa/eval/error.go",
+		},
+		"module cache version": {
+			in:   "/home/me/go/pkg/mod/goa.design/goa/v3@v3.23.2/dsl/result_type.go",
+			want: "/home/me/go/pkg/mod/goa.design/goa/v3/dsl/result_type.go",
+		},
+		"multiple @ segments": {
+			in:   "/home/me/go/pkg/mod/example.com/foo@v1.2.3/bar@v0.1.0/baz.go",
+			want: "/home/me/go/pkg/mod/example.com/foo/bar/baz.go",
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			got := normalizeFileForPackageMatch(tc.in)
+			if got != tc.want {
+				t.Errorf("got %q, want %q", got, tc.want)
+			}
+		})
+	}
+}
+
diff --git a/eval/incompatible_dsl_context_test.go b/eval/incompatible_dsl_context_test.go
new file mode 100644
index 00000000..b563db5a
--- /dev/null
+++ b/eval/incompatible_dsl_context_test.go
@@ -0,0 +1,29 @@
+package eval_test
+
+import (
+	"strings"
+	"testing"
+
+	. "goa.design/goa/v3/dsl"
+	"goa.design/goa/v3/expr"
+)
+
+func TestIncompatibleDSLIncludesTypeContext(t *testing.T) {
+	dsl := func() {
+		Type("SomeType", func() {
+			Attribute("attr")
+			View("default", func() {
+				Attribute("attr")
+			})
+		})
+	}
+
+	err := expr.RunInvalidDSL(t, dsl)
+	if err == nil {
+		t.Fatalf("expected error")
+	}
+	if !strings.Contains(err.Error(), "in type \"SomeType\"") {
+		t.Fatalf("unexpected error: %s", err.Error())
+	}
+}
+