From 14fe8c120c222b0b9f0569a13fffbbf372030c46 Mon Sep 17 00:00:00 2001 From: Daniel Pupius Date: Tue, 21 May 2024 15:45:59 -0700 Subject: [PATCH 1/8] Introduce explicit template payload --- data/file.go | 7 ------- generator/generator.go | 26 +++++++++++++++----------- generator/template.go | 7 +++++++ 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/data/file.go b/data/file.go index c03bb7a..8390a2c 100644 --- a/data/file.go +++ b/data/file.go @@ -26,13 +26,6 @@ type File struct { TSFileName string // PackageNonScalarType stores the type inside the same packages within the file, which will be used to figure out external dependencies inside the same package (different files) PackageNonScalarType []Type - - // TODO: These generator options are shoe-horned into the File struct so that - // they can be passed to the template. They should be removed and passed in - // separately. - EnableStylingCheck bool - UseStaticClasses bool - // Dependencies is a list of dependencies for the file, which will be rendered at the top of the file as import statements dependencies []*Dependency } diff --git a/generator/generator.go b/generator/generator.go index 5957154..6c1d7a7 100644 --- a/generator/generator.go +++ b/generator/generator.go @@ -10,7 +10,6 @@ import ( plugin "github.com/golang/protobuf/protoc-gen-go/plugin" log "github.com/sirupsen/logrus" // nolint: depguard - "github.com/dpup/protoc-gen-grpc-gateway-ts/data" "github.com/dpup/protoc-gen-grpc-gateway-ts/registry" "github.com/pkg/errors" ) @@ -41,16 +40,18 @@ func (t *TypeScriptGRPCGatewayGenerator) Generate(req *plugin.CodeGeneratorReque needToGenerateFetchModule := false // feed fileData into rendering process for _, fileData := range filesData { - fileData.EnableStylingCheck = t.Registry.EnableStylingCheck - fileData.UseStaticClasses = t.Registry.UseStaticClasses - if !t.Registry.IsFileToGenerate(fileData.Name) { log.Debugf("file %s is not the file to generate, skipping", fileData.Name) continue } log.Debugf("generating file for %s", fileData.TSFileName) - generated, err := t.generateFile(fileData, tmpl) + data := &templateData{ + File: fileData, + EnableStylingCheck: t.Registry.EnableStylingCheck, + UseStaticClasses: t.Registry.UseStaticClasses, + } + generated, err := t.generateFile(data, tmpl) if err != nil { return nil, errors.Wrap(err, "error generating file") } @@ -73,19 +74,19 @@ func (t *TypeScriptGRPCGatewayGenerator) Generate(req *plugin.CodeGeneratorReque return resp, nil } -func (t *TypeScriptGRPCGatewayGenerator) generateFile(fileData *data.File, tmpl *template.Template) (*plugin.CodeGeneratorResponse_File, error) { +func (t *TypeScriptGRPCGatewayGenerator) generateFile(data *templateData, tmpl *template.Template) (*plugin.CodeGeneratorResponse_File, error) { w := bytes.NewBufferString("") - if fileData.IsEmpty() { + if data.IsEmpty() { w.Write([]byte(fmt.Sprintln("export default {}"))) } else { - err := tmpl.Execute(w, fileData) + err := tmpl.Execute(w, data) if err != nil { - return nil, errors.Wrapf(err, "error generating ts file for %s", fileData.Name) + return nil, errors.Wrapf(err, "error generating ts file for %s", data.Name) } } - fileName := fileData.TSFileName + fileName := data.TSFileName content := strings.TrimSpace(w.String()) return &plugin.CodeGeneratorResponse_File{ @@ -98,7 +99,10 @@ func (t *TypeScriptGRPCGatewayGenerator) generateFile(fileData *data.File, tmpl func (t *TypeScriptGRPCGatewayGenerator) generateFetchModule(tmpl *template.Template) (*plugin.CodeGeneratorResponse_File, error) { w := bytes.NewBufferString("") fileName := filepath.Join(t.Registry.FetchModuleDirectory, t.Registry.FetchModuleFilename) - err := tmpl.Execute(w, &data.File{EnableStylingCheck: t.Registry.EnableStylingCheck}) + err := tmpl.Execute(w, &templateData{ + EnableStylingCheck: t.Registry.EnableStylingCheck, + UseStaticClasses: t.Registry.UseStaticClasses, + }) if err != nil { return nil, errors.Wrapf(err, "error generating fetch module at %s", fileName) } diff --git a/generator/template.go b/generator/template.go index c00b5d1..99bb26d 100644 --- a/generator/template.go +++ b/generator/template.go @@ -34,6 +34,13 @@ var fetchTmplHeader = `{{- if not .EnableStylingCheck}} var fetchTmpl = fetchTmplHeader + fetchTmplScript +// Data object injected into the templates. +type templateData struct { + *data.File + EnableStylingCheck bool + UseStaticClasses bool +} + // GetTemplate gets the templates to for the typescript file func GetTemplate(r *registry.Registry) *template.Template { t := template.New("file") From 09fdc8180622b76af6a9e24324b3580498db6aaf Mon Sep 17 00:00:00 2001 From: Daniel Pupius Date: Tue, 21 May 2024 15:54:30 -0700 Subject: [PATCH 2/8] Rename template related methods and fields --- data/service.go | 4 ++-- generator/generator.go | 17 ++++++++--------- generator/template.go | 18 +++++++++--------- registry/file.go | 2 +- 4 files changed, 20 insertions(+), 21 deletions(-) diff --git a/data/service.go b/data/service.go index 18e2d46..142280f 100644 --- a/data/service.go +++ b/data/service.go @@ -35,8 +35,8 @@ func (s Services) HasUnaryCallMethod() bool { return false } -// NeedsFetchModule returns whether the given services needs fetch module support -func (s Services) NeedsFetchModule() bool { +// RequiresFetchModule returns whether the given services needs fetch module support +func (s Services) RequiresFetchModule() bool { hasServices := len(s) > 0 return hasServices && (s.HasUnaryCallMethod() || s.HasServerStreamingMethod()) } diff --git a/generator/generator.go b/generator/generator.go index 6c1d7a7..ab36a1d 100644 --- a/generator/generator.go +++ b/generator/generator.go @@ -34,10 +34,10 @@ func (t *TypeScriptGRPCGatewayGenerator) Generate(req *plugin.CodeGeneratorReque if err != nil { return nil, errors.Wrap(err, "error analysing proto files") } - tmpl := GetTemplate(t.Registry) + tmpl := ServiceTemplate(t.Registry) log.Debugf("files to generate %v", req.GetFileToGenerate()) - needToGenerateFetchModule := false + requiresFetchModule := false // feed fileData into rendering process for _, fileData := range filesData { if !t.Registry.IsFileToGenerate(fileData.Name) { @@ -46,7 +46,7 @@ func (t *TypeScriptGRPCGatewayGenerator) Generate(req *plugin.CodeGeneratorReque } log.Debugf("generating file for %s", fileData.TSFileName) - data := &templateData{ + data := &TemplateData{ File: fileData, EnableStylingCheck: t.Registry.EnableStylingCheck, UseStaticClasses: t.Registry.UseStaticClasses, @@ -56,12 +56,11 @@ func (t *TypeScriptGRPCGatewayGenerator) Generate(req *plugin.CodeGeneratorReque return nil, errors.Wrap(err, "error generating file") } resp.File = append(resp.File, generated) - needToGenerateFetchModule = needToGenerateFetchModule || fileData.Services.NeedsFetchModule() + requiresFetchModule = requiresFetchModule || fileData.Services.RequiresFetchModule() } - if needToGenerateFetchModule { - // generate fetch module - fetchTmpl := GetFetchModuleTemplate() + if requiresFetchModule { + fetchTmpl := FetchModuleTemplate() log.Debugf("generate fetch template") generatedFetch, err := t.generateFetchModule(fetchTmpl) if err != nil { @@ -74,7 +73,7 @@ func (t *TypeScriptGRPCGatewayGenerator) Generate(req *plugin.CodeGeneratorReque return resp, nil } -func (t *TypeScriptGRPCGatewayGenerator) generateFile(data *templateData, tmpl *template.Template) (*plugin.CodeGeneratorResponse_File, error) { +func (t *TypeScriptGRPCGatewayGenerator) generateFile(data *TemplateData, tmpl *template.Template) (*plugin.CodeGeneratorResponse_File, error) { w := bytes.NewBufferString("") if data.IsEmpty() { @@ -99,7 +98,7 @@ func (t *TypeScriptGRPCGatewayGenerator) generateFile(data *templateData, tmpl * func (t *TypeScriptGRPCGatewayGenerator) generateFetchModule(tmpl *template.Template) (*plugin.CodeGeneratorResponse_File, error) { w := bytes.NewBufferString("") fileName := filepath.Join(t.Registry.FetchModuleDirectory, t.Registry.FetchModuleFilename) - err := tmpl.Execute(w, &templateData{ + err := tmpl.Execute(w, &TemplateData{ EnableStylingCheck: t.Registry.EnableStylingCheck, UseStaticClasses: t.Registry.UseStaticClasses, }) diff --git a/generator/template.go b/generator/template.go index 99bb26d..7b0cb87 100644 --- a/generator/template.go +++ b/generator/template.go @@ -35,14 +35,14 @@ var fetchTmplHeader = `{{- if not .EnableStylingCheck}} var fetchTmpl = fetchTmplHeader + fetchTmplScript // Data object injected into the templates. -type templateData struct { +type TemplateData struct { *data.File EnableStylingCheck bool UseStaticClasses bool } -// GetTemplate gets the templates to for the typescript file -func GetTemplate(r *registry.Registry) *template.Template { +// ServiceTemplate gets the template for the primary typescript file. +func ServiceTemplate(r *registry.Registry) *template.Template { t := template.New("file") t = t.Funcs(sprig.TxtFuncMap()) @@ -63,6 +63,12 @@ func GetTemplate(r *registry.Registry) *template.Template { return t } +// FetchModuleTemplate returns the go template for fetch module. +func FetchModuleTemplate() *template.Template { + t := template.New("fetch") + return template.Must(t.Parse(fetchTmpl)) +} + func fieldName(r *registry.Registry) func(name string) string { return func(name string) string { if r.UseProtoNames { @@ -124,12 +130,6 @@ func buildInitReq(method data.Method) string { return strings.Join(fields, ", ") } -// GetFetchModuleTemplate returns the go template for fetch module -func GetFetchModuleTemplate() *template.Template { - t := template.New("fetch") - return template.Must(t.Parse(fetchTmpl)) -} - // include is the include template functions copied from // copied from: https://github.com/helm/helm/blob/8648ccf5d35d682dcd5f7a9c2082f0aaf071e817/pkg/engine/engine.go#L147-L154 func include(t *template.Template) func(name string, data interface{}) (string, error) { diff --git a/registry/file.go b/registry/file.go index b9e6bb0..00b498d 100644 --- a/registry/file.go +++ b/registry/file.go @@ -51,7 +51,7 @@ func (r *Registry) analyseFile(f *descriptorpb.FileDescriptorProto) (*data.File, } func (r *Registry) addFetchModuleDependencies(fileData *data.File) error { - if !fileData.Services.NeedsFetchModule() { + if !fileData.Services.RequiresFetchModule() { log.Debugf("no services found for %s, skipping fetch module", fileData.Name) return nil } From e550a8cb0e80bf9788c1004123e692d14c40f30f Mon Sep 17 00:00:00 2001 From: Daniel Pupius Date: Tue, 21 May 2024 16:07:20 -0700 Subject: [PATCH 3/8] Embed Options inside Registry to remove duplicated defs --- generator/template_test.go | 2 +- main.go | 2 +- registry/registry.go | 44 +++++++++----------------------------- 3 files changed, 12 insertions(+), 36 deletions(-) diff --git a/generator/template_test.go b/generator/template_test.go index c99344f..77fc7b9 100644 --- a/generator/template_test.go +++ b/generator/template_test.go @@ -27,7 +27,7 @@ func TestFieldName(t *testing.T) { } for _, tt := range tests { t.Run(tt.input, func(t *testing.T) { - r := ®istry.Registry{UseProtoNames: tt.useProtoNames} + r := ®istry.Registry{Options: registry.Options{UseProtoNames: tt.useProtoNames}} fn := fieldName(r) if got := fn(tt.input); got != tt.want { assert.Equal(t, got, tt.want, "fieldName(%s) = %s, want %s", tt.input, got, tt.want) diff --git a/main.go b/main.go index 79c79c0..d68b643 100644 --- a/main.go +++ b/main.go @@ -58,7 +58,7 @@ func run() error { EnableStylingCheck: *enableStylingCheck, EmitUnpopulated: *emitUnpopulated, FetchModuleDirectory: *fetchModuleDirectory, - FetchModuleFileName: *fetchModuleFilename, + FetchModuleFilename: *fetchModuleFilename, TSImportRoots: *tsImportRoots, TSImportRootAliases: *tsImportRootAliases, }) diff --git a/registry/registry.go b/registry/registry.go index b59c699..2e8ce50 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -23,8 +23,8 @@ type Options struct { TSImportRootAliases string // FetchModuleDirectory is the parameter for directory where fetch module will live FetchModuleDirectory string - // FetchModuleFileName is the file name for the individual fetch module - FetchModuleFileName string + // FetchModuleFilename is the file name for the individual fetch module + FetchModuleFilename string // UseProtoNames will generate field names the same as defined in the proto UseProtoNames bool // UseStaticClasses will cause the generator to generate a static class in the form ServiceName.MethodName, which is @@ -41,6 +41,8 @@ type Options struct { // Registry analyze generation request, spits out the data the the rendering process // it also holds the information about all the types type Registry struct { + Options + // Types stores the type information keyed by the fully qualified name of a type Types map[string]*TypeInformation @@ -53,27 +55,6 @@ type Registry struct { // TSImportRootAliases if not empty will substitutes the common import root when writing the import into the js file TSImportRootAliases []string - // FetchModuleDirectory is the directory to place fetch module file - FetchModuleDirectory string - - // FetchModuleFilename is the filename for the fetch module - FetchModuleFilename string - - // UseProtoNames will cause the generator to generate field name the same as defined in the proto - UseProtoNames bool - - // UseStaticClasses will cause the generator to generate a static class in the form ServiceName.MethodName, which is - // the legacy behavior for this generator. If set to false, the generator will generate a client class with methods - // as well as static methods exported for each service method. - UseStaticClasses bool - - // EmitUnpopulated mirrors the grpc gateway protojson configuration of the same name and allows - // clients to differentiate between zero values and optional values that aren't set. - EmitUnpopulated bool - - // EnableStylingCheck enables both eslint and tsc check for the generated code - EnableStylingCheck bool - // TSPackages stores the package name keyed by the TS file name TSPackages map[string]string } @@ -88,19 +69,14 @@ func NewRegistry(opts Options) (*Registry, error) { } log.Debugf("found fetch module directory %s", opts.FetchModuleDirectory) - log.Debugf("found fetch module name %s", opts.FetchModuleFileName) + log.Debugf("found fetch module name %s", opts.FetchModuleFilename) return &Registry{ - Types: make(map[string]*TypeInformation), - TSImportRoots: tsImportRoots, - TSImportRootAliases: tsImportRootAliases, - FetchModuleDirectory: opts.FetchModuleDirectory, - FetchModuleFilename: opts.FetchModuleFileName, - TSPackages: make(map[string]string), - UseProtoNames: opts.UseProtoNames, - UseStaticClasses: opts.UseStaticClasses, - EmitUnpopulated: opts.EmitUnpopulated, - EnableStylingCheck: opts.EnableStylingCheck, + Options: opts, + Types: make(map[string]*TypeInformation), + TSPackages: make(map[string]string), + TSImportRoots: tsImportRoots, + TSImportRootAliases: tsImportRootAliases, }, nil } From cb485f814f96545e12bfdb07f908011017962541 Mon Sep 17 00:00:00 2001 From: Daniel Pupius Date: Tue, 21 May 2024 16:58:45 -0700 Subject: [PATCH 4/8] Add stricter lint config and fix a number of issues --- .golangci.yaml | 349 +++++++++++++++++++++++++++++++++ main.go | 33 ++-- registry/enum.go | 10 +- registry/field.go | 91 +++++---- registry/file.go | 10 +- registry/message.go | 18 +- registry/registry.go | 78 ++++---- registry/service.go | 9 +- test/{test.go => init_test.go} | 0 9 files changed, 486 insertions(+), 112 deletions(-) create mode 100644 .golangci.yaml rename test/{test.go => init_test.go} (100%) diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 0000000..bf91977 --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,349 @@ +# This code is licensed under the terms of the MIT license https://opensource.org/license/mit +# Copyright (c) 2021 Marat Reymers + +## Golden config for golangci-lint v1.58.2 +# +# This is the best config for golangci-lint based on my experience and opinion. +# It is very strict, but not extremely strict. +# Feel free to adapt and change it for your needs. + +run: + # Timeout for analysis, e.g. 30s, 5m. + # Default: 1m + timeout: 3m + + +# This file contains only configs which differ from defaults. +# All possible options can be found here https://github.com/golangci/golangci-lint/blob/master/.golangci.reference.yml +linters-settings: + cyclop: + # The maximal code complexity to report. + # Default: 10 + max-complexity: 30 + # The maximal average package complexity. + # If it's higher than 0.0 (float) the check is enabled + # Default: 0.0 + package-average: 10.0 + + errcheck: + # Report about not checking of errors in type assertions: `a := b.(MyStruct)`. + # Such cases aren't reported by default. + # Default: false + check-type-assertions: true + + exhaustive: + # Program elements to check for exhaustiveness. + # Default: [ switch ] + check: + - switch + - map + + exhaustruct: + # List of regular expressions to exclude struct packages and their names from checks. + # Regular expressions must match complete canonical struct package/name/structname. + # Default: [] + exclude: + # std libs + - "^net/http.Client$" + - "^net/http.Cookie$" + - "^net/http.Request$" + - "^net/http.Response$" + - "^net/http.Server$" + - "^net/http.Transport$" + - "^net/url.URL$" + - "^os/exec.Cmd$" + - "^reflect.StructField$" + # public libs + - "^github.com/Shopify/sarama.Config$" + - "^github.com/Shopify/sarama.ProducerMessage$" + - "^github.com/mitchellh/mapstructure.DecoderConfig$" + - "^github.com/prometheus/client_golang/.+Opts$" + - "^github.com/spf13/cobra.Command$" + - "^github.com/spf13/cobra.CompletionOptions$" + - "^github.com/stretchr/testify/mock.Mock$" + - "^github.com/testcontainers/testcontainers-go.+Request$" + - "^github.com/testcontainers/testcontainers-go.FromDockerfile$" + - "^golang.org/x/tools/go/analysis.Analyzer$" + - "^google.golang.org/protobuf/.+Options$" + - "^gopkg.in/yaml.v3.Node$" + + funlen: + # Checks the number of lines in a function. + # If lower than 0, disable the check. + # Default: 60 + lines: 100 + # Checks the number of statements in a function. + # If lower than 0, disable the check. + # Default: 40 + statements: 50 + # Ignore comments when counting lines. + # Default false + ignore-comments: true + + gocognit: + # Minimal code complexity to report. + # Default: 30 (but we recommend 10-20) + min-complexity: 45 + + gocritic: + # Settings passed to gocritic. + # The settings key is the name of a supported gocritic checker. + # The list of supported checkers can be find in https://go-critic.github.io/overview. + settings: + captLocal: + # Whether to restrict checker to params only. + # Default: true + paramsOnly: false + underef: + # Whether to skip (*x).method() calls where x is a pointer receiver. + # Default: true + skipRecvDeref: false + + gomodguard: + blocked: + # List of blocked modules. + # Default: [] + modules: + - github.com/golang/protobuf: + recommendations: + - google.golang.org/protobuf + reason: "see https://developers.google.com/protocol-buffers/docs/reference/go/faq#modules" + - github.com/satori/go.uuid: + recommendations: + - github.com/google/uuid + reason: "satori's package is not maintained" + - github.com/gofrs/uuid: + recommendations: + - github.com/gofrs/uuid/v5 + reason: "gofrs' package was not go module before v5" + + govet: + # Enable all analyzers. + # Default: false + enable-all: true + # Disable analyzers by name. + # Run `go tool vet help` to see all analyzers. + # Default: [] + disable: + - fieldalignment # too strict + # Settings per analyzer. + settings: + shadow: + # Whether to be strict about shadowing; can be noisy. + # Default: false + strict: false + + inamedparam: + # Skips check for interface methods with only a single parameter. + # Default: false + skip-single-param: true + + mnd: + # List of function patterns to exclude from analysis. + # Values always ignored: `time.Date`, + # `strconv.FormatInt`, `strconv.FormatUint`, `strconv.FormatFloat`, + # `strconv.ParseInt`, `strconv.ParseUint`, `strconv.ParseFloat`. + # Default: [] + ignored-functions: + - args.Error + - flag.Arg + - flag.Duration.* + - flag.Float.* + - flag.Int.* + - flag.Uint.* + - os.Chmod + - os.Mkdir.* + - os.OpenFile + - os.WriteFile + - prometheus.ExponentialBuckets.* + - prometheus.LinearBuckets + + nakedret: + # Make an issue if func has more lines of code than this setting, and it has naked returns. + # Default: 30 + max-func-lines: 0 + + nolintlint: + # Exclude following linters from requiring an explanation. + # Default: [] + allow-no-explanation: [funlen, gocognit, lll] + # Enable to require an explanation of nonzero length after each nolint directive. + # Default: false + require-explanation: true + # Enable to require nolint directives to mention the specific linter being suppressed. + # Default: false + require-specific: true + + perfsprint: + # Optimizes into strings concatenation. + # Default: true + strconcat: false + + rowserrcheck: + # database/sql is always checked + # Default: [] + packages: + - github.com/jmoiron/sqlx + + sloglint: + # Enforce not using global loggers. + # Values: + # - "": disabled + # - "all": report all global loggers + # - "default": report only the default slog logger + # Default: "" + no-global: "all" + # Enforce using methods that accept a context. + # Values: + # - "": disabled + # - "all": report all contextless calls + # - "scope": report only if a context exists in the scope of the outermost function + # Default: "" + context: "scope" + + tenv: + # The option `all` will run against whole test files (`_test.go`) regardless of method/function signatures. + # Otherwise, only methods that take `*testing.T`, `*testing.B`, and `testing.TB` as arguments are checked. + # Default: false + all: true + + +linters: + disable-all: true + enable: + ## enabled by default + - errcheck # checking for unchecked errors, these unchecked errors can be critical bugs in some cases + - gosimple # specializes in simplifying a code + - govet # reports suspicious constructs, such as Printf calls whose arguments do not align with the format string + - ineffassign # detects when assignments to existing variables are not used + - staticcheck # is a go vet on steroids, applying a ton of static analysis checks + - typecheck # like the front-end of a Go compiler, parses and type-checks Go code + - unused # checks for unused constants, variables, functions and types + ## disabled by default + - asasalint # checks for pass []any as any in variadic func(...any) + - asciicheck # checks that your code does not contain non-ASCII identifiers + - bidichk # checks for dangerous unicode character sequences + - bodyclose # checks whether HTTP response body is closed successfully + - canonicalheader # checks whether net/http.Header uses canonical header + - copyloopvar # detects places where loop variables are copied + - cyclop # checks function and package cyclomatic complexity + - dupl # tool for code clone detection + - durationcheck # checks for two durations multiplied together + - errname # checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error + - errorlint # finds code that will cause problems with the error wrapping scheme introduced in Go 1.13 + - exhaustive # checks exhaustiveness of enum switch statements + - exportloopref # checks for pointers to enclosing loop variables + - fatcontext # detects nested contexts in loops + - forbidigo # forbids identifiers + - funlen # tool for detection of long functions + - gocheckcompilerdirectives # validates go compiler directive comments (//go:) + - gochecknoglobals # checks that no global variables exist + - gochecknoinits # checks that no init functions are present in Go code + - gochecksumtype # checks exhaustiveness on Go "sum types" + - gocognit # computes and checks the cognitive complexity of functions + - goconst # finds repeated strings that could be replaced by a constant + - gocritic # provides diagnostics that check for bugs, performance and style issues + - gocyclo # computes and checks the cyclomatic complexity of functions + - godot # checks if comments end in a period + - goimports # in addition to fixing imports, goimports also formats your code in the same style as gofmt + - gomoddirectives # manages the use of 'replace', 'retract', and 'excludes' directives in go.mod + - gomodguard # allow and block lists linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations + - goprintffuncname # checks that printf-like functions are named with f at the end + - gosec # inspects source code for security problems + - intrange # finds places where for loops could make use of an integer range + - lll # reports long lines + - loggercheck # checks key value pairs for common logger libraries (kitlog,klog,logr,zap) + - makezero # finds slice declarations with non-zero initial length + - mirror # reports wrong mirror patterns of bytes/strings usage + - mnd # detects magic numbers + - musttag # enforces field tags in (un)marshaled structs + - nakedret # finds naked returns in functions greater than a specified function length + - nilerr # finds the code that returns nil even if it checks that the error is not nil + - nilnil # checks that there is no simultaneous return of nil error and an invalid value + - noctx # finds sending http request without context.Context + - nolintlint # reports ill-formed or insufficient nolint directives + - nonamedreturns # reports all named returns + - nosprintfhostport # checks for misuse of Sprintf to construct a host with port in a URL + - perfsprint # checks that fmt.Sprintf can be replaced with a faster alternative + - predeclared # finds code that shadows one of Go's predeclared identifiers + - promlinter # checks Prometheus metrics naming via promlint + - reassign # checks that package variables are not reassigned + - revive # fast, configurable, extensible, flexible, and beautiful linter for Go, drop-in replacement of golint + - rowserrcheck # checks whether Err of rows is checked successfully + - sloglint # ensure consistent code style when using log/slog + - spancheck # checks for mistakes with OpenTelemetry/Census spans + - sqlclosecheck # checks that sql.Rows and sql.Stmt are closed + - stylecheck # is a replacement for golint + - tenv # detects using os.Setenv instead of t.Setenv since Go1.17 + - testableexamples # checks if examples are testable (have an expected output) + - testifylint # checks usage of github.com/stretchr/testify + - testpackage # makes you use a separate _test package + - tparallel # detects inappropriate usage of t.Parallel() method in your Go test codes + - unconvert # removes unnecessary type conversions + - unparam # reports unused function parameters + - usestdlibvars # detects the possibility to use variables/constants from the Go standard library + - wastedassign # finds wasted assignment statements + - whitespace # detects leading and trailing whitespace + + ## you may want to enable + #- protogetter # reports direct reads from proto message fields when getters should be used + #- nestif # reports deeply nested if statements + #- decorder # checks declaration order and count of types, constants, variables and functions + #- exhaustruct # [highly recommend to enable] checks if all structure fields are initialized + #- gci # controls golang package import order and makes it always deterministic + #- ginkgolinter # [if you use ginkgo/gomega] enforces standards of using ginkgo and gomega + #- godox # detects FIXME, TODO and other comment keywords + #- goheader # checks is file header matches to pattern + #- inamedparam # [great idea, but too strict, need to ignore a lot of cases by default] reports interfaces with unnamed method parameters + #- interfacebloat # checks the number of methods inside an interface + #- ireturn # accept interfaces, return concrete types + #- prealloc # [premature optimization, but can be used in some cases] finds slice declarations that could potentially be preallocated + #- tagalign # checks that struct tags are well aligned + #- varnamelen # [great idea, but too many false positives] checks that the length of a variable's name matches its scope + #- wrapcheck # checks that errors returned from external packages are wrapped + #- zerologlint # detects the wrong usage of zerolog that a user forgets to dispatch zerolog.Event + + ## disabled + #- containedctx # detects struct contained context.Context field + #- contextcheck # [too many false positives] checks the function whether use a non-inherited context + #- depguard # [replaced by gomodguard] checks if package imports are in a list of acceptable packages + #- dogsled # checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) + #- dupword # [useless without config] checks for duplicate words in the source code + #- err113 # [too strict] checks the errors handling expressions + #- errchkjson # [don't see profit + I'm against of omitting errors like in the first example https://github.com/breml/errchkjson] checks types passed to the json encoding functions. Reports unsupported types and optionally reports occasions, where the check for the returned error can be omitted + #- execinquery # [deprecated] checks query string in Query function which reads your Go src files and warning it finds + #- forcetypeassert # [replaced by errcheck] finds forced type assertions + #- gofmt # [replaced by goimports] checks whether code was gofmt-ed + #- gofumpt # [replaced by goimports, gofumports is not available yet] checks whether code was gofumpt-ed + #- gosmopolitan # reports certain i18n/l10n anti-patterns in your Go codebase + #- grouper # analyzes expression groups + #- importas # enforces consistent import aliases + #- maintidx # measures the maintainability index of each function + #- misspell # [useless] finds commonly misspelled English words in comments + #- nlreturn # [too strict and mostly code is not more readable] checks for a new line before return and branch statements to increase code clarity + #- paralleltest # [too many false positives] detects missing usage of t.Parallel() method in your Go test + #- tagliatelle # checks the struct tags + #- thelper # detects golang test helpers without t.Helper() call and checks the consistency of test helpers + #- wsl # [too strict and mostly code is not more readable] whitespace linter forces you to use empty lines + + +issues: + # Maximum count of issues with the same text. + # Set to 0 to disable. + # Default: 3 + max-same-issues: 50 + + exclude-rules: + - source: "(noinspection|TODO)" + linters: [godot] + - source: "//noinspection" + linters: [gocritic] + - path: "_test\\.go" + linters: + - bodyclose + - dupl + - funlen + - goconst + - gosec + - noctx + - wrapcheck diff --git a/main.go b/main.go index d68b643..344928f 100644 --- a/main.go +++ b/main.go @@ -22,23 +22,26 @@ func main() { } func run() error { - var useProtoNames = flag.Bool("use_proto_names", false, "field names will match the protofile instead of lowerCamelCase") - var useStaticClasses = flag.Bool("use_static_classes", true, "generate static classes instead of functions and a client class") - var emitUnpopulated = flag.Bool("emit_unpopulated", false, "expect the gRPC Gateway to send zero values over the wire") + var ( + useProtoNames = flag.Bool("use_proto_names", false, "field names will match the protofile instead of lowerCamelCase") + useStaticClasses = flag.Bool("use_static_classes", true, "generate static classes instead of functions and a client class") + emitUnpopulated = flag.Bool("emit_unpopulated", false, "expect the gRPC Gateway to send zero values over the wire") - var fetchModuleDirectory = flag.String("fetch_module_directory", ".", "where shared typescript file should be placed, default $(pwd)") - var fetchModuleFilename = flag.String("fetch_module_filename", "fetch.pb.ts", "name of shard typescript file") - var tsImportRoots = flag.String("ts_import_roots", "", "defaults to $(pwd)") - var tsImportRootAliases = flag.String("ts_import_root_aliases", "", "use import aliases instead of relative paths") + fetchModuleDirectory = flag.String("fetch_module_directory", ".", "where shared typescript file should be placed, default $(pwd)") + fetchModuleFilename = flag.String("fetch_module_filename", "fetch.pb.ts", "name of shard typescript file") + tsImportRoots = flag.String("ts_import_roots", "", "defaults to $(pwd)") + tsImportRootAliases = flag.String("ts_import_root_aliases", "", "use import aliases instead of relative paths") - var enableStylingCheck = flag.Bool("enable_styling_check", false, "TODO") + enableStylingCheck = flag.Bool("enable_styling_check", false, "TODO") - var logtostderr = flag.Bool("logtostderr", false, "turn on logging to stderr") - var loglevel = flag.String("loglevel", "info", "defines the logging level. Values are debug, info, warn, error") + logtostderr = flag.Bool("logtostderr", false, "turn on logging to stderr") + loglevel = flag.String("loglevel", "info", "defines the logging level. Values are debug, info, warn, error") + ) flag.Parse() req := decodeReq() + paramsMap := getParamsMap(req) for k, v := range paramsMap { if k != "" { @@ -70,15 +73,19 @@ func run() error { if err != nil { return fmt.Errorf("error instantiating a new generator: %w", err) } + log.Debug("Starts generating file request") + resp, err := g.Generate(req) if err != nil { return fmt.Errorf("error generating output: %w", err) } + features := uint64(pluginpb.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL) resp.SupportedFeatures = &features encodeResponse(resp) + log.Debug("generation finished") return nil @@ -92,11 +99,11 @@ func configureLogging(enableLogging bool, levelStr string) error { log.SetOutput(os.Stderr) log.Debugf("Logging configured completed, logging has been enabled") if levelStr != "" { - level, err := log.ParseLevel(levelStr) - if err != nil { + if level, err := log.ParseLevel(levelStr); err != nil { return fmt.Errorf("error parsing log level %s: %s", levelStr, err) + } else { + log.SetLevel(level) } - log.SetLevel(level) } else { log.SetLevel(log.InfoLevel) } diff --git a/registry/enum.go b/registry/enum.go index 8aa01a9..6ebb8fb 100644 --- a/registry/enum.go +++ b/registry/enum.go @@ -1,12 +1,15 @@ package registry import ( - descriptorpb "github.com/golang/protobuf/protoc-gen-go/descriptor" - "github.com/dpup/protoc-gen-grpc-gateway-ts/data" + "google.golang.org/protobuf/types/descriptorpb" ) -func (r *Registry) analyseEnumType(fileData *data.File, packageName, fileName string, parents []string, enum *descriptorpb.EnumDescriptorProto) { +func (r *Registry) analyseEnumType( + fileData *data.File, + packageName, fileName string, + parents []string, + enum *descriptorpb.EnumDescriptorProto) { packageIdentifier := r.getNameOfPackageLevelIdentifier(parents, enum.GetName()) fqName := r.getFullQualifiedName(packageName, parents, enum.GetName()) protoType := descriptorpb.FieldDescriptorProto_TYPE_ENUM @@ -27,5 +30,4 @@ func (r *Registry) analyseEnumType(fileData *data.File, packageName, fileName st } fileData.Enums = append(fileData.Enums, enumData) - } diff --git a/registry/field.go b/registry/field.go index 23ce725..ad588a8 100644 --- a/registry/field.go +++ b/registry/field.go @@ -1,57 +1,62 @@ package registry import ( - descriptorpb "github.com/golang/protobuf/protoc-gen-go/descriptor" - "github.com/dpup/protoc-gen-grpc-gateway-ts/data" + "google.golang.org/protobuf/types/descriptorpb" ) -// getFieldType generates an intermediate type and leave the rendering logic to choose what to render +// getFieldType generates an intermediate type and leave the rendering logic to +// choose what to render. func (r *Registry) getFieldType(f *descriptorpb.FieldDescriptorProto) string { - typeName := "" - if f.Type != nil { - switch *f.Type { - case descriptorpb.FieldDescriptorProto_TYPE_MESSAGE, descriptorpb.FieldDescriptorProto_TYPE_ENUM: - typeName = f.GetTypeName() - case descriptorpb.FieldDescriptorProto_TYPE_STRING: - typeName = "string" - case descriptorpb.FieldDescriptorProto_TYPE_BOOL: - typeName = "bool" - case descriptorpb.FieldDescriptorProto_TYPE_BYTES: - typeName = "bytes" - case descriptorpb.FieldDescriptorProto_TYPE_FLOAT: - typeName = "float" - case descriptorpb.FieldDescriptorProto_TYPE_DOUBLE: - typeName = "double" - case descriptorpb.FieldDescriptorProto_TYPE_FIXED32: - typeName = "fixed32" - case descriptorpb.FieldDescriptorProto_TYPE_SFIXED32: - typeName = "sfixed32" - case descriptorpb.FieldDescriptorProto_TYPE_INT32: - typeName = "int32" - case descriptorpb.FieldDescriptorProto_TYPE_SINT32: - typeName = "sint32" - case descriptorpb.FieldDescriptorProto_TYPE_UINT32: - typeName = "uint32" - case descriptorpb.FieldDescriptorProto_TYPE_FIXED64: - typeName = "fixed64" - case descriptorpb.FieldDescriptorProto_TYPE_SFIXED64: - typeName = "sfixed64" - case descriptorpb.FieldDescriptorProto_TYPE_INT64: - typeName = "int64" - case descriptorpb.FieldDescriptorProto_TYPE_SINT64: - typeName = "sint64" - case descriptorpb.FieldDescriptorProto_TYPE_UINT64: - typeName = "uint64" - } + if f.Type == nil { + return "" + } + switch *f.Type { + case descriptorpb.FieldDescriptorProto_TYPE_MESSAGE, + descriptorpb.FieldDescriptorProto_TYPE_ENUM, + descriptorpb.FieldDescriptorProto_TYPE_GROUP: + return f.GetTypeName() + case descriptorpb.FieldDescriptorProto_TYPE_STRING: + return "string" + case descriptorpb.FieldDescriptorProto_TYPE_BOOL: + return "bool" + case descriptorpb.FieldDescriptorProto_TYPE_BYTES: + return "bytes" + case descriptorpb.FieldDescriptorProto_TYPE_FLOAT: + return "float" + case descriptorpb.FieldDescriptorProto_TYPE_DOUBLE: + return "double" + case descriptorpb.FieldDescriptorProto_TYPE_FIXED32: + return "fixed32" + case descriptorpb.FieldDescriptorProto_TYPE_SFIXED32: + return "sfixed32" + case descriptorpb.FieldDescriptorProto_TYPE_INT32: + return "int32" + case descriptorpb.FieldDescriptorProto_TYPE_SINT32: + return "sint32" + case descriptorpb.FieldDescriptorProto_TYPE_UINT32: + return "uint32" + case descriptorpb.FieldDescriptorProto_TYPE_FIXED64: + return "fixed64" + case descriptorpb.FieldDescriptorProto_TYPE_SFIXED64: + return "sfixed64" + case descriptorpb.FieldDescriptorProto_TYPE_INT64: + return "int64" + case descriptorpb.FieldDescriptorProto_TYPE_SINT64: + return "sint64" + case descriptorpb.FieldDescriptorProto_TYPE_UINT64: + return "uint64" + default: + return "" } - - return typeName } -func (r *Registry) analyseField(fileData *data.File, msgData *data.Message, packageName string, f *descriptorpb.FieldDescriptorProto) { +func (r *Registry) analyseField( + fileData *data.File, + msgData *data.Message, + packageName string, + f *descriptorpb.FieldDescriptorProto) { fqTypeName := r.getFieldType(f) - isExternal := r.isExternalDependenciesOutsidePackage(fqTypeName, packageName) fieldData := &data.Field{ diff --git a/registry/file.go b/registry/file.go index 00b498d..5eac2e8 100644 --- a/registry/file.go +++ b/registry/file.go @@ -6,10 +6,10 @@ import ( "github.com/dpup/protoc-gen-grpc-gateway-ts/data" "github.com/dpup/protoc-gen-grpc-gateway-ts/options" - descriptorpb "github.com/golang/protobuf/protoc-gen-go/descriptor" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" // nolint: depguard + log "github.com/sirupsen/logrus" //nolint: depguard // Need to remove "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/descriptorpb" ) func (r *Registry) analyseFile(f *descriptorpb.FileDescriptorProto) (*data.File, error) { @@ -21,7 +21,7 @@ func (r *Registry) analyseFile(f *descriptorpb.FileDescriptorProto) (*data.File, fileData.Name = fileName fileData.TSFileName = data.GetTSFileName(fileName) if proto.HasExtension(f.Options, options.E_TsPackage) { - r.TSPackages[fileData.TSFileName] = proto.GetExtension(f.Options, options.E_TsPackage).(string) + r.TSPackages[fileData.TSFileName], _ = proto.GetExtension(f.Options, options.E_TsPackage).(string) } // analyse enums @@ -63,7 +63,6 @@ func (r *Registry) addFetchModuleDependencies(fileData *data.File) error { foundAtRoot, alias, err := r.findRootAliasForPath(func(absRoot string) (bool, error) { return strings.HasPrefix(absDir, absRoot), nil - }) if err != nil { return errors.Wrapf(err, "error looking up root alias for fetch module directory %s", r.FetchModuleDirectory) @@ -101,7 +100,8 @@ func (r *Registry) analyseFilePackageTypeDependencies(fileData *data.File) { // or the type has appeared in another file different to the current file // in this case we will put the type as external in the fileData // and also mutate the IsExternal field of the given type:w - log.Debugf("type %s is external to file %s, mutating the external dependencies information", fqTypeName, fileData.Name) + log.Debugf("type %s is external to file %s, mutating the external dependencies information", + fqTypeName, fileData.Name) fileData.ExternalDependingTypes = append(fileData.ExternalDependingTypes, fqTypeName) t.SetExternal(true) diff --git a/registry/message.go b/registry/message.go index 7688133..3fe4c9a 100644 --- a/registry/message.go +++ b/registry/message.go @@ -1,15 +1,18 @@ package registry import ( - descriptorpb "github.com/golang/protobuf/protoc-gen-go/descriptor" - "github.com/dpup/protoc-gen-grpc-gateway-ts/data" + "google.golang.org/protobuf/types/descriptorpb" ) -func (r *Registry) analyseMessage(fileData *data.File, packageName, fileName string, parents []string, message *descriptorpb.DescriptorProto) { +func (r *Registry) analyseMessage( + fileData *data.File, + packageName, fileName string, + parents []string, + message *descriptorpb.DescriptorProto) { packageIdentifier := r.getNameOfPackageLevelIdentifier(parents, message.GetName()) - fqName := r.getFullQualifiedName(packageName, parents, message.GetName()) // "." + packageName + "." + parentsPrefix + message.GetName() + fqName := r.getFullQualifiedName(packageName, parents, message.GetName()) protoType := descriptorpb.FieldDescriptorProto_TYPE_MESSAGE typeInfo := &TypeInformation{ @@ -42,13 +45,12 @@ func (r *Registry) analyseMessage(fileData *data.File, packageName, fileName str IsExternal: r.isExternalDependenciesOutsidePackage(f.GetTypeName(), packageName), } } - } + fileData.TrackPackageNonScalarType(typeInfo.KeyType) fileData.TrackPackageNonScalarType(typeInfo.ValueType) // no need to add a map type into return - } } @@ -57,7 +59,9 @@ func (r *Registry) analyseMessage(fileData *data.File, packageName, fileName str data.FQType = fqName data.IsDeprecated = message.GetOptions().GetDeprecated() - newParents := append(parents, message.GetName()) + newParents := []string{} + newParents = append(newParents, parents...) + newParents = append(newParents, message.GetName()) // handle enums, by pulling the enums out to the top level for _, enum := range message.EnumType { diff --git a/registry/registry.go b/registry/registry.go index 2e8ce50..bd6e479 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -7,14 +7,14 @@ import ( "strings" "github.com/dpup/protoc-gen-grpc-gateway-ts/data" - descriptorpb "github.com/golang/protobuf/protoc-gen-go/descriptor" - plugin "github.com/golang/protobuf/protoc-gen-go/plugin" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" // nolint: depguard + log "github.com/sirupsen/logrus" //nolint: depguard // not sure, will remove + "google.golang.org/protobuf/types/descriptorpb" + "google.golang.org/protobuf/types/pluginpb" ) -// TSImportRootSeparator separates the ts import root inside ts_import_roots & ts_import_root_aliases -const TSImportRootSeparator = ";" // nolint: gochecknoglobals +// importRootSeparator separates the ts import root inside ts_import_roots & ts_import_root_aliases. +const importRootSeparator = ";" type Options struct { // TSImportRootParamsKey contains the key for common_import_root in parameters @@ -39,27 +39,30 @@ type Options struct { } // Registry analyze generation request, spits out the data the the rendering process -// it also holds the information about all the types +// it also holds the information about all the types. type Registry struct { Options // Types stores the type information keyed by the fully qualified name of a type Types map[string]*TypeInformation - // FilesToGenerate contains a list of actual file to generate, different from all the files from the request, some of which are import files + // FilesToGenerate contains a list of actual file to generate, different from all the files from + // the request, some of which are import files FilesToGenerate map[string]bool - // TSImportRoots represents the ts import root for the generator to figure out required import path, will default to cwd + // TSImportRoots represents the ts import root for the generator to figure out required import + // path, will default to cwd TSImportRoots []string - // TSImportRootAliases if not empty will substitutes the common import root when writing the import into the js file + // TSImportRootAliases if not empty will substitutes the common import root when writing the + // import into the js file TSImportRootAliases []string // TSPackages stores the package name keyed by the TS file name TSPackages map[string]string } -// NewRegistry initialise the registry and return the instance +// NewRegistry initialise the registry and return the instance. func NewRegistry(opts Options) (*Registry, error) { tsImportRoots, tsImportRootAliases, err := getTSImportRootInformation(opts) log.Debugf("found ts import roots %v", tsImportRoots) @@ -86,7 +89,7 @@ func getTSImportRootInformation(opts Options) ([]string, []string, error) { tsImportRootsValue = "." } - splittedImportRoots := strings.Split(tsImportRootsValue, TSImportRootSeparator) + splittedImportRoots := strings.Split(tsImportRootsValue, importRootSeparator) numImportRoots := len(splittedImportRoots) tsImportRoots := make([]string, 0, numImportRoots) @@ -106,7 +109,7 @@ func getTSImportRootInformation(opts Options) ([]string, []string, error) { } tsImportRootAliasValue := opts.TSImportRootAliases - splittedImportRootAliases := strings.Split(tsImportRootAliasValue, TSImportRootSeparator) + splittedImportRootAliases := strings.Split(tsImportRootAliasValue, importRootSeparator) tsImportRootAliases := make([]string, numImportRoots) for i, ra := range splittedImportRootAliases { if i >= numImportRoots { @@ -114,21 +117,23 @@ func getTSImportRootInformation(opts Options) ([]string, []string, error) { break } tsImportRootAliases[i] = ra - } return tsImportRoots, tsImportRootAliases, nil } -// TypeInformation store the information about a given type +// TypeInformation store the information about a given type. type TypeInformation struct { - // Fully qualified name of the type, it starts with `.` and followed by packages and the nested structure path. + // Fully qualified name of the type, it starts with `.` and followed by packages and the nested + // structure path. FullyQualifiedName string // Package is the package of the type it belongs to Package string - // Files is the file of the type belongs to, this is important in Typescript as modules is the namespace for types defined inside + // Files is the file of the type belongs to, this is important in Typescript as modules is the + // namespace for types defined inside File string - // ModuleIdentifier is the identifier of the type inside the package, this will be useful for enum and nested enum. + // ModuleIdentifier is the identifier of the type inside the package, this will be useful for enum + // and nested enum. PackageIdentifier string // LocalIdentifier is the identifier inside the types local scope LocalIdentifier string @@ -142,14 +147,14 @@ type TypeInformation struct { ValueType *data.MapEntryType } -// IsFileToGenerate contains the file to be generated in the request +// IsFileToGenerate contains the file to be generated in the request. func (r *Registry) IsFileToGenerate(name string) bool { result, ok := r.FilesToGenerate[name] return ok && result } -// Analyse analyses the the file inputs, stores types information and spits out the rendering data -func (r *Registry) Analyse(req *plugin.CodeGeneratorRequest) (map[string]*data.File, error) { +// Analyse analyses the the file inputs, stores types information and spits out the rendering data. +func (r *Registry) Analyse(req *pluginpb.CodeGeneratorRequest) (map[string]*data.File, error) { r.FilesToGenerate = make(map[string]bool) for _, f := range req.GetFileToGenerate() { r.FilesToGenerate[f] = true @@ -183,7 +188,8 @@ func (r *Registry) getNameOfPackageLevelIdentifier(parents []string, name string } func (r *Registry) getFullQualifiedName(packageName string, parents []string, name string) string { - namesToConcat := make([]string, 0, 2+len(parents)) + additionalNames := 2 + namesToConcat := make([]string, 0, additionalNames+len(parents)) if packageName != "" { namesToConcat = append(namesToConcat, packageName) @@ -196,17 +202,18 @@ func (r *Registry) getFullQualifiedName(packageName string, parents []string, na namesToConcat = append(namesToConcat, name) return "." + strings.Join(namesToConcat, ".") - } func (r *Registry) isExternalDependenciesOutsidePackage(fqTypeName, packageName string) bool { return strings.Index(fqTypeName, "."+packageName) != 0 && strings.Index(fqTypeName, ".") == 0 } -// findRootAliasForPath iterate through all ts_import_roots and try to find an alias with the first matching the ts_import_root -func (r *Registry) findRootAliasForPath(predicate func(root string) (bool, error)) (foundAtRoot, alias string, err error) { - foundAtRoot = "" - alias = "" +// findRootAliasForPath iterate through all ts_import_roots and try to find an alias with the first +// matching the ts_import_root. +func (r *Registry) findRootAliasForPath( + predicate func(root string) (bool, error)) (string, string, error) { + foundAtRoot := "" + alias := "" for i, root := range r.TSImportRoots { absRoot, err := filepath.Abs(root) if err != nil { @@ -235,15 +242,16 @@ func (r *Registry) findRootAliasForPath(predicate func(root string) (bool, error // getSourceFileForImport will return source file for import use. // if alias is provided it will try to replace the absolute root with target's absolute path with alias -// if no alias then it will try to return a relative path to the source file +// if no alias then it will try to return a relative path to the source file. func (r *Registry) getSourceFileForImport(source, target, root, alias string) (string, error) { - ret := "" + var ret string absTarget, err := filepath.Abs(target) if err != nil { return "", errors.Wrapf(err, "error looking up absolute path for target %s", target) } - if alias != "" { // if an alias has been provided, that means there's no need to get relative path + // if an alias has been provided, that means there's no need to get relative path + if alias != "" { absRoot, err := filepath.Abs(root) if err != nil { return "", errors.Wrapf(err, "error looking up absolute path for root %s", root) @@ -266,7 +274,8 @@ func (r *Registry) getSourceFileForImport(source, target, root, alias string) (s slashPath := filepath.ToSlash(ret) log.Debugf("got relative path %s for %s", target, slashPath) - if !strings.HasPrefix(slashPath, "../") { // sub directory will not have relative path ./, if this happens, prepend one + // sub directory will not have relative path ./, if this happens, prepend one + if !strings.HasPrefix(slashPath, "../") { ret = filepath.FromSlash("./" + slashPath) } @@ -280,7 +289,6 @@ func (r *Registry) getSourceFileForImport(source, target, root, alias string) (s } return ret, nil - } func (r *Registry) collectExternalDependenciesFromData(filesData map[string]*data.File) error { @@ -308,7 +316,7 @@ func (r *Registry) collectExternalDependenciesFromData(filesData map[string]*dat // Referencing types will be [ModuleIdentifier].[PackageIdentifier] base := fileData.TSFileName target := data.GetTSFileName(typeInfo.File) - sourceFile := "" + var sourceFile string if pkg, ok := r.TSPackages[target]; ok { log.Debugf("package import override %s has been found for file %s", pkg, target) sourceFile = pkg @@ -320,13 +328,9 @@ func (r *Registry) collectExternalDependenciesFromData(filesData map[string]*dat if os.IsNotExist(err) { return false, nil } - return false, err - - } else { - return true, nil } - + return true, nil }) if err != nil { return errors.WithStack(err) diff --git a/registry/service.go b/registry/service.go index 4518ee4..f6e8c6d 100644 --- a/registry/service.go +++ b/registry/service.go @@ -3,9 +3,9 @@ package registry import ( "fmt" - descriptorpb "github.com/golang/protobuf/protoc-gen-go/descriptor" "google.golang.org/genproto/googleapis/api/annotations" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/descriptorpb" "github.com/dpup/protoc-gen-grpc-gateway-ts/data" ) @@ -19,7 +19,7 @@ func hasHTTPAnnotation(m *descriptorpb.MethodDescriptorProto) bool { return getHTTPAnnotation(m) != nil } -func getHTTPMethodPath(m *descriptorpb.MethodDescriptorProto) (method, path string) { +func getHTTPMethodPath(m *descriptorpb.MethodDescriptorProto) (string, string) { if !hasHTTPAnnotation(m) { return "", "" } @@ -58,7 +58,10 @@ func getHTTPBody(m *descriptorpb.MethodDescriptorProto) *string { } } -func (r *Registry) analyseService(fileData *data.File, packageName string, fileName string, service *descriptorpb.ServiceDescriptorProto) { +func (r *Registry) analyseService( + fileData *data.File, + packageName, fileName string, + service *descriptorpb.ServiceDescriptorProto) { packageIdentifier := service.GetName() fqName := "." + packageName + "." + packageIdentifier diff --git a/test/test.go b/test/init_test.go similarity index 100% rename from test/test.go rename to test/init_test.go From 38d047bafcd84c1a636c4cb67260fd4bded6ecb2 Mon Sep 17 00:00:00 2001 From: Daniel Pupius Date: Tue, 21 May 2024 17:00:11 -0700 Subject: [PATCH 5/8] Dont run goreleaser against PRs --- .github/workflows/goreleaser.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/goreleaser.yml b/.github/workflows/goreleaser.yml index 8401c66..a7e2e2a 100644 --- a/.github/workflows/goreleaser.yml +++ b/.github/workflows/goreleaser.yml @@ -1,6 +1,5 @@ name: Go Release on: - pull_request: push: tags: - "*" From 1407affa5b6a9df4bf9c1dbf3bef80ad0ffcb805 Mon Sep 17 00:00:00 2001 From: Daniel Pupius Date: Tue, 21 May 2024 17:01:39 -0700 Subject: [PATCH 6/8] Bump golangci-lint version --- .github/workflows/lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index e9537bc..9a7e7ed 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -27,5 +27,5 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@v4 with: - version: v1.54 + version: v1.58 only-new-issues: true From 22db8b7fac7c64d969a4f8b944cfafad39f06a7f Mon Sep 17 00:00:00 2001 From: Daniel Pupius Date: Tue, 21 May 2024 17:25:45 -0700 Subject: [PATCH 7/8] More lint fixes that were missed due to VSCode config --- .golangci.yaml | 9 ++++++--- data/enum.go | 2 +- data/file.go | 25 ++++++++++++++----------- data/message.go | 26 ++++++++++++++------------ data/service.go | 20 ++++++++++---------- generator/generator.go | 28 ++++++++++++++++------------ generator/strings.go | 6 +++--- generator/template.go | 17 +++++++++-------- generator/template_test.go | 2 +- main.go | 23 +++++++++++------------ 10 files changed, 85 insertions(+), 73 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index bf91977..add8ff9 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -237,7 +237,6 @@ linters: - forbidigo # forbids identifiers - funlen # tool for detection of long functions - gocheckcompilerdirectives # validates go compiler directive comments (//go:) - - gochecknoglobals # checks that no global variables exist - gochecknoinits # checks that no init functions are present in Go code - gochecksumtype # checks exhaustiveness on Go "sum types" - gocognit # computes and checks the cognitive complexity of functions @@ -277,7 +276,6 @@ linters: - tenv # detects using os.Setenv instead of t.Setenv since Go1.17 - testableexamples # checks if examples are testable (have an expected output) - testifylint # checks usage of github.com/stretchr/testify - - testpackage # makes you use a separate _test package - tparallel # detects inappropriate usage of t.Parallel() method in your Go test codes - unconvert # removes unnecessary type conversions - unparam # reports unused function parameters @@ -286,6 +284,8 @@ linters: - whitespace # detects leading and trailing whitespace ## you may want to enable + # - gochecknoglobals # checks that no global variables exist + # - testpackage # makes you use a separate _test package #- protogetter # reports direct reads from proto message fields when getters should be used #- nestif # reports deeply nested if statements #- decorder # checks declaration order and count of types, constants, variables and functions @@ -338,7 +338,7 @@ issues: linters: [godot] - source: "//noinspection" linters: [gocritic] - - path: "_test\\.go" + - path: "(_test\\.go)" linters: - bodyclose - dupl @@ -347,3 +347,6 @@ issues: - gosec - noctx - wrapcheck + + exclude-dirs: + - test diff --git a/data/enum.go b/data/enum.go index 672e91e..babd8f7 100644 --- a/data/enum.go +++ b/data/enum.go @@ -2,7 +2,7 @@ package data // Enum is the data out to render Enums in a file // Enums that nested inside messages will be pulled out to the top level -// Because the way it works in typescript +// Because the way it works in typescript. type Enum struct { // Name will be the package level unique name // Nested names will concat with their parent messages so that it will remain unique diff --git a/data/file.go b/data/file.go index 8390a2c..6f33638 100644 --- a/data/file.go +++ b/data/file.go @@ -9,9 +9,10 @@ import ( "github.com/iancoleman/strcase" ) -// File store the information about rendering a file +// File store the information about rendering a file. type File struct { - // Enums is a list of enums to render, due to the fact that there cannot be any enum defined nested in the class in Typescript. + // Enums is a list of enums to render, due to the fact that there cannot be + // any enum defined nested in the class in Typescript. // All Enums will be rendered at the top level Enums []*Enum // Messages represents top level messages inside the file. @@ -24,9 +25,12 @@ type File struct { Name string // TSFileName is the name of the output file TSFileName string - // PackageNonScalarType stores the type inside the same packages within the file, which will be used to figure out external dependencies inside the same package (different files) + // PackageNonScalarType stores the type inside the same packages within the + // file, which will be used to figure out external dependencies inside the + // same package (different files) PackageNonScalarType []Type - // Dependencies is a list of dependencies for the file, which will be rendered at the top of the file as import statements + // Dependencies is a list of dependencies for the file, which will be rendered + // at the top of the file as import statements dependencies []*Dependency } @@ -50,7 +54,7 @@ func (f *File) Dependencies() []*Dependency { return out } -// NeedsOneOfSupport indicates the file needs one of support type utilities +// NeedsOneOfSupport indicates the file needs one of support type utilities. func (f *File) NeedsOneOfSupport() bool { for _, m := range f.Messages { if m.HasOneOfFields() { @@ -71,7 +75,7 @@ func (f *File) NeedsStructPBSupport() bool { return false } -// TrackPackageNonScalarType tracks the supplied non scala type in the same package +// TrackPackageNonScalarType tracks the supplied non scala type in the same package. func (f *File) TrackPackageNonScalarType(t Type) { isNonScalarType := strings.Index(t.GetType().Type, ".") == 0 if isNonScalarType { @@ -83,7 +87,7 @@ func (f *File) IsEmpty() bool { return len(f.Enums) == 0 && len(f.Messages) == 0 && len(f.Services) == 0 } -// NewFile returns an initialised new file +// NewFile returns an initialised new file. func NewFile() *File { return &File{ dependencies: make([]*Dependency, 0), @@ -92,7 +96,6 @@ func NewFile() *File { Services: make([]*Service, 0), ExternalDependingTypes: make([]string, 0), } - } // Dependency stores the information about dependencies. @@ -117,7 +120,7 @@ func GetModuleName(packageName, fileName string) string { return strcase.ToCamel(packageName) + strcase.ToCamel(name) } -// GetTSFileName gets the typescript filename out of the proto file name +// GetTSFileName gets the typescript filename out of the proto file name. func GetTSFileName(fileName string) string { baseName := filepath.Base(fileName) ext := filepath.Ext(fileName) @@ -125,7 +128,7 @@ func GetTSFileName(fileName string) string { return path.Join(filepath.Dir(fileName), name+".pb.ts") } -// Type is an interface to get type out of field and method arguments +// Type is an interface to get type out of field and method arguments. type Type interface { // GetType returns some information of the type to aid the rendering GetType() *TypeInfo @@ -133,7 +136,7 @@ type Type interface { SetExternal(bool) } -// TypeInfo stores some common type information for rendering +// TypeInfo stores some common type information for rendering. type TypeInfo struct { // Type Type string diff --git a/data/message.go b/data/message.go index 733d6c5..c111054 100644 --- a/data/message.go +++ b/data/message.go @@ -1,6 +1,6 @@ package data -// Message stores the rendering information about message +// Message stores the rendering information about message. type Message struct { // Nested shows whether this message is a nested message and needs to be exported Nested bool @@ -8,7 +8,7 @@ type Message struct { Name string // Message has been marked as deprecated IsDeprecated bool - //FQType is the fully qualified type name for the message itself + // FQType is the fully qualified type name for the message itself FQType string // Enums is a list of NestedEnums inside Enums []*NestedEnum @@ -20,9 +20,11 @@ type Message struct { OptionalFields []*Field // Message is the nested messages defined inside the message Messages []*Message - // OneOfFieldsGroups is the grouped list of one of fields with same index. so that renderer can render the clearing of other fields on set. + // OneOfFieldsGroups is the grouped list of one of fields with same index. so + // that renderer can render the clearing of other fields on set. OneOfFieldsGroups map[int32][]*Field - // OneOfFieldNames is the names of one of fields with same index. so that renderer can render the clearing of other fields on set. + // OneOfFieldNames is the names of one of fields with same index. so that + // renderer can render the clearing of other fields on set. OneOfFieldsNames map[int32]string } @@ -47,7 +49,7 @@ func (m *Message) HasStructPBFields() bool { return false } -// NewMessage initialises and return a Message +// NewMessage initialises and return a Message. func NewMessage() *Message { return &Message{ Nested: false, @@ -60,7 +62,7 @@ func NewMessage() *Message { } } -// NestedEnum stores the information of enums defined inside a message +// NestedEnum stores the information of enums defined inside a message. type NestedEnum struct { // Name of the Enum inside the class, which will be identical to the name // defined inside the message @@ -75,7 +77,7 @@ type NestedEnum struct { Type string } -// Field stores the information about a field inside message +// Field stores the information about a field inside message. type Field struct { Name string // Type will be similar to NestedEnum.Type. Where scalar type and types inside @@ -100,7 +102,7 @@ type Field struct { IsRepeated bool } -// GetType returns some information of the type to aid the rendering +// GetType returns some information of the type to aid the rendering. func (f *Field) GetType() *TypeInfo { return &TypeInfo{ Type: f.Type, @@ -110,12 +112,12 @@ func (f *Field) GetType() *TypeInfo { } } -// SetExternal mutate the IsExternal attribute +// SetExternal mutate the IsExternal attribute. func (f *Field) SetExternal(external bool) { f.IsExternal = external } -// MapEntryType is the generic entry type for both key and value +// MapEntryType is the generic entry type for both key and value. type MapEntryType struct { // Type of the map entry Type string @@ -123,7 +125,7 @@ type MapEntryType struct { IsExternal bool } -// GetType returns the type information for the type entry +// GetType returns the type information for the type entry. func (m *MapEntryType) GetType() *TypeInfo { return &TypeInfo{ Type: m.Type, @@ -132,7 +134,7 @@ func (m *MapEntryType) GetType() *TypeInfo { } } -// SetExternal mutate the IsExternal attribute inside +// SetExternal mutate the IsExternal attribute inside. func (m *MapEntryType) SetExternal(external bool) { m.IsExternal = external } diff --git a/data/service.go b/data/service.go index 142280f..d8578b8 100644 --- a/data/service.go +++ b/data/service.go @@ -1,6 +1,6 @@ package data -// Service is the data representation of Service in proto +// Service is the data representation of Service in proto. type Service struct { // Name is the name of the Service Name string @@ -8,10 +8,10 @@ type Service struct { Methods []*Method } -// Services is an alias of Service array +// Services is an alias of Service array. type Services []*Service -// HasServerStreamingMethod indicates whether there is server side streaming calls inside any of the services +// HasServerStreamingMethod indicates whether there is server side streaming calls inside any of the services. func (s Services) HasServerStreamingMethod() bool { for _, service := range s { for _, method := range service.Methods { @@ -23,7 +23,7 @@ func (s Services) HasServerStreamingMethod() bool { return false } -// HasUnaryCallMethod indicates whether there is unary methods inside any of the services +// HasUnaryCallMethod indicates whether there is unary methods inside any of the services. func (s Services) HasUnaryCallMethod() bool { for _, service := range s { for _, method := range service.Methods { @@ -35,20 +35,20 @@ func (s Services) HasUnaryCallMethod() bool { return false } -// RequiresFetchModule returns whether the given services needs fetch module support +// RequiresFetchModule returns whether the given services needs fetch module support. func (s Services) RequiresFetchModule() bool { hasServices := len(s) > 0 return hasServices && (s.HasUnaryCallMethod() || s.HasServerStreamingMethod()) } -// NewService returns an initialised service +// NewService returns an initialised service. func NewService() *Service { return &Service{ Methods: make([]*Method, 0), } } -// Method represents the rpc calls in protobuf service +// Method represents the rpc calls in protobuf service. type Method struct { // Name is the name of the method Name string @@ -68,7 +68,7 @@ type Method struct { HTTPRequestBody *string } -// MethodArgument stores the type information about method argument +// MethodArgument stores the type information about method argument. type MethodArgument struct { // Type is the type of the argument Type string @@ -78,7 +78,7 @@ type MethodArgument struct { IsRepeated bool } -// GetType returns some information of the type to aid the rendering +// GetType returns some information of the type to aid the rendering. func (m *MethodArgument) GetType() *TypeInfo { return &TypeInfo{ Type: m.Type, @@ -87,7 +87,7 @@ func (m *MethodArgument) GetType() *TypeInfo { } } -// SetExternal mutates the IsExternal attribute of the type +// SetExternal mutates the IsExternal attribute of the type. func (m *MethodArgument) SetExternal(external bool) { m.IsExternal = external } diff --git a/generator/generator.go b/generator/generator.go index ab36a1d..ccbae7c 100644 --- a/generator/generator.go +++ b/generator/generator.go @@ -7,28 +7,30 @@ import ( "strings" "text/template" - plugin "github.com/golang/protobuf/protoc-gen-go/plugin" - log "github.com/sirupsen/logrus" // nolint: depguard + log "github.com/sirupsen/logrus" //nolint: depguard // Need to remove + "google.golang.org/protobuf/types/pluginpb" "github.com/dpup/protoc-gen-grpc-gateway-ts/registry" "github.com/pkg/errors" ) -// TypeScriptGRPCGatewayGenerator is the protobuf generator for typescript +// TypeScriptGRPCGatewayGenerator is the protobuf generator for typescript. type TypeScriptGRPCGatewayGenerator struct { Registry *registry.Registry } -// New returns an initialised generator +// New returns an initialised generator. func New(reg *registry.Registry) (*TypeScriptGRPCGatewayGenerator, error) { return &TypeScriptGRPCGatewayGenerator{ Registry: reg, }, nil } -// Generate take a code generator request and returns a response. it analyse request with registry and use the generated data to render ts files -func (t *TypeScriptGRPCGatewayGenerator) Generate(req *plugin.CodeGeneratorRequest) (*plugin.CodeGeneratorResponse, error) { - resp := &plugin.CodeGeneratorResponse{} +// Generate take a code generator request and returns a response. it analyse request with registry +// and use the generated data to render ts files. +func (t *TypeScriptGRPCGatewayGenerator) Generate( + req *pluginpb.CodeGeneratorRequest) (*pluginpb.CodeGeneratorResponse, error) { + resp := &pluginpb.CodeGeneratorResponse{} filesData, err := t.Registry.Analyse(req) if err != nil { @@ -73,11 +75,12 @@ func (t *TypeScriptGRPCGatewayGenerator) Generate(req *plugin.CodeGeneratorReque return resp, nil } -func (t *TypeScriptGRPCGatewayGenerator) generateFile(data *TemplateData, tmpl *template.Template) (*plugin.CodeGeneratorResponse_File, error) { +func (t *TypeScriptGRPCGatewayGenerator) generateFile( + data *TemplateData, tmpl *template.Template) (*pluginpb.CodeGeneratorResponse_File, error) { w := bytes.NewBufferString("") if data.IsEmpty() { - w.Write([]byte(fmt.Sprintln("export default {}"))) + w.WriteString(fmt.Sprintln("export default {}")) } else { err := tmpl.Execute(w, data) if err != nil { @@ -88,14 +91,15 @@ func (t *TypeScriptGRPCGatewayGenerator) generateFile(data *TemplateData, tmpl * fileName := data.TSFileName content := strings.TrimSpace(w.String()) - return &plugin.CodeGeneratorResponse_File{ + return &pluginpb.CodeGeneratorResponse_File{ Name: &fileName, InsertionPoint: nil, Content: &content, }, nil } -func (t *TypeScriptGRPCGatewayGenerator) generateFetchModule(tmpl *template.Template) (*plugin.CodeGeneratorResponse_File, error) { +func (t *TypeScriptGRPCGatewayGenerator) generateFetchModule( + tmpl *template.Template) (*pluginpb.CodeGeneratorResponse_File, error) { w := bytes.NewBufferString("") fileName := filepath.Join(t.Registry.FetchModuleDirectory, t.Registry.FetchModuleFilename) err := tmpl.Execute(w, &TemplateData{ @@ -107,7 +111,7 @@ func (t *TypeScriptGRPCGatewayGenerator) generateFetchModule(tmpl *template.Temp } content := strings.TrimSpace(w.String()) - return &plugin.CodeGeneratorResponse_File{ + return &pluginpb.CodeGeneratorResponse_File{ Name: &fileName, InsertionPoint: nil, Content: &content, diff --git a/generator/strings.go b/generator/strings.go index df79ebf..b12eea1 100644 --- a/generator/strings.go +++ b/generator/strings.go @@ -5,11 +5,11 @@ import "strings" // JSONCamelCase converts a snake_case identifier to a camelCase identifier, // according to the protobuf JSON specification. // -// Copied from: google.golang.org/protobuf/internal/strs +// Copied from: google.golang.org/protobuf/internal/strs. func JSONCamelCase(s string) string { var b []byte var wasUnderscore bool - for i := 0; i < len(s); i++ { // proto identifiers are always ASCII + for i := range len(s) { // proto identifiers are always ASCII c := s[i] if c != '_' { if wasUnderscore && isASCIILower(c) { @@ -35,7 +35,7 @@ func functionCase(s string) string { // Find the position of the first non-uppercase letter after the initial uppercase sequence firstLowerPos := -1 - for i := 0; i < len(s); i++ { + for i := range len(s) { if isASCIILower(s[i]) { firstLowerPos = i break diff --git a/generator/template.go b/generator/template.go index 7b0cb87..86178a6 100644 --- a/generator/template.go +++ b/generator/template.go @@ -24,7 +24,7 @@ var serviceTmplScript string //go:embed fetch_tmpl.ts var fetchTmplScript string -var fetchTmplHeader = `{{- if not .EnableStylingCheck}} +const fetchTmplHeader = `{{- if not .EnableStylingCheck}} /* eslint-disable */ // @ts-nocheck {{- else -}} @@ -130,8 +130,8 @@ func buildInitReq(method data.Method) string { return strings.Join(fields, ", ") } -// include is the include template functions copied from -// copied from: https://github.com/helm/helm/blob/8648ccf5d35d682dcd5f7a9c2082f0aaf071e817/pkg/engine/engine.go#L147-L154 +// include is the include template functions copied from copied from: +// https://github.com/helm/helm/blob/8648ccf5d35d682dcd5f7a9c2082f0aaf071e817/pkg/engine/engine.go#L147-L154 func include(t *template.Template) func(name string, data interface{}) (string, error) { return func(name string, data interface{}) (string, error) { buf := bytes.NewBufferString("") @@ -179,14 +179,15 @@ func tsType(r *registry.Registry, fieldType data.Type) string { return fmt.Sprintf("Record<%s, %s>", keyType, valueType) } - typeStr := "" - if mapWellKnownType(info.Type) != "" { + var typeStr string + switch { + case mapWellKnownType(info.Type) != "": typeStr = mapWellKnownType(info.Type) - } else if strings.Index(info.Type, ".") != 0 { + case strings.Index(info.Type, ".") != 0: typeStr = mapScalaType(info.Type) - } else if !info.IsExternal { + case !info.IsExternal: typeStr = typeInfo.PackageIdentifier - } else { + default: typeStr = data.GetModuleName(typeInfo.Package, typeInfo.File) + "." + typeInfo.PackageIdentifier } diff --git a/generator/template_test.go b/generator/template_test.go index 77fc7b9..61b34af 100644 --- a/generator/template_test.go +++ b/generator/template_test.go @@ -30,7 +30,7 @@ func TestFieldName(t *testing.T) { r := ®istry.Registry{Options: registry.Options{UseProtoNames: tt.useProtoNames}} fn := fieldName(r) if got := fn(tt.input); got != tt.want { - assert.Equal(t, got, tt.want, "fieldName(%s) = %s, want %s", tt.input, got, tt.want) + assert.Equal(t, tt.want, got, "fieldName(%s) = %s, want %s", tt.input, got, tt.want) } }) } diff --git a/main.go b/main.go index 344928f..0298692 100644 --- a/main.go +++ b/main.go @@ -9,8 +9,7 @@ import ( "github.com/dpup/protoc-gen-grpc-gateway-ts/generator" "github.com/dpup/protoc-gen-grpc-gateway-ts/registry" - plugin "github.com/golang/protobuf/protoc-gen-go/plugin" - log "github.com/sirupsen/logrus" // nolint: depguard + log "github.com/sirupsen/logrus" //nolint: depguard // need to remove "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/pluginpb" ) @@ -23,11 +22,11 @@ func main() { func run() error { var ( - useProtoNames = flag.Bool("use_proto_names", false, "field names will match the protofile instead of lowerCamelCase") - useStaticClasses = flag.Bool("use_static_classes", true, "generate static classes instead of functions and a client class") + useProtoNames = flag.Bool("use_proto_names", false, "field names will match the proto file") + useStaticClasses = flag.Bool("use_static_classes", true, "use static classes rather than functions and a client") emitUnpopulated = flag.Bool("emit_unpopulated", false, "expect the gRPC Gateway to send zero values over the wire") - fetchModuleDirectory = flag.String("fetch_module_directory", ".", "where shared typescript file should be placed, default $(pwd)") + fetchModuleDirectory = flag.String("fetch_module_directory", ".", "where shared typescript file should be placed") fetchModuleFilename = flag.String("fetch_module_filename", "fetch.pb.ts", "name of shard typescript file") tsImportRoots = flag.String("ts_import_roots", "", "defaults to $(pwd)") tsImportRootAliases = flag.String("ts_import_root_aliases", "", "use import aliases instead of relative paths") @@ -99,11 +98,11 @@ func configureLogging(enableLogging bool, levelStr string) error { log.SetOutput(os.Stderr) log.Debugf("Logging configured completed, logging has been enabled") if levelStr != "" { - if level, err := log.ParseLevel(levelStr); err != nil { - return fmt.Errorf("error parsing log level %s: %s", levelStr, err) - } else { - log.SetLevel(level) + level, err := log.ParseLevel(levelStr) + if err != nil { + return fmt.Errorf("error parsing log level %s: %w", levelStr, err) } + log.SetLevel(level) } else { log.SetLevel(log.InfoLevel) } @@ -111,7 +110,7 @@ func configureLogging(enableLogging bool, levelStr string) error { return nil } -func getParamsMap(req *plugin.CodeGeneratorRequest) map[string]string { +func getParamsMap(req *pluginpb.CodeGeneratorRequest) map[string]string { paramsMap := make(map[string]string) params := req.GetParameter() @@ -126,8 +125,8 @@ func getParamsMap(req *plugin.CodeGeneratorRequest) map[string]string { return paramsMap } -func decodeReq() *plugin.CodeGeneratorRequest { - req := &plugin.CodeGeneratorRequest{} +func decodeReq() *pluginpb.CodeGeneratorRequest { + req := &pluginpb.CodeGeneratorRequest{} data, err := io.ReadAll(os.Stdin) if err != nil { panic(err) From 1a452a7a0c141b4cc3ececfb263edb969cbad998 Mon Sep 17 00:00:00 2001 From: Daniel Pupius Date: Tue, 21 May 2024 17:48:35 -0700 Subject: [PATCH 8/8] Remove logrus in-favor of log/slog --- .golangci.yaml | 2 +- Makefile | 8 ++++---- generator/generator.go | 10 +++++----- generator/template.go | 5 ++--- go.mod | 2 -- go.sum | 9 --------- main.go | 34 ++++++++++++++++++++-------------- registry/file.go | 16 +++++++++------- registry/registry.go | 25 +++++++++++++------------ 9 files changed, 54 insertions(+), 57 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index add8ff9..b3b4185 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -192,7 +192,7 @@ linters-settings: # - "all": report all global loggers # - "default": report only the default slog logger # Default: "" - no-global: "all" + no-global: "" # Enforce using methods that accept a context. # Values: # - "": disabled diff --git a/Makefile b/Makefile index 99cd993..4c8f5ce 100644 --- a/Makefile +++ b/Makefile @@ -55,7 +55,7 @@ integration-test-client: build ## Generates the typescript client code used by e -I ../ \ --grpc-gateway-ts_out ./test/integration/defaultConfig \ --grpc-gateway-ts_opt logtostderr=true \ - --grpc-gateway-ts_opt loglevel=debug \ + --grpc-gateway-ts_opt loglevel=info \ --grpc-gateway-ts_opt use_proto_names=false \ --grpc-gateway-ts_opt emit_unpopulated=false \ --grpc-gateway-ts_opt enable_styling_check=true \ @@ -67,7 +67,7 @@ integration-test-client: build ## Generates the typescript client code used by e -I ../ \ --grpc-gateway-ts_out ./test/integration/useProtoNames \ --grpc-gateway-ts_opt logtostderr=true \ - --grpc-gateway-ts_opt loglevel=debug \ + --grpc-gateway-ts_opt loglevel=info \ --grpc-gateway-ts_opt use_proto_names=true \ --grpc-gateway-ts_opt emit_unpopulated=false \ --grpc-gateway-ts_opt enable_styling_check=true \ @@ -79,7 +79,7 @@ integration-test-client: build ## Generates the typescript client code used by e -I ../ \ --grpc-gateway-ts_out ./test/integration/emitUnpopulated \ --grpc-gateway-ts_opt logtostderr=true \ - --grpc-gateway-ts_opt loglevel=debug \ + --grpc-gateway-ts_opt loglevel=info \ --grpc-gateway-ts_opt use_proto_names=false \ --grpc-gateway-ts_opt emit_unpopulated=true \ --grpc-gateway-ts_opt enable_styling_check=true \ @@ -91,7 +91,7 @@ integration-test-client: build ## Generates the typescript client code used by e -I ../ \ --grpc-gateway-ts_out ./test/integration/noStaticClasses \ --grpc-gateway-ts_opt logtostderr=true \ - --grpc-gateway-ts_opt loglevel=debug \ + --grpc-gateway-ts_opt loglevel=info \ --grpc-gateway-ts_opt use_proto_names=false \ --grpc-gateway-ts_opt emit_unpopulated=false \ --grpc-gateway-ts_opt enable_styling_check=true \ diff --git a/generator/generator.go b/generator/generator.go index ccbae7c..a186d2b 100644 --- a/generator/generator.go +++ b/generator/generator.go @@ -3,11 +3,11 @@ package generator import ( "bytes" "fmt" + "log/slog" "path/filepath" "strings" "text/template" - log "github.com/sirupsen/logrus" //nolint: depguard // Need to remove "google.golang.org/protobuf/types/pluginpb" "github.com/dpup/protoc-gen-grpc-gateway-ts/registry" @@ -37,17 +37,17 @@ func (t *TypeScriptGRPCGatewayGenerator) Generate( return nil, errors.Wrap(err, "error analysing proto files") } tmpl := ServiceTemplate(t.Registry) - log.Debugf("files to generate %v", req.GetFileToGenerate()) + slog.Debug("generating files", slog.Any("files", req.GetFileToGenerate())) requiresFetchModule := false // feed fileData into rendering process for _, fileData := range filesData { if !t.Registry.IsFileToGenerate(fileData.Name) { - log.Debugf("file %s is not the file to generate, skipping", fileData.Name) + slog.Debug("file is not the file to generate, skipping", slog.String("fileName", fileData.Name)) continue } - log.Debugf("generating file for %s", fileData.TSFileName) + slog.Debug("generating file", slog.String("fileName", fileData.TSFileName)) data := &TemplateData{ File: fileData, EnableStylingCheck: t.Registry.EnableStylingCheck, @@ -63,7 +63,7 @@ func (t *TypeScriptGRPCGatewayGenerator) Generate( if requiresFetchModule { fetchTmpl := FetchModuleTemplate() - log.Debugf("generate fetch template") + slog.Debug("generate fetch template") generatedFetch, err := t.generateFetchModule(fetchTmpl) if err != nil { return nil, errors.Wrap(err, "error generating fetch module") diff --git a/generator/template.go b/generator/template.go index 86178a6..c1baeb4 100644 --- a/generator/template.go +++ b/generator/template.go @@ -3,13 +3,12 @@ package generator import ( "bytes" "fmt" + "log/slog" "net/url" "regexp" "strings" "text/template" - log "github.com/sirupsen/logrus" - "github.com/Masterminds/sprig" "github.com/dpup/protoc-gen-grpc-gateway-ts/data" @@ -86,7 +85,7 @@ func renderURL(r *registry.Registry) func(method data.Method) string { matches := reg.FindAllStringSubmatch(methodURL, -1) fieldsInPath := make([]string, 0, len(matches)) if len(matches) > 0 { - log.Debugf("url matches %v", matches) + slog.Debug("url matches", slog.Any("matches", matches)) for _, m := range matches { expToReplace := m[0] fieldName := fieldNameFn(m[1]) diff --git a/go.mod b/go.mod index 94f9b02..f452271 100644 --- a/go.mod +++ b/go.mod @@ -4,11 +4,9 @@ go 1.22 require ( github.com/Masterminds/sprig v2.22.0+incompatible - github.com/golang/protobuf v1.5.4 github.com/grpc-ecosystem/grpc-gateway/v2 v2.19.1 github.com/iancoleman/strcase v0.3.0 github.com/pkg/errors v0.9.1 - github.com/sirupsen/logrus v1.9.3 github.com/stretchr/testify v1.9.0 google.golang.org/genproto/googleapis/api v0.0.0-20240415180920-8c6c420018be google.golang.org/grpc v1.63.2 diff --git a/go.sum b/go.sum index 1adcfc1..7eeb490 100644 --- a/go.sum +++ b/go.sum @@ -5,11 +5,8 @@ github.com/Masterminds/semver v1.5.0/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF0 github.com/Masterminds/sprig v2.22.0+incompatible h1:z4yfnGrZ7netVz+0EDJ0Wi+5VZCSYp4Z0m2dk6cEM60= github.com/Masterminds/sprig v2.22.0+incompatible/go.mod h1:y6hNFY5UBTIWBxnzTeuNhlNS5hqE0NB0E6fgfo2Br3o= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= -github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= -github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= @@ -36,17 +33,12 @@ github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZb github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8= github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4= -github.com/sirupsen/logrus v1.9.3 h1:dueUQJ1C2q9oE3F7wvmSGAaVtTmUizReu6fjN8uqzbQ= -github.com/sirupsen/logrus v1.9.3/go.mod h1:naHLuLoDiP4jHNo9R0sCBMtWGeIprob74mVsIT4qYEQ= -github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= golang.org/x/crypto v0.22.0 h1:g1v0xeRhjcugydODzvb3mEM9SQ0HGp9s/nh3COQ/C30= golang.org/x/crypto v0.22.0/go.mod h1:vr6Su+7cTlO45qkww3VDJlzDn0ctJvRgYbC2NvXHt+M= golang.org/x/net v0.24.0 h1:1PcaxkF854Fu3+lvBIx5SYn9wRlBzzcnHZSiaFFAb0w= golang.org/x/net v0.24.0/go.mod h1:2Q7sJY5mzlzWjKtYUEXSlBWCdyaioyXzRB2RtU8KVE8= -golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o= golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= @@ -64,6 +56,5 @@ google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHh gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/main.go b/main.go index 0298692..8924798 100644 --- a/main.go +++ b/main.go @@ -4,12 +4,12 @@ import ( "flag" "fmt" "io" + "log/slog" "os" "strings" "github.com/dpup/protoc-gen-grpc-gateway-ts/generator" "github.com/dpup/protoc-gen-grpc-gateway-ts/registry" - log "github.com/sirupsen/logrus" //nolint: depguard // need to remove "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/pluginpb" ) @@ -73,7 +73,7 @@ func run() error { return fmt.Errorf("error instantiating a new generator: %w", err) } - log.Debug("Starts generating file request") + slog.Debug("Starts generating file request") resp, err := g.Generate(req) if err != nil { @@ -85,27 +85,33 @@ func run() error { encodeResponse(resp) - log.Debug("generation finished") + slog.Debug("generation finished") return nil } func configureLogging(enableLogging bool, levelStr string) error { if enableLogging { - log.SetFormatter(&log.TextFormatter{ - DisableTimestamp: true, - }) - log.SetOutput(os.Stderr) - log.Debugf("Logging configured completed, logging has been enabled") + level := slog.LevelInfo if levelStr != "" { - level, err := log.ParseLevel(levelStr) - if err != nil { - return fmt.Errorf("error parsing log level %s: %w", levelStr, err) + switch levelStr { + case "debug": + level = slog.LevelDebug + case "info": + level = slog.LevelInfo + case "warn": + level = slog.LevelWarn + case "error": + level = slog.LevelError + default: + return fmt.Errorf("invalid log level %s", levelStr) } - log.SetLevel(level) - } else { - log.SetLevel(log.InfoLevel) } + opts := &slog.HandlerOptions{Level: level} + logger := slog.New(slog.NewTextHandler(os.Stderr, opts)) + slog.SetDefault(logger) + } else { + slog.SetDefault(slog.New(slog.NewTextHandler(io.Discard, nil))) } return nil } diff --git a/registry/file.go b/registry/file.go index 5eac2e8..1949292 100644 --- a/registry/file.go +++ b/registry/file.go @@ -1,19 +1,20 @@ package registry import ( + "fmt" + "log/slog" "path/filepath" "strings" "github.com/dpup/protoc-gen-grpc-gateway-ts/data" "github.com/dpup/protoc-gen-grpc-gateway-ts/options" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" //nolint: depguard // Need to remove "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/descriptorpb" ) func (r *Registry) analyseFile(f *descriptorpb.FileDescriptorProto) (*data.File, error) { - log.Debugf("analysing %s", f.GetName()) + slog.Debug("analysing file", slog.String("filename", f.GetName())) fileData := data.NewFile() fileName := f.GetName() packageName := f.GetPackage() @@ -52,7 +53,7 @@ func (r *Registry) analyseFile(f *descriptorpb.FileDescriptorProto) (*data.File, func (r *Registry) addFetchModuleDependencies(fileData *data.File) error { if !fileData.Services.RequiresFetchModule() { - log.Debugf("no services found for %s, skipping fetch module", fileData.Name) + slog.Debug("no services found for, skipping fetch module", slog.String("name", fileData.Name)) return nil } @@ -75,7 +76,7 @@ func (r *Registry) addFetchModuleDependencies(fileData *data.File) error { return errors.Wrapf(err, "error replacing source file with alias for %s", fileName) } - log.Debugf("added fetch dependency %s for %s", sourceFile, fileData.TSFileName) + slog.Debug("added fetch dependency %s for %s", sourceFile, fileData.TSFileName) fileData.AddDependency(&data.Dependency{ ModuleIdentifier: "fm", SourceFile: sourceFile, @@ -92,7 +93,8 @@ func (r *Registry) analyseFilePackageTypeDependencies(fileData *data.File) { // also need to change the type's IsExternal information for rendering purpose typeInfo := t.GetType() fqTypeName := typeInfo.Type - log.Debugf("checking whether non scala type %s in the same message is external to the current file", fqTypeName) + slog.Debug("checking whether non scala type in the same message is external to the current file", + slog.Any("type", fqTypeName)) registryType, foundInRegistry := r.Types[fqTypeName] if !foundInRegistry || registryType.File != fileData.Name { @@ -100,8 +102,8 @@ func (r *Registry) analyseFilePackageTypeDependencies(fileData *data.File) { // or the type has appeared in another file different to the current file // in this case we will put the type as external in the fileData // and also mutate the IsExternal field of the given type:w - log.Debugf("type %s is external to file %s, mutating the external dependencies information", - fqTypeName, fileData.Name) + slog.Debug(fmt.Sprintf("type %s is external to file %s, mutating the external dependencies information", + fqTypeName, fileData.Name)) fileData.ExternalDependingTypes = append(fileData.ExternalDependingTypes, fqTypeName) t.SetExternal(true) diff --git a/registry/registry.go b/registry/registry.go index bd6e479..c365923 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -1,6 +1,8 @@ package registry import ( + "fmt" + "log/slog" "os" "path" "path/filepath" @@ -8,7 +10,6 @@ import ( "github.com/dpup/protoc-gen-grpc-gateway-ts/data" "github.com/pkg/errors" - log "github.com/sirupsen/logrus" //nolint: depguard // not sure, will remove "google.golang.org/protobuf/types/descriptorpb" "google.golang.org/protobuf/types/pluginpb" ) @@ -65,14 +66,14 @@ type Registry struct { // NewRegistry initialise the registry and return the instance. func NewRegistry(opts Options) (*Registry, error) { tsImportRoots, tsImportRootAliases, err := getTSImportRootInformation(opts) - log.Debugf("found ts import roots %v", tsImportRoots) - log.Debugf("found ts import root aliases %v", tsImportRootAliases) + slog.Debug("found ts import roots", slog.Any("importRoots", tsImportRoots)) + slog.Debug("found ts import root aliases", slog.Any("importRootAliases", tsImportRootAliases)) if err != nil { return nil, errors.Wrap(err, "error getting common import root information") } - log.Debugf("found fetch module directory %s", opts.FetchModuleDirectory) - log.Debugf("found fetch module name %s", opts.FetchModuleFilename) + slog.Debug("found fetch module directory", slog.String("moduleDir", opts.FetchModuleDirectory)) + slog.Debug("found fetch module name", slog.String("moduleName", opts.FetchModuleFilename)) return &Registry{ Options: opts, @@ -161,7 +162,7 @@ func (r *Registry) Analyse(req *pluginpb.CodeGeneratorRequest) (map[string]*data } files := req.GetProtoFile() - log.Debugf("about to start anaylyse files, %d in total", len(files)) + slog.Debug("about to start anaylyse files", slog.Int("count", len(files))) data := make(map[string]*data.File) // analyse all files in the request first for _, f := range files { @@ -258,9 +259,9 @@ func (r *Registry) getSourceFileForImport(source, target, root, alias string) (s } ret = strings.ReplaceAll(absTarget, absRoot, alias) - log.Debugf("replacing root alias %s for %s, result: %s", alias, target, ret) + slog.Debug(fmt.Sprintf("replacing root alias %s for %s, result: %s", alias, target, ret)) } else { // return relative path here - log.Debugf("no root alias found, trying to get the relative path for %s", target) + slog.Debug("no root alias found, trying to get the relative path", slog.String("target", target)) absSource, err := filepath.Abs(source) if err != nil { return "", errors.Wrapf(err, "error looking up absolute directory with base dir: %s", source) @@ -272,14 +273,14 @@ func (r *Registry) getSourceFileForImport(source, target, root, alias string) (s } slashPath := filepath.ToSlash(ret) - log.Debugf("got relative path %s for %s", target, slashPath) + slog.Debug(fmt.Sprintf("got relative path %s for %s", target, slashPath)) // sub directory will not have relative path ./, if this happens, prepend one if !strings.HasPrefix(slashPath, "../") { ret = filepath.FromSlash("./" + slashPath) } - log.Debugf("no root alias found, trying to get the relative path for %s, result: %s", target, ret) + slog.Debug(fmt.Sprintf("no root alias found, trying to get the relative path for %s, result: %s", target, ret)) } // remove .ts suffix if there's any @@ -293,7 +294,7 @@ func (r *Registry) getSourceFileForImport(source, target, root, alias string) (s func (r *Registry) collectExternalDependenciesFromData(filesData map[string]*data.File) error { for _, fileData := range filesData { - log.Debugf("collecting dependencies information for %s", fileData.TSFileName) + slog.Debug("collecting dependencies information", slog.String("fileName", fileData.TSFileName)) // dependency group up the dependency by package+file dependencies := make(map[string]*data.Dependency) for _, typeName := range fileData.ExternalDependingTypes { @@ -318,7 +319,7 @@ func (r *Registry) collectExternalDependenciesFromData(filesData map[string]*dat target := data.GetTSFileName(typeInfo.File) var sourceFile string if pkg, ok := r.TSPackages[target]; ok { - log.Debugf("package import override %s has been found for file %s", pkg, target) + slog.Debug("package import override has been found", slog.String("pkg", pkg), slog.String("target", target)) sourceFile = pkg } else { foundAtRoot, alias, err := r.findRootAliasForPath(func(absRoot string) (bool, error) {