foster/bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-03-05 14:06:27 -05:00
Code Issues Activity

Merge bitcoin/bitcoin#26706: doc: Properly report optional RPC args

Browse source 
fad56f7dd6 doc: Properly report optional RPC args (MarcoFalke)
fa09cb6086 refactor: Introduce is_top_level_arg (MarcoFalke)
fab92a5a5a refactor: Remove const to fix performance-move-const-arg clang-tidy errors (MarcoFalke)

Pull request description:

  `OMITTED_NAMED_ARG` and `OMITTED` are a confusing burden:

  * It puts the burden on developers to pick the right one of the two
  * They can be interchanged without introducing a compile failure or other error
  * Picking the wrong one is leading to incorrect docs
  * They are redundant, because the correct one can already be determined by the surrounding type

  Fix all issues by making them an alias of each other; Pick the right one based on the outer type.

ACKs for top commit:
  fanquake:
    ACK fad56f7dd6

Tree-SHA512: 6e7193a05a852ba1618a9cb3261220c7ad3282bc5595325c04437aa811f529a88e2851e9c7dbf9878389b8aa42e98f8817b7eb0260fbb9e123da0893cbae6ca2
This commit is contained in:
fanquake 2023-01-18 15:39:45 +00:00
commit 3fef2944ad
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
2 changed files with 52 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
src/rpc/util.cpp
View file

@ -388,6 +388,7 @@ struct Sections {
const auto indent = std::string(current_indent, ' '); const auto indent = std::string(current_indent, ' ');
const auto indent_next = std::string(current_indent + 2, ' '); const auto indent_next = std::string(current_indent + 2, ' ');
const bool push_name{outer_type == OuterType::OBJ}; // Dictionary keys must have a name const bool push_name{outer_type == OuterType::OBJ}; // Dictionary keys must have a name
const bool is_top_level_arg{outer_type == OuterType::NONE}; // True on the first recursion
switch (arg.m_type) { switch (arg.m_type) {
case RPCArg::Type::STR_HEX: case RPCArg::Type::STR_HEX:
@ -396,7 +397,7 @@ struct Sections {
case RPCArg::Type::AMOUNT: case RPCArg::Type::AMOUNT:
case RPCArg::Type::RANGE: case RPCArg::Type::RANGE:
case RPCArg::Type::BOOL: { case RPCArg::Type::BOOL: {
if (outer_type == OuterType::NONE) return; // Nothing more to do for non-recursive types on first recursion if (is_top_level_arg) return; // Nothing more to do for non-recursive types on first recursion
auto left = indent; auto left = indent;
if (arg.m_opts.type_str.size() != 0 && push_name) { if (arg.m_opts.type_str.size() != 0 && push_name) {
left += "\"" + arg.GetName() + "\": " + arg.m_opts.type_str.at(0); left += "\"" + arg.GetName() + "\": " + arg.m_opts.type_str.at(0);
@ -404,12 +405,12 @@ struct Sections {
left += push_name ? arg.ToStringObj(/*oneline=*/false) : arg.ToString(/*oneline=*/false); left += push_name ? arg.ToStringObj(/*oneline=*/false) : arg.ToString(/*oneline=*/false);
} }
left += ","; left += ",";
PushSection({left, arg.ToDescriptionString()}); PushSection({left, arg.ToDescriptionString(/*is_named_arg=*/push_name)});
break; break;
} }
case RPCArg::Type::OBJ: case RPCArg::Type::OBJ:
case RPCArg::Type::OBJ_USER_KEYS: { case RPCArg::Type::OBJ_USER_KEYS: {
const auto right = outer_type == OuterType::NONE ? "" : arg.ToDescriptionString(); const auto right = is_top_level_arg ? "" : arg.ToDescriptionString(/*is_named_arg=*/push_name);
PushSection({indent + (push_name ? "\"" + arg.GetName() + "\": " : "") + "{", right}); PushSection({indent + (push_name ? "\"" + arg.GetName() + "\": " : "") + "{", right});
for (const auto& arg_inner : arg.m_inner) { for (const auto& arg_inner : arg.m_inner) {
Push(arg_inner, current_indent + 2, OuterType::OBJ); Push(arg_inner, current_indent + 2, OuterType::OBJ);
@ -417,20 +418,20 @@ struct Sections {
if (arg.m_type != RPCArg::Type::OBJ) { if (arg.m_type != RPCArg::Type::OBJ) {
PushSection({indent_next + "...", ""}); PushSection({indent_next + "...", ""});
} }
PushSection({indent + "}" + (outer_type != OuterType::NONE ? "," : ""), ""}); PushSection({indent + "}" + (is_top_level_arg ? "" : ","), ""});
break; break;
} }
case RPCArg::Type::ARR: { case RPCArg::Type::ARR: {
auto left = indent; auto left = indent;
left += push_name ? "\"" + arg.GetName() + "\": " : ""; left += push_name ? "\"" + arg.GetName() + "\": " : "";
left += "["; left += "[";
const auto right = outer_type == OuterType::NONE ? "" : arg.ToDescriptionString(); const auto right = is_top_level_arg ? "" : arg.ToDescriptionString(/*is_named_arg=*/push_name);
PushSection({left, right}); PushSection({left, right});
for (const auto& arg_inner : arg.m_inner) { for (const auto& arg_inner : arg.m_inner) {
Push(arg_inner, current_indent + 2, OuterType::ARR); Push(arg_inner, current_indent + 2, OuterType::ARR);
} }
PushSection({indent_next + "...", ""}); PushSection({indent_next + "...", ""});
PushSection({indent + "]" + (outer_type != OuterType::NONE ? "," : ""), ""}); PushSection({indent + "]" + (is_top_level_arg ? "" : ","), ""});
break; break;
} }
} // no default case, so the compiler can warn about missing cases } // no default case, so the compiler can warn about missing cases
@ -627,7 +628,7 @@ std::string RPCHelpMan::ToString() const
if (i == 0) ret += "\nArguments:\n"; if (i == 0) ret += "\nArguments:\n";
// Push named argument name and description // Push named argument name and description
sections.m_sections.emplace_back(::ToString(i + 1) + ". " + arg.GetFirstName(), arg.ToDescriptionString()); sections.m_sections.emplace_back(::ToString(i + 1) + ". " + arg.GetFirstName(), arg.ToDescriptionString(/*is_named_arg=*/true));
sections.m_max_pad = std::max(sections.m_max_pad, sections.m_sections.back().m_left.size()); sections.m_max_pad = std::max(sections.m_max_pad, sections.m_sections.back().m_left.size());
// Recursively push nested args // Recursively push nested args
@ -721,7 +722,7 @@ bool RPCArg::IsOptional() const
} }
} }
std::string RPCArg::ToDescriptionString() const std::string RPCArg::ToDescriptionString(bool is_named_arg) const
{ {
std::string ret; std::string ret;
ret += "("; ret += "(";
@ -767,14 +768,12 @@ std::string RPCArg::ToDescriptionString() const
ret += ", optional, default=" + std::get<RPCArg::Default>(m_fallback).write(); ret += ", optional, default=" + std::get<RPCArg::Default>(m_fallback).write();
} else { } else {
switch (std::get<RPCArg::Optional>(m_fallback)) { switch (std::get<RPCArg::Optional>(m_fallback)) {
case RPCArg::Optional::OMITTED_NAMED_ARG: // Deprecated alias for OMITTED, can be removed
case RPCArg::Optional::OMITTED: { case RPCArg::Optional::OMITTED: {
if (is_named_arg) ret += ", optional"; // Default value is "null" in dicts. Otherwise,
// nothing to do. Element is treated as if not present and has no default value // nothing to do. Element is treated as if not present and has no default value
break; break;
} }
case RPCArg::Optional::OMITTED_NAMED_ARG: {
ret += ", optional"; // Default value is "null"
break;
}
case RPCArg::Optional::NO: { case RPCArg::Optional::NO: {
ret += ", required"; ret += ", required";
break; break;

79
src/rpc/util.h
View file

@ -154,21 +154,24 @@ struct RPCArg {
/** Required arg */ /** Required arg */
NO, NO,
/** /**
* The arg is optional for one of two reasons:
*
* Optional arg that is a named argument and has a default value of * Optional arg that is a named argument and has a default value of
* `null`. When possible, the default value should be specified. * `null`.
*/ *
OMITTED_NAMED_ARG,
/**
* Optional argument with default value omitted because they are * Optional argument with default value omitted because they are
* implicitly clear. That is, elements in an array or object may not * implicitly clear. That is, elements in an array may not
* exist by default. * exist by default.
* When possible, the default value should be specified. * When possible, the default value should be specified.
*/ */
OMITTED, OMITTED,
OMITTED_NAMED_ARG, // Deprecated alias for OMITTED, can be removed
}; };
/** Hint for default value */
using DefaultHint = std::string; using DefaultHint = std::string;
/** Default constant value */
using Default = UniValue; using Default = UniValue;
using Fallback = std::variant<Optional, /* hint for default value */ DefaultHint, /* default constant value */ Default>; using Fallback = std::variant<Optional, DefaultHint, Default>;
const std::string m_names; //!< The name of the arg (can be empty for inner args, can contain multiple aliases separated by | for named request arguments) const std::string m_names; //!< The name of the arg (can be empty for inner args, can contain multiple aliases separated by | for named request arguments)
const Type m_type; const Type m_type;
@ -178,10 +181,10 @@ struct RPCArg {
const RPCArgOptions m_opts; const RPCArgOptions m_opts;
RPCArg( RPCArg(
const std::string name, std::string name,
const Type type, Type type,
const Fallback fallback, Fallback fallback,
const std::string description, std::string description,
RPCArgOptions opts = {}) RPCArgOptions opts = {})
: m_names{std::move(name)}, : m_names{std::move(name)},
m_type{std::move(type)}, m_type{std::move(type)},
@ -193,11 +196,11 @@ struct RPCArg {
} }
RPCArg( RPCArg(
const std::string name, std::string name,
const Type type, Type type,
const Fallback fallback, Fallback fallback,
const std::string description, std::string description,
const std::vector<RPCArg> inner, std::vector<RPCArg> inner,
RPCArgOptions opts = {}) RPCArgOptions opts = {})
: m_names{std::move(name)}, : m_names{std::move(name)},
m_type{std::move(type)}, m_type{std::move(type)},
@ -234,7 +237,7 @@ struct RPCArg {
* Return the description string, including the argument type and whether * Return the description string, including the argument type and whether
* the argument is required. * the argument is required.
*/ */
std::string ToDescriptionString() const; std::string ToDescriptionString(bool is_named_arg) const;
}; };
struct RPCResult { struct RPCResult {
@ -263,12 +266,12 @@ struct RPCResult {
const std::string m_cond; const std::string m_cond;
RPCResult( RPCResult(
const std::string cond, std::string cond,
const Type type, Type type,
const std::string m_key_name, std::string m_key_name,
const bool optional, bool optional,
const std::string description, std::string description,
const std::vector<RPCResult> inner = {}) std::vector<RPCResult> inner = {})
: m_type{std::move(type)}, : m_type{std::move(type)},
m_key_name{std::move(m_key_name)}, m_key_name{std::move(m_key_name)},
m_inner{std::move(inner)}, m_inner{std::move(inner)},
@ -282,19 +285,19 @@ struct RPCResult {
} }
RPCResult( RPCResult(
const std::string cond, std::string cond,
const Type type, Type type,
const std::string m_key_name, std::string m_key_name,
const std::string description, std::string description,
const std::vector<RPCResult> inner = {}) std::vector<RPCResult> inner = {})
: RPCResult{cond, type, m_key_name, false, description, inner} {} : RPCResult{std::move(cond), type, std::move(m_key_name), /*optional=*/false, std::move(description), std::move(inner)} {}
RPCResult( RPCResult(
const Type type, Type type,
const std::string m_key_name, std::string m_key_name,
const bool optional, bool optional,
const std::string description, std::string description,
const std::vector<RPCResult> inner = {}, std::vector<RPCResult> inner = {},
bool skip_type_check = false) bool skip_type_check = false)
: m_type{std::move(type)}, : m_type{std::move(type)},
m_key_name{std::move(m_key_name)}, m_key_name{std::move(m_key_name)},
@ -308,12 +311,12 @@ struct RPCResult {
} }
RPCResult( RPCResult(
const Type type, Type type,
const std::string m_key_name, std::string m_key_name,
const std::string description, std::string description,
const std::vector<RPCResult> inner = {}, std::vector<RPCResult> inner = {},
bool skip_type_check = false) bool skip_type_check = false)
: RPCResult{type, m_key_name, false, description, inner, skip_type_check} {} : RPCResult{type, std::move(m_key_name), /*optional=*/false, std::move(description), std::move(inner), skip_type_check} {}
/** Append the sections of the result. */ /** Append the sections of the result. */
void ToSections(Sections& sections, OuterType outer_type = OuterType::NONE, const int current_indent = 0) const; void ToSections(Sections& sections, OuterType outer_type = OuterType::NONE, const int current_indent = 0) const;