(fix) remove internal comments from generated protoComments

[kiambogo]

References to other Issues or PRs

Fixes #3778 .

Brief description of what is fixed or changed

Internal comments on enum types within proto schemas were leaking into the description field of generated query parameter definitions.

This fix leverages the existing removeInternalComments method for removing internal comments from the enum proto type.

Other comments

I added a unit test that constructs an enum type with a similar definition as to the one that I saw was generated through the code generator. If there is a better unit testing strategy here please let me know and I'll update the PR.

Verified fixed through reproducing the bug before and after this code change.
#3778 (comment)

Verification steps:

1. Build protoc-gen-openapiv2 by executing go build in protoc-gen-openapiv2/.
2. Generate code with binary present in path

$ PATH=$PATH:<path_to_dir>/protoc-gen-openapiv2/ buf generate

buf.yaml

version: v1
breaking:
  use:
    - FILE
lint:
  use:
    - DEFAULT
deps:
  - buf.build/googleapis/googleapis

buf.gen.yaml

version: v1
plugins:
  - plugin: openapiv2 # references local binary built with fix
    out: gen/openapiv2
    opt:
      - remove_internal_comments=true

example/v1/example.proto

syntax = "proto3";

package example.v1;

import "google/api/annotations.proto";

option go_package = "./";

service ExampleService {
  rpc Example(ExampleRequest) returns (ExampleResponse) {
    option (google.api.http) = {
      post: "/example"
      body: "example"
    };
  }
}

enum ExampleKind {
  // (-- aip.dev/not-precedent: This preceded the AIP standards
  // buf:lint:ignore ENUM_ZERO_VALUE_SUFFIX --)
  FIRST = 0;
  SECOND = 1;
}

message Example {}

message ExampleRequest {
  Example example = 1;
  ExampleKind kind = 2;
}

message ExampleResponse {}

Full Output of generated swagger against this change:

{
  "swagger": "2.0",
  "info": {
    "title": "example/v1/example.proto",
    "version": "version not set"
  },
  "tags": [
    {
      "name": "ExampleService"
    }
  ],
  "consumes": [
    "application/json"
  ],
  "produces": [
    "application/json"
  ],
  "paths": {
    "/example": {
      "post": {
        "operationId": "ExampleService_Example",
        "responses": {
          "200": {
            "description": "A successful response.",
            "schema": {
              "$ref": "#/definitions/v1ExampleResponse"
            }
          },
          "default": {
            "description": "An unexpected error response.",
            "schema": {
              "$ref": "#/definitions/rpcStatus"
            }
          }
        },
        "parameters": [
          {
            "name": "example",
            "in": "body",
            "required": true,
            "schema": {
              "$ref": "#/definitions/examplev1Example"
            }
          },
          {
            "name": "kind",
            "in": "query",
            "required": false,
            "type": "string",
            "enum": [
              "FIRST",
              "SECOND"
            ],
            "default": "FIRST"
          }
        ],
        "tags": [
          "ExampleService"
        ]
      }
    }
  },
  "definitions": {
    "examplev1Example": {
      "type": "object"
    },
    "protobufAny": {
      "type": "object",
      "properties": {
        "@type": {
          "type": "string"
        }
      },
      "additionalProperties": {}
    },
    "rpcStatus": {
      "type": "object",
      "properties": {
        "code": {
          "type": "integer",
          "format": "int32"
        },
        "message": {
          "type": "string"
        },
        "details": {
          "type": "array",
          "items": {
            "type": "object",
            "$ref": "#/definitions/protobufAny"
          }
        }
      }
    },
    "v1ExampleKind": {
      "type": "string",
      "enum": [
        "FIRST",
        "SECOND"
      ],
      "default": "FIRST"
    },
    "v1ExampleResponse": {
      "type": "object"
    }
  }
}

Summary by CodeRabbit

• Refactor

  • Internal comments are now automatically removed to enhance the clarity of public documentation.

• Tests

  • Implemented new tests to ensure internal comments are correctly stripped from enum values in documentation.

[coderabbitai]

Walkthrough

The recent update addresses a bug where internal comments were not being properly stripped from the descriptions of enum query parameters in generated OpenAPI documentation. A function has been implemented to ensure that internal comments are removed, and a test has been added to verify the correct behavior for enum values.

Changes

File Path	Change Summary
.../genopenapi/template.go	Added call to removeInternalComments for comments.
.../genopenapi/template_test.go	Added TestEnumValueProtoComments to verify comment removal from enums.

Assessment against linked issues

Objective	Addressed	Explanation
Identify and fix the issue with internal comments in enum query parameter's description (#3778)	✅
Ensure the enum query parameter's description excludes internal comments (#3778)	✅
Verify the enum's description in definitions strips internal comments correctly (#3778)	✅
The query parameter's description should only include enum values without internal comments (#3778)	✅

Poem

🐇🌟
In the realm of code, where bugs do hide,
A rabbit hopped through, with a graceful stride.
With every leap, a glitch untied,
Leaving clean docs where comments once lied.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

• X
• Mastodon
• Reddit
• LinkedIn

---

Tips

Chat

There are 3 ways to chat with CodeRabbit's AI:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit-tests for this file.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit tests for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository from git and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit tests.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid.
  • @coderabbitai read the files in the src/scheduler package and generate README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

[coderabbitai]

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between 5ea0aa6 and a4506f4.

Files selected for processing (2)

• protoc-gen-openapiv2/internal/genopenapi/template.go (1 hunks)
• protoc-gen-openapiv2/internal/genopenapi/template_test.go (1 hunks)

Additional comments: 2

protoc-gen-openapiv2/internal/genopenapi/template.go (2)

• 2479-2479: The removeInternalComments function is now being called to process the comments variable. Ensure that this function is thoroughly tested, especially for edge cases where internal comments may be part of a larger comment block or inline with code.

Verification successful

---

The test TestEnumValueProtoComments in template_test.go indirectly verifies the functionality of removeInternalComments by expecting an empty string after processing an enum with a comment. This suggests that the removeInternalComments function is being tested as part of the enum comment processing.

Scripts Executed

The following scripts were executed for the analysis:

---

Script:

#!/bin/bash
# Search for the removeInternalComments function usage across the codebase.
rg --type go 'removeInternalComments'
# Verify that there are tests covering the removeInternalComments function.
rg --type go 'TestRemoveInternalComments'

Length of output: 1321

---

---

Script:

#!/bin/bash
# View the implementation of the removeInternalComments function.
cat protoc-gen-openapiv2/internal/genopenapi/template.go | grep -A 10 'func removeInternalComments'

# Search for tests in template_test.go that might cover the removeInternalComments functionality indirectly.
rg --type go 'func Test.*' protoc-gen-openapiv2/internal/genopenapi/template_test.go

Length of output: 4167

---

---

Script:

#!/bin/bash
# View the contents of the TestEnumValueProtoComments test to check if it covers removeInternalComments functionality.
rg --type go -A 30 'func TestEnumValueProtoComments' protoc-gen-openapiv2/internal/genopenapi/template_test.go

Length of output: 905

* 2476-2480: The `removeInternalComments` function is used to clean up comments by removing internal annotations. It's important to ensure that the function correctly handles various comment formats and does not alter the meaning of the remaining comments.

[coderabbitai]

The test function TestEnumValueProtoComments is well-structured and tests the removal of internal comments from enum values. However, it would be beneficial to include a case where comments are expected to remain to ensure that only internal comments are removed and other comments are left intact.

+ // Test that non-internal comments are not removed
+ func TestEnumValueNonInternalComments(t *testing.T) {
+     // ... setup similar to TestEnumValueProtoComments ...
+     comments := "This is a public comment."
+     // ... create enum with comments ...
+     comments = enumValueProtoComments(reg, enum)
+     if comments == "" {
+         t.Errorf("expected 'This is a public comment.', got '%v'", comments)
+     }
+ }

---

Committable suggestion

❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

	// Test that enum values have internal comments removed
	func TestEnumValueProtoComments(t *testing.T) {
	reg := descriptor.NewRegistry()
	name := "kind"
	comments := "(-- this is a comment --)"
	enum := &descriptor.Enum{
	EnumDescriptorProto: &descriptorpb.EnumDescriptorProto{
	Name: &name,
	},
	File: &descriptor.File{
	FileDescriptorProto: &descriptorpb.FileDescriptorProto{
	Name: new(string),
	Package: new(string),
	SourceCodeInfo: &descriptorpb.SourceCodeInfo{
	Location: []*descriptorpb.SourceCodeInfo_Location{
	&descriptorpb.SourceCodeInfo_Location{
	LeadingComments: &comments,
	},
	},
	},
	},
	},
	}
	comments = enumValueProtoComments(reg, enum)
	if comments != "" {
	t.Errorf("expected '', got '%v'", comments)
	}
	}
	// Test that enum values have internal comments removed
	func TestEnumValueProtoComments(t *testing.T) {
	reg := descriptor.NewRegistry()
	name := "kind"
	comments := "(-- this is a comment --)"
	enum := &descriptor.Enum{
	EnumDescriptorProto: &descriptorpb.EnumDescriptorProto{
	Name: &name,
	},
	File: &descriptor.File{
	FileDescriptorProto: &descriptorpb.FileDescriptorProto{
	Name: new(string),
	Package: new(string),
	SourceCodeInfo: &descriptorpb.SourceCodeInfo{
	Location: []*descriptorpb.SourceCodeInfo_Location{
	&descriptorpb.SourceCodeInfo_Location{
	LeadingComments: &comments,
	},
	},
	},
	},
	},
	}
	comments = enumValueProtoComments(reg, enum)
	if comments != "" {
	t.Errorf("expected '', got '%v'", comments)
	}
	}
	// Test that non-internal comments are not removed
	func TestEnumValueNonInternalComments(t *testing.T) {
	// ... setup similar to TestEnumValueProtoComments ...
	comments := "This is a public comment."
	// ... create enum with comments ...
	comments = enumValueProtoComments(reg, enum)
	if comments == "" {
	t.Errorf("expected 'This is a public comment.', got '%v'", comments)
	}
	}

[johanbrandhorst]

LGTM, thank you!