lint: Move assertion linter into lint runner

On failure, this makes the output more consistent with the other linter.
Each failure will be marked with an '⚠️ ' emoji and explanation, making
it easier to spot.

Also, add --line-number to the filesystem linter.

Also, add newlines after each failing check, to visually separate
different failures from each other.

Can be reviewed with:
"--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space"
This commit is contained in:
MarcoFalke 2024-12-06 09:55:54 +01:00
parent beac62e541
commit fa9aacf614
No known key found for this signature in database
2 changed files with 69 additions and 56 deletions

View file

@ -1,54 +0,0 @@
#!/usr/bin/env python3
#
# Copyright (c) 2018-2022 The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
#
# Check for assertions with obvious side effects.
import sys
import subprocess
def git_grep(params: [], error_msg: ""):
try:
output = subprocess.check_output(["git", "grep", *params], text=True, encoding="utf8")
print(error_msg)
print(output)
return 1
except subprocess.CalledProcessError as ex1:
if ex1.returncode > 1:
raise ex1
return 0
def main():
# Aborting the whole process is undesirable for RPC code. So nonfatal
# checks should be used over assert. See: src/util/check.h
# src/rpc/server.cpp is excluded from this check since it's mostly meta-code.
exit_code = git_grep([
"--line-number",
"--extended-regexp",
r"\<(A|a)ss(ume|ert)\(",
"--",
"src/rpc/",
"src/wallet/rpc*",
":(exclude)src/rpc/server.cpp",
], "CHECK_NONFATAL(condition) or NONFATAL_UNREACHABLE should be used instead of assert for RPC code.")
# The `BOOST_ASSERT` macro requires to `#include boost/assert.hpp`,
# which is an unnecessary Boost dependency.
exit_code |= git_grep([
"--line-number",
"--extended-regexp",
r"BOOST_ASSERT\(",
"--",
"*.cpp",
"*.h",
], "BOOST_ASSERT must be replaced with Assert, BOOST_REQUIRE, or BOOST_CHECK.")
sys.exit(exit_code)
if __name__ == "__main__":
main()

View file

@ -48,6 +48,16 @@ fn get_linter_list() -> Vec<&'static Linter> {
name: "std_filesystem",
lint_fn: lint_std_filesystem
},
&Linter {
description: "Check that fatal assertions are not used in RPC code",
name: "rpc_assert",
lint_fn: lint_rpc_assert
},
&Linter {
description: "Check that boost assertions are not used",
name: "boost_assert",
lint_fn: lint_boost_assert
},
&Linter {
description: "Check that release note snippets are in the right folder",
name: "doc_release_note_snippets",
@ -273,6 +283,7 @@ fn lint_std_filesystem() -> LintResult {
let found = git()
.args([
"grep",
"--line-number",
"std::filesystem",
"--",
"./src/",
@ -293,6 +304,62 @@ fs:: namespace, which has unsafe filesystem functions marked as deleted.
}
}
fn lint_rpc_assert() -> LintResult {
let found = git()
.args([
"grep",
"--line-number",
"--extended-regexp",
r"\<(A|a)ss(ume|ert)\(",
"--",
"src/rpc/",
"src/wallet/rpc*",
":(exclude)src/rpc/server.cpp",
// src/rpc/server.cpp is excluded from this check since it's mostly meta-code.
])
.status()
.expect("command error")
.success();
if found {
Err(r#"
^^^
CHECK_NONFATAL(condition) or NONFATAL_UNREACHABLE should be used instead of assert for RPC code.
Aborting the whole process is undesirable for RPC code. So nonfatal
checks should be used over assert. See: src/util/check.h
"#
.to_string())
} else {
Ok(())
}
}
fn lint_boost_assert() -> LintResult {
let found = git()
.args([
"grep",
"--line-number",
"--extended-regexp",
r"BOOST_ASSERT\(",
"--",
"*.cpp",
"*.h",
])
.status()
.expect("command error")
.success();
if found {
Err(r#"
^^^
BOOST_ASSERT must be replaced with Assert, BOOST_REQUIRE, or BOOST_CHECK to avoid an unnecessary
include of the boost/assert.hpp dependency.
"#
.to_string())
} else {
Ok(())
}
}
fn lint_doc_release_note_snippets() -> LintResult {
let non_release_notes = check_output(git().args([
"ls-files",
@ -593,7 +660,7 @@ fn main() -> ExitCode {
"{err}\n^---- ⚠️ Failure generated from lint check '{}'!",
linter.name
);
println!("{}", linter.description);
println!("{}\n\n", linter.description);
test_failed = true;
}
}