owncloud / protoc-gen-microweb

Protoc generator for Micro web services
Apache License 2.0
7 stars 6 forks source link

Code from a message-only proto file includes unused code #8

Open jvillafanez opened 3 years ago

jvillafanez commented 3 years ago

Steps to reproduce

  1. Create a message.proto file with only "Message" definition. It's intended to be used as common data type definition
  2. Create a service.proto file with a "Service" definition that imports from the message.proto file.
  3. Generate the code.

Current result

The generated code includes imports "net/http", "github.com/go-chi/chi/v5" and "github.com/go-chi/render" which aren't used.

Example generated code: https://github.com/jvillafanez/prototest001/blob/abf38bb9e4aa6da50ff0c97d59a26ba9905cffd6/gen/ocis/messages/v1/config.pb.web.go

Trying to use / compile the generated code fails.

...../github.com/jvillafanez/prototest001/gen@v0.0.0-20211202132711-abf38bb9e4aa/ocis/messages/v1/accounts.pb.web.go:9:2: imported and not used: "net/http"
...../github.com/jvillafanez/prototest001/gen@v0.0.0-20211202132711-abf38bb9e4aa/ocis/messages/v1/accounts.pb.web.go:11:2: imported and not used: "github.com/go-chi/chi/v5" as chi
...../github.com/jvillafanez/prototest001/gen@v0.0.0-20211202132711-abf38bb9e4aa/ocis/messages/v1/accounts.pb.web.go:12:2: imported and not used: "github.com/go-chi/render"
...../github.com/jvillafanez/prototest001/gen@v0.0.0-20211202132711-abf38bb9e4aa/ocis/messages/v1/config.pb.web.go:9:2: imported and not used: "net/http"
...../github.com/jvillafanez/prototest001/gen@v0.0.0-20211202132711-abf38bb9e4aa/ocis/messages/v1/config.pb.web.go:11:2: imported and not used: "github.com/go-chi/chi/v5" as chi
...../github.com/jvillafanez/prototest001/gen@v0.0.0-20211202132711-abf38bb9e4aa/ocis/messages/v1/config.pb.web.go:12:2: imported and not used: "github.com/go-chi/render"
jvillafanez commented 3 years ago

Imported definitions aren't handled properly neither.

Comparing with both the generated go and go-micro code, there is a missing import statement for the imported definitions, and the usage also needs some touches.

(after commenting the unused imports)

..../github.com/jvillafanez/prototest001/gen@v0.0.0-20211202141858-aff6436eb7dd/ocis/services/accounts/v1/accounts.pb.web.go:55:11: undefined: Account
..../github.com/jvillafanez/prototest001/gen@v0.0.0-20211202141858-aff6436eb7dd/ocis/services/accounts/v1/accounts.pb.web.go:79:11: undefined: Account
..../github.com/jvillafanez/prototest001/gen@v0.0.0-20211202141858-aff6436eb7dd/ocis/services/accounts/v1/accounts.pb.web.go:103:11: undefined: Account
..../github.com/jvillafanez/prototest001/gen@v0.0.0-20211202141858-aff6436eb7dd/ocis/services/accounts/v1/accounts.pb.web.go:196:11: undefined: Group
..../github.com/jvillafanez/prototest001/gen@v0.0.0-20211202141858-aff6436eb7dd/ocis/services/accounts/v1/accounts.pb.web.go:220:11: undefined: Group
..../github.com/jvillafanez/prototest001/gen@v0.0.0-20211202141858-aff6436eb7dd/ocis/services/accounts/v1/accounts.pb.web.go:244:11: undefined: Group
..../github.com/jvillafanez/prototest001/gen@v0.0.0-20211202141858-aff6436eb7dd/ocis/services/accounts/v1/accounts.pb.web.go:291:11: undefined: Group
..../github.com/jvillafanez/prototest001/gen@v0.0.0-20211202141858-aff6436eb7dd/ocis/services/accounts/v1/accounts.pb.web.go:315:11: undefined: Group
jvillafanez commented 3 years ago

Unfinished patch:

diff --git a/microweb.go b/microweb.go
index fc0e2ab..374b628 100644
--- a/microweb.go
+++ b/microweb.go
@@ -4,8 +4,8 @@ import (
    "text/template"

    "github.com/golang/protobuf/proto"
-   "github.com/lyft/protoc-gen-star"
-   "github.com/lyft/protoc-gen-star/lang/go"
+   pgs "github.com/lyft/protoc-gen-star"
+   pgsgo "github.com/lyft/protoc-gen-star/lang/go"
    "google.golang.org/genproto/googleapis/api/annotations"
 )

@@ -35,6 +35,7 @@ func (p *MicroWebModule) InitContext(ctx pgs.BuildContext) {
    tpl := template.New("microweb").Funcs(map[string]interface{}{
        "package":        p.ctx.PackageName,
        "name":           p.ctx.Name,
+       "importPath":     p.ctx.ImportPath,
        "marshaler":      p.marshaler,
        "unmarshaler":    p.unmarshaler,
        "handlerPrefix":  p.handlerPrefix,
@@ -140,6 +141,13 @@ package {{ package . }}
    {{ range .Methods }}
        {{ if or (eq .Input.Name "Empty") (eq .Output.Name "Empty") -}}
            {{ $imports = extraImports "github.com/golang/protobuf/ptypes/empty" }}
+       {{ else }}
+           {{ if ne (importPath .Input) (importPath .File) }}
+               {{ $imports = extraImports (importPath .Input) }}
+           {{ end }}
+           {{ if ne (importPath .Output) (importPath .File) }}
+               {{ $imports = extraImports (importPath .Output) }}
+           {{ end }}
        {{- end }}
    {{ end }}
 {{ end }}
@@ -147,11 +155,15 @@ package {{ package . }}
 import (
    "bytes"
    "encoding/json"
+   {{ if gt (len .Services) 0 -}}
    "net/http"
+   {{- end }}

    "github.com/golang/protobuf/jsonpb"
+   {{ if gt (len .Services) 0 -}}
    "github.com/go-chi/render"
    "github.com/go-chi/chi/v5"
+   {{- end }}
    {{ range $imports }}
        "{{ . }}"
    {{- end }}

That would cover the "message-only" proto files, and the import paths. The generated file doesn't use the imported paths yet.