Repository: googleapis/api-linter
Pull request #369 (2019-11-21 00:18:38)

[AIP-191] Check csharp_namespace for idiomaticity.

lukesneeringer: This will need a slight change after #370 goes in. Fine to approve both (tests will just start failing after it is merged).

Patch:
diff --git a/docs/rules/0191/csharp-namespace.md b/docs/rules/0191/csharp-namespace.md
new file mode 100644
index 0000000..c275b23
--- /dev/null
+++ b/docs/rules/0191/csharp-namespace.md
@@ -0,0 +1,101 @@
+---
+rule:
+  aip: 191
+  name: [core, '0191', csharp-namespace]
+  summary: The `option csharp_namespace` annotation should be idiomatic if set.
+permalink: /191/java-package
+redirect_from:
+  - /0191/csharp-namespace
+---
+
+# C# namespace annotation
+
+This rule enforces that if a proto file for a public API surface sets
+`option csharp_namespace`, that it uses language idiomatic conventions, as
+mandated in [AIP-191][].
+
+## Details
+
+This rule looks at each proto file, and complains if the `csharp_namespace`
+file annotation uses anything other than upper camel case, or includes
+characters other than letters, numbers, and `.`.
+
+It also ensures that versions with stability (e.g. `V1Beta1`) are capitalized
+appropriately.
+
+## Examples
+
+### Case
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+syntax = "proto3";
+
+package google.example.v1;
+
+option csharp_namespace = "google.example.v1";
+```
+
+```proto
+// Incorrect.
+syntax = "proto3";
+
+package google.example.v1;
+
+option csharp_namespace = "Google::Example::V1";
+```
+
+**Correct** code for this rule:
+
+```proto
+// Correct.
+syntax = "proto3";
+
+package google.example.v1;
+
+option csharp_namespace = "Google.Example.V1";
+```
+
+### Versions with stability
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+syntax = "proto3";
+
+package google.example.v1beta1;
+
+option csharp_namespace = "Google.Example.V1beta1"; // Should be V1Beta1.
+```
+
+**Correct** code for this rule:
+
+```proto
+// Correct.
+syntax = "proto3";
+
+package google.example.v1beta1;
+
+option csharp_namespace = "Google.Example.V1Beta1";
+```
+
+## Disabling
+
+If you need to violate this rule, use a comment at the top of the file.
+Remember to also include an [aip.dev/not-precedent][] comment explaining why.
+
+```proto
+// (-- api-linter: core::0191::java-package=disabled
+//     aip.dev/not-precedent: We need to do this because reasons. --)
+syntax = "proto3";
+
+package google.example.v1;
+
+option csharp_namespace = "google.example.v1";
+```
+
+[aip-191]: https://aip.dev/191
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
diff --git a/locations/file_locations.go b/locations/file_locations.go
index 6813005..d1d82e6 100644
--- a/locations/file_locations.go
+++ b/locations/file_locations.go
@@ -34,3 +34,12 @@ func FileSyntax(f *desc.FileDescriptor) *dpb.SourceCodeInfo_Location {
 func FilePackage(f *desc.FileDescriptor) *dpb.SourceCodeInfo_Location {
 	return pathLocation(f, 2) // FileDescriptor.package == 2
 }
+
+// FileCsharpNamespace returns the location of the csharp_namespace file option
+// in a file descriptor.
+//
+// If the location can not be found (for example, because there is no
+// csharp_namespace option), it returns nil.
+func FileCsharpNamespace(f *desc.FileDescriptor) *dpb.SourceCodeInfo_Location {
+	return pathLocation(f, 8, 37) // 8 == options, 37 == csharp_namespace
+}
diff --git a/rules/aip0191/aip0191.go b/rules/aip0191/aip0191.go
index 9c74eb9..5156e2e 100644
--- a/rules/aip0191/aip0191.go
+++ b/rules/aip0191/aip0191.go
@@ -26,6 +26,7 @@ import (
 func AddRules(r lint.RuleRegistry) error {
 	return r.Register(
 		191,
+		csharpNamespace,
 		filename,
 		javaMultipleFiles,
 		javaOuterClassname,
diff --git a/rules/aip0191/csharp_namespace.go b/rules/aip0191/csharp_namespace.go
new file mode 100644
index 0000000..1f10b90
--- /dev/null
+++ b/rules/aip0191/csharp_namespace.go
@@ -0,0 +1,67 @@
+// Copyright 2019 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package aip0191
+
+import (
+	"fmt"
+	"regexp"
+	"strings"
+
+	"github.com/googleapis/api-linter/lint"
+	"github.com/googleapis/api-linter/locations"
+	"github.com/jhump/protoreflect/desc"
+	"github.com/stoewer/go-strcase"
+)
+
+var csharpNamespace = &lint.FileRule{
+	Name: lint.NewRuleName(191, "csharp-namespace"),
+	LintFile: func(f *desc.FileDescriptor) []lint.Problem {
+		if ns := f.GetFileOptions().GetCsharpNamespace(); ns != "" {
+			// Check for invalid characters.
+			if !csharpValidChars.MatchString(ns) {
+				return []lint.Problem{{
+					Message:    "Invalid characters: C# namespaces only allow [A-Za-z0-9.].",
+					Descriptor: f,
+					Location:   locations.FileCsharpNamespace(f),
+				}}
+			}
+
+			// Check that upper camel case is used.
+			upperCamel := []string{}
+			for _, segment := range strings.Split(ns, ".") {
+				wantSegment := csharpVersionRegexp.ReplaceAllStringFunc(
+					strcase.UpperCamelCase(segment),
+					func(s string) string {
+						stability := csharpVersionRegexp.FindStringSubmatch(s)[1]
+						return strings.ReplaceAll(s, stability, strings.Title(stability))
+					},
+				)
+				upperCamel = append(upperCamel, wantSegment)
+			}
+			if want := strings.Join(upperCamel, "."); ns != want {
+				return []lint.Problem{{
+					Message:    "C# namespaces use UpperCamelCase.",
+					Suggestion: fmt.Sprintf("option csharp_namespace = %q;", want),
+					Descriptor: f,
+					Location:   locations.FileCsharpNamespace(f),
+				}}
+			}
+		}
+		return nil
+	},
+}
+
+var csharpValidChars = regexp.MustCompile("^[A-Za-z0-9.]+$")
+var csharpVersionRegexp = regexp.MustCompile("[0-9]+(alpha|beta)")


Test patch:
diff --git a/locations/file_locations_test.go b/locations/file_locations_test.go
index f3f559b..80992ce 100644
--- a/locations/file_locations_test.go
+++ b/locations/file_locations_test.go
@@ -30,6 +30,8 @@ func TestLocations(t *testing.T) {
 
 		package google.api.linter;
 
+		option csharp_namespace = "Google.Api.Linter";
+
 		message Foo {
 			string bar = 1;
 		}
@@ -44,6 +46,7 @@ func TestLocations(t *testing.T) {
 		}{
 			{"Syntax", FileSyntax, []int32{1, 0, int32(len("syntax = \"proto3\";"))}},
 			{"Package", FilePackage, []int32{3, 0, int32(len("package google.api.linter;"))}},
+			{"CsharpNamespace", FileCsharpNamespace, []int32{5, 0, int32(len(`option csharp_namespace = "Google.Api.Linter";`))}},
 		}
 		for _, test := range tests {
 			t.Run(test.testName, func(t *testing.T) {
diff --git a/rules/aip0191/csharp_namespace_test.go b/rules/aip0191/csharp_namespace_test.go
new file mode 100644
index 0000000..a24ee3c
--- /dev/null
+++ b/rules/aip0191/csharp_namespace_test.go
@@ -0,0 +1,54 @@
+// Copyright 2019 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package aip0191
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/googleapis/api-linter/rules/internal/testutils"
+)
+
+func TestCsharpNamespace(t *testing.T) {
+	for _, test := range []struct {
+		name            string
+		CsharpNamespace string
+		problems        testutils.Problems
+	}{
+		{"Valid", "Google.Example.V1", testutils.Problems{}},
+		{"ValidBeta", "Google.Example.V1Beta1", testutils.Problems{}},
+		{"InvalidBadChars", "Google:Example:V1", testutils.Problems{{Message: "Invalid characters"}}},
+		{"Invalid", "google.example.v1", testutils.Problems{{
+			Suggestion: fmt.Sprintf("option csharp_namespace = %q;", "Google.Example.V1"),
+		}}},
+		{"InvalidVersion", "Google.Example.v1", testutils.Problems{{
+			Suggestion: fmt.Sprintf("option csharp_namespace = %q;", "Google.Example.V1"),
+		}}},
+		{"InvalidBeta", "Google.Example.V1beta1", testutils.Problems{{
+			Suggestion: fmt.Sprintf("option csharp_namespace = %q;", "Google.Example.V1Beta1"),
+		}}},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			f := testutils.ParseProto3Tmpl(t, `
+				package google.example.v1;
+
+				option csharp_namespace = "{{.CsharpNamespace}}";
+			`, test)
+			if diff := test.problems.SetDescriptor(f).Diff(csharpNamespace.Lint(f)); diff != "" {
+				t.Errorf(diff)
+			}
+		})
+	}
+}