mirror of
https://github.com/bitcoin/bitcoin.git
synced 2025-03-05 14:06:27 -05:00
Merge bitcoin/bitcoin#30553: lint: Find function calls in default arguments
fac7b7ff7f
lint: Find function calls in default arguments (MarcoFalke) Pull request description: This type of bug in the test code keeps biting back regularly, is hard to debug, and wastes review cycles: https://github.com/bitcoin/bitcoin/issues/30543#issuecomment-2259260024 . Fix all issues by catching it with a linter that checks for rule B008: https://docs.astral.sh/ruff/rules/function-call-in-default-argument/ This also allows to drop the hand-written linter that checks for rule B006: https://docs.astral.sh/ruff/rules/mutable-argument-default/ ACKs for top commit: davidgumberg: reACKfac7b7ff7f
Tree-SHA512: a47a28a35ec9c81947cb8c3e625f1eb8c0d7e780a4d768491ef94b2beed43b9710bf6c1044da18c7fd677ea5576fb9077c7f77b4465033fedfdca9920c185bf7
This commit is contained in:
commit
37a6d7643c
3 changed files with 35 additions and 72 deletions
|
@ -53,6 +53,7 @@ ${CI_RETRY_EXE} pip3 install \
|
||||||
lief==0.13.2 \
|
lief==0.13.2 \
|
||||||
mypy==1.4.1 \
|
mypy==1.4.1 \
|
||||||
pyzmq==25.1.0 \
|
pyzmq==25.1.0 \
|
||||||
|
ruff==0.5.5 \
|
||||||
vulture==2.6
|
vulture==2.6
|
||||||
|
|
||||||
SHELLCHECK_VERSION=v0.8.0
|
SHELLCHECK_VERSION=v0.8.0
|
||||||
|
|
|
@ -1,72 +0,0 @@
|
||||||
#!/usr/bin/env python3
|
|
||||||
#
|
|
||||||
# Copyright (c) 2019-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.
|
|
||||||
|
|
||||||
"""
|
|
||||||
Detect when a mutable list or dict is used as a default parameter value in a Python function.
|
|
||||||
"""
|
|
||||||
|
|
||||||
import subprocess
|
|
||||||
import sys
|
|
||||||
|
|
||||||
|
|
||||||
def main():
|
|
||||||
command = [
|
|
||||||
"git",
|
|
||||||
"grep",
|
|
||||||
"-E",
|
|
||||||
r"^\s*def [a-zA-Z0-9_]+\(.*=\s*(\[|\{)",
|
|
||||||
"--",
|
|
||||||
"*.py",
|
|
||||||
]
|
|
||||||
output = subprocess.run(command, stdout=subprocess.PIPE, text=True)
|
|
||||||
if len(output.stdout) > 0:
|
|
||||||
error_msg = (
|
|
||||||
"A mutable list or dict seems to be used as default parameter value:\n\n"
|
|
||||||
f"{output.stdout}\n"
|
|
||||||
f"{example()}"
|
|
||||||
)
|
|
||||||
print(error_msg)
|
|
||||||
sys.exit(1)
|
|
||||||
else:
|
|
||||||
sys.exit(0)
|
|
||||||
|
|
||||||
|
|
||||||
def example():
|
|
||||||
return """This is how mutable list and dict default parameter values behave:
|
|
||||||
|
|
||||||
>>> def f(i, j=[], k={}):
|
|
||||||
... j.append(i)
|
|
||||||
... k[i] = True
|
|
||||||
... return j, k
|
|
||||||
...
|
|
||||||
>>> f(1)
|
|
||||||
([1], {1: True})
|
|
||||||
>>> f(1)
|
|
||||||
([1, 1], {1: True})
|
|
||||||
>>> f(2)
|
|
||||||
([1, 1, 2], {1: True, 2: True})
|
|
||||||
|
|
||||||
The intended behaviour was likely:
|
|
||||||
|
|
||||||
>>> def f(i, j=None, k=None):
|
|
||||||
... if j is None:
|
|
||||||
... j = []
|
|
||||||
... if k is None:
|
|
||||||
... k = {}
|
|
||||||
... j.append(i)
|
|
||||||
... k[i] = True
|
|
||||||
... return j, k
|
|
||||||
...
|
|
||||||
>>> f(1)
|
|
||||||
([1], {1: True})
|
|
||||||
>>> f(1)
|
|
||||||
([1], {1: True})
|
|
||||||
>>> f(2)
|
|
||||||
([2], {2: True})"""
|
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
|
||||||
main()
|
|
|
@ -35,6 +35,11 @@ fn get_linter_list() -> Vec<&'static Linter> {
|
||||||
name: "markdown",
|
name: "markdown",
|
||||||
lint_fn: lint_markdown
|
lint_fn: lint_markdown
|
||||||
},
|
},
|
||||||
|
&Linter {
|
||||||
|
description: "Check the default arguments in python",
|
||||||
|
name: "py_mut_arg_default",
|
||||||
|
lint_fn: lint_py_mut_arg_default,
|
||||||
|
},
|
||||||
&Linter {
|
&Linter {
|
||||||
description: "Check that std::filesystem is not used directly",
|
description: "Check that std::filesystem is not used directly",
|
||||||
name: "std_filesystem",
|
name: "std_filesystem",
|
||||||
|
@ -180,6 +185,35 @@ fn lint_subtree() -> LintResult {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn lint_py_mut_arg_default() -> LintResult {
|
||||||
|
let bin_name = "ruff";
|
||||||
|
let checks = ["B006", "B008"]
|
||||||
|
.iter()
|
||||||
|
.map(|c| format!("--select={}", c))
|
||||||
|
.collect::<Vec<_>>();
|
||||||
|
let files = check_output(
|
||||||
|
git()
|
||||||
|
.args(["ls-files", "--", "*.py"])
|
||||||
|
.args(get_pathspecs_exclude_subtrees()),
|
||||||
|
)?;
|
||||||
|
|
||||||
|
let mut cmd = Command::new(bin_name);
|
||||||
|
cmd.arg("check").args(checks).args(files.lines());
|
||||||
|
|
||||||
|
match cmd.status() {
|
||||||
|
Ok(status) if status.success() => Ok(()),
|
||||||
|
Ok(_) => Err(format!("`{}` found errors!", bin_name)),
|
||||||
|
Err(e) if e.kind() == ErrorKind::NotFound => {
|
||||||
|
println!(
|
||||||
|
"`{}` was not found in $PATH, skipping those checks.",
|
||||||
|
bin_name
|
||||||
|
);
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
Err(e) => Err(format!("Error running `{}`: {}", bin_name, e)),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fn lint_std_filesystem() -> LintResult {
|
fn lint_std_filesystem() -> LintResult {
|
||||||
let found = git()
|
let found = git()
|
||||||
.args([
|
.args([
|
||||||
|
|
Loading…
Add table
Reference in a new issue