nebius/SWE-rebench-V2-PRs huggingface dataset:
Repository: googleapis/api-linter
Pull request #1159 (2023-05-26 22:04:12)

fix(AIP-122): allow display_name suffix
The field `display_name` is an allowed use of `_name` suffix. I've seen some Custom Methods that create multiple resources and want to allow specifying the resulting `display_name` fields, so they prefix them with the resource it would correlate to post-creation e.g. `instance_display_name` and `network_display_name`, while other resource fields would be populated on behalf of the user (if there are any to begin with). 

Fixes http://b/270956831


Patch:
diff --git a/rules/aip0122/name_suffix.go b/rules/aip0122/name_suffix.go
index 3918a75..47a3e72 100644
--- a/rules/aip0122/name_suffix.go
+++ b/rules/aip0122/name_suffix.go
@@ -26,18 +26,20 @@ import (
 var nameSuffix = &lint.FieldRule{
 	Name: lint.NewRuleName(122, "name-suffix"),
 	OnlyIf: func(f *desc.FieldDescriptor) bool {
-		return !stringset.New(
+		n := f.GetName()
+		// Ignore `{prefix}_display_name` fields as this seems like a reasonable suffix.
+		return strings.HasSuffix(n, "_name") && !strings.HasSuffix(n, "_display_name")
+	},
+	LintField: func(f *desc.FieldDescriptor) []lint.Problem {
+		allowedNameFields := stringset.New(
 			"display_name",
 			"family_name",
 			"given_name",
 			"full_resource_name",
 			"crypto_key_name",
 			"cmek_key_name",
-			"name",
-		).Contains(f.GetName())
-	},
-	LintField: func(f *desc.FieldDescriptor) []lint.Problem {
-		if n := f.GetName(); strings.HasSuffix(n, "_name") {
+		)
+		if n := f.GetName(); !allowedNameFields.Contains(n) {
 			return []lint.Problem{{
 				Message:    "Fields should not use the `_name` suffix.",
 				Suggestion: strings.TrimSuffix(n, "_name"),


Test patch:
diff --git a/rules/aip0122/name_suffix_test.go b/rules/aip0122/name_suffix_test.go
index 688b2b3..786f3af 100644
--- a/rules/aip0122/name_suffix_test.go
+++ b/rules/aip0122/name_suffix_test.go
@@ -31,6 +31,7 @@ func TestNameSuffix(t *testing.T) {
 		{"ValidStandardGiven", "given_name", testutils.Problems{}},
 		{"ValidStandardFamily", "family_name", testutils.Problems{}},
 		{"ValidStandardFull", "full_resource_name", testutils.Problems{}},
+		{"SkipValidDisplayNameSuffix", "foo_display_name", testutils.Problems{}},
 		{"Invalid", "author_name", testutils.Problems{{Suggestion: "author"}}},
 	} {
 		f := testutils.ParseProto3Tmpl(t, `
Repository: googleapis/api-linter
Pull request #1198 (2023-07-13 19:58:25)

feat(AIP-142): adding purge_time field
Updating checking for purge_time to align with
https://github.com/aip-dev/google.aip.dev/pull/1169.

Continued to ignore expire_time for soft-deleted resources
for backwards-compatibility.


Patch:
diff --git a/rules/aip0142/time_field_names.go b/rules/aip0142/time_field_names.go
index 52c937a..dd54763 100644
--- a/rules/aip0142/time_field_names.go
+++ b/rules/aip0142/time_field_names.go
@@ -32,6 +32,7 @@ var fieldNames = &lint.FieldRule{
 			"expired":  "expire_time",
 			"modified": "update_time",
 			"updated":  "update_time",
+			"purged":   "purged_time",
 		}
 		for got, want := range mistakes {
 			if strings.Contains(f.GetName(), got) {
diff --git a/rules/aip0164/resource_expire_time_field.go b/rules/aip0164/resource_expire_time_field.go
index dfa8217..05e6208 100644
--- a/rules/aip0164/resource_expire_time_field.go
+++ b/rules/aip0164/resource_expire_time_field.go
@@ -14,11 +14,16 @@ var resourceExpireTimeField = &lint.MessageRule{
 		return utils.FindMethod(m.GetFile(), "Undelete"+resource) != nil
 	},
 	LintMessage: func(m *desc.MessageDescriptor) []lint.Problem {
+		// for backwards compatibility, do not lint on expire_time.
+		// previously expire_time was the recommended term.
 		if m.FindFieldByName("expire_time") != nil {
 			return nil
 		}
+		if m.FindFieldByName("purge_time") != nil {
+			return nil
+		}
 		return []lint.Problem{{
-			Message:    "Resources supporting soft delete must have a `google.protobuf.Timestamp expire_time` field.",
+			Message:    "Resources supporting soft delete must have a `google.protobuf.Timestamp purge_time` field.",
 			Descriptor: m,
 		}}
 	},


Test patch:
diff --git a/rules/aip0164/resource_expire_time_field_test.go b/rules/aip0164/resource_expire_time_field_test.go
index 377136d..7a5c5c1 100644
--- a/rules/aip0164/resource_expire_time_field_test.go
+++ b/rules/aip0164/resource_expire_time_field_test.go
@@ -13,7 +13,8 @@ func TestResourceExpireTimeField(t *testing.T) {
 		ResourceField string
 		problems      testutils.Problems
 	}{
-		{"Valid", `UndeleteBook`, `google.protobuf.Timestamp expire_time = 1;`, nil},
+		{"Valid", `UndeleteBook`, `google.protobuf.Timestamp purge_time = 1;`, nil},
+		{"Valid", `UndeleteBookLegacyExpireTime`, `google.protobuf.Timestamp expire_time = 1;`, nil},
 		{"Invalid", `UndeleteBook`, ``, testutils.Problems{{Message: "Resources supporting soft delete"}}},
 		{"IrrelevantNoSoftDelete", `GetBook`, ``, nil},
 	} {
Repository: googleapis/api-linter
Pull request #1199 (2023-07-15 06:12:17)

fix(AIP-203): handle recursive field behavior check
Some messages can be recursive, which lead to stackoverflows.


Patch:
diff --git a/rules/aip0142/time_field_names.go b/rules/aip0142/time_field_names.go
index dd54763..854c936 100644
--- a/rules/aip0142/time_field_names.go
+++ b/rules/aip0142/time_field_names.go
@@ -32,7 +32,7 @@ var fieldNames = &lint.FieldRule{
 			"expired":  "expire_time",
 			"modified": "update_time",
 			"updated":  "update_time",
-			"purged":   "purged_time",
+			"purged":   "purge_time",
 		}
 		for got, want := range mistakes {
 			if strings.Contains(f.GetName(), got) {
diff --git a/rules/aip0203/field_behavior_required.go b/rules/aip0203/field_behavior_required.go
index bafb050..0581043 100644
--- a/rules/aip0203/field_behavior_required.go
+++ b/rules/aip0203/field_behavior_required.go
@@ -32,7 +32,7 @@ var fieldBehaviorRequired = &lint.MethodRule{
 	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
 		req := m.GetInputType()
 		p := m.GetFile().GetPackage()
-		ps := problems(req, p)
+		ps := problems(req, p, map[desc.Descriptor]bool{})
 		if len(ps) == 0 {
 			return nil
 		}
@@ -41,10 +41,16 @@ var fieldBehaviorRequired = &lint.MethodRule{
 	},
 }
 
-func problems(m *desc.MessageDescriptor, pkg string) []lint.Problem {
+func problems(m *desc.MessageDescriptor, pkg string, visited map[desc.Descriptor]bool) []lint.Problem {
 	var ps []lint.Problem
 
 	for _, f := range m.GetFields() {
+		// ignore the field if it was already visited
+		if ok := visited[f]; ok {
+			continue
+		}
+		visited[f] = true
+
 		if utils.IsResource(m) && f.GetName() == "name" {
 			continue
 		}
@@ -58,7 +64,7 @@ func problems(m *desc.MessageDescriptor, pkg string) []lint.Problem {
 		}
 
 		if mt := f.GetMessageType(); mt != nil && !mt.IsMapEntry() && mt.GetFile().GetPackage() == pkg {
-			ps = append(ps, problems(mt, pkg)...)
+			ps = append(ps, problems(mt, pkg, visited)...)
 		}
 	}
 


Test patch:
diff --git a/rules/aip0203/field_behavior_required_test.go b/rules/aip0203/field_behavior_required_test.go
index ff81bb9..339bb7a 100644
--- a/rules/aip0203/field_behavior_required_test.go
+++ b/rules/aip0203/field_behavior_required_test.go
@@ -72,6 +72,13 @@ func TestFieldBehaviorRequired_SingleFile_SingleMessage(t *testing.T) {
 			];`,
 			nil,
 		},
+		{
+			"ValidRecursiveMessage",
+			`message Foo { Foo foo = 1 [(google.api.field_behavior) = OPTIONAL]; }
+			 Foo foo = 1 [(google.api.field_behavior) = OPTIONAL];
+			`,
+			nil,
+		},
 		{
 			"InvalidEmpty",
 			"int32 page_count = 1;",
Repository: googleapis/api-linter
Pull request #1213 (2023-07-28 21:39:06)

fix(locations): add arbitrary field option helper
Similar to the `MethodOption` helper, this enables retrieving source code info for an arbitrary Field option.


Patch:
diff --git a/locations/field_locations.go b/locations/field_locations.go
index 73880d5..e58cdc2 100644
--- a/locations/field_locations.go
+++ b/locations/field_locations.go
@@ -18,8 +18,17 @@ import (
 	dpb "github.com/golang/protobuf/protoc-gen-go/descriptor"
 	"github.com/jhump/protoreflect/desc"
 	apb "google.golang.org/genproto/googleapis/api/annotations"
+	protoimpl "google.golang.org/protobuf/runtime/protoimpl"
 )
 
+// FieldOption returns the precise location for the given extension defintion on
+// the given field. This is useful for writing rules against custom extensions.
+//
+// Example: locations.FieldOption(field, fieldbehaviorpb.E_FieldBehavior)
+func FieldOption(f *desc.FieldDescriptor, e *protoimpl.ExtensionInfo) *dpb.SourceCodeInfo_Location {
+	return pathLocation(f, 8, int(e.TypeDescriptor().Number())) // FieldDescriptor.options == 8
+}
+
 // FieldResourceReference returns the precise location for a field's
 // resource reference annotation.
 func FieldResourceReference(f *desc.FieldDescriptor) *dpb.SourceCodeInfo_Location {


Test patch:
diff --git a/locations/field_locations_test.go b/locations/field_locations_test.go
index afb05ce..5bb7c7b 100644
--- a/locations/field_locations_test.go
+++ b/locations/field_locations_test.go
@@ -19,6 +19,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/jhump/protoreflect/desc"
+	apb "google.golang.org/genproto/googleapis/api/annotations"
 )
 
 func TestFieldLocations(t *testing.T) {
@@ -81,6 +82,22 @@ func TestFieldResourceReference(t *testing.T) {
 		}
 	`)
 	loc := FieldResourceReference(f.GetMessageTypes()[0].GetFields()[0])
+	// resource_reference annotation location is roughly line 4, column 19.
+	if diff := cmp.Diff(loc.GetSpan(), []int32{4, 19, 6, 3}); diff != "" {
+		t.Errorf(diff)
+	}
+}
+
+func TestFieldOption(t *testing.T) {
+	f := parse(t, `
+		import "google/api/resource.proto";
+		message GetBookRequest {
+		  string name = 1 [(google.api.resource_reference) = {
+		    type: "library.googleapis.com/Book"
+		  }];
+		}
+	`)
+	loc := FieldOption(f.GetMessageTypes()[0].GetFields()[0], apb.E_ResourceReference)
 	if diff := cmp.Diff(loc.GetSpan(), []int32{4, 19, 6, 3}); diff != "" {
 		t.Errorf(diff)
 	}
Repository: googleapis/api-linter
Pull request #1230 (2023-08-10 20:59:04)

feat(AIP-156): allow singleton list
Allow Singleton resources to have Standard List as per https://github.com/aip-dev/google.aip.dev/pull/1195


Patch:
diff --git a/docs/rules/0156/forbidden-methods.md b/docs/rules/0156/forbidden-methods.md
index 0963b9f..14d3dbf 100644
--- a/docs/rules/0156/forbidden-methods.md
+++ b/docs/rules/0156/forbidden-methods.md
@@ -2,7 +2,7 @@
 rule:
   aip: 156
   name: [core, '0156', forbidden-methods]
-  summary: Singletons must not define List, Create, or Delete methods.
+  summary: Singletons must not define Create, or Delete methods.
 permalink: /156/forbidden-methods
 redirect_from:
   - /0156/forbidden-methods
@@ -10,15 +10,15 @@ redirect_from:
 
 # Singletons: Forbidden methods
 
-This rule enforces that singleton resources do not define `List`, `Create`, or
-`Delete` methods, as mandated in [AIP-156][].
+This rule enforces that singleton resources do not define `Create`, or `Delete`
+methods, as mandated in [AIP-156][].
 
 ## Details
 
 This rule looks at any message with a `name` variable in the URI where the name
 ends in anything other than `*`. It assumes that this is a method operating on
-a singleton resource, and complains if the method is a `List`, `Create`, or
-`Delete` standard method.
+a singleton resource, and complains if the method is a `Create`, or `Delete`
+standard method.
 
 ## Examples
 
diff --git a/rules/aip0156/forbidden_methods.go b/rules/aip0156/forbidden_methods.go
index eb1caed..56f109c 100644
--- a/rules/aip0156/forbidden_methods.go
+++ b/rules/aip0156/forbidden_methods.go
@@ -42,8 +42,8 @@ var forbiddenMethods = &lint.MethodRule{
 		return false
 	},
 	LintMethod: func(m *desc.MethodDescriptor) []lint.Problem {
-		// Singletons should not use Create, List, or Delete.
-		for _, badPrefix := range []string{"Create", "List", "Delete"} {
+		// Singletons should not use Create, or Delete.
+		for _, badPrefix := range []string{"Create", "Delete"} {
 			if strings.HasPrefix(m.GetName(), badPrefix) {
 				return []lint.Problem{{
 					Message:    fmt.Sprintf("Singletons must not define %q methods.", badPrefix),


Test patch:
diff --git a/rules/aip0156/forbidden_methods_test.go b/rules/aip0156/forbidden_methods_test.go
index 1542365..8b3e144 100644
--- a/rules/aip0156/forbidden_methods_test.go
+++ b/rules/aip0156/forbidden_methods_test.go
@@ -29,8 +29,8 @@ func TestForbiddenMethods(t *testing.T) {
 	}{
 		{"ValidGet", "GetPublisherSettings", "/settings", testutils.Problems{}},
 		{"ValidUpdate", "UpdatePublisherSettings", "/settings", testutils.Problems{}},
+		{"ValidList", "ListPublisherSettings", "/settings", testutils.Problems{}},
 		{"InvalidCreate", "CreatePublisherSettings", "/settings", testutils.Problems{{Message: "Create"}}},
-		{"InvalidList", "ListPublisherSettings", "/settings", testutils.Problems{{Message: "List"}}},
 		{"InvalidDelete", "DeletePublisherSettings", "/settings", testutils.Problems{{Message: "Delete"}}},
 		{"Irrelevant", "CreatePublisher", "", testutils.Problems{}},
 	} {
Repository: googleapis/api-linter
Pull request #1250 (2023-09-11 23:33:05)

fix(AIP-121): ignore standard method lookalikes
Ignore methods that look like standard methods but don't actually operate on a resource as a standard method would. This prevents false positive findings related for messages that aren't actually resources but have some standard method lookalikes. Similar to #1235


Patch:
diff --git a/rules/aip0121/resource_must_support_get.go b/rules/aip0121/resource_must_support_get.go
index ff414fa..7dc66c3 100644
--- a/rules/aip0121/resource_must_support_get.go
+++ b/rules/aip0121/resource_must_support_get.go
@@ -33,16 +33,16 @@ var resourceMustSupportGet = &lint.ServiceRule{
 		// Iterate all RPCs and try to find resources. Mark the
 		// resources which have a Get method, and which ones do not.
 		for _, m := range s.GetMethods() {
-			if utils.IsGetMethod(m) {
+			if utils.IsGetMethod(m) && utils.IsResource(utils.GetResponseType(m)) {
 				t := utils.GetResource(m.GetOutputType()).GetType()
 				resourcesWithGet.Add(t)
 			} else if utils.IsCreateMethod(m) || utils.IsUpdateMethod(m) {
-				if msg := utils.GetResponseType(m); msg != nil {
+				if msg := utils.GetResponseType(m); msg != nil && utils.IsResource(msg) {
 					t := utils.GetResource(msg).GetType()
 					resourcesWithOtherMethods.Add(t)
 				}
 			} else if utils.IsListMethod(m) {
-				if msg := utils.GetListResourceMessage(m); msg != nil {
+				if msg := utils.GetListResourceMessage(m); msg != nil && utils.IsResource(msg) {
 					t := utils.GetResource(msg).GetType()
 					resourcesWithOtherMethods.Add(t)
 				}


Test patch:
diff --git a/rules/aip0121/resource_must_support_get_test.go b/rules/aip0121/resource_must_support_get_test.go
index 25134dd..07af817 100644
--- a/rules/aip0121/resource_must_support_get_test.go
+++ b/rules/aip0121/resource_must_support_get_test.go
@@ -51,6 +51,15 @@ func TestResourceMustSupportGet(t *testing.T) {
 			rpc GetBook(GetBookRequest) returns (Book) {};
 			rpc UpdateBook(UpdateBookRequest) returns (Book) {};
 		`, nil},
+		{"ValidIgnoreNonResourceUpdate", `
+			rpc UpdateBook(UpdateBookRequest) returns (Other) {};
+		`, nil},
+		{"ValidIgnoreNonResourceCreate", `
+			rpc CreateBook(CreateBookRequest) returns (Other) {};
+		`, nil},
+		{"ValidIgnoreNonResourceList", `
+			rpc ListBooks(ListBooksRequest) returns (RepeatedOther) {};
+		`, nil},
 		{"InvalidCreateOnly", `
 			rpc CreateBook(CreateBookRequest) returns (Book) {};
 		`, []lint.Problem{
@@ -124,6 +133,12 @@ func TestResourceMustSupportGet(t *testing.T) {
 					repeated Book books = 1;
 					string next_page_token = 2;
 				 }
+
+				 message Other {}
+
+				 message RepeatedOther {
+					repeated Other others = 1;
+				 }
 			`, test)
 			s := file.GetServices()[0]
 			got := resourceMustSupportGet.Lint(file)
Repository: googleapis/api-linter
Pull request #1263 (2023-10-04 22:49:29)

fix(AIP-203): remove resource name identifier suggestion
As [reported internally](https://b.corp.google.com/issues/302714576), the suggestion from this finding replaces the entire line rather than just adding the extension.


Patch:
diff --git a/rules/aip0203/resource_name_identifier.go b/rules/aip0203/resource_name_identifier.go
index 3f42b0f..556bc51 100644
--- a/rules/aip0203/resource_name_identifier.go
+++ b/rules/aip0203/resource_name_identifier.go
@@ -33,7 +33,6 @@ var resourceNameIdentifier = &lint.MessageRule{
 				Message:    "resource name field must have field_behavior IDENTIFIER",
 				Descriptor: f,
 				Location:   locations.FieldOption(f, fpb.E_FieldBehavior),
-				Suggestion: "(google.api.field_behavior) = IDENTIFIER",
 			}}
 		}
 


Test patch:
diff --git a/rules/aip0203/resource_name_identifier_test.go b/rules/aip0203/resource_name_identifier_test.go
index 436562f..02c206d 100644
--- a/rules/aip0203/resource_name_identifier_test.go
+++ b/rules/aip0203/resource_name_identifier_test.go
@@ -40,12 +40,12 @@ func TestResourceNameIdentifier(t *testing.T) {
 		{
 			name:     "InvalidNoFieldBehavior",
 			Field:    "string name = 1;",
-			problems: testutils.Problems{{Message: "field_behavior IDENTIFIER", Suggestion: "(google.api.field_behavior) = IDENTIFIER"}},
+			problems: testutils.Problems{{Message: "field_behavior IDENTIFIER"}},
 		},
 		{
 			name:     "InvalidMissingIdentifier",
 			Field:    "string name = 1 [(google.api.field_behavior) = REQUIRED];",
-			problems: testutils.Problems{{Message: "field_behavior IDENTIFIER", Suggestion: "(google.api.field_behavior) = IDENTIFIER"}},
+			problems: testutils.Problems{{Message: "field_behavior IDENTIFIER"}},
 		},
 	}
Repository: googleapis/api-linter
Pull request #1293 (2023-12-01 15:01:22)

fix(AIP-123): disallow spaces in pattern
Closes https://github.com/googleapis/api-linter/issues/1090.


Patch:
diff --git a/rules/aip0123/resource_pattern.go b/rules/aip0123/resource_pattern.go
index f0c7bc5..8a37816 100644
--- a/rules/aip0123/resource_pattern.go
+++ b/rules/aip0123/resource_pattern.go
@@ -46,9 +46,11 @@ func lintResourcePattern(resource *annotations.ResourceDescriptor, desc desc.Des
 	}
 
 	// Ensure that the constant segments of the pattern uses camel case,
-	// not snake case.
+	// not snake case, and there are no spaces.
 	for _, pattern := range resource.GetPattern() {
-		if strings.Contains(getPlainPattern(pattern), "_") {
+		plainPattern := getPlainPattern(pattern)
+
+		if strings.Contains(plainPattern, "_") {
 			return []lint.Problem{{
 				Message: fmt.Sprintf(
 					"Resource patterns should use camel case (apart from the variable names), such as %q.",
@@ -58,6 +60,13 @@ func lintResourcePattern(resource *annotations.ResourceDescriptor, desc desc.Des
 				Location:   loc,
 			}}
 		}
+		if strings.Contains(plainPattern, " ") {
+			return []lint.Problem{{
+				Message:    "Resource patterns should not have spaces",
+				Descriptor: desc,
+				Location:   loc,
+			}}
+		}
 	}
 	return nil
 }


Test patch:
diff --git a/rules/aip0123/resource_pattern_test.go b/rules/aip0123/resource_pattern_test.go
index 6595655..276de85 100644
--- a/rules/aip0123/resource_pattern_test.go
+++ b/rules/aip0123/resource_pattern_test.go
@@ -32,6 +32,9 @@ func TestResourcePattern(t *testing.T) {
 		{"SnakeCase", `pattern: "book_publishers/{book_publisher}/books/{book}"`, testutils.Problems{{
 			Message: "bookPublishers/{book_publisher}/books/{book}",
 		}}},
+		{"HasSpaces", `pattern: "publishers/{publisher}/ books /{book}"`, testutils.Problems{{
+			Message: "Resource patterns should not have spaces",
+		}}},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			f := testutils.ParseProto3Tmpl(t, `
Repository: googleapis/gapic-generator-go
Pull request #20 (2018-10-01 18:51:12)

sample: check for error when initializing client

pongad: @vchudnov-g PTAL
pongad: I'll merge since reviewers already LGTMed.

Patch:
diff --git a/cmd/gen-go-sample/main.go b/cmd/gen-go-sample/main.go
index 92d4728..f4a079d 100644
--- a/cmd/gen-go-sample/main.go
+++ b/cmd/gen-go-sample/main.go
@@ -244,9 +244,13 @@ func (g *generator) genSample(ifaceName, methName, regTag string, valSet SampleV
 
 	p("func sample%s(%s) {", methName, argStr)
 	p("  ctx := context.Background()")
-	p("  c := %s.New%sClient(ctx)", g.clientPkg.Name, pbinfo.ReduceServName(serv.GetName(), g.clientPkg.Name))
+	p("  c, err := %s.New%sClient(ctx)", g.clientPkg.Name, pbinfo.ReduceServName(serv.GetName(), g.clientPkg.Name))
+	p("  if err != nil {")
+	p("    log.Fatal(err)")
+	p("  }")
 	p("")
 	g.imports[pbinfo.ImportSpec{Path: "context"}] = true
+	g.imports[pbinfo.ImportSpec{Path: "log"}] = true
 
 	for i, name := range argNames {
 		var sb strings.Builder


Test patch:
diff --git a/cmd/gen-go-sample/main_test.go b/cmd/gen-go-sample/main_test.go
index e5bd016..c2660af 100644
--- a/cmd/gen-go-sample/main_test.go
+++ b/cmd/gen-go-sample/main_test.go
@@ -84,12 +84,12 @@ func TestSample(t *testing.T) {
 	}
 	file := &descriptor.FileDescriptorProto{
 		Options: &descriptor.FileOptions{
-			GoPackage: proto.String("path/to/pb/foo;foo"),
+			GoPackage: proto.String("path.to/pb/foo;foo"),
 		},
 	}
 
 	g := generator{
-		clientPkg: pbinfo.ImportSpec{Path: "path/to/client/foo", Name: "foo"},
+		clientPkg: pbinfo.ImportSpec{Path: "path.to/client/foo", Name: "foo"},
 		imports:   map[pbinfo.ImportSpec]bool{},
 		descInfo: pbinfo.Info{
 			Serv: map[string]*descriptor.ServiceDescriptorProto{
diff --git a/cmd/gen-go-sample/testdata/sample.want b/cmd/gen-go-sample/testdata/sample.want
index 56559c5..0006387 100644
--- a/cmd/gen-go-sample/testdata/sample.want
+++ b/cmd/gen-go-sample/testdata/sample.want
@@ -19,14 +19,19 @@ import(
  "context"
  "flag"
  "fmt"
-foo "path/to/client/foo"
-foopb "path/to/pb/foo"
+ "log"
+
+foo "path.to/client/foo"
+foopb "path.to/pb/foo"
 )
 // [START awesome_region]
 
 func sampleMyMethod(arg0 string) {
 	ctx := context.Background()
-	c := foo.NewClient(ctx)
+	c, err := foo.NewClient(ctx)
+	if err != nil {
+		log.Fatal(err)
+	}
 
 // arg0 := 42
 req := &foopb.InputType{
Repository: googleapis/gapic-generator-go
Pull request #29 (2018-10-10 22:18:06)

sample: make sample handle error properly


Patch:
diff --git a/cmd/gen-go-sample/main.go b/cmd/gen-go-sample/main.go
index 1d68c74..8279c3b 100644
--- a/cmd/gen-go-sample/main.go
+++ b/cmd/gen-go-sample/main.go
@@ -253,15 +253,14 @@ func (g *generator) genSample(ifaceName, methName, regTag string, valSet SampleV
 		argStr = sb.String()[2:]
 	}
 
-	p("func sample%s(%s) {", methName, argStr)
+	p("func sample%s(%s) error {", methName, argStr)
 	p("  ctx := context.Background()")
 	p("  c, err := %s.New%sClient(ctx)", g.clientPkg.Name, pbinfo.ReduceServName(serv.GetName(), g.clientPkg.Name))
 	p("  if err != nil {")
-	p("    log.Fatal(err)")
+	p("    return err")
 	p("  }")
 	p("")
 	g.imports[pbinfo.ImportSpec{Path: "context"}] = true
-	g.imports[pbinfo.ImportSpec{Path: "log"}] = true
 
 	for i, name := range argNames {
 		var sb strings.Builder
@@ -298,7 +297,7 @@ func (g *generator) genSample(ifaceName, methName, regTag string, valSet SampleV
 	// TODO(pongad): handle non-unary
 	p("  resp, err := c.%s(ctx, req)", methName)
 	p("  if err != nil {")
-	p("    // TODO: Handle error.")
+	p("    return err")
 	p("  }")
 	p("")
 
@@ -318,6 +317,7 @@ func (g *generator) genSample(ifaceName, methName, regTag string, valSet SampleV
 		}
 	}
 
+	p("return nil")
 	p("}")
 	p("")
 	p("// [END %s]", regTag)


Test patch:
diff --git a/cmd/gen-go-sample/testdata/sample.want b/cmd/gen-go-sample/testdata/sample.want
index fe4ba61..16137c4 100644
--- a/cmd/gen-go-sample/testdata/sample.want
+++ b/cmd/gen-go-sample/testdata/sample.want
@@ -19,18 +19,17 @@ import(
  "context"
  "flag"
  "fmt"
- "log"
 
 foo "path.to/client/foo"
 foopb "path.to/pb/foo"
 )
 // [START awesome_region]
 
-func sampleMyMethod(arg0 int64, arg1 string) {
+func sampleMyMethod(arg0 int64, arg1 string) error {
 	ctx := context.Background()
 	c, err := foo.NewClient(ctx)
 	if err != nil {
-		log.Fatal(err)
+		return err
 	}
 
 // arg0 := 42
@@ -48,12 +47,13 @@ req := &foopb.InputType{
 }
 	resp, err := c.MyMethod(ctx, req)
 	if err != nil {
-		// TODO: Handle error.
+		return err
 	}
 
 outA := resp.A
 fmt.Printf("x = %v\n", resp.A.X)
 fmt.Printf("y = %v\n", outA.Y)
+	return nil
 }
 
 // [END awesome_region]