Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add package_suffix option to protoc-gen-connect-go #803

Merged
merged 10 commits into from
Jan 7, 2025
Merged

Add package_suffix option to protoc-gen-connect-go #803

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
acb8029
Add same_package option to protoc-gen-connect-go
bufdev Dec 18, 2024
7b04e44
Add testcase for same_package generation
emcfarlane Dec 19, 2024
a2b22c5
Fix import for self
emcfarlane Dec 19, 2024
c71a431
Fix schema initialization order for same package
emcfarlane Dec 19, 2024
70ee2a2
Remove var for method descriptors
emcfarlane Dec 19, 2024
189737f
Add package_suffix option (#804)
bufdev Dec 20, 2024
26ef768
Add testdata for generator
emcfarlane Dec 20, 2024
f94f80d
Update godocs for protoc-gen-go
emcfarlane Dec 20, 2024
c8a29a0
Fix windows test
emcfarlane Dec 20, 2024
7349179
Test client handler
emcfarlane Dec 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export PATH := $(BIN):$(PATH)
export GOBIN := $(abspath $(BIN))
COPYRIGHT_YEARS := 2021-2024
LICENSE_IGNORE := --ignore /testdata/
BUF_VERSION := 1.34.0
BUF_VERSION := 1.47.2

.PHONY: help
help: ## Describe useful make targets
Expand Down
1 change: 1 addition & 0 deletions buf.gen.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ plugins:
- local: protoc-gen-connect-go
out: internal/gen
opt: paths=source_relative
clean: true
37 changes: 23 additions & 14 deletions cmd/protoc-gen-connect-go/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ package main

import (
"bytes"
"flag"
"fmt"
"os"
"path"
Expand Down Expand Up @@ -89,39 +90,47 @@ func main() {
fmt.Fprintln(os.Stderr, usage)
os.Exit(1)
}
protogen.Options{}.Run(
var flagSet flag.FlagSet
samePackage := flagSet.Bool("same_package", false, "Generate files into the same Go package as the .pb.go base files.")
protogen.Options{
ParamFunc: flagSet.Set,
}.Run(
func(plugin *protogen.Plugin) error {
plugin.SupportedFeatures = uint64(pluginpb.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL) | uint64(pluginpb.CodeGeneratorResponse_FEATURE_SUPPORTS_EDITIONS)
plugin.SupportedEditionsMinimum = descriptorpb.Edition_EDITION_PROTO2
plugin.SupportedEditionsMaximum = descriptorpb.Edition_EDITION_2023
for _, file := range plugin.Files {
if file.Generate {
generate(plugin, file)
generate(plugin, file, *samePackage)
}
}
return nil
},
)
}

func generate(plugin *protogen.Plugin, file *protogen.File) {
func generate(plugin *protogen.Plugin, file *protogen.File, samePackage bool) {
if len(file.Services) == 0 {
return
}
file.GoPackageName += generatedPackageSuffix

generatedFilenamePrefixToSlash := filepath.ToSlash(file.GeneratedFilenamePrefix)
file.GeneratedFilenamePrefix = path.Join(
path.Dir(generatedFilenamePrefixToSlash),
string(file.GoPackageName),
path.Base(generatedFilenamePrefixToSlash),
)
generatedFile := plugin.NewGeneratedFile(
file.GeneratedFilenamePrefix+generatedFilenameExtension,
protogen.GoImportPath(path.Join(
goImportPath := file.GoImportPath
if !samePackage {
file.GoPackageName += generatedPackageSuffix
generatedFilenamePrefixToSlash := filepath.ToSlash(file.GeneratedFilenamePrefix)
file.GeneratedFilenamePrefix = path.Join(
path.Dir(generatedFilenamePrefixToSlash),
string(file.GoPackageName),
path.Base(generatedFilenamePrefixToSlash),
)
goImportPath = protogen.GoImportPath(path.Join(
string(file.GoImportPath),
string(file.GoPackageName),
)),
))
}
generatedFile := plugin.NewGeneratedFile(
file.GeneratedFilenamePrefix+generatedFilenameExtension,
goImportPath,
)
generatedFile.Import(file.GoImportPath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we only want to do this when !samePackage?

I think we need to come up with a testing strategy. I need to look at the repo a little more to see how existing tests work. Hopefully there's an easy way to plugin a test for this new option.

Copy link
Contributor

@emcfarlane emcfarlane Dec 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put up a testing strategy that runs the protoc-gen-connect-go binary by re-compiling the main.go file. Wanted to bounce this off before extending. Currently it will assert that the expected files in the output but not the contents. We could add a testdata/ directory of .pb.go files to check contents match if we wanted better testing of contents.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely need to test contents, though I don't think "golden files" are sufficient since they are verified by human eyes. We actually need to compile them to make sure we are generating valid code: this option changes references, packages, and imports, which is a high risk of a compiler error if not done correctly.

To get as much coverage as possible with this option, we may even want to generate all test files that we're currently using to test protoc-gen-connect-go, to a different package with this option and make sure it all compiles.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the testdata directory to a set of module proto files which is generated as part of make generate. I found having the golden files output in testdata nice to be able to browse the output. The test code imports these, which asserts that it is compilable and valid code, and then uses it for comparisons when running the different options.

generatePreamble(generatedFile, file)
Expand Down
Loading