From 098d1487c2d50fd52ef9bc7f00dfc9e8e022c161 Mon Sep 17 00:00:00 2001 From: noahdietz Date: Thu, 29 Feb 2024 12:28:14 -0800 Subject: [PATCH] feat: remove legacy list revisions rules --- docs/rules/0162/list-revisions-http-body.md | 66 -------------- docs/rules/0162/list-revisions-http-method.md | 65 ------------- .../0162/list-revisions-http-uri-suffix.md | 64 ------------- .../list-revisions-request-message-name.md | 66 -------------- .../list-revisions-request-name-behavior.md | 81 ----------------- .../0162/list-revisions-request-name-field.md | 91 ------------------- .../list-revisions-request-name-reference.md | 77 ---------------- ...ist-revisions-request-no-order-by-field.md | 85 ----------------- .../list-revisions-response-message-name.md | 66 -------------- rules/aip0162/aip0162.go | 13 --- rules/aip0162/list_revisions_http_body.go | 27 ------ .../aip0162/list_revisions_http_body_test.go | 57 ------------ rules/aip0162/list_revisions_http_method.go | 27 ------ .../list_revisions_http_method_test.go | 56 ------------ .../aip0162/list_revisions_http_uri_suffix.go | 41 --------- .../list_revisions_http_uri_suffix_test.go | 56 ------------ .../list_revisions_request_message_name.go | 27 ------ ...ist_revisions_request_message_name_test.go | 50 ---------- .../list_revisions_request_name_behavior.go | 29 ------ ...st_revisions_request_name_behavior_test.go | 49 ---------- .../list_revisions_request_name_field.go | 27 ------ .../list_revisions_request_name_field_test.go | 51 ----------- .../list_revisions_request_name_reference.go | 29 ------ ...t_revisions_request_name_reference_test.go | 49 ---------- ...ist_revisions_request_no_order_by_field.go | 34 ------- ...evisions_request_no_order_by_field_test.go | 46 ---------- .../list_revisions_response_message_name.go | 26 ------ ...st_revisions_response_message_name_test.go | 48 ---------- rules/internal/utils/method.go | 46 +++++++--- rules/internal/utils/method_test.go | 59 ++++++++++-- 30 files changed, 84 insertions(+), 1424 deletions(-) delete mode 100644 docs/rules/0162/list-revisions-http-body.md delete mode 100644 docs/rules/0162/list-revisions-http-method.md delete mode 100644 docs/rules/0162/list-revisions-http-uri-suffix.md delete mode 100644 docs/rules/0162/list-revisions-request-message-name.md delete mode 100644 docs/rules/0162/list-revisions-request-name-behavior.md delete mode 100644 docs/rules/0162/list-revisions-request-name-field.md delete mode 100644 docs/rules/0162/list-revisions-request-name-reference.md delete mode 100644 docs/rules/0162/list-revisions-request-no-order-by-field.md delete mode 100644 docs/rules/0162/list-revisions-response-message-name.md delete mode 100644 rules/aip0162/list_revisions_http_body.go delete mode 100644 rules/aip0162/list_revisions_http_body_test.go delete mode 100644 rules/aip0162/list_revisions_http_method.go delete mode 100644 rules/aip0162/list_revisions_http_method_test.go delete mode 100644 rules/aip0162/list_revisions_http_uri_suffix.go delete mode 100644 rules/aip0162/list_revisions_http_uri_suffix_test.go delete mode 100644 rules/aip0162/list_revisions_request_message_name.go delete mode 100644 rules/aip0162/list_revisions_request_message_name_test.go delete mode 100644 rules/aip0162/list_revisions_request_name_behavior.go delete mode 100644 rules/aip0162/list_revisions_request_name_behavior_test.go delete mode 100644 rules/aip0162/list_revisions_request_name_field.go delete mode 100644 rules/aip0162/list_revisions_request_name_field_test.go delete mode 100644 rules/aip0162/list_revisions_request_name_reference.go delete mode 100644 rules/aip0162/list_revisions_request_name_reference_test.go delete mode 100644 rules/aip0162/list_revisions_request_no_order_by_field.go delete mode 100644 rules/aip0162/list_revisions_request_no_order_by_field_test.go delete mode 100644 rules/aip0162/list_revisions_response_message_name.go delete mode 100644 rules/aip0162/list_revisions_response_message_name_test.go diff --git a/docs/rules/0162/list-revisions-http-body.md b/docs/rules/0162/list-revisions-http-body.md deleted file mode 100644 index fe49b6028..000000000 --- a/docs/rules/0162/list-revisions-http-body.md +++ /dev/null @@ -1,66 +0,0 @@ ---- -rule: - aip: 162 - name: [core, '0162', list-revisions-http-body] - summary: List Revisions methods must not have an HTTP body. -permalink: /162/list-revisions-http-body -redirect_from: - - /0162/list-revisions-http-body ---- - -# List Revisions methods: No HTTP body - -This rule enforces that all List Revisions RPCs omit the HTTP `body`, as mandated in -[AIP-162][]. - -## Details - -This rule looks at any method matching `List*Revisions`, and complains -if the HTTP `body` field is set. - -## Examples - -**Incorrect** code for this rule: - -```proto -// Incorrect. -rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListBookRevisionsResponse) { - option (google.api.http) = { - get: "/v1/{name=publishers/*/books/*}:listRevisions" - body: "*" // This should be absent. - }; -} -``` - -**Correct** code for this rule: - -```proto -// Correct. -rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListBookRevisionsResponse) { - option (google.api.http) = { - get: "/v1/{name=publishers/*/books/*}:listRevisions" - }; -} -``` - -## Disabling - -If you need to violate this rule, use a leading comment above the method. -Remember to also include an [aip.dev/not-precedent][] comment explaining why. - -```proto -// (-- api-linter: core::0162::list-revisions-http-body=disabled -// aip.dev/not-precedent: We need to do this because reasons. --) -rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListBookRevisionsResponse) { - option (google.api.http) = { - get: "/v1/{name=publishers/*/books/*}:listRevisions" - body: "*" - }; -} -``` - -If you need to violate this rule for an entire file, place the comment at the -top of the file. - -[aip-162]: https://aip.dev/162 -[aip.dev/not-precedent]: https://aip.dev/not-precedent diff --git a/docs/rules/0162/list-revisions-http-method.md b/docs/rules/0162/list-revisions-http-method.md deleted file mode 100644 index ebd617e73..000000000 --- a/docs/rules/0162/list-revisions-http-method.md +++ /dev/null @@ -1,65 +0,0 @@ ---- -rule: - aip: 162 - name: [core, '0162', list-revisions-http-method] - summary: List Revisions methods must use the GET HTTP verb. -permalink: /162/list-revisions-http-method -redirect_from: - - /0162/list-revisions-http-method ---- - -# List Revisions methods: GET HTTP verb - -This rule enforces that all List Revisions RPCs use the `GET` HTTP verb, as -mandated in [AIP-162][]. - -## Details - -This rule looks at any method matching `List*Revisions`, and complains -if the HTTP verb is anything other than `GET`. It _does_ check additional -bindings if they are present. - -## Examples - -**Incorrect** code for this rule: - -```proto -// Incorrect. -rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListBookRevisionsResponse) { - option (google.api.http) = { - post: "/v1/{name=publishers/*/books/*}:listRevisions" // Should be `get:`. - }; -} -``` - -**Correct** code for this rule: - -```proto -// Correct. -rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListBookRevisionsResponse) { - option (google.api.http) = { - get: "/v1/{name=publishers/*/books/*}:listRevisions" - }; -} -``` - -## Disabling - -If you need to violate this rule, use a leading comment above the method. -Remember to also include an [aip.dev/not-precedent][] comment explaining why. - -```proto -// (-- api-linter: core::0162::list-revisions-http-method=disabled -// aip.dev/not-precedent: We need to do this because reasons. --) -rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListBookRevisionsResponse) { - option (google.api.http) = { - post: "/v1/{name=publishers/*/books/*}:listRevisions" - }; -} -``` - -If you need to violate this rule for an entire file, place the comment at the -top of the file. - -[aip-162]: https://aip.dev/162 -[aip.dev/not-precedent]: https://aip.dev/not-precedent diff --git a/docs/rules/0162/list-revisions-http-uri-suffix.md b/docs/rules/0162/list-revisions-http-uri-suffix.md deleted file mode 100644 index b36c8b969..000000000 --- a/docs/rules/0162/list-revisions-http-uri-suffix.md +++ /dev/null @@ -1,64 +0,0 @@ ---- -rule: - aip: 162 - name: [core, '0162', list-revisions-http-uri-suffix] - summary: List Revisions methods must have the correct URI suffix -permalink: /162/list-revisions-http-uri-suffix -redirect_from: - - /0162/list-revisions-http-uri-suffix ---- - -# List Revisions methods: URI suffix - -This rule enforces that List Revisions methods include the `:listRevisions` suffix -in the REST URI, as mandated in [AIP-162][]. - -## Details - -This rule looks at any method matching `List*Revisions`, and -complains if the HTTP URI does not end with `:listRevisions`. - -## Examples - -**Incorrect** code for this rule: - -```proto -// Incorrect. -rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListBookRevisionsResponse) { - option (google.api.http) = { - get: "/v1/{name=publishers/*/books/*}:list" // Should end with `:listRevisions` - }; -} -``` - -**Correct** code for this rule: - -```proto -// Correct. -rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListBookRevisionsResponse) { - option (google.api.http) = { - get: "/v1/{name=publishers/*/books/*}:listRevisions" - }; -} -``` - -## Disabling - -If you need to violate this rule, use a leading comment above the method. -Remember to also include an [aip.dev/not-precedent][] comment explaining why. - -```proto -// (-- api-linter: core::0162::list-revisions-http-uri-suffix=disabled -// aip.dev/not-precedent: We need to do this because reasons. --) -rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListBookRevisionsResponse) { - option (google.api.http) = { - get: "/v1/{name=publishers/*/books/*}:list" - }; -} -``` - -If you need to violate this rule for an entire file, place the comment at the -top of the file. - -[aip-162]: https://aip.dev/162 -[aip.dev/not-precedent]: https://aip.dev/not-precedent diff --git a/docs/rules/0162/list-revisions-request-message-name.md b/docs/rules/0162/list-revisions-request-message-name.md deleted file mode 100644 index 3215d999c..000000000 --- a/docs/rules/0162/list-revisions-request-message-name.md +++ /dev/null @@ -1,66 +0,0 @@ ---- -rule: - aip: 162 - name: [core, '0162', list-revisions-request-message-name] - summary: List Revisions methods must have standardized request message names. -permalink: /162/list-revisions-request-message-name -redirect_from: - - /0162/list-revisions-request-message-name ---- - -# List Revisions methods: Request message - -This rule enforces that all List Revisions RPCs have a request message name of -`List*RevisionsRequest`, as mandated in [AIP-162][]. - -## Details - -This rule looks at any method matching `List*Revisions`, and complains -if the name of the corresponding input message does not match the name of the -RPC with the suffix `Request` appended. - -## Examples - -**Incorrect** code for this rule: - -```proto -// Incorrect. -// Should be `ListBookRevisionsRequest`. -rpc ListBookRevisions(ListRevisionsRequest) returns (ListBookRevisionsResponse) { - option (google.api.http) = { - get: "/v1/{name=publishers/*/books/*}:listRevisions" - }; -} -``` - -**Correct** code for this rule: - -```proto -// Correct. -rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListBookRevisionsResponse) { - option (google.api.http) = { - get: "/v1/{name=publishers/*/books/*}:listRevisions" - }; -} -``` - -## Disabling - -If you need to violate this rule, use a leading comment above the method. -Remember to also include an [aip.dev/not-precedent][] comment explaining why. - -```proto -// (-- api-linter: core::0162::list-revisions-request-message-name=disabled -// aip.dev/not-precedent: We need to do this because reasons. --) -rpc ListBookRevisions(ListRevisionsRequest) returns (ListBookRevisionsResponse) { - option (google.api.http) = { - get: "/v1/{name=publishers/*/books/*}:listRevisions" - }; -} -``` - -If you need to violate this rule for an entire file, place the comment at the -top of the file. - -[aip-162]: https://aip.dev/162 -[aip.dev/not-precedent]: https://aip.dev/not-precedent diff --git a/docs/rules/0162/list-revisions-request-name-behavior.md b/docs/rules/0162/list-revisions-request-name-behavior.md deleted file mode 100644 index 6313ee2bb..000000000 --- a/docs/rules/0162/list-revisions-request-name-behavior.md +++ /dev/null @@ -1,81 +0,0 @@ ---- -rule: - aip: 162 - name: [core, '0162', list-revisions-request-name-behavior] - summary: | - List Revisions requests should annotate the `name` field with `google.api.field_behavior`. -permalink: /162/list-revisions-request-name-behavior -redirect_from: - - /0162/list-revisions-request-name-behavior ---- - -# List Revisions requests: Name field behavior - -This rule enforces that all List Revisions requests have -`google.api.field_behavior` set to `REQUIRED` on their `string name` field, as -mandated in [AIP-162][]. - -## Details - -This rule looks at any message matching `List*RevisionsRequest` and complains if -the `name` field does not have a `google.api.field_behavior` annotation with a -value of `REQUIRED`. - -## Examples - -**Incorrect** code for this rule: - -```proto -// Incorrect. -message ListBookRevisionsRequest { - // The `google.api.field_behavior` annotation should also be included. - string name = 1 [ - (google.api.resource_reference).type = "library.googleapis.com/Book" - ]; - - int32 page_size = 2; - - string page_token = 3; -} -``` - -**Correct** code for this rule: - -```proto -// Correct. -message ListBookRevisionsRequest { - string name = 1 [ - (google.api.field_behavior) = REQUIRED, - (google.api.resource_reference).type = "library.googleapis.com/Book" - ]; - - int32 page_size = 2; - - string page_token = 3; -} -``` - -## 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 -message ListBookRevisionsRequest { - // (-- api-linter: core::0162::list-revisions-request-name-behavior=disabled - // aip.dev/not-precedent: We need to do this because reasons. --) - string name = 1 [ - (google.api.resource_reference).type = "library.googleapis.com/Book" - ]; - - int32 page_size = 2; - - string page_token = 3; -} -``` - -If you need to violate this rule for an entire file, place the comment at the -top of the file. - -[aip-162]: https://aip.dev/162 -[aip.dev/not-precedent]: https://aip.dev/not-precedent diff --git a/docs/rules/0162/list-revisions-request-name-field.md b/docs/rules/0162/list-revisions-request-name-field.md deleted file mode 100644 index 77f79af6e..000000000 --- a/docs/rules/0162/list-revisions-request-name-field.md +++ /dev/null @@ -1,91 +0,0 @@ ---- -rule: - aip: 162 - name: [core, '0162', list-revisions-request-name-field] - summary: List Revisions RPCs must have a `name` field in the request. -permalink: /162/list-revisions-request-name-field -redirect_from: - - /0162/list-revisions-request-name-field ---- - -# List Revisions requests: Name field - -This rule enforces that all List Revisions methods have a `string name` -field in the request message, as mandated in [AIP-162][]. - -## Details - -This rule looks at any message matching `List*RevisionsRequest` and complains if -either the `name` field is missing or it has any type other than `string`. - -## Examples - -**Incorrect** code for this rule: - -```proto -// Incorrect. -// Should include a `string name` field. -message ListBookRevisionsRequest { - int32 page_size = 1; - - string page_token = 2; -} -``` - -```proto -// Incorrect. -message ListBookRevisionsRequest { - // Field type should be `string`. - bytes name = 1 [ - (google.api.field_behavior) = REQUIRED, - (google.api.resource_reference).type = "library.googleapis.com/Book" - ]; - - int32 page_size = 2; - - string page_token = 3; -} -``` - -**Correct** code for this rule: - -```proto -// Correct. -message ListBookRevisionsRequest { - string name = 1 [ - (google.api.field_behavior) = REQUIRED, - (google.api.resource_reference).type = "library.googleapis.com/Book" - ]; - - int32 page_size = 2; - - string page_token = 3; -} -``` - -## Disabling - -If you need to violate this rule, use a leading comment above the message (if -the `name` field is missing) or above the field (if it is the wrong type). -Remember to also include an [aip.dev/not-precedent][] comment explaining why. - -```proto -message ListBookRevisionsRequest { - // (-- api-linter: core::0162::list-revisions-request-name-field=disabled - // aip.dev/not-precedent: We need to do this because reasons. --) - bytes name = 1 [ - (google.api.field_behavior) = REQUIRED, - (google.api.resource_reference).type = "library.googleapis.com/Book" - ]; - - int32 page_size = 2; - - string page_token = 3; -} -``` - -If you need to violate this rule for an entire file, place the comment at the -top of the file. - -[aip-162]: https://aip.dev/162 -[aip.dev/not-precedent]: https://aip.dev/not-precedent diff --git a/docs/rules/0162/list-revisions-request-name-reference.md b/docs/rules/0162/list-revisions-request-name-reference.md deleted file mode 100644 index 53cbe1854..000000000 --- a/docs/rules/0162/list-revisions-request-name-reference.md +++ /dev/null @@ -1,77 +0,0 @@ ---- -rule: - aip: 162 - name: [core, '0162', list-revisions-request-name-reference] - summary: | - List Revisions requests should annotate the `name` field with `google.api.resource_reference`. -permalink: /162/list-revisions-request-name-reference -redirect_from: - - /0162/list-revisions-request-name-reference ---- - -# List Revisions requests: Name resource reference - -This rule enforces that all List Revisions requests have -`google.api.resource_reference` on their `string name` field, as mandated in -[AIP-162][]. - -## Details - -This rule looks at the `name` field of any message matching -`List*RevisionsRequest` and complains if it does not have a -`google.api.resource_reference` annotation. - -## Examples - -**Incorrect** code for this rule: - -```proto -// Incorrect. -message ListBookRevisionsRequest { - // The `google.api.resource_reference` annotation should also be included. - string name = 1 [(google.api.field_behavior) = REQUIRED]; - - int32 page_size = 2; - - string page_token = 3; -} -``` - -**Correct** code for this rule: - -```proto -// Correct. -message ListBookRevisionsRequest { - string name = 1 [ - (google.api.field_behavior) = REQUIRED, - (google.api.resource_reference).type = "library.googleapis.com/Book" - ]; - - int32 page_size = 2; - - string page_token = 3; -} -``` - -## 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 -message ListBookRevisionsRequest { - // (-- api-linter: core::0162::list-revisions-request-name-reference=disabled - // aip.dev/not-precedent: We need to do this because reasons. --) - string name = 1 [(google.api.field_behavior) = REQUIRED]; - - int32 page_size = 2; - - string page_token = 3; -} -``` - -If you need to violate this rule for an entire file, place the comment at the -top of the file. - -[aip-162]: https://aip.dev/162 -[aip.dev/not-precedent]: https://aip.dev/not-precedent diff --git a/docs/rules/0162/list-revisions-request-no-order-by-field.md b/docs/rules/0162/list-revisions-request-no-order-by-field.md deleted file mode 100644 index 2d9267216..000000000 --- a/docs/rules/0162/list-revisions-request-no-order-by-field.md +++ /dev/null @@ -1,85 +0,0 @@ ---- -rule: - aip: 162 - name: [core, '0162', list-revisions-request-no-order-by-field] - summary: List Revisions requests should not have an `order_by` field. -permalink: /162/list-revisions-request-no-order-by-field -redirect_from: - - /0162/list-revisions-request-no-order-by-field ---- - -# List Revisions requests: No `order_by` field - -This rule enforces that List Revisions requests do not have a `string order_by` -field, as mandated in [AIP-162][]. - -## Details - -This rule looks at any message matching `List*RevisionsRequest` and complains if -an `order_by` field is present, as revisions in the response must be ordered in -reverse chronological order. - -## Examples - -**Incorrect** code for this rule: - -```proto -// Incorrect. -message ListBookRevisionsRequest { - string name = 1 [ - (google.api.field_behavior) = REQUIRED, - (google.api.resource_reference).type = "library.googleapis.com/Book" - ]; - - int32 page_size = 2; - - string page_token = 3; - - // Field should be removed. - string order_by = 4; -} -``` - -**Correct** code for this rule: - -```proto -// Correct. -message ListBookRevisionsRequest { - string name = 1 [ - (google.api.field_behavior) = REQUIRED, - (google.api.resource_reference).type = "library.googleapis.com/Book" - ]; - - int32 page_size = 2; - - string page_token = 3; -} -``` - -## 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 -message ListBookRevisionsRequest { - string name = 1 [ - (google.api.field_behavior) = REQUIRED, - (google.api.resource_reference).type = "library.googleapis.com/Book" - ]; - - int32 page_size = 2; - - string page_token = 3; - - // (-- api-linter: core::0162::list-revisions-request-no-order-by-field=disabled - // aip.dev/not-precedent: We need to do this because reasons. --) - string order_by = 4; -} -``` - -If you need to violate this rule for an entire file, place the comment at the -top of the file. - -[aip-162]: https://aip.dev/162 -[aip.dev/not-precedent]: https://aip.dev/not-precedent diff --git a/docs/rules/0162/list-revisions-response-message-name.md b/docs/rules/0162/list-revisions-response-message-name.md deleted file mode 100644 index 7bf34415a..000000000 --- a/docs/rules/0162/list-revisions-response-message-name.md +++ /dev/null @@ -1,66 +0,0 @@ ---- -rule: - aip: 162 - name: [core, '0162', list-revisions-response-message-name] - summary: List Revisions methods must have standardized response message names. -permalink: /162/list-revisions-response-message-name -redirect_from: - - /0162/list-revisions-response-message-name ---- - -# List Revisions methods: Response message - -This rule enforces that all List Revisions RPCs have a response message of -`List*RevisionsResponse`, as mandated in [AIP-162][]. - -## Details - -This rule looks at any method matching `List*Revisions`, and complains -if the name of the corresponding output message does not match the name of the -RPC with the suffix `Response` appended. - -## Examples - -**Incorrect** code for this rule: - -```proto -// Incorrect. -// Should return `ListBookRevisionsResponse`. -rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListRevisionsResponse) { - option (google.api.http) = { - get: "/v1/{name=publishers/*/books/*}:listRevisions" - }; -} -``` - -**Correct** code for this rule: - -```proto -// Correct. -rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListBookRevisionsResponse) { - option (google.api.http) = { - get: "/v1/{name=publishers/*/books/*}:listRevisions" - }; -} -``` - -## Disabling - -If you need to violate this rule, use a leading comment above the method. -Remember to also include an [aip.dev/not-precedent][] comment explaining why. - -```proto -// (-- api-linter: core::0162::list-revisions-response-message-name=disabled -// aip.dev/not-precedent: We need to do this because reasons. --) -rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListRevisionsResponse) { - option (google.api.http) = { - get: "/v1/{name=publishers/*/books/*}:listRevisions" - }; -} -``` - -If you need to violate this rule for an entire file, place the comment at the -top of the file. - -[aip-162]: https://aip.dev/162 -[aip.dev/not-precedent]: https://aip.dev/not-precedent diff --git a/rules/aip0162/aip0162.go b/rules/aip0162/aip0162.go index 88ae14b63..2f0884b1b 100644 --- a/rules/aip0162/aip0162.go +++ b/rules/aip0162/aip0162.go @@ -44,15 +44,6 @@ func AddRules(r lint.RuleRegistry) error { deleteRevisionRequestNameField, deleteRevisionRequestNameReference, deleteRevisionResponseMessageName, - listRevisionsHTTPBody, - listRevisionsHTTPMethod, - listRevisionsHTTPURISuffix, - listRevisionsRequestMessageName, - listRevisionsRequestNameBehavior, - listRevisionsRequestNameField, - listRevisionsRequestNameReference, - listRevisionsRequestNoOrderByField, - listRevisionsResponseMessageName, rollbackHTTPBody, rollbackHTTPMethod, rollbackHTTPURISuffix, @@ -140,10 +131,6 @@ func isDeleteRevisionRequestMessage(m *desc.MessageDescriptor) bool { return deleteRevisionReqMessageRegexp.MatchString(m.GetName()) } -var ( - listRevisionsURINameRegexp = regexp.MustCompile(`:listRevisions$`) -) - // IsListRevisionsRequestMessage returns true if this is an AIP-162 List // Revisions request message, false otherwise. // diff --git a/rules/aip0162/list_revisions_http_body.go b/rules/aip0162/list_revisions_http_body.go deleted file mode 100644 index 20a8386d2..000000000 --- a/rules/aip0162/list_revisions_http_body.go +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "github.com/googleapis/api-linter/lint" - "github.com/googleapis/api-linter/rules/internal/utils" -) - -// List Revisions methods should have no HTTP body. -var listRevisionsHTTPBody = &lint.MethodRule{ - Name: lint.NewRuleName(162, "list-revisions-http-body"), - OnlyIf: utils.IsListRevisionsMethod, - LintMethod: utils.LintNoHTTPBody, -} diff --git a/rules/aip0162/list_revisions_http_body_test.go b/rules/aip0162/list_revisions_http_body_test.go deleted file mode 100644 index 50a1992e6..000000000 --- a/rules/aip0162/list_revisions_http_body_test.go +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "testing" - - "github.com/googleapis/api-linter/rules/internal/testutils" -) - -func TestListRevisionsHTTPBody(t *testing.T) { - tests := []struct { - testName string - Body string - MethodName string - problems testutils.Problems - }{ - {"Valid", "", "ListBookRevisions", nil}, - {"Invalid", "*", "ListBookRevisions", testutils.Problems{{Message: "HTTP body"}}}, - {"Irrelevant", "*", "ListBooks", nil}, - } - - for _, test := range tests { - t.Run(test.testName, func(t *testing.T) { - file := testutils.ParseProto3Tmpl(t, ` - import "google/api/annotations.proto"; - service Library { - rpc {{.MethodName}}({{.MethodName}}Request) returns ({{.MethodName}}Response) { - option (google.api.http) = { - get: "/v1/{name=publishers/*/books/*}:listRevisions" - body: "{{.Body}}" - }; - } - } - message {{.MethodName}}Request {} - message {{.MethodName}}Response {} - `, test) - method := file.GetServices()[0].GetMethods()[0] - problems := listRevisionsHTTPBody.Lint(file) - if diff := test.problems.SetDescriptor(method).Diff(problems); diff != "" { - t.Error(diff) - } - }) - } -} diff --git a/rules/aip0162/list_revisions_http_method.go b/rules/aip0162/list_revisions_http_method.go deleted file mode 100644 index b17d5994e..000000000 --- a/rules/aip0162/list_revisions_http_method.go +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "github.com/googleapis/api-linter/lint" - "github.com/googleapis/api-linter/rules/internal/utils" -) - -// List Revisions methods should use the HTTP GET method. -var listRevisionsHTTPMethod = &lint.MethodRule{ - Name: lint.NewRuleName(162, "list-revisions-http-method"), - OnlyIf: utils.IsListRevisionsMethod, - LintMethod: utils.LintHTTPMethod("GET"), -} diff --git a/rules/aip0162/list_revisions_http_method_test.go b/rules/aip0162/list_revisions_http_method_test.go deleted file mode 100644 index 33694b7d0..000000000 --- a/rules/aip0162/list_revisions_http_method_test.go +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "testing" - - "github.com/googleapis/api-linter/rules/internal/testutils" -) - -func TestListRevisionsHTTPMethod(t *testing.T) { - tests := []struct { - testName string - Method string - MethodName string - problems testutils.Problems - }{ - {"Valid", "get", "ListBookRevisions", nil}, - {"Invalid", "delete", "ListBookRevisions", testutils.Problems{{Message: "HTTP GET"}}}, - {"Irrelevant", "delete", "ListBooks", nil}, - } - - for _, test := range tests { - t.Run(test.testName, func(t *testing.T) { - file := testutils.ParseProto3Tmpl(t, ` - import "google/api/annotations.proto"; - service Library { - rpc {{.MethodName}}({{.MethodName}}Request) returns ({{.MethodName}}Response) { - option (google.api.http) = { - {{.Method}}: "/v1/{name=publishers/*/books/*}:listRevisions" - }; - } - } - message {{.MethodName}}Request {} - message {{.MethodName}}Response {} - `, test) - method := file.GetServices()[0].GetMethods()[0] - problems := listRevisionsHTTPMethod.Lint(file) - if diff := test.problems.SetDescriptor(method).Diff(problems); diff != "" { - t.Error(diff) - } - }) - } -} diff --git a/rules/aip0162/list_revisions_http_uri_suffix.go b/rules/aip0162/list_revisions_http_uri_suffix.go deleted file mode 100644 index aaf8cbc72..000000000 --- a/rules/aip0162/list_revisions_http_uri_suffix.go +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "github.com/googleapis/api-linter/lint" - "github.com/googleapis/api-linter/locations" - "github.com/googleapis/api-linter/rules/internal/utils" - "github.com/jhump/protoreflect/desc" -) - -// List Revisions methods should have a proper HTTP pattern. -var listRevisionsHTTPURISuffix = &lint.MethodRule{ - Name: lint.NewRuleName(162, "list-revisions-http-uri-suffix"), - OnlyIf: utils.IsListRevisionsMethod, - LintMethod: func(m *desc.MethodDescriptor) []lint.Problem { - for _, httpRule := range utils.GetHTTPRules(m) { - if !listRevisionsURINameRegexp.MatchString(httpRule.URI) { - return []lint.Problem{{ - Message: `List Revisions URI should end with ":listRevisions".`, - Descriptor: m, - Location: locations.MethodHTTPRule(m), - }} - } - } - - return nil - }, -} diff --git a/rules/aip0162/list_revisions_http_uri_suffix_test.go b/rules/aip0162/list_revisions_http_uri_suffix_test.go deleted file mode 100644 index 34040dea4..000000000 --- a/rules/aip0162/list_revisions_http_uri_suffix_test.go +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "testing" - - "github.com/googleapis/api-linter/rules/internal/testutils" -) - -func TestListRevisionsHTTPURISuffix(t *testing.T) { - tests := []struct { - testName string - URI string - MethodName string - problems testutils.Problems - }{ - {"Valid", "/v1/{name=publishers/*/books/*}:listRevisions", "ListBookRevisions", nil}, - {"InvalidSuffix", "/v1/{name=publishers/*/books/*}:list", "ListBookRevisions", testutils.Problems{{Message: ":listRevisions"}}}, - {"Irrelevant", "/v1/{name=publishers/*/books/*}:list", "ListBooks", nil}, - } - - for _, test := range tests { - t.Run(test.testName, func(t *testing.T) { - file := testutils.ParseProto3Tmpl(t, ` - import "google/api/annotations.proto"; - service Library { - rpc {{.MethodName}}({{.MethodName}}Request) returns ({{.MethodName}}Response) { - option (google.api.http) = { - get: "{{.URI}}" - }; - } - } - message {{.MethodName}}Request {} - message {{.MethodName}}Response {} - `, test) - - problems := listRevisionsHTTPURISuffix.Lint(file) - if diff := test.problems.SetDescriptor(file.GetServices()[0].GetMethods()[0]).Diff(problems); diff != "" { - t.Errorf(diff) - } - }) - } -} diff --git a/rules/aip0162/list_revisions_request_message_name.go b/rules/aip0162/list_revisions_request_message_name.go deleted file mode 100644 index f8a513ad7..000000000 --- a/rules/aip0162/list_revisions_request_message_name.go +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "github.com/googleapis/api-linter/lint" - "github.com/googleapis/api-linter/rules/internal/utils" -) - -// List Revisions messages should have a properly named request message. -var listRevisionsRequestMessageName = &lint.MethodRule{ - Name: lint.NewRuleName(162, "list-revisions-request-message-name"), - OnlyIf: utils.IsListRevisionsMethod, - LintMethod: utils.LintMethodHasMatchingRequestName, -} diff --git a/rules/aip0162/list_revisions_request_message_name_test.go b/rules/aip0162/list_revisions_request_message_name_test.go deleted file mode 100644 index ec67253f5..000000000 --- a/rules/aip0162/list_revisions_request_message_name_test.go +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "testing" - - "github.com/googleapis/api-linter/rules/internal/testutils" -) - -func TestListRevisionsRequestMessageName(t *testing.T) { - tests := []struct { - testName string - MethodName string - ReqMessageName string - problems testutils.Problems - }{ - {"Valid", "ListBookRevisions", "ListBookRevisionsRequest", nil}, - {"Invalid", "ListBookRevisions", "ListRevisionsRequest", testutils.Problems{{Suggestion: "ListBookRevisionsRequest"}}}, - {"Irrelevant", "ListBooks", "GetBookRequest", nil}, - } - - for _, test := range tests { - t.Run(test.testName, func(t *testing.T) { - f := testutils.ParseProto3Tmpl(t, ` - service Library { - rpc {{.MethodName}}({{.ReqMessageName}}) returns ({{.MethodName}}Response) {} - } - message {{.ReqMessageName}} {} - message {{.MethodName}}Response {} - `, test) - m := f.GetServices()[0].GetMethods()[0] - if diff := test.problems.SetDescriptor(m).Diff(listRevisionsRequestMessageName.Lint(f)); diff != "" { - t.Error(diff) - } - }) - } -} diff --git a/rules/aip0162/list_revisions_request_name_behavior.go b/rules/aip0162/list_revisions_request_name_behavior.go deleted file mode 100644 index 6449589a0..000000000 --- a/rules/aip0162/list_revisions_request_name_behavior.go +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "github.com/googleapis/api-linter/lint" - "github.com/googleapis/api-linter/rules/internal/utils" - "github.com/jhump/protoreflect/desc" -) - -var listRevisionsRequestNameBehavior = &lint.FieldRule{ - Name: lint.NewRuleName(162, "list-revisions-request-name-behavior"), - OnlyIf: func(f *desc.FieldDescriptor) bool { - return IsListRevisionsRequestMessage(f.GetOwner()) && f.GetName() == "name" - }, - LintField: utils.LintRequiredField, -} diff --git a/rules/aip0162/list_revisions_request_name_behavior_test.go b/rules/aip0162/list_revisions_request_name_behavior_test.go deleted file mode 100644 index 5d6d8b31a..000000000 --- a/rules/aip0162/list_revisions_request_name_behavior_test.go +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "testing" - - "github.com/googleapis/api-linter/rules/internal/testutils" -) - -func TestListRevisionsRequestNameBehavior(t *testing.T) { - for _, test := range []struct { - name string - RPC string - Field string - FieldOpts string - problems testutils.Problems - }{ - {"Valid", "ListBookRevisions", "name", " [(google.api.field_behavior) = REQUIRED]", nil}, - {"Missing", "ListBookRevisions", "name", "", testutils.Problems{{Message: "(google.api.field_behavior) = REQUIRED"}}}, - {"IrrelevantMessage", "ListBooks", "name", "", nil}, - {"IrrelevantField", "ListBookRevisions", "something_else", "", nil}, - } { - t.Run(test.name, func(t *testing.T) { - f := testutils.ParseProto3Tmpl(t, ` - import "google/api/field_behavior.proto"; - message {{.RPC}}Request { - string {{.Field}} = 1{{.FieldOpts}}; - } - `, test) - field := f.GetMessageTypes()[0].GetFields()[0] - if diff := test.problems.SetDescriptor(field).Diff(listRevisionsRequestNameBehavior.Lint(f)); diff != "" { - t.Error(diff) - } - }) - } -} diff --git a/rules/aip0162/list_revisions_request_name_field.go b/rules/aip0162/list_revisions_request_name_field.go deleted file mode 100644 index 5b0cd594b..000000000 --- a/rules/aip0162/list_revisions_request_name_field.go +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "github.com/googleapis/api-linter/lint" - "github.com/googleapis/api-linter/rules/internal/utils" -) - -// The List Revisions request message should have a name field. -var listRevisionsRequestNameField = &lint.MessageRule{ - Name: lint.NewRuleName(162, "list-revisions-request-name-field"), - OnlyIf: IsListRevisionsRequestMessage, - LintMessage: utils.LintFieldPresentAndSingularString("name"), -} diff --git a/rules/aip0162/list_revisions_request_name_field_test.go b/rules/aip0162/list_revisions_request_name_field_test.go deleted file mode 100644 index f15710ac9..000000000 --- a/rules/aip0162/list_revisions_request_name_field_test.go +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "testing" - - "github.com/googleapis/api-linter/rules/internal/testutils" - "github.com/jhump/protoreflect/desc" -) - -func TestListRevisionsRequestNameField(t *testing.T) { - for _, test := range []struct { - name string - RPC string - Field string - problems testutils.Problems - }{ - {"Valid", "ListBookRevisions", "string name = 1;", nil}, - {"Missing", "ListBookRevisions", "", testutils.Problems{{Message: "no `name`"}}}, - {"InvalidType", "ListBookRevisions", "int32 name = 1;", testutils.Problems{{Suggestion: "string"}}}, - {"IrrelevantRPCName", "ListBooks", "", nil}, - } { - t.Run(test.name, func(t *testing.T) { - f := testutils.ParseProto3Tmpl(t, ` - message {{.RPC}}Request { - {{.Field}} - } - `, test) - var d desc.Descriptor = f.GetMessageTypes()[0] - if test.name == "InvalidType" { - d = f.GetMessageTypes()[0].GetFields()[0] - } - if diff := test.problems.SetDescriptor(d).Diff(listRevisionsRequestNameField.Lint(f)); diff != "" { - t.Error(diff) - } - }) - } -} diff --git a/rules/aip0162/list_revisions_request_name_reference.go b/rules/aip0162/list_revisions_request_name_reference.go deleted file mode 100644 index ca218d0d9..000000000 --- a/rules/aip0162/list_revisions_request_name_reference.go +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "github.com/googleapis/api-linter/lint" - "github.com/googleapis/api-linter/rules/internal/utils" - "github.com/jhump/protoreflect/desc" -) - -var listRevisionsRequestNameReference = &lint.FieldRule{ - Name: lint.NewRuleName(162, "list-revisions-request-name-reference"), - OnlyIf: func(f *desc.FieldDescriptor) bool { - return IsListRevisionsRequestMessage(f.GetOwner()) && f.GetName() == "name" - }, - LintField: utils.LintFieldResourceReference, -} diff --git a/rules/aip0162/list_revisions_request_name_reference_test.go b/rules/aip0162/list_revisions_request_name_reference_test.go deleted file mode 100644 index de76b41c6..000000000 --- a/rules/aip0162/list_revisions_request_name_reference_test.go +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "testing" - - "github.com/googleapis/api-linter/rules/internal/testutils" -) - -func TestListRevisionsRequestNameReference(t *testing.T) { - for _, test := range []struct { - name string - RPC string - Field string - FieldOpts string - problems testutils.Problems - }{ - {"Valid", "ListBookRevisions", "name", ` [(google.api.resource_reference) = { type: "foo" }]`, nil}, - {"Missing", "ListBookRevisions", "name", "", testutils.Problems{{Message: "google.api.resource_reference"}}}, - {"IrrelevantMessage", "ListBooks", "name", "", nil}, - {"IrrelevantField", "ListBookRevisions", "something_else", "", nil}, - } { - t.Run(test.name, func(t *testing.T) { - f := testutils.ParseProto3Tmpl(t, ` - import "google/api/resource.proto"; - message {{.RPC}}Request { - repeated string {{.Field}} = 1{{.FieldOpts}}; - } - `, test) - field := f.GetMessageTypes()[0].GetFields()[0] - if diff := test.problems.SetDescriptor(field).Diff(listRevisionsRequestNameReference.Lint(f)); diff != "" { - t.Error(diff) - } - }) - } -} diff --git a/rules/aip0162/list_revisions_request_no_order_by_field.go b/rules/aip0162/list_revisions_request_no_order_by_field.go deleted file mode 100644 index e5686a0f4..000000000 --- a/rules/aip0162/list_revisions_request_no_order_by_field.go +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "github.com/googleapis/api-linter/lint" - "github.com/jhump/protoreflect/desc" -) - -// List Revisions requests should not have an order_by field. -var listRevisionsRequestNoOrderByField = &lint.FieldRule{ - Name: lint.NewRuleName(162, "list-revisions-request-no-order-by-field"), - OnlyIf: func(f *desc.FieldDescriptor) bool { - return IsListRevisionsRequestMessage(f.GetOwner()) && f.GetName() == "order_by" - }, - LintField: func(f *desc.FieldDescriptor) []lint.Problem { - return []lint.Problem{{ - Message: "List Revisions requests should not contain an `order_by` field, as revisions must be ordered in reverse chronological order.", - Descriptor: f, - }} - }, -} diff --git a/rules/aip0162/list_revisions_request_no_order_by_field_test.go b/rules/aip0162/list_revisions_request_no_order_by_field_test.go deleted file mode 100644 index 2f2472ef9..000000000 --- a/rules/aip0162/list_revisions_request_no_order_by_field_test.go +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "testing" - - "github.com/googleapis/api-linter/rules/internal/testutils" -) - -func TestListRevisionsRequestNoOrderByField(t *testing.T) { - for _, test := range []struct { - name string - Message string - Field string - problems testutils.Problems - }{ - {"Valid", "ListBookRevisionsRequest", "string name = 1;", nil}, - {"Invalid", "ListBookRevisionsRequest", "string order_by = 1;", testutils.Problems{{Message: "not contain an `order_by`"}}}, - {"IrrelevantMessage", "ListBooksRequest", "string order_by = 1;", nil}, - } { - t.Run(test.name, func(t *testing.T) { - f := testutils.ParseProto3Tmpl(t, ` - message {{.Message}} { - {{.Field}} - } - `, test) - d := f.GetMessageTypes()[0].GetFields()[0] - if diff := test.problems.SetDescriptor(d).Diff(listRevisionsRequestNoOrderByField.Lint(f)); diff != "" { - t.Error(diff) - } - }) - } -} diff --git a/rules/aip0162/list_revisions_response_message_name.go b/rules/aip0162/list_revisions_response_message_name.go deleted file mode 100644 index 63e72f5e7..000000000 --- a/rules/aip0162/list_revisions_response_message_name.go +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "github.com/googleapis/api-linter/lint" - "github.com/googleapis/api-linter/rules/internal/utils" -) - -var listRevisionsResponseMessageName = &lint.MethodRule{ - Name: lint.NewRuleName(162, "list-revisions-response-message-name"), - OnlyIf: utils.IsListRevisionsMethod, - LintMethod: utils.LintMethodHasMatchingResponseName, -} diff --git a/rules/aip0162/list_revisions_response_message_name_test.go b/rules/aip0162/list_revisions_response_message_name_test.go deleted file mode 100644 index 8bbeaab1a..000000000 --- a/rules/aip0162/list_revisions_response_message_name_test.go +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2021 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 aip0162 - -import ( - "testing" - - "github.com/googleapis/api-linter/rules/internal/testutils" -) - -func TestListRevisionsResponseMessageName(t *testing.T) { - for _, test := range []struct { - name string - Method string - ResponseType string - problems testutils.Problems - }{ - {"Valid", "ListBookRevisions", "ListBookRevisionsResponse", nil}, - {"Invalid", "ListBookRevisions", "ListRevisionsResponse", testutils.Problems{{Suggestion: "ListBookRevisionsResponse"}}}, - {"Irrelevant", "ListBooks", "Book", nil}, - } { - t.Run(test.name, func(t *testing.T) { - f := testutils.ParseProto3Tmpl(t, ` - service Library { - rpc {{.Method}}({{.Method}}Request) returns ({{.ResponseType}}); - } - message {{.Method}}Request {} - message {{.ResponseType}} {} - `, test) - m := f.GetServices()[0].GetMethods()[0] - if diff := test.problems.SetDescriptor(m).Diff(listRevisionsResponseMessageName.Lint(f)); diff != "" { - t.Error(diff) - } - }) - } -} diff --git a/rules/internal/utils/method.go b/rules/internal/utils/method.go index 8eaca4b10..3de11d923 100644 --- a/rules/internal/utils/method.go +++ b/rules/internal/utils/method.go @@ -21,13 +21,14 @@ import ( ) var ( - createMethodRegexp = regexp.MustCompile("^Create(?:[A-Z]|$)") - getMethodRegexp = regexp.MustCompile("^Get(?:[A-Z]|$)") - listMethodRegexp = regexp.MustCompile("^List(?:[A-Z]|$)") - listRevisionsMethodRegexp = regexp.MustCompile(`^List(?:[A-Za-z0-9]+)Revisions$`) - updateMethodRegexp = regexp.MustCompile("^Update(?:[A-Z]|$)") - deleteMethodRegexp = regexp.MustCompile("^Delete(?:[A-Z]|$)") - deleteRevisionMethodRegexp = regexp.MustCompile("^Delete[A-Za-z0-9]*Revision$") + createMethodRegexp = regexp.MustCompile("^Create(?:[A-Z]|$)") + getMethodRegexp = regexp.MustCompile("^Get(?:[A-Z]|$)") + listMethodRegexp = regexp.MustCompile("^List(?:[A-Z]|$)") + listRevisionsMethodRegexp = regexp.MustCompile(`^List(?:[A-Za-z0-9]+)Revisions$`) + updateMethodRegexp = regexp.MustCompile("^Update(?:[A-Z]|$)") + deleteMethodRegexp = regexp.MustCompile("^Delete(?:[A-Z]|$)") + deleteRevisionMethodRegexp = regexp.MustCompile("^Delete[A-Za-z0-9]*Revision$") + legacyListRevisionsURINameRegexp = regexp.MustCompile(`:listRevisions$`) ) // IsCreateMethod returns true if this is a AIP-133 Create method. @@ -46,13 +47,34 @@ func IsGetMethod(m *desc.MethodDescriptor) bool { // IsListMethod return true if this is an AIP-132 List method func IsListMethod(m *desc.MethodDescriptor) bool { - return listMethodRegexp.MatchString(m.GetName()) && !IsListRevisionsMethod(m) + return listMethodRegexp.MatchString(m.GetName()) && !IsLegacyListRevisionsMethod(m) } -// IsListRevisionsMethod returns true if this is an AIP-162 List Revisions method, -// false otherwise. -func IsListRevisionsMethod(m *desc.MethodDescriptor) bool { - return listRevisionsMethodRegexp.MatchString(m.GetName()) +// IsLegacyListRevisions identifies such a method by having the appropriate +// method name, having a `name` field instead of parent, and a HTTP suffix of +// `listRevisions`. +func IsLegacyListRevisionsMethod(m *desc.MethodDescriptor) bool { + // Must be named like List{Resource}Revisions. + if !listRevisionsMethodRegexp.MatchString(m.GetName()) { + return false + } + + // Must have a `name` field instead of `parent`. + if m.GetInputType().FindFieldByName("name") == nil { + return false + } + + // Must have the `:listRevisions` HTTP URI suffix. + if !HasHTTPRules(m) { + // If it doesn't have HTTP bindings, we shouldn't proceed to the next + // check, but a List{Resource}Revisions method with a `name` field is + // probably enough to be sure in the absence of HTTP bindings. + return true + } + + // Just check the first bidning as they should all have the same suffix. + h := GetHTTPRules(m)[0].GetPlainURI() + return legacyListRevisionsURINameRegexp.MatchString(h) } // IsUpdateMethod returns true if this is a AIP-134 Update method diff --git a/rules/internal/utils/method_test.go b/rules/internal/utils/method_test.go index eff2c4e56..8e60910e2 100644 --- a/rules/internal/utils/method_test.go +++ b/rules/internal/utils/method_test.go @@ -124,9 +124,9 @@ func TestIsListMethod(t *testing.T) { {"ValidList", ` rpc ListBooks(ListBooksRequest) returns (ListBooksResponse) {}; `, true}, - {"InvalidListRevisionsMethod", ` - rpc ListBookRevisions(ListBooksRequest) returns (ListBooksResponse) {}; - `, false}, + {"ValidListRevisionsMethod", ` + rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListBookRevisionsResponse) {}; + `, true}, {"InvalidNonList", ` rpc EnumerateBooks(ListBooksRequest) returns (ListBooksResponse) {}; `, false}, @@ -150,6 +150,17 @@ func TestIsListMethod(t *testing.T) { }; } + // This is at the top to make it retrievable + // by the test code. + message BookRevision { + option (google.api.resource) = { + type: "library.googleapis.com/BookRevision" + pattern: "books/{book}/revisions/{revision}" + singular: "bookRevision" + plural: "bookRevisions" + }; + } + message ListBooksRequest { string parent = 1; int32 page_size = 2; @@ -160,6 +171,17 @@ func TestIsListMethod(t *testing.T) { repeated Book books = 1; string next_page_token = 2; } + + message ListBookRevisionsRequest { + string parent = 1; + int32 page_size = 2; + string page_token = 3; + } + + message ListBookRevisionsResponse { + repeated BookRevision book_revisions = 1; + string next_page_token = 2; + } `, test) method := file.GetServices()[0].GetMethods()[0] got := IsListMethod(method) @@ -170,21 +192,29 @@ func TestIsListMethod(t *testing.T) { } } -func TestIsListRevisionsMethod(t *testing.T) { +func TestIsLegacyListRevisionsMethod(t *testing.T) { for _, test := range []struct { name string RPCs string want bool }{ - {"ValidListRevisionsMethod", ` - rpc ListBookRevisions(ListBooksRequest) returns (ListBooksResponse) {}; + {"ValidLegacyListRevisionsMethod", ` + rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListBookRevisionsResponse) { + option (google.api.http) = { + get: "/v1/{name=books/*}:listRevisions" + }; + }; `, true}, - {"InvalidList", ` + {"ValidLegacyListRevisionsMethodWithoutHTTP", ` + rpc ListBookRevisions(ListBookRevisionsRequest) returns (ListBookRevisionsResponse) {}; + `, true}, + {"InvalidLegacyListRevisionsMethod", ` rpc ListBooks(ListBooksRequest) returns (ListBooksResponse) {}; `, false}, } { t.Run(test.name, func(t *testing.T) { file := testutils.ParseProto3Tmpl(t, ` + import "google/api/annotations.proto"; import "google/api/resource.proto"; import "google/protobuf/field_mask.proto"; service Foo { @@ -212,11 +242,22 @@ func TestIsListRevisionsMethod(t *testing.T) { repeated Book books = 1; string next_page_token = 2; } + + message ListBookRevisionsRequest { + string name = 1; + int32 page_size = 2; + string page_token = 3; + } + + message ListBookRevisionsResponse { + repeated Book books = 1; + string next_page_token = 2; + } `, test) method := file.GetServices()[0].GetMethods()[0] - got := IsListRevisionsMethod(method) + got := IsLegacyListRevisionsMethod(method) if got != test.want { - t.Errorf("IsListRevisionsMethod got %v, want %v", got, test.want) + t.Errorf("IsLegacyListRevisionsMethod got %v, want %v", got, test.want) } }) }