Repository: goadesign/goa (go)

Issue:
v3: $ref generated in OpenAPI are not RFC3986 compliant
For a method named `Resolve IP` the generated references look like:
```
$ref: '#/definitions/DNSResolve IPNot FoundResponseBody'
```
Those must be percent-encoded as [URIs](https://www.ietf.org/rfc/rfc3986.txt) (the white-spaces in that case) or changed to underscores to avoid.

Pull request description:
Goify response definitions in openapi
Fixes #2118 

Patch:
diff --git a/README.md b/README.md
index d381f24c..c1999d11 100644
--- a/README.md
+++ b/README.md
@@ -11,8 +11,6 @@ design-first approach.
 [![Godoc](https://godoc.org/goa.design/goa?status.svg)](https://godoc.org/goa.design/goa)
 [![Slack](https://img.shields.io/badge/slack-gophers-orange.svg?style=flat)](https://gophers.slack.com/messages/goa/)
 
-## [Goa v2 and v3 released!!!](https://goa.design/blog/009-v_3_0_0/)
-
 ## Overview
 
 Goa takes a different approach to building services by making it possible to
diff --git a/codegen/scope.go b/codegen/scope.go
index cf930668..ae9c239a 100644
--- a/codegen/scope.go
+++ b/codegen/scope.go
@@ -53,26 +53,26 @@ func (s *NameScope) HashedUnique(key Hasher, name string, suffix ...string) stri
 // name if given name is not unique. If suffixed name is still not unique, a
 // counter value is added to the suffixed name until unique.
 func (s *NameScope) Unique(name string, suffix ...string) string {
-	var (
-		i   int
-		suf string
-	)
-	_, ok := s.counts[name]
+	c, ok := s.counts[name]
 	if !ok {
-		goto done
+		s.counts[name]++
+		return name
 	}
 	if len(suffix) > 0 {
-		suf = suffix[0]
+		name += suffix[0]
+		c, ok = s.counts[name]
+		if !ok {
+			s.counts[name]++
+			return name
+		}
 	}
-	name += suf
-	i, ok = s.counts[name]
-	if !ok {
-		goto done
+	for i := c; ; i++ {
+		ret := name + strconv.Itoa(i)
+		if _, ok := s.counts[ret]; !ok {
+			s.counts[ret]++
+			return ret
+		}
 	}
-	name += strconv.Itoa(i + 1)
-done:
-	s.counts[name] = i + 1
-	return name
 }
 
 // Name returns a unique name for the given name by adding a counter value to
diff --git a/expr/attribute.go b/expr/attribute.go
index d7d922a6..62e98481 100644
--- a/expr/attribute.go
+++ b/expr/attribute.go
@@ -159,7 +159,11 @@ func TaggedAttribute(a *AttributeExpr, tag string) string {
 		}
 	}
 	for _, b := range a.Bases {
-		if n := TaggedAttribute(&AttributeExpr{Type: b}, tag); n != "" {
+		at := &AttributeExpr{Type: b}
+		if ut, ok := b.(UserType); ok {
+			at = ut.Attribute()
+		}
+		if n := TaggedAttribute(at, tag); n != "" {
 			return n
 		}
 	}
diff --git a/http/codegen/openapi/json_schema.go b/http/codegen/openapi/json_schema.go
index 10a18971..1960e314 100644
--- a/http/codegen/openapi/json_schema.go
+++ b/http/codegen/openapi/json_schema.go
@@ -230,19 +230,15 @@ func ResultTypeRefWithPrefix(api *expr.APIExpr, mt *expr.ResultTypeExpr, view st
 		panic(fmt.Sprintf("failed to project media type %#v: %s", mt.Identifier, err)) // bug
 	}
 	if _, ok := Definitions[projected.TypeName]; !ok {
-		projected.TypeName = codegen.Goify(prefix, true) + projected.TypeName
+		projected.TypeName = codegen.Goify(prefix, true) + codegen.Goify(projected.TypeName, true)
 		GenerateResultTypeDefinition(api, projected, "default")
 	}
-	ref := fmt.Sprintf("#/definitions/%s", projected.TypeName)
-	return ref
+	return fmt.Sprintf("#/definitions/%s", projected.TypeName)
 }
 
 // TypeRef produces the JSON reference to the type definition.
 func TypeRef(api *expr.APIExpr, ut *expr.UserTypeExpr) string {
-	if _, ok := Definitions[ut.TypeName]; !ok {
-		GenerateTypeDefinition(api, ut)
-	}
-	return fmt.Sprintf("#/definitions/%s", ut.TypeName)
+	return TypeRefWithPrefix(api, ut, "")
 }
 
 // TypeRefWithPrefix produces the JSON reference to the type definition and adds the provided prefix
@@ -255,7 +251,6 @@ func TypeRefWithPrefix(api *expr.APIExpr, ut *expr.UserTypeExpr, prefix string)
 	if _, ok := Definitions[typeName]; !ok {
 		GenerateTypeDefinitionWithName(api, ut, typeName)
 	}
-
 	return fmt.Sprintf("#/definitions/%s", typeName)
 }
 
diff --git a/http/codegen/service_data.go b/http/codegen/service_data.go
index accec45b..349961a2 100644
--- a/http/codegen/service_data.go
+++ b/http/codegen/service_data.go
@@ -769,8 +769,12 @@ func (d ServicesData) analyze(hs *expr.HTTPServiceExpr) *ServiceData {
 			)
 			{
 				name = fmt.Sprintf("Build%sRequest", ep.VarName)
+				s := codegen.NewNameScope()
+				s.Unique("c") // 'c' is reserved as the client's receiver name.
 				for _, ca := range routes[0].PathInit.ClientArgs {
 					if ca.FieldName != "" {
+						ca.Name = s.Unique(ca.Name)
+						ca.Ref = ca.Name
 						args = append(args, ca)
 					}
 				}
@@ -2560,9 +2564,9 @@ const (
 	// requestInitT is the template used to render the code of HTTP
 	// request constructors.
 	requestInitT = `
-{{- if .PathInit.ClientArgs }}
+{{- if .Args }}
 	var (
-	{{- range .PathInit.ClientArgs }}
+	{{- range .Args }}
 	{{ .Name }} {{ .TypeRef }}
 	{{- end }}
 	)
@@ -2593,7 +2597,7 @@ const (
 			scheme = "wss"
 		}
 	{{- end }}
-	u := &url.URL{Scheme: {{ if .IsStreaming }}scheme{{ else }}c.scheme{{ end }}, Host: c.host, Path: {{ .PathInit.Name }}({{ range .PathInit.ClientArgs }}{{ .Ref }}, {{ end }})}
+	u := &url.URL{Scheme: {{ if .IsStreaming }}scheme{{ else }}c.scheme{{ end }}, Host: c.host, Path: {{ .PathInit.Name }}({{ range .Args }}{{ .Ref }}, {{ end }})}
 	req, err := http.NewRequest("{{ .Verb }}", u.String(), nil)
 	if err != nil {
 		return nil, goahttp.ErrInvalidURL("{{ .ServiceName }}", "{{ .EndpointName }}", u.String(), err)


Test patch:
diff --git a/codegen/scope_test.go b/codegen/scope_test.go
new file mode 100644
index 00000000..bb653ef3
--- /dev/null
+++ b/codegen/scope_test.go
@@ -0,0 +1,36 @@
+package codegen
+
+import (
+	"testing"
+)
+
+func TestNameScope_Unique(t *testing.T) {
+	sequence := []struct {
+		Input    string
+		Suffix   []string
+		Expected string
+	}{
+		{Input: "a", Expected: "a"},
+		{Input: "a", Expected: "a1"},
+		{Input: "a", Expected: "a2"},
+		{Input: "a", Expected: "a3"},
+		{Input: "b", Expected: "b"},
+		{Input: "c", Expected: "c"},
+		{Input: "hel", Expected: "hel"},
+		{Input: "hel", Suffix: []string{"lo"}, Expected: "hello"},
+		{Input: "hello", Expected: "hello1"},
+		{Input: "hello", Suffix: []string{"1"}, Expected: "hello11"},
+		{Input: "hello", Suffix: []string{"1"}, Expected: "hello12"},
+		{Input: "hello,world", Expected: "hello,world"},
+		{Input: "hello,world1", Expected: "hello,world1"},
+		{Input: "hello,world2", Expected: "hello,world2"},
+		{Input: "hello", Suffix: []string{",world"}, Expected: "hello,world3"},
+	}
+
+	scope := NewNameScope()
+	for i, v := range sequence {
+		if got := scope.Unique(v.Input, v.Suffix...); v.Expected != got {
+			t.Fatalf("#%v, expected %v, got %v", i, v.Expected, got)
+		}
+	}
+}
diff --git a/expr/attribute_test.go b/expr/attribute_test.go
index 3a57d20d..0639e898 100644
--- a/expr/attribute_test.go
+++ b/expr/attribute_test.go
@@ -8,23 +8,22 @@ import (
 )
 
 func TestTaggedAttribute(t *testing.T) {
+	tagged := &AttributeExpr{
+		Type: &Object{
+			&NamedAttributeExpr{
+				Name: "Foo",
+				Attribute: &AttributeExpr{
+					Meta: MetaExpr{"foo": []string{"foo"}},
+				},
+			},
+		},
+	}
 	cases := map[string]struct {
 		a        *AttributeExpr
 		expected string
 	}{
 		"tagged attribute": {
-			a: &AttributeExpr{
-				Type: &Object{
-					&NamedAttributeExpr{
-						Name: "Foo",
-						Attribute: &AttributeExpr{
-							Meta: MetaExpr{
-								"foo": []string{"foo"},
-							},
-						},
-					},
-				},
-			},
+			a:        tagged,
 			expected: "Foo",
 		},
 		"not object": {
@@ -37,19 +36,70 @@ func TestTaggedAttribute(t *testing.T) {
 			a: &AttributeExpr{
 				Type: &Object{
 					&NamedAttributeExpr{
-						Name:      "foo",
+						Name:      "Foo",
 						Attribute: &AttributeExpr{},
 					},
 				},
 			},
 			expected: "",
 		},
+		"extended": {
+			a: &AttributeExpr{
+				Type: &Object{
+					&NamedAttributeExpr{
+						Name:      "bar",
+						Attribute: &AttributeExpr{Type: String},
+					},
+				},
+				Bases: []DataType{
+					&UserTypeExpr{
+						TypeName:      "Extended",
+						AttributeExpr: tagged,
+					},
+				},
+			},
+			expected: "Foo",
+		},
+		"recursively extended": {
+			a: &AttributeExpr{
+				Type: &Object{
+					&NamedAttributeExpr{
+						Name:      "bar",
+						Attribute: &AttributeExpr{Type: String},
+					},
+				},
+				Bases: []DataType{
+					&UserTypeExpr{
+						TypeName: "Extended",
+						AttributeExpr: &AttributeExpr{
+							Type: &Object{
+								&NamedAttributeExpr{
+									Name: "foobar",
+									Attribute: &AttributeExpr{
+										Meta: MetaExpr{"foobar": []string{"foobar"}},
+									},
+								},
+							},
+							Bases: []DataType{
+								&UserTypeExpr{
+									TypeName:      "AnotherExtended",
+									AttributeExpr: tagged,
+								},
+							},
+						},
+					},
+				},
+			},
+			expected: "Foo",
+		},
 	}
 
 	for k, tc := range cases {
-		if actual := TaggedAttribute(tc.a, "foo"); tc.expected != actual {
-			t.Errorf("%s: got %#v, expected %#v", k, actual, tc.expected)
-		}
+		t.Run(k, func(t *testing.T) {
+			if actual := TaggedAttribute(tc.a, "foo"); tc.expected != actual {
+				t.Errorf("got %#v, expected %#v", actual, tc.expected)
+			}
+		})
 	}
 }
 
diff --git a/grpc/codegen/testdata/proto_code.go b/grpc/codegen/testdata/proto_code.go
index 90e66686..e788bcb3 100644
--- a/grpc/codegen/testdata/proto_code.go
+++ b/grpc/codegen/testdata/proto_code.go
@@ -159,10 +159,10 @@ service MyNameConflicts {
 }
 
 message MyNameConflictsMethodRequest {
-	MyNameConflicts2 conflict = 1;
+	MyNameConflicts1 conflict = 1;
 }
 
-message MyNameConflicts2 {
+message MyNameConflicts1 {
 	bool boolean_field = 1;
 }
 
diff --git a/http/codegen/openapi_test.go b/http/codegen/openapi_test.go
index d8e2b30d..5136ab6e 100644
--- a/http/codegen/openapi_test.go
+++ b/http/codegen/openapi_test.go
@@ -78,6 +78,7 @@ func TestSections(t *testing.T) {
 		{"explicit-view", testdata.ExplicitViewDSL},
 		{"security", testdata.SecurityDSL},
 		{"server-host-with-variables", testdata.ServerHostWithVariablesDSL},
+		{"with-spaces", testdata.WithSpacesDSL},
 	}
 	for _, c := range cases {
 		t.Run(c.Name, func(t *testing.T) {
diff --git a/http/codegen/testdata/openapi_v2/TestSections/with-spaces_file0.golden b/http/codegen/testdata/openapi_v2/TestSections/with-spaces_file0.golden
new file mode 100644
index 00000000..63004da2
--- /dev/null
+++ b/http/codegen/testdata/openapi_v2/TestSections/with-spaces_file0.golden
@@ -0,0 +1,1 @@
+{"swagger":"2.0","info":{"title":"","version":""},"host":"localhost:80","consumes":["application/json","application/xml","application/gob"],"produces":["application/json","application/xml","application/gob"],"paths":{"/":{"post":{"tags":["test service"],"summary":"test endpoint test service","operationId":"test service#test endpoint","parameters":[{"name":"Test EndpointRequestBody","in":"body","required":true,"schema":{"$ref":"#/definitions/TestServiceTestEndpointRequestBody"}}],"responses":{"200":{"description":"OK response.","schema":{"$ref":"#/definitions/TestServiceTestEndpointOKResponseBody"}},"404":{"description":"Not Found response.","schema":{"$ref":"#/definitions/TestServiceTestEndpointNotFoundResponseBody"}}},"schemes":["http"]}}},"definitions":{"TestServiceTestEndpointNotFoundResponseBody":{"title":"Mediatype identifier: application/vnd.goa.foobar; view=default","type":"object","properties":{"bar":{"type":"array","items":{"$ref":"#/definitions/barResponseBody"},"example":[{"string":""},{"string":""}]},"foo":{"type":"string","example":""}},"description":"Test EndpointNot FoundResponseBody result type (default view)","example":{"bar":[{"string":""},{"string":""},{"string":""},{"string":""}],"foo":""}},"TestServiceTestEndpointOKResponseBody":{"title":"Mediatype identifier: application/vnd.goa.foobar; view=default","type":"object","properties":{"bar":{"type":"array","items":{"$ref":"#/definitions/barResponseBody"},"example":[{"string":""},{"string":""},{"string":""},{"string":""}]},"foo":{"type":"string","example":""}},"description":"Test EndpointOKResponseBody result type (default view)","example":{"bar":[{"string":""},{"string":""}],"foo":""}},"TestServiceTestEndpointRequestBody":{"title":"TestServiceTestEndpointRequestBody","type":"object","properties":{"string":{"type":"string","example":""}},"example":{"string":""}},"barResponseBody":{"title":"barResponseBody","type":"object","properties":{"string":{"type":"string","example":""}},"example":{"string":""}}}}
\ No newline at end of file
diff --git a/http/codegen/testdata/openapi_v2/TestSections/with-spaces_file1.golden b/http/codegen/testdata/openapi_v2/TestSections/with-spaces_file1.golden
new file mode 100644
index 00000000..db3edb9e
--- /dev/null
+++ b/http/codegen/testdata/openapi_v2/TestSections/with-spaces_file1.golden
@@ -0,0 +1,100 @@
+swagger: "2.0"
+info:
+  title: ""
+  version: ""
+host: localhost:80
+consumes:
+- application/json
+- application/xml
+- application/gob
+produces:
+- application/json
+- application/xml
+- application/gob
+paths:
+  /:
+    post:
+      tags:
+      - test service
+      summary: test endpoint test service
+      operationId: test service#test endpoint
+      parameters:
+      - name: Test EndpointRequestBody
+        in: body
+        required: true
+        schema:
+          $ref: '#/definitions/TestServiceTestEndpointRequestBody'
+      responses:
+        "200":
+          description: OK response.
+          schema:
+            $ref: '#/definitions/TestServiceTestEndpointOKResponseBody'
+        "404":
+          description: Not Found response.
+          schema:
+            $ref: '#/definitions/TestServiceTestEndpointNotFoundResponseBody'
+      schemes:
+      - http
+definitions:
+  TestServiceTestEndpointNotFoundResponseBody:
+    title: 'Mediatype identifier: application/vnd.goa.foobar; view=default'
+    type: object
+    properties:
+      bar:
+        type: array
+        items:
+          $ref: '#/definitions/barResponseBody'
+        example:
+        - string: ""
+        - string: ""
+      foo:
+        type: string
+        example: ""
+    description: Test EndpointNot FoundResponseBody result type (default view)
+    example:
+      bar:
+      - string: ""
+      - string: ""
+      - string: ""
+      - string: ""
+      foo: ""
+  TestServiceTestEndpointOKResponseBody:
+    title: 'Mediatype identifier: application/vnd.goa.foobar; view=default'
+    type: object
+    properties:
+      bar:
+        type: array
+        items:
+          $ref: '#/definitions/barResponseBody'
+        example:
+        - string: ""
+        - string: ""
+        - string: ""
+        - string: ""
+      foo:
+        type: string
+        example: ""
+    description: Test EndpointOKResponseBody result type (default view)
+    example:
+      bar:
+      - string: ""
+      - string: ""
+      foo: ""
+  TestServiceTestEndpointRequestBody:
+    title: TestServiceTestEndpointRequestBody
+    type: object
+    properties:
+      string:
+        type: string
+        example: ""
+    example:
+      string: ""
+  barResponseBody:
+    title: barResponseBody
+    type: object
+    properties:
+      string:
+        type: string
+        example: ""
+    example:
+      string: ""
diff --git a/http/codegen/testdata/openapi_v2_dsls.go b/http/codegen/testdata/openapi_v2_dsls.go
index 3a0d5bec..1cf4f91b 100644
--- a/http/codegen/testdata/openapi_v2_dsls.go
+++ b/http/codegen/testdata/openapi_v2_dsls.go
@@ -369,3 +369,29 @@ var ServerHostWithVariablesDSL = func() {
 		})
 	})
 }
+
+var WithSpacesDSL = func() {
+	var Bar = Type("bar", func() {
+		Attribute("string", String, func() {
+			Example("")
+		})
+	})
+	var FooBar = ResultType("application/vnd.goa.foobar", func() {
+		TypeName("Foo Bar")
+		Attribute("foo", String, func() {
+			Example("")
+		})
+		Attribute("bar", ArrayOf(Bar))
+	})
+	Service("test service", func() {
+		Method("test endpoint", func() {
+			Payload(Bar)
+			Result(FooBar)
+			HTTP(func() {
+				POST("/")
+				Response(StatusOK)
+				Response(StatusNotFound)
+			})
+		})
+	})
+}