Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix(client/v2): fix short command description if not set and skip unsupported commands #18324

Merged
merged 13 commits into from
Nov 6, 2023
7 changes: 6 additions & 1 deletion client/v2/autocli/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ func (b *Builder) buildMethodCommandCommon(descriptor protoreflect.MethodDescrip
options = &autocliv1.RpcCommandOptions{}
}

short := options.Short
if short == "" {
short = fmt.Sprintf("Execute the %s RPC method", descriptor.Name())
}

long := options.Long
if long == "" {
long = util.DescriptorDocs(descriptor)
Expand All @@ -44,7 +49,7 @@ func (b *Builder) buildMethodCommandCommon(descriptor protoreflect.MethodDescrip
SilenceUsage: false,
Use: use,
Long: long,
Short: options.Short,
Short: short,
Example: options.Example,
Aliases: options.Alias,
SuggestFor: options.SuggestFor,
Expand Down
5 changes: 5 additions & 0 deletions client/v2/autocli/msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

autocliv1 "cosmossdk.io/api/cosmos/autocli/v1"
"cosmossdk.io/client/v2/autocli/flag"
"cosmossdk.io/client/v2/internal/util"

"github.com/cosmos/cosmos-sdk/client"
clienttx "github.com/cosmos/cosmos-sdk/client/tx"
Expand Down Expand Up @@ -85,6 +86,10 @@ func (b *Builder) AddMsgServiceCommands(cmd *cobra.Command, cmdDescriptor *autoc
continue
}

if !util.IsSupportedVersion(util.DescriptorDocs(methodDescriptor)) {
continue
}

methodCmd, err := b.BuildMsgMethodCommand(methodDescriptor, methodOpts)
if err != nil {
return err
Expand Down
4 changes: 4 additions & 0 deletions client/v2/autocli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ func (b *Builder) AddQueryServiceCommands(cmd *cobra.Command, cmdDescriptor *aut
continue
}

if !util.IsSupportedVersion(util.DescriptorDocs(methodDescriptor)) {
continue
}
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved

methodCmd, err := b.BuildQueryMethodCommand(methodDescriptor, methodOpts)
if err != nil {
return err
Expand Down
2 changes: 2 additions & 0 deletions client/v2/autocli/testdata/help-deprecated.golden
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
Command "echo" is deprecated, don't use this
Execute the Echo RPC method

Usage:
test deprecatedecho echo [flags]

Expand Down
8 changes: 4 additions & 4 deletions client/v2/autocli/testdata/help-toplevel-msg.golden
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ Usage:
test [command]

Available Commands:
burn
burn Execute the Burn RPC method
completion Generate the autocompletion script for the specified shell
help Help about any command
multi-send
multi-send Execute the MultiSend RPC method
send Send coins from one account to another
set-send-enabled
update-params
set-send-enabled Execute the SetSendEnabled RPC method
update-params Execute the UpdateParams RPC method

Flags:
-h, --help help for test
Expand Down
65 changes: 65 additions & 0 deletions client/v2/internal/util/util.go
Original file line number Diff line number Diff line change
@@ -1,17 +1,29 @@
package util

import (
"regexp"
"runtime/debug"
"strings"

"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/reflect/protoregistry"
"google.golang.org/protobuf/types/dynamicpb"

"cosmossdk.io/client/v2/internal/strcase"
)

// get build info to verify later if comment is supported
// this is a hack in because of the global api module package
// later versions unsupported by the current version can be added
var buildInfo, _ = debug.ReadBuildInfo()
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved

// DescriptorName returns the name of the descriptor in kebab case.
func DescriptorKebabName(descriptor protoreflect.Descriptor) string {
return strcase.ToKebab(string(descriptor.Name()))
}

// DescriptorDocs returns the leading comments of the descriptor.
// TODO this does not work, to fix.
func DescriptorDocs(descriptor protoreflect.Descriptor) string {
return descriptor.ParentFile().SourceLocations().ByDescriptor(descriptor).LeadingComments
}
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -24,3 +36,56 @@ func ResolveMessageType(resolver protoregistry.MessageTypeResolver, descriptor p

return dynamicpb.NewMessageType(descriptor)
}

// IsSupportedVersion is used to determine in which version of a module / sdk a rpc was introduced.
// It returns false if the rpc has comment for an higher version than the current one.
func IsSupportedVersion(input string) bool {
return isSupportedVersion(input, buildInfo)
}
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved

// isSupportedVersion is used to determine in which version of a module / sdk a rpc was introduced.
// It returns false if the rpc has comment for an higher version than the current one.
// It takes a buildInfo as argument to be able to test it.
func isSupportedVersion(input string, buildInfo *debug.BuildInfo) bool {
if input == "" || buildInfo == nil {
return true
}

moduleName, version := parseSinceComment(input)
for _, dep := range buildInfo.Deps {
if !strings.Contains(dep.Path, moduleName) {
continue
}

if version <= dep.Version {
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
return true
}

return false
}

return true // if cannot find the module consider it's supported
}

// parseSinceComment parses the `// Since: cosmos-sdk v0.xx` comment on rpc.
func parseSinceComment(input string) (string, string) {
var (
moduleName string
version string
)

input = strings.ToLower(input)
input = strings.ReplaceAll(input, "cosmos sdk", "cosmos-sdk")

re := regexp.MustCompile(`\/\/\s?since: (\S+) (\S+)`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The comment above says "Since", yet this regex uses "since", but we aren't using the case insensitive flag "(?i)" We could simply use "[sS]ince". I highly recommend removing the strings.ToLower then simply using that regexp suggestion that I've made. This way if some module's name is for example PbValidator, it won't be returned as "pbvalidator"

Copy link
Member Author

@julienrbrt julienrbrt Nov 5, 2023

Choose a reason for hiding this comment

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

Case insensitive definitely makes sense, but I am getting that with strings.ToLower.
Additionally, I actually really need to get the module name lowercase to match it properly with the build info.
Lastly, it is easier to replace any writing of CosMoS SdK / Cosmos-SDK -> cosmos-sdk that way.

julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
matches := re.FindStringSubmatch(input)
if len(matches) >= 3 {
moduleName, version = matches[1], matches[2]

if !strings.Contains(version, "v") {
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
version = "v" + version
}
}

return moduleName, version
}
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
151 changes: 151 additions & 0 deletions client/v2/internal/util/util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
package util

import (
"runtime/debug"
"testing"
)

func TestIsSupportedVersion(t *testing.T) {
mockBuildInfo := &debug.BuildInfo{
Deps: []*debug.Module{
{
Path: "github.com/cosmos/cosmos-sdk",
Version: "v0.50.0",
},
{
Path: "cosmossdk.io/feegrant",
Version: "v0.1.0",
},
},
}

cases := []struct {
input string
expected bool
}{
{
input: "",
expected: true,
},
{
input: "not a since comment",
expected: true,
},
{
input: "// Since: cosmos-sdk v0.47",
expected: true,
},
{
input: "// since: Cosmos-SDK 0.50",
expected: true,
},
{
input: "// Since: cosmos-sdk v0.51",
expected: false,
},
{
input: "// Since: cosmos-sdk v1.0.0",
expected: false,
},
{
input: "// since: x/feegrant v0.1.0",
expected: true,
},
{
input: "// since: feegrant v0.0.1",
expected: true,
},
{
input: "// since: feegrant v0.1.0",
expected: true,
},
{
input: "// since: feegrant v0.1",
expected: true,
},
{
input: "// since: feegrant v0.1.1",
expected: false,
},
{
input: "// since: feegrant v0.2.0",
expected: false,
},
}

for _, tc := range cases {
resp := isSupportedVersion(tc.input, mockBuildInfo)
if resp != tc.expected {
t.Errorf("expected %v, got %v", tc.expected, resp)
}

resp = isSupportedVersion(tc.input, &debug.BuildInfo{})
if !resp {
t.Errorf("expected %v, got %v", true, resp)
}
}
}

func TestParseSinceComment(t *testing.T) {
cases := []struct {
input string
expectedModuleName string
expectedVersion string
}{
{
input: "",
expectedModuleName: "",
expectedVersion: "",
},
{
input: "not a since comment",
expectedModuleName: "",
expectedVersion: "",
},
{
input: "// since: Cosmos SDK 0.50",
expectedModuleName: "cosmos-sdk",
expectedVersion: "v0.50",
},
{
input: "// since: cosmos sdk 0.50",
expectedModuleName: "cosmos-sdk",
expectedVersion: "v0.50",
},
{
input: "// since: Cosmos-SDK 0.50",
expectedModuleName: "cosmos-sdk",
expectedVersion: "v0.50",
},
{
input: "// Since: cosmos-sdk v0.50",
expectedModuleName: "cosmos-sdk",
expectedVersion: "v0.50",
},
{
input: "//since: cosmos-sdk v0.50.1",
expectedModuleName: "cosmos-sdk",
expectedVersion: "v0.50.1",
},
{
input: "// Since: x/feegrant v0.1.0",
expectedModuleName: "x/feegrant",
expectedVersion: "v0.1.0",
},
{
input: "// since: x/feegrant 0.1",
expectedModuleName: "x/feegrant",
expectedVersion: "v0.1",
},
}

for _, tc := range cases {
moduleName, version := parseSinceComment(tc.input)
if moduleName != tc.expectedModuleName {
t.Errorf("expected module name %s, got %s", tc.expectedModuleName, moduleName)
}
if version != tc.expectedVersion {
t.Errorf("expected version %s, got %s", tc.expectedVersion, version)
}
}
}
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved