Reimplement cli hints based on command arg docs (#10515)

Now that the command argument specs are available at runtime (#9656), this PR addresses
#8084 by implementing a complete solution for command-line hinting in `redis-cli`.

It correctly handles nearly every case in Redis's complex command argument definitions, including
`BLOCK` and `ONEOF` arguments, reordering of optional arguments, and repeated arguments
(even when followed by mandatory arguments). It also validates numerically-typed arguments.
It may not correctly handle all possible combinations of those, but overall it is quite robust.

Arguments are only matched after the space bar is typed, so partial word matching is not
supported - that proved to be more confusing than helpful. When the user's current input
cannot be matched against the argument specs, hinting is disabled.

Partial support has been implemented for legacy (pre-7.0) servers that do not support
`COMMAND DOCS`, by falling back to a statically-compiled command argument table.
On startup, if the server does not support `COMMAND DOCS`, `redis-cli` will now issue
an `INFO SERVER` command to retrieve the server version (unless `HELLO` has already
been sent, in which case the server version will be extracted from the reply to `HELLO`).
The server version will be used to filter the commands and arguments in the command table,
removing those not supported by that version of the server. However, the static table only
includes core Redis commands, so with a legacy server hinting will not be supported for
module commands. The auto generated help.h and the scripts that generates it are gone.

Command and argument tables for the server and CLI use different structs, due primarily
to the need to support different runtime data. In order to generate code for both, macros
have been added to `commands.def` (previously `commands.c`) to make it possible to
configure the code generation differently for different use cases (one linked with redis-server,
and one with redis-cli).

Also adding a basic testing framework for the command hints based on new (undocumented)
command line options to `redis-cli`: `--test_hint 'INPUT'` prints out the command-line hint for
a given input string, and `--test_hint_file <filename>` runs a suite of test cases for the hinting
mechanism. The test suite is in `tests/assets/test_cli_hint_suite.txt`, and it is run from
`tests/integration/redis-cli.tcl`.

Co-authored-by: Oran Agra <oran@redislabs.com>
Co-authored-by: Viktor Söderqvist <viktor.soderqvist@est.tech>

2 files changed, 88 insertions, 189 deletions

diff --git a/utils/generate-command-code.py b/utils/generate-command-code.py
index 81d8c19f1..dc66ce81f 100755
--- a/utils/generate-command-code.py
+++ b/utils/generate-command-code.py
@@ -196,7 +196,7 @@ class Argument(object):
     def struct_code(self):
         """
         Output example:
-        "expiration",ARG_TYPE_ONEOF,NULL,NULL,NULL,CMD_ARG_OPTIONAL,.value.subargs=SET_expiration_Subargs
+        MAKE_ARG("expiration",ARG_TYPE_ONEOF,-1,NULL,NULL,NULL,CMD_ARG_OPTIONAL,5,NULL),.subargs=GETEX_expiration_Subargs
         """
 
         def _flags_code():
@@ -210,7 +210,7 @@ class Argument(object):
                 s += "CMD_ARG_MULTIPLE_TOKEN|"
             return s[:-1] if s else "CMD_ARG_NONE"
 
-        s = "\"%s\",%s,%d,%s,%s,%s,%s" % (
+        s = "MAKE_ARG(\"%s\",%s,%d,%s,%s,%s,%s,%d,%s)" % (
             self.name,
             ARG_TYPES[self.type],
             self.desc.get("key_spec_index", -1),
@@ -218,9 +218,9 @@ class Argument(object):
             get_optional_desc_string(self.desc, "summary"),
             get_optional_desc_string(self.desc, "since"),
             _flags_code(),
+            len(self.subargs),
+            get_optional_desc_string(self.desc, "deprecated_since"),
         )
-        if "deprecated_since" in self.desc:
-            s += ",.deprecated_since=\"%s\"" % self.desc["deprecated_since"]
         if "display" in self.desc:
             s += ",.display_text=\"%s\"" % self.desc["display"].lower()
         if self.subargs:
@@ -234,10 +234,9 @@ class Argument(object):
                 subarg.write_internal_structs(f)
 
             f.write("/* %s argument table */\n" % self.fullname())
-            f.write("struct redisCommandArg %s[] = {\n" % self.subarg_table_name())
+            f.write("struct COMMAND_ARG %s[] = {\n" % self.subarg_table_name())
             for subarg in self.subargs:
                 f.write("{%s},\n" % subarg.struct_code())
-            f.write("{0}\n")
             f.write("};\n\n")
 
 
@@ -339,11 +338,14 @@ class Command(object):
         return "%s_History" % (self.fullname().replace(" ", "_"))
 
     def tips_table_name(self):
-        return "%s_tips" % (self.fullname().replace(" ", "_"))
+        return "%s_Tips" % (self.fullname().replace(" ", "_"))
 
     def arg_table_name(self):
         return "%s_Args" % (self.fullname().replace(" ", "_"))
 
+    def key_specs_table_name(self):
+        return "%s_Keyspecs" % (self.fullname().replace(" ", "_"))
+
     def reply_schema_name(self):
         return "%s_ReplySchema" % (self.fullname().replace(" ", "_"))
 
@@ -356,22 +358,37 @@ class Command(object):
         s = ""
         for tupl in self.desc["history"]:
             s += "{\"%s\",\"%s\"},\n" % (tupl[0], tupl[1])
-        s += "{0}"
         return s
 
+    def num_history(self):
+        if not self.desc.get("history"):
+            return 0
+        return len(self.desc["history"])
+
     def tips_code(self):
         if not self.desc.get("command_tips"):
             return ""
         s = ""
         for hint in self.desc["command_tips"]:
             s += "\"%s\",\n" % hint.lower()
-        s += "NULL"
         return s
 
+    def num_tips(self):
+        if not self.desc.get("command_tips"):
+            return 0
+        return len(self.desc["command_tips"])
+
+    def key_specs_code(self):
+        s = ""
+        for spec in self.key_specs:
+            s += "{%s}," % KeySpec(spec).struct_code()
+        return s[:-1]
+
+
     def struct_code(self):
         """
         Output example:
-        "set","Set the string value of a key","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,COMMAND_GROUP_STRING,SET_History,SET_tips,setCommand,-3,"write denyoom @string",{{"write read",KSPEC_BS_INDEX,.bs.index={1},KSPEC_FK_RANGE,.fk.range={0,1,0}}},.args=SET_Args
+        MAKE_CMD("set","Set the string value of a key","O(1)","1.0.0",CMD_DOC_NONE,NULL,NULL,"string",COMMAND_GROUP_STRING,SET_History,4,SET_Tips,0,setCommand,-3,CMD_WRITE|CMD_DENYOOM,ACL_CATEGORY_STRING,SET_Keyspecs,1,setGetKeys,5),.args=SET_Args
         """
 
         def _flags_code():
@@ -392,13 +409,7 @@ class Command(object):
                 s += "CMD_DOC_%s|" % flag
             return s[:-1] if s else "CMD_DOC_NONE"
 
-        def _key_specs_code():
-            s = ""
-            for spec in self.key_specs:
-                s += "{%s}," % KeySpec(spec).struct_code()
-            return s[:-1]
-
-        s = "\"%s\",%s,%s,%s,%s,%s,%s,%s,%s,%s,%s,%d,%s,%s," % (
+        s = "MAKE_CMD(\"%s\",%s,%s,%s,%s,%s,%s,%s,%s,%s,%d,%s,%d,%s,%d,%s,%s,%s,%d,%s,%d)," % (
             self.name.lower(),
             get_optional_desc_string(self.desc, "summary"),
             get_optional_desc_string(self.desc, "complexity"),
@@ -406,22 +417,22 @@ class Command(object):
             _doc_flags_code(),
             get_optional_desc_string(self.desc, "replaced_by"),
             get_optional_desc_string(self.desc, "deprecated_since"),
+            "\"%s\"" % self.group,
             GROUPS[self.group],
             self.history_table_name(),
+            self.num_history(),
             self.tips_table_name(),
+            self.num_tips(),
             self.desc.get("function", "NULL"),
             self.desc["arity"],
             _flags_code(),
-            _acl_categories_code()
+            _acl_categories_code(),
+            self.key_specs_table_name(),
+            len(self.key_specs),
+            self.desc.get("get_keys_function", "NULL"),
+            len(self.args),
         )
 
-        specs = _key_specs_code()
-        if specs:
-            s += "{%s}," % specs
-
-        if self.desc.get("get_keys_function"):
-            s += "%s," % self.desc["get_keys_function"]
-
         if self.subcommands:
             s += ".subcommands=%s," % self.subcommand_table_name()
 
@@ -440,7 +451,7 @@ class Command(object):
                 subcommand.write_internal_structs(f)
 
             f.write("/* %s command table */\n" % self.fullname())
-            f.write("struct redisCommand %s[] = {\n" % self.subcommand_table_name())
+            f.write("struct COMMAND_STRUCT %s[] = {\n" % self.subcommand_table_name())
             for subcommand in subcommand_list:
                 f.write("{%s},\n" % subcommand.struct_code())
             f.write("{0}\n")
@@ -448,33 +459,47 @@ class Command(object):
 
         f.write("/********** %s ********************/\n\n" % self.fullname())
 
+        f.write("#ifndef SKIP_CMD_HISTORY_TABLE\n")
         f.write("/* %s history */\n" % self.fullname())
         code = self.history_code()
         if code:
             f.write("commandHistory %s[] = {\n" % self.history_table_name())
-            f.write("%s\n" % code)
-            f.write("};\n\n")
+            f.write("%s" % code)
+            f.write("};\n")
         else:
-            f.write("#define %s NULL\n\n" % self.history_table_name())
+            f.write("#define %s NULL\n" % self.history_table_name())
+        f.write("#endif\n\n")
 
+        f.write("#ifndef SKIP_CMD_TIPS_TABLE\n")
         f.write("/* %s tips */\n" % self.fullname())
         code = self.tips_code()
         if code:
             f.write("const char *%s[] = {\n" % self.tips_table_name())
+            f.write("%s" % code)
+            f.write("};\n")
+        else:
+            f.write("#define %s NULL\n" % self.tips_table_name())
+        f.write("#endif\n\n")
+
+        f.write("#ifndef SKIP_CMD_KEY_SPECS_TABLE\n")
+        f.write("/* %s key specs */\n" % self.fullname())
+        code = self.key_specs_code()
+        if code:
+            f.write("keySpec %s[%d] = {\n" % (self.key_specs_table_name(), len(self.key_specs)))
             f.write("%s\n" % code)
-            f.write("};\n\n")
+            f.write("};\n")
         else:
-            f.write("#define %s NULL\n\n" % self.tips_table_name())
+            f.write("#define %s NULL\n" % self.key_specs_table_name())
+        f.write("#endif\n\n")
 
         if self.args:
             for arg in self.args:
                 arg.write_internal_structs(f)
 
             f.write("/* %s argument table */\n" % self.fullname())
-            f.write("struct redisCommandArg %s[] = {\n" % self.arg_table_name())
+            f.write("struct COMMAND_ARG %s[] = {\n" % self.arg_table_name())
             for arg in self.args:
                 f.write("{%s},\n" % arg.struct_code())
-            f.write("{0}\n")
             f.write("};\n\n")
 
         if self.reply_schema and args.with_reply_schema:
@@ -543,15 +568,40 @@ if check_command_error_counter != 0:
     exit(1)
 
 commands_filename = "commands_with_reply_schema" if args.with_reply_schema else "commands"
-print("Generating %s.c..." % commands_filename)
-with open("%s/%s.c" % (srcdir, commands_filename), "w") as f:
+print("Generating %s.def..." % commands_filename)
+with open("%s/%s.def" % (srcdir, commands_filename), "w") as f:
     f.write("/* Automatically generated by %s, do not edit. */\n\n" % os.path.basename(__file__))
-    f.write("#include \"server.h\"\n")
     f.write(
 """
 /* We have fabulous commands from
  * the fantastic
- * Redis Command Table! */\n
+ * Redis Command Table! */
+
+/* Must match redisCommandGroup */
+const char *COMMAND_GROUP_STR[] = {
+    "generic",
+    "string",
+    "list",
+    "set",
+    "sorted-set",
+    "hash",
+    "pubsub",
+    "transactions",
+    "connection",
+    "server",
+    "scripting",
+    "hyperloglog",
+    "cluster",
+    "sentinel",
+    "geo",
+    "stream",
+    "bitmap",
+    "module"
+};
+
+const char *commandGroupStr(int index) {
+    return COMMAND_GROUP_STR[index];
+}
 """
     )
 
@@ -560,7 +610,7 @@ with open("%s/%s.c" % (srcdir, commands_filename), "w") as f:
         command.write_internal_structs(f)
 
     f.write("/* Main command table */\n")
-    f.write("struct redisCommand redisCommandTable[] = {\n")
+    f.write("struct COMMAND_STRUCT redisCommandTable[] = {\n")
     curr_group = None
     for command in command_list:
         if curr_group != command.group:
diff --git a/utils/generate-command-help.rb b/utils/generate-command-help.rb
deleted file mode 100755
index 1042ce6d2..000000000
--- a/utils/generate-command-help.rb
+++ /dev/null
@@ -1,151 +0,0 @@
-#!/usr/bin/env ruby -w
-# Usage: generate-command-help.r [path/to/commands.json]
-#    or: generate-commands-json.py | generate-command-help.rb -
-#
-# Defaults to downloading commands.json from the redis-doc repo if not provided
-# or STDINed.
-
-GROUPS = [
-  "generic",
-  "string",
-  "list",
-  "set",
-  "sorted-set",
-  "hash",
-  "pubsub",
-  "transactions",
-  "connection",
-  "server",
-  "scripting",
-  "hyperloglog",
-  "cluster",
-  "geo",
-  "stream",
-  "bitmap"
-].freeze
-
-GROUPS_BY_NAME = Hash[*
-  GROUPS.each_with_index.map do |n,i|
-    [n,i]
-  end.flatten
-].freeze
-
-def argument arg
-  if "block" == arg["type"]
-    name = arg["arguments"].map do |entry|
-      argument entry
-    end.join " "
-  elsif "oneof" == arg["type"]
-    name = arg["arguments"].map do |entry|
-      argument entry
-    end.join "|"
-  elsif "pure-token" == arg["type"]
-    name = nil    # prepended later
-  else
-    name = arg["name"].is_a?(Array) ? arg["name"].join(" ") : arg["name"]
-  end
-  if arg["multiple"]
-    if arg["multiple_token"]
-      name = "#{name} [#{arg["token"]} #{name} ...]"
-    else
-      name = "#{name} [#{name} ...]"
-    end
-  end
-  if arg["token"]
-    name = [arg["token"], name].compact.join " "
-  end
-  if arg["optional"]
-    name = "[#{name}]"
-  end
-  name
-end
-
-def arguments command
-  return "" unless command["arguments"]
-  command["arguments"].map do |arg|
-    argument arg
-  end.join " "
-end
-
-def commands
-  return @commands if @commands
-
-  require "rubygems"
-  require "net/http"
-  require "net/https"
-  require "json"
-  require "uri"
-  if ARGV.length > 0
-    if ARGV[0] == '-'
-      data = STDIN.read
-    elsif FileTest.exist? ARGV[0]
-      data = File.read(ARGV[0])
-    else
-      raise Exception.new "File not found: #{ARGV[0]}"
-    end
-  else
-    url = URI.parse "https://raw.githubusercontent.com/redis/redis-doc/master/commands.json"
-    client = Net::HTTP.new url.host, url.port
-    client.use_ssl = true
-    response = client.get url.path
-    if !response.is_a?(Net::HTTPSuccess)
-      response.error!
-      return
-    else
-      data = response.body
-    end
-  end
-  @commands = JSON.parse(data)
-end
-
-def generate_groups
-  GROUPS.map do |n|
-    "\"#{n}\""
-  end.join(",\n    ");
-end
-
-def generate_commands
-  commands.to_a.sort do |x,y|
-    x[0] <=> y[0]
-  end.map do |key, command|
-    group = GROUPS_BY_NAME[command["group"]]
-    if group.nil?
-      STDERR.puts "Please update groups array in #{__FILE__}"
-      raise "Unknown group #{command["group"]}"
-    end
-
-    ret = <<-SPEC
-{ "#{key}",
-    "#{arguments(command)}",
-    "#{command["summary"]}",
-    #{group},
-    "#{command["since"]}" }
-    SPEC
-    ret.strip
-  end.join(",\n    ")
-end
-
-# Write to stdout
-puts <<-HELP_H
-/* Automatically generated by #{__FILE__}, do not edit. */
-
-#ifndef __REDIS_HELP_H
-#define __REDIS_HELP_H
-
-static char *commandGroups[] = {
-    #{generate_groups}
-};
-
-struct commandHelp {
-  char *name;
-  char *params;
-  char *summary;
-  int group;
-  char *since;
-} commandHelp[] = {
-    #{generate_commands}
-};
-
-#endif
-HELP_H
-