refactor: Add simple clang-tidy readability checks (#6556)

This change enables the following clang-tidy checks:
-  readability-avoid-nested-conditional-operator,
-  readability-avoid-return-with-void-value,
-  readability-braces-around-statements,
-  readability-const-return-type,
-  readability-container-contains,
-  readability-container-size-empty,
-  readability-else-after-return,
-  readability-make-member-function-const,
-  readability-redundant-casting,
-  readability-redundant-inline-specifier,
-  readability-redundant-member-init,
-  readability-redundant-string-init,
-  readability-reference-to-constructed-temporary,
-  readability-static-definition
This commit is contained in:
Alex Kremer
2026-03-18 16:41:49 +00:00
committed by GitHub
parent b92a9a3053
commit 57e4cbbcd9
328 changed files with 4415 additions and 1176 deletions

View File

@@ -53,8 +53,6 @@ AMM::AMM(
, log_(log)
, doClose_(close)
, lastPurchasePrice_(0)
, bidMin_()
, bidMax_()
, msig_(ms)
, fee_(fee)
, ammAccount_(create(tfee, flags, seq, ter))
@@ -123,9 +121,13 @@ AMM::create(
if (flags)
jv[jss::Flags] = *flags;
if (fee_ != 0)
{
jv[sfFee] = std::to_string(fee_);
}
else
{
jv[jss::Fee] = std::to_string(env_.current()->fees().increment.drops());
}
submit(jv, seq, ter);
if (!ter || env_.ter() == tesSUCCESS)
@@ -218,6 +220,7 @@ IOUAmount
AMM::getLPTokensBalance(std::optional<AccountID> const& account) const
{
if (account)
{
return accountHolds(
*env_.current(),
*account,
@@ -225,6 +228,7 @@ AMM::getLPTokensBalance(std::optional<AccountID> const& account) const
FreezeHandling::fhZERO_IF_FROZEN,
env_.journal)
.iou();
}
if (auto const amm = env_.current()->read(keylet::amm(asset1_.issue(), asset2_.issue())))
return amm->getFieldAmount(sfLPTokenBalance).iou();
return IOUAmount{0};
@@ -442,15 +446,25 @@ AMM::deposit(
if (!(jvFlags & tfDepositSubTx))
{
if (tokens && !asset1In)
{
jvFlags |= tfLPToken;
}
else if (tokens && asset1In)
{
jvFlags |= tfOneAssetLPToken;
}
else if (asset1In && asset2In)
{
jvFlags |= tfTwoAsset;
}
else if (maxEP && asset1In)
{
jvFlags |= tfLimitLPToken;
}
else if (asset1In)
{
jvFlags |= tfSingleAsset;
}
}
jv[jss::Flags] = jvFlags;
return deposit(account, jv, assets, seq, ter);
@@ -562,15 +576,25 @@ AMM::withdraw(
if (!(jvFlags & tfWithdrawSubTx))
{
if (tokens && !asset1Out)
{
jvFlags |= tfLPToken;
}
else if (asset1Out && asset2Out)
{
jvFlags |= tfTwoAsset;
}
else if (tokens && asset1Out)
{
jvFlags |= tfOneAssetLPToken;
}
else if (asset1Out && maxEP)
{
jvFlags |= tfLimitLPToken;
}
else if (asset1Out)
{
jvFlags |= tfSingleAsset;
}
}
jv[jss::Flags] = jvFlags;
return withdraw(account, jv, seq, assets, ter);
@@ -615,7 +639,7 @@ AMM::vote(
void
AMM::vote(VoteArg const& arg)
{
return vote(arg.account, arg.tfee, arg.flags, arg.seq, arg.assets, arg.err);
vote(arg.account, arg.tfee, arg.flags, arg.seq, arg.assets, arg.err);
}
Json::Value
@@ -641,11 +665,15 @@ AMM::bid(BidArg const& arg)
setTokens(jv, arg.assets);
auto getBid = [&](auto const& bid) {
if (std::holds_alternative<int>(bid))
{
return STAmount{lptIssue_, std::get<int>(bid)};
else if (std::holds_alternative<IOUAmount>(bid))
}
if (std::holds_alternative<IOUAmount>(bid))
{
return toSTAmount(std::get<IOUAmount>(bid), lptIssue_);
else
return std::get<STAmount>(bid);
}
return std::get<STAmount>(bid);
};
if (arg.bidMin)
{
@@ -659,7 +687,7 @@ AMM::bid(BidArg const& arg)
saTokens.setJson(jv[jss::BidMax]);
bidMax_ = saTokens.iou();
}
if (arg.authAccounts.size() > 0)
if (!arg.authAccounts.empty())
{
Json::Value accounts(Json::arrayValue);
for (auto const& account : arg.authAccounts)
@@ -691,22 +719,38 @@ AMM::submit(
if (msig_)
{
if (seq && ter)
{
env_(jv, *msig_, *seq, *ter);
}
else if (seq)
{
env_(jv, *msig_, *seq);
}
else if (ter)
{
env_(jv, *msig_, *ter);
}
else
{
env_(jv, *msig_);
}
}
else if (seq && ter)
{
env_(jv, *seq, *ter);
}
else if (seq)
{
env_(jv, *seq);
}
else if (ter)
{
env_(jv, *ter);
}
else
{
env_(jv);
}
if (doClose_)
env_.close();
}

View File

@@ -129,11 +129,17 @@ AMMTestBase::testAMM(std::function<void(jtx::AMM&, jtx::Env&)> const& cb, TestAM
BEAST_EXPECT(asset1 <= toFund1 && asset2 <= toFund2);
if (!asset1.native() && !asset2.native())
{
fund(env, gw, {alice, carol}, {toFund1, toFund2}, Fund::All);
}
else if (asset1.native())
{
fund(env, gw, {alice, carol}, toFund1, {toFund2}, Fund::All);
}
else if (asset2.native())
{
fund(env, gw, {alice, carol}, toFund2, {toFund1}, Fund::All);
}
AMM ammAlice(
env, alice, asset1, asset2, CreateArg{.log = false, .tfee = arg.tfee, .err = arg.ter});

View File

@@ -107,7 +107,9 @@ Env::close(NetClock::time_point closeTime, std::optional<std::chrono::millisecon
// Go through the rpc interface unless we need to simulate
// a specific consensus delay.
if (consensusDelay)
{
app().getOPs().acceptLedger(consensusDelay);
}
else
{
auto resp = rpc("ledger_accept");
@@ -115,11 +117,17 @@ Env::close(NetClock::time_point closeTime, std::optional<std::chrono::millisecon
{
std::string reason = "internal error";
if (resp.isMember("error_what"))
{
reason = resp["error_what"].asString();
}
else if (resp.isMember("error_message"))
{
reason = resp["error_message"].asString();
}
else if (resp.isMember("error"))
{
reason = resp["error"].asString();
}
JLOG(journal.error()) << "Env::close() failed: " << reason;
res = false;
@@ -199,16 +207,14 @@ Env::balance(Account const& account, MPTIssue const& mptIssue) const
STAmount const amount{mptIssue, sle->getFieldU64(sfOutstandingAmount), 0, true};
return {amount, lookup(issuer).name()};
}
else
{
// Holder balance
auto const sle = le(keylet::mptoken(id, account));
if (!sle)
return {STAmount(mptIssue, 0), account.name()};
STAmount const amount{mptIssue, sle->getFieldU64(sfMPTAmount)};
return {amount, lookup(issuer).name()};
}
// Holder balance
auto const sle = le(keylet::mptoken(id, account));
if (!sle)
return {STAmount(mptIssue, 0), account.name()};
STAmount const amount{mptIssue, sle->getFieldU64(sfMPTAmount)};
return {amount, lookup(issuer).name()};
}
PrettyAmount
@@ -336,10 +342,14 @@ Env::parseResult(Json::Value const& jr)
parsed.rpcCode.emplace(rpcSUCCESS);
}
else
{
error(parsed, result);
}
}
else
{
error(parsed, jr);
}
return parsed;
}
@@ -362,16 +372,14 @@ Env::submit(JTx const& jt, std::source_location const& loc)
return jr;
}
else
{
// Parsing failed or the JTx is
// otherwise missing the stx field.
parsedResult.ter = ter_ = temMALFORMED;
return Json::Value();
}
// Parsing failed or the JTx is
// otherwise missing the stx field.
parsedResult.ter = ter_ = temMALFORMED;
return Json::Value();
}();
return postconditions(jt, parsedResult, jr, loc);
postconditions(jt, parsedResult, jr, loc);
}
void
@@ -409,7 +417,7 @@ Env::sign_and_submit(JTx const& jt, Json::Value params, std::source_location con
test.expect(parsedResult.ter, "ter uninitialized!");
ter_ = parsedResult.ter.value_or(telENV_RPC_FAILED);
return postconditions(jt, parsedResult, jr, loc);
postconditions(jt, parsedResult, jr, loc);
}
void
@@ -532,9 +540,13 @@ Env::autofill_sig(JTx& jt)
}
auto const ar = le(account);
if (ar && ar->isFieldPresent(sfRegularKey))
{
jtx::sign(jv, lookup(ar->getAccountID(sfRegularKey)));
}
else
{
jtx::sign(jv, account);
}
}
void

View File

@@ -35,8 +35,10 @@ class JSONRPCClient : public AbstractClient
continue;
using namespace boost::asio::ip;
if (pp.ip && pp.ip->is_unspecified())
{
*pp.ip = pp.ip->is_v6() ? address{address_v6::loopback()}
: address{address_v4::loopback()};
}
if (!pp.port)
Throw<std::runtime_error>("Use fixConfigPorts with auto ports");

View File

@@ -12,7 +12,7 @@ namespace test {
namespace jtx {
namespace oracle {
Oracle::Oracle(Env& env, CreateArg const& arg, bool submit) : env_(env), owner_{}, documentID_{}
Oracle::Oracle(Env& env, CreateArg const& arg, bool submit) : env_(env), documentID_{}
{
// LastUpdateTime is checked to be in range
// {close-maxLastUpdateTimeDelta, close+maxLastUpdateTimeDelta}.
@@ -38,11 +38,17 @@ Oracle::remove(RemoveArg const& arg)
jv[jss::Account] = to_string(arg.owner.value_or(owner_));
toJson(jv[jss::OracleDocumentID], arg.documentID.value_or(documentID_));
if (Oracle::fee != 0)
{
jv[jss::Fee] = std::to_string(Oracle::fee);
}
else if (arg.fee != 0)
{
jv[jss::Fee] = std::to_string(arg.fee);
}
else
{
jv[jss::Fee] = std::to_string(env_.current()->fees().increment.drops());
}
if (arg.flags != 0)
jv[jss::Flags] = arg.flags;
submit(jv, arg.msig, arg.seq, arg.err);
@@ -58,22 +64,38 @@ Oracle::submit(
if (msig)
{
if (seq && err)
{
env_(jv, *msig, *seq, *err);
}
else if (seq)
{
env_(jv, *msig, *seq);
}
else if (err)
{
env_(jv, *msig, *err);
}
else
{
env_(jv, *msig);
}
}
else if (seq && err)
{
env_(jv, *seq, *err);
}
else if (seq)
{
env_(jv, *seq);
}
else if (err)
{
env_(jv, *err);
}
else
{
env_(jv);
}
env_.close();
}
@@ -90,7 +112,7 @@ Oracle::expectPrice(DataSeries const& series) const
if (auto const sle = env_.le(keylet::oracle(owner_, documentID_)))
{
auto const& leSeries = sle->getFieldArray(sfPriceDataSeries);
if (leSeries.size() == 0 || leSeries.size() != series.size())
if (leSeries.empty() || leSeries.size() != series.size())
return false;
for (auto const& data : series)
{
@@ -157,9 +179,13 @@ Oracle::aggregatePrice(
if (jr.isObject())
{
if (jr.isMember(jss::result) && jr[jss::result].isMember(jss::status))
{
return jr[jss::result];
else if (jr.isMember(jss::error))
}
if (jr.isMember(jss::error))
{
return jr;
}
}
return Json::nullValue;
}
@@ -177,9 +203,13 @@ Oracle::set(UpdateArg const& arg)
jv[jss::OracleDocumentID] = documentID_;
}
else if (arg.documentID)
{
toJson(jv[jss::OracleDocumentID], *arg.documentID);
}
else
{
jv[jss::OracleDocumentID] = documentID_;
}
jv[jss::TransactionType] = jss::OracleSet;
jv[jss::Account] = to_string(owner_);
if (arg.assetClass)
@@ -191,24 +221,36 @@ Oracle::set(UpdateArg const& arg)
if (arg.flags != 0)
jv[jss::Flags] = arg.flags;
if (Oracle::fee != 0)
{
jv[jss::Fee] = std::to_string(Oracle::fee);
}
else if (arg.fee != 0)
{
jv[jss::Fee] = std::to_string(arg.fee);
}
else
{
jv[jss::Fee] = std::to_string(env_.current()->fees().increment.drops());
}
// lastUpdateTime if provided is offset from testStartTime
if (arg.lastUpdateTime)
{
if (std::holds_alternative<std::uint32_t>(*arg.lastUpdateTime))
{
jv[jss::LastUpdateTime] =
to_string(testStartTime.count() + std::get<std::uint32_t>(*arg.lastUpdateTime));
}
else
{
toJson(jv[jss::LastUpdateTime], *arg.lastUpdateTime);
}
}
else
{
jv[jss::LastUpdateTime] = to_string(
duration_cast<seconds>(env_.current()->header().closeTime.time_since_epoch()).count() +
epoch_offset.count());
}
Json::Value dataSeries(Json::arrayValue);
auto assetToStr = [](std::string const& s) {
// assume standard currency
@@ -225,8 +267,10 @@ Oracle::set(UpdateArg const& arg)
price[jss::BaseAsset] = assetToStr(std::get<0>(data));
price[jss::QuoteAsset] = assetToStr(std::get<1>(data));
if (std::get<2>(data))
{
price[jss::AssetPrice] =
*std::get<2>(data); // NOLINT(bugprone-unchecked-optional-access)
}
if (std::get<3>(data))
price[jss::Scale] = *std::get<3>(data); // NOLINT(bugprone-unchecked-optional-access)
priceData[jss::PriceData] = price;
@@ -265,9 +309,13 @@ Oracle::ledgerEntry(
if (account)
{
if (std::holds_alternative<AccountID>(*account))
{
jvParams[jss::oracle][jss::account] = to_string(std::get<AccountID>(*account));
}
else
{
jvParams[jss::oracle][jss::account] = std::get<std::string>(*account);
}
}
if (documentID)
toJson(jvParams[jss::oracle][jss::oracle_document_id], *documentID);
@@ -275,9 +323,13 @@ Oracle::ledgerEntry(
{
std::uint32_t i = 0;
if (boost::conversion::try_lexical_convert(*index, i))
{
jvParams[jss::oracle][jss::ledger_index] = i;
}
else
{
jvParams[jss::oracle][jss::ledger_index] = *index;
}
}
// Convert "%None%" to None
auto str = to_string(jvParams);
@@ -308,12 +360,18 @@ toJsonHex(Json::Value& jv, AnyValue const& v)
if constexpr (std::is_same_v<T, std::string const&>)
{
if (arg.starts_with("##"))
{
jv = arg.substr(2);
}
else
{
jv = strHex(arg);
}
}
else
{
jv = arg;
}
},
v);
}

View File

@@ -49,8 +49,10 @@ class WSClientImpl : public WSClient
continue;
using namespace boost::asio::ip;
if (pp.ip && pp.ip->is_unspecified())
{
*pp.ip = pp.ip->is_v6() ? address{address_v6::loopback()}
: address{address_v4::loopback()};
}
if (!pp.port)
Throw<std::runtime_error>("Use fixConfigPorts with auto ports");
@@ -178,7 +180,9 @@ public:
jp[jss::id] = 5;
}
else
{
jp[jss::command] = cmd;
}
auto const s = to_string(jp);
ws_.write_some(true, buffer(s));
}

View File

@@ -62,9 +62,13 @@ operator<<(std::ostream& os, PrettyAmount const& amount)
if (n < c)
{
if (amount.value().negative())
{
os << "-" << n << " drops";
}
else
{
os << n << " drops";
}
return os;
}
auto const d = double(n) / dropsPerXRP.drops();

View File

@@ -66,7 +66,7 @@ doBalance(
void
balance::operator()(Env& env) const
{
return std::visit(
std::visit(
[&](auto const& issue) { doBalance(env, account_.id(), none_, value_, issue); },
value_.asset().value());
}

View File

@@ -72,7 +72,9 @@ bumpLastPage(
// Adjust root previous and previous node's next
sleRoot->setFieldU64(sfIndexPrevious, newLastPage);
if (prevIndex.value_or(0) == 0)
{
sleRoot->setFieldU64(sfIndexNext, newLastPage);
}
else
{
auto slePrev = sb.peek(keylet::page(directory, *prevIndex));
@@ -88,6 +90,7 @@ bumpLastPage(
// Fixup page numbers in the objects referred by indexes
if (adjust)
{
for (auto const key : indexes)
{
if (!adjust(sb, key, newLastPage))
@@ -96,6 +99,7 @@ bumpLastPage(
return false;
}
}
}
sb.apply(view);
return true;

View File

@@ -14,9 +14,13 @@ fee::operator()(Env& env, JTx& jt) const
jt.fill_fee = false;
assert(!increment_ || !amount_);
if (increment_)
{
jt[sfFee] = STAmount(env.current()->fees().increment).getJson();
}
else if (amount_)
{
jt[sfFee] = amount_->getJson(JsonOptions::none);
}
}
} // namespace jtx

View File

@@ -24,11 +24,17 @@ flags::operator()(Env& env) const
{
auto const sle = env.le(account_);
if (!sle)
{
env.test.fail();
}
else if (sle->isFieldPresent(sfFlags))
{
env.test.expect((sle->getFieldU32(sfFlags) & mask_) == mask_);
}
else
{
env.test.expect(mask_ == 0);
}
}
void
@@ -36,11 +42,17 @@ nflags::operator()(Env& env) const
{
auto const sle = env.le(account_);
if (!sle)
{
env.test.fail();
}
else if (sle->isFieldPresent(sfFlags))
{
env.test.expect((sle->getFieldU32(sfFlags) & mask_) == 0);
}
else
{
env.test.pass();
}
}
} // namespace jtx

View File

@@ -77,12 +77,14 @@ static MPTCreate
makeMPTCreate(MPTInitDef const& arg)
{
if (arg.pay)
{
return {
.maxAmt = arg.maxAmt,
.transferFee = arg.transferFee,
.pay = {{arg.holders, *arg.pay}},
.flags = arg.flags,
.authHolder = arg.authHolder};
}
return {
.maxAmt = arg.maxAmt,
.transferFee = arg.transferFee,
@@ -174,16 +176,24 @@ MPTTester::create(MPTCreate const& arg)
if (arg.authorize)
{
if (arg.authorize->empty())
{
authAndPay(holders_, [](auto const& it) { return it.second; });
}
else
{
authAndPay(*arg.authorize, [](auto const& it) { return it; });
}
}
else if (arg.pay)
{
if (arg.pay->first.empty())
{
authAndPay(holders_, [](auto const& it) { return it.second; });
}
else
{
authAndPay(arg.pay->first, [](auto const& it) { return it; });
}
}
}
}
@@ -253,10 +263,14 @@ MPTTester::authorize(MPTAuthorize const& arg)
auto const flags = getFlags(arg.holder);
// issuer un-authorizes the holder
if (arg.flags.value_or(0) == tfMPTUnauthorize)
{
env_.require(mptflags(*this, flags, arg.holder));
// issuer authorizes the holder
// issuer authorizes the holder
}
else
{
env_.require(mptflags(*this, flags | lsfMPTAuthorized, arg.holder));
}
}
// Holder authorizes
else if (arg.flags.value_or(0) != tfMPTUnauthorize)
@@ -315,9 +329,13 @@ MPTTester::setJV(MPTSet const& arg)
std::visit(
[&jv]<typename T>(T const& holder) {
if constexpr (std::is_same_v<T, Account>)
{
jv[sfHolder] = holder.human();
}
else if constexpr (std::is_same_v<T, AccountID>)
{
jv[sfHolder] = toBase58(holder);
}
},
*arg.holder);
}
@@ -360,42 +378,70 @@ MPTTester::set(MPTSet const& arg)
if (arg.flags)
{
if (*arg.flags & tfMPTLock)
{
flags |= lsfMPTLocked;
}
else if (*arg.flags & tfMPTUnlock)
{
flags &= ~lsfMPTLocked;
}
}
if (arg.mutableFlags)
{
if (*arg.mutableFlags & tmfMPTSetCanLock)
{
flags |= lsfMPTCanLock;
}
else if (*arg.mutableFlags & tmfMPTClearCanLock)
{
flags &= ~lsfMPTCanLock;
}
if (*arg.mutableFlags & tmfMPTSetRequireAuth)
{
flags |= lsfMPTRequireAuth;
}
else if (*arg.mutableFlags & tmfMPTClearRequireAuth)
{
flags &= ~lsfMPTRequireAuth;
}
if (*arg.mutableFlags & tmfMPTSetCanEscrow)
{
flags |= lsfMPTCanEscrow;
}
else if (*arg.mutableFlags & tmfMPTClearCanEscrow)
{
flags &= ~lsfMPTCanEscrow;
}
if (*arg.mutableFlags & tmfMPTSetCanClawback)
{
flags |= lsfMPTCanClawback;
}
else if (*arg.mutableFlags & tmfMPTClearCanClawback)
{
flags &= ~lsfMPTCanClawback;
}
if (*arg.mutableFlags & tmfMPTSetCanTrade)
{
flags |= lsfMPTCanTrade;
}
else if (*arg.mutableFlags & tmfMPTClearCanTrade)
{
flags &= ~lsfMPTCanTrade;
}
if (*arg.mutableFlags & tmfMPTSetCanTransfer)
{
flags |= lsfMPTCanTransfer;
}
else if (*arg.mutableFlags & tmfMPTClearCanTransfer)
{
flags &= ~lsfMPTCanTransfer;
}
}
}
env_.require(mptflags(*this, flags, holder));
@@ -498,12 +544,16 @@ MPTTester::pay(
auto const outstandingAmt = getBalance(issuer_);
if (credentials)
{
env_(
jtx::pay(src, dest, mpt(amount)),
ter(err.value_or(tesSUCCESS)),
credentials::ids(*credentials));
}
else
{
env_(jtx::pay(src, dest, mpt(amount)), ter(err.value_or(tesSUCCESS)));
}
if (!isTesSuccess(env_.ter()))
amount = 0;

View File

@@ -53,9 +53,13 @@ msig::operator()(Env& env, JTx& jt) const
// The signing pub key is only required at the top level.
if (!subField)
{
sigObject[sfSigningPubKey] = "";
}
else if (sigObject.isNull())
{
sigObject = Json::Value(Json::objectValue);
}
std::optional<STObject> st;
try
{
@@ -80,9 +84,13 @@ msig::operator()(Env& env, JTx& jt) const
}
};
if (!subField)
{
jt.mainSigners.emplace_back(callback);
}
else
{
jt.postSigners.emplace_back(callback);
}
}
} // namespace jtx

View File

@@ -23,9 +23,13 @@ sig::operator()(Env&, JTx& jt) const
jtx::sign(jtx.jv, account, sigObject);
};
if (!subField_)
{
jt.mainSigners.emplace_back(callback);
}
else
{
jt.postSigners.emplace_back(callback);
}
}
}

View File

@@ -12,7 +12,7 @@ namespace test {
namespace jtx {
std::tuple<Json::Value, Keylet>
Vault::create(CreateArgs const& args)
Vault::create(CreateArgs const& args) const
{
auto keylet = keylet::vault(args.owner.id(), env.seq(args.owner));
Json::Value jv;

View File

@@ -286,6 +286,7 @@ claim_attestations(
JValueVec vec;
vec.reserve(numAtts);
for (auto i = fromIdx; i < fromIdx + numAtts; ++i)
{
vec.emplace_back(claim_attestation(
submittingAccount,
jvBridge,
@@ -296,6 +297,7 @@ claim_attestations(
claimID,
dst,
signers[i]));
}
return vec;
}
@@ -319,6 +321,7 @@ create_account_attestations(
JValueVec vec;
vec.reserve(numAtts);
for (auto i = fromIdx; i < fromIdx + numAtts; ++i)
{
vec.emplace_back(create_account_attestation(
submittingAccount,
jvBridge,
@@ -330,6 +333,7 @@ create_account_attestations(
createCount,
dst,
signers[i]));
}
return vec;
}