Repository: googleapis/api-linter
Pull request #1127 (2023-03-30 21:51:32)

feat(AIP-216): Enforce OUTPUT_ONLY behavior for State enums
AIP-216 calls for `State` enums to be
OUTPUT_ONLY.

Add a rule that requires OUTPUT_ONLY for fields which have an enum type ending with `State`.

Note that the AIP does not mention rules for the *field name* for state fields, we ignore the field name in the linter.

noahdietz: You will need to add a documentation page for this to ./docs/rules/aip0216. Take a look at some other examples, copy one and change it up to match this rule.
hoskeri: > You will need to add a documentation page for this to ./docs/rules/aip0216. Take a look at some other examples, copy one and change it up to match this rule.

Added the documentation.

shwoodard: Yeah, changing the naming is all I'm advocating for since it's the field and that is OUTPUT_ONLY and not the enum. Thanks @noahdietz!
shwoodard: I approved this but left one comment. Please update.
shwoodard: > Member

![image](https://user-images.githubusercontent.com/34149/230123833-4b478b82-252f-4323-9ae1-d718e4f55cd6.png)

This image depicts a good reason why **not** to force push.
noahdietz: I will set this for automerge since we got double approval.

Patch:
diff --git a/docs/rules/0216/state-field-output-only.md b/docs/rules/0216/state-field-output-only.md
new file mode 100644
index 0000000..46cde23
--- /dev/null
+++ b/docs/rules/0216/state-field-output-only.md
@@ -0,0 +1,78 @@
+---
+rule:
+  aip: 216
+  name: [core, '0216', state-field-output-only]
+  summary: Lifecycle State enum fields should be marked OUTPUT_ONLY
+permalink: /216/state-field-output-only
+redirect_from:
+  - /0216/state-field-output-only
+---
+
+# States
+
+This rule requires that all lifecycle state fields whose names end with `State`
+are marked as `OUTPUT_ONLY`, as mandated in [AIP-216][].
+
+## Details
+
+This rule iterates over message fields that have an `enum` type, and the type
+name ends with `State`. Each field should have the annotation
+`[(google.api.field_behavior) = OUTPUT_ONLY]`.
+
+Note that the field name is ignored for the purposes of this rule.
+
+## Examples
+
+**Incorrect** code for this rule:
+
+```proto
+// Incorrect.
+enum State {  // Should be `State`.
+  STATUS_UNSPECIFIED = 0;
+}
+
+State state = 1; // Should be marked OUTPUT_ONLY
+
+```
+
+```proto
+// Incorrect.
+enum BookState {
+  BOOK_STATUS_UNSPECIFIED = 0;
+  HARDCOVER = 1;
+}
+
+BookState state = 1; // Should be marked OUTPUT_ONLY
+```
+
+**Correct** code for this rule:
+
+```proto
+// Correct.
+enum State {
+  STATE_UNSPECIFIED = 0;
+}
+
+State state = 1 [(google.api.field_behavior) = OUTPUT_ONLY];
+```
+
+## Disabling
+
+If you need to violate this rule, use a leading comment above the field
+Remember to also include an [aip.dev/not-precedent][] comment explaining why.
+
+```proto
+enum BookState {
+  UNSPECIFIED = 0;
+}
+
+// (-- api-linter: core::0216::state-field-output-only=disabled
+//     aip.dev/not-precedent: We need to do this because reasons. --)
+BookState state = 1;
+```
+
+If you need to violate this rule for an entire file, place the comment at the
+top of the file.
+
+[aip-216]: https://aip.dev/216
+[aip.dev/not-precedent]: https://aip.dev/not-precedent
diff --git a/rules/aip0216/aip0216.go b/rules/aip0216/aip0216.go
index 362fb7f..d2d84d2 100644
--- a/rules/aip0216/aip0216.go
+++ b/rules/aip0216/aip0216.go
@@ -25,5 +25,6 @@ func AddRules(r lint.RuleRegistry) error {
 		nesting,
 		synonyms,
 		valueSynonyms,
+		stateFieldOutputOnly,
 	)
 }
diff --git a/rules/aip0216/state_field_output_only.go b/rules/aip0216/state_field_output_only.go
new file mode 100644
index 0000000..fa63aed
--- /dev/null
+++ b/rules/aip0216/state_field_output_only.go
@@ -0,0 +1,53 @@
+// 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 aip0216
+
+import (
+	"github.com/googleapis/api-linter/lint"
+	"github.com/googleapis/api-linter/rules/internal/utils"
+	"github.com/jhump/protoreflect/desc"
+	"google.golang.org/genproto/googleapis/api/annotations"
+	"strings"
+)
+
+var stateFieldOutputOnly = &lint.FieldRule{
+	Name: lint.NewRuleName(216, "state-field-output-only"),
+	OnlyIf: func(f *desc.FieldDescriptor) bool {
+		// We care about the name of the State enum type.
+		// AIP 0216 makes no mention of the state field name.
+		et := f.GetEnumType()
+		if et == nil {
+			return false
+		}
+
+		if !strings.HasSuffix(et.GetName(), "State") {
+			return false
+		}
+
+		return true
+	},
+	LintField: func(f *desc.FieldDescriptor) []lint.Problem {
+		behaviors := utils.GetFieldBehavior(f)
+		if !behaviors.Contains(annotations.FieldBehavior_OUTPUT_ONLY.String()) {
+			return []lint.Problem{
+				{
+					Message:    "state fields must have field_behavior OUTPUT_ONLY",
+					Descriptor: f,
+				},
+			}
+		}
+		return nil
+	},
+}


Test patch:
diff --git a/rules/aip0216/state_field_output_only_test.go b/rules/aip0216/state_field_output_only_test.go
new file mode 100644
index 0000000..00d2542
--- /dev/null
+++ b/rules/aip0216/state_field_output_only_test.go
@@ -0,0 +1,83 @@
+// 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 aip0216
+
+import (
+	"testing"
+
+	"github.com/googleapis/api-linter/rules/internal/testutils"
+)
+
+func TestStateFieldOutputOnly(t *testing.T) {
+	tests := []struct {
+		name          string
+		FieldName     string
+		FieldType     string
+		FieldBehavior string
+		problems      testutils.Problems
+	}{
+		// Accepted
+		{"ValidState", "state", "State", "[(google.api.field_behavior) = OUTPUT_ONLY]", testutils.Problems{}},
+		{"ValidOtherFieldName", "country", "State", "[(google.api.field_behavior) = OUTPUT_ONLY]", testutils.Problems{}},
+		{"ValidStateSuffix", "state", "WritersBlockState", "[(google.api.field_behavior) = OUTPUT_ONLY]", testutils.Problems{}},
+
+		// No Annotation
+		{"InvalidState", "state", "State", "", testutils.Problems{{Message: "OUTPUT_ONLY"}}},
+		{"InvalidWithSuffix", "state", "WritersBlockState", "", testutils.Problems{{Message: "OUTPUT_ONLY"}}},
+		{"InvalidWithFieldName", "city", "State", "", testutils.Problems{{Message: "OUTPUT_ONLY"}}},
+
+		// Ignored
+		{"NotAStateField", "state", "StateOfDespair", "", testutils.Problems{}},
+		{"NotAnEnum", "state", "StateOfState", "", testutils.Problems{}},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			f := testutils.ParseProto3Tmpl(t, `
+			import "google/api/field_behavior.proto";
+
+			message Book {
+				enum State {
+					STATE_UNSPECIFIED = 0;
+					ACTIVE = 1;
+				}
+
+				message StateOfState {
+					string name = 1;
+				}
+
+				// state enums end with 'State'
+				enum WritersBlockState {
+					WRITERS_BLOCK_STATE_UNSPECIFIED = 0;
+					BLOCKED = 1;
+				}
+
+				// not a state enum
+				enum StateOfDespair {
+					STATE_OF_DESPAIR_UNSPECIFIED = 0;
+					NOTREALLY = 1;
+				}
+
+				string other_state = 1;
+				{{.FieldType}} {{.FieldName}} = 2 {{.FieldBehavior}};
+			}
+		`, test)
+
+			field := f.GetMessageTypes()[0].GetFields()[1]
+			if diff := test.problems.SetDescriptor(field).Diff(stateFieldOutputOnly.Lint(f)); diff != "" {
+				t.Errorf(diff)
+			}
+		})
+	}
+}