Copilot commented on code in PR #2951: URL: https://github.com/apache/dubbo-go/pull/2951#discussion_r2207442585
########## tools/protoc-gen-triple-openapi/internal/options/options.go: ########## @@ -0,0 +1,59 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 + * + * http://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 options + +import ( + "fmt" + "strings" +) + +type Options struct { + // yaml or json + Format string +} + +func defaultOptions() Options { + return Options{ + // default yaml format + Format: "yaml", + } +} + +func Generate(s string) (Options, error) { + opts := defaultOptions() + + for _, param := range strings.Split(s, ",") { + switch { + case param == "": + case strings.HasPrefix(param, "format="): + format := param[7:] + switch format { + case "yaml": + opts.Format = "yaml" + case "json": + opts.Format = "json" + default: + return opts, fmt.Errorf("not support '%s' format", format) Review Comment: The error message 'not support '%s' format' is grammatically incorrect and unclear; consider changing it to 'format '%s' is not supported'. ```suggestion return opts, fmt.Errorf("format '%s' is not supported", format) ``` ########## tools/protoc-gen-triple-openapi/internal/converter/util.go: ########## @@ -0,0 +1,30 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 + * + * http://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 converter + +import ( + "github.com/pb33f/libopenapi/datamodel/high/base" + openapimodel "github.com/pb33f/libopenapi/datamodel/high/v3" + "github.com/pb33f/libopenapi/orderedmap" +) + +func makeMediaTypes(s *base.SchemaProxy) *orderedmap.Map[string, *openapimodel.MediaType] { + mediaTypes := orderedmap.New[string, *openapimodel.MediaType]() + mediaTypes.Set("application/json", &openapimodel.MediaType{Schema: s}) Review Comment: [nitpick] Rename the parameter 's' to a more descriptive name (e.g., 'schemaProxy') to improve readability. ```suggestion func makeMediaTypes(schemaProxy *base.SchemaProxy) *orderedmap.Map[string, *openapimodel.MediaType] { mediaTypes := orderedmap.New[string, *openapimodel.MediaType]() mediaTypes.Set("application/json", &openapimodel.MediaType{Schema: schemaProxy}) ``` ########## tools/protoc-gen-triple-openapi/internal/converter/schema/util.go: ########## @@ -0,0 +1,159 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 + * + * http://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 schema + +import ( + "fmt" +) + +import ( + "github.com/pb33f/libopenapi/datamodel/high/base" + "github.com/pb33f/libopenapi/orderedmap" + + "google.golang.org/protobuf/reflect/protoreflect" + + "gopkg.in/yaml.v3" +) + +func messageToSchema(tt protoreflect.MessageDescriptor) (string, *base.Schema) { + s := &base.Schema{ + Title: string(tt.Name()), + // TODO: add Description + Description: "", + Type: []string{"object"}, + } + + props := orderedmap.New[string, *base.SchemaProxy]() + + fields := tt.Fields() + for i := 0; i < fields.Len(); i++ { + field := fields.Get(i) + // TODO: handle oneof + prop := fieldToSchema(base.CreateSchemaProxy(s), field) + props.Set(field.JSONName(), prop) + } + + s.Properties = props + + return string(tt.FullName()), s +} + +func fieldToSchema(parent *base.SchemaProxy, tt protoreflect.FieldDescriptor) *base.SchemaProxy { + if tt.IsMap() { + // Handle maps + root := ScalarFieldToSchema(parent, tt, false) + root.Title = string(tt.Name()) + root.Type = []string{"object"} + // TODO: todo + root.Description = "" + return base.CreateSchemaProxy(root) + } else if tt.IsList() { + var itemSchema *base.SchemaProxy + switch tt.Kind() { + case protoreflect.MessageKind: + itemSchema = ReferenceFieldToSchema(parent, tt) + case protoreflect.EnumKind: + itemSchema = ReferenceFieldToSchema(parent, tt) + default: + itemSchema = base.CreateSchemaProxy(ScalarFieldToSchema(parent, tt, true)) + } + s := &base.Schema{ + Title: string(tt.Name()), + ParentProxy: parent, + // TODO: todo + Description: "", + Type: []string{"array"}, + Items: &base.DynamicValue[*base.SchemaProxy, bool]{A: itemSchema}, + } + return base.CreateSchemaProxy(s) + } else { + switch tt.Kind() { + case protoreflect.MessageKind, protoreflect.EnumKind: + msg := ScalarFieldToSchema(parent, tt, false) + ref := ReferenceFieldToSchema(parent, tt) + extensions := orderedmap.New[string, *yaml.Node]() + extensions.Set("$ref", CreateStringNode(ref.GetReference())) + msg.Extensions = extensions + return base.CreateSchemaProxy(msg) + } + + s := ScalarFieldToSchema(parent, tt, false) + return base.CreateSchemaProxy(s) + } +} + +func ScalarFieldToSchema(parent *base.SchemaProxy, tt protoreflect.FieldDescriptor, inContainer bool) *base.Schema { + s := &base.Schema{ + ParentProxy: parent, + } + if !inContainer { + s.Title = string(tt.Name()) + // TODO: add description + s.Description = "" + } + + switch tt.Kind() { + case protoreflect.BoolKind: + s.Type = []string{"boolean"} + case protoreflect.Int32Kind, protoreflect.Sfixed32Kind, protoreflect.Sint32Kind: // int32 types + s.Type = []string{"integer"} + s.Format = "int32" + case protoreflect.Fixed32Kind, protoreflect.Uint32Kind: // uint32 types + s.Type = []string{"integer"} + case protoreflect.Int64Kind, protoreflect.Sint64Kind, protoreflect.Sfixed64Kind: // int64 types + // NOTE: 64-bit integer types can be strings or numbers because they sometimes + // cannot fit into a JSON number type + s.Type = []string{"integer", "string"} + s.Format = "int64" + case protoreflect.Uint64Kind, protoreflect.Fixed64Kind: // uint64 types + s.Type = []string{"integer", "string"} + s.Format = "int64" + case protoreflect.DoubleKind: + s.Type = []string{"number"} + s.Format = "double" + case protoreflect.FloatKind: + s.Type = []string{"number"} + s.Format = "float" + case protoreflect.StringKind: + s.Type = []string{"string"} + case protoreflect.BytesKind: + s.Type = []string{"string"} + s.Format = "byte" + } + return s +} + +func ReferenceFieldToSchema(parent *base.SchemaProxy, tt protoreflect.FieldDescriptor) *base.SchemaProxy { + switch tt.Kind() { + case protoreflect.MessageKind: + return base.CreateSchemaProxyRef("#/components/schemas/" + string(tt.Message().FullName())) + case protoreflect.EnumKind: + return base.CreateSchemaProxyRef("#/components/schemas/" + string(tt.Enum().FullName())) + default: + panic(fmt.Errorf("ReferenceFieldToSchema called with unknown kind: %T", tt.Kind())) Review Comment: Panicking on unknown field kinds can crash the plugin; consider returning an error to allow graceful handling. ```suggestion func ReferenceFieldToSchema(parent *base.SchemaProxy, tt protoreflect.FieldDescriptor) (*base.SchemaProxy, error) { switch tt.Kind() { case protoreflect.MessageKind: return base.CreateSchemaProxyRef("#/components/schemas/" + string(tt.Message().FullName())), nil case protoreflect.EnumKind: return base.CreateSchemaProxyRef("#/components/schemas/" + string(tt.Enum().FullName())), nil default: return nil, fmt.Errorf("ReferenceFieldToSchema called with unknown kind: %T", tt.Kind()) ``` ########## tools/protoc-gen-triple-openapi/Makefile: ########## @@ -0,0 +1,52 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You 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 +# +# http://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. +# + +SHELL := bash +.DELETE_ON_ERROR: +.SHELLFLAGS := -eu -o pipefail -c +MAKEFLAGS += --warn-undefined-variables +MAKEFLAGS += --no-builtin-rules + +# Variables +BINARY_NAME=protoc-gen-triple-openapi +PROTO_FILE=./example/greet.proto +OUTPUT_DIR=./example + +.PHONY: help +help: + @echo "Usage: make [target]" + @echo "" + @echo "Targets:" + @echo " build Build the binary" + @echo " generate Generate OpenAPI spec from the example proto file" + @echo " clean Remove generated files" + @echo " help Show this help message" + +.PHONY: build +build: + @echo "Building $(BINARY_NAME)..." + go build -o $(BINARY_NAME) . + +.PHONY: generate +generate: build + @echo "Generating OpenAPI spec from $(PROTO_FILE)..." + protoc --plugin=$(BINARY_NAME)=./$(BINARY_NAME) --triple-openapi_out=. $(PROTO_FILE) + +.PHONY: clean +clean: + @echo "Cleaning up..." + rm -f $(BINARY_NAME) $(OUTPUT_DIR)/greet.openapi.yaml Review Comment: The Makefile's clean target removes 'greet.openapi.yaml' but the generated file is named 'greet.triple.openapi.yaml'; update the pattern to match the actual output. ```suggestion rm -f $(BINARY_NAME) $(OUTPUT_DIR)/greet.triple.openapi.yaml ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org For additional commands, e-mail: notifications-h...@dubbo.apache.org
