OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

RIIR update lints: Add check mode (update_lints.py rewrite complete)

Browse Source 
This finishes up the rewrite of `update_lints.py` in Rust. More
specifically, this

* adds the `--check` flag and handling to clippy_dev
* tracks file changes over the different calls to `replace_region_in_file`
* only writes changes to files if the `--check` flag is *not* used
* runs `./util/dev update_lints --check` on CI instead of the old script
* replaces usage of the `update_lints.py` script with an error

`./util/dev update_lints` behaves 99% the same as the python script.
The only difference that I'm aware of is an ordering change to
`clippy_lints/src/lib.rs` because underscores seem to be sorted
differently in Rust and in Python.

🏁
This commit is contained in:
Philipp Hansch 2018-11-04 09:41:28 +01:00
parent 7e417d4cbd
commit 90f31e21ab
No known key found for this signature in database
GPG Key ID: B6FA06A6E0E2665B
6 changed files with 81 additions and 278 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
CONTRIBUTING.md
View File

@ -180,7 +180,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
The [`rustc_plugin::PluginRegistry`][plugin_registry] provides two methods to register lints: [register_early_lint_pass][reg_early_lint_pass] and [register_late_lint_pass][reg_late_lint_pass].
Both take an object that implements an [`EarlyLintPass`][early_lint_pass] or [`LateLintPass`][late_lint_pass] respectively. This is done in every single lint.
It's worth noting that the majority of `clippy_lints/src/lib.rs` is autogenerated by `util/update_lints.py` and you don't have to add anything by hand. When you are writing your own lint, you can use that script to save you some time.
It's worth noting that the majority of `clippy_lints/src/lib.rs` is autogenerated by `util/dev update_lints` and you don't have to add anything by hand. When you are writing your own lint, you can use that script to save you some time.
```rust
// ./clippy_lints/src/else_if_without_else.rs

5
ci/base-tests.sh
View File

@ -23,8 +23,9 @@ cargo test --features debugging
cd clippy_lints && cargo test && cd ..
cd rustc_tools_util && cargo test && cd ..
cd clippy_dev && cargo test && cd ..
# check that the lint lists are up-to-date
./util/update_lints.py -c
# Perform various checks for lint registration
./util/dev update_lints --check
CLIPPY="`pwd`/target/debug/cargo-clippy clippy"
# run clippy on its own codebase...

31
clippy_dev/src/lib.rs
View File

@ -168,24 +168,34 @@ fn lint_files() -> impl Iterator<Item=walkdir::DirEntry> {
.filter(|f| f.path().extension() == Some(OsStr::new("rs")))
}
/// Whether a file has had its text changed or not
#[derive(PartialEq, Debug)]
pub struct FileChange {
pub changed: bool,
pub new_lines: String,
}
/// Replace a region in a file delimited by two lines matching regexes.
///
/// `path` is the relative path to the file on which you want to perform the replacement.
///
/// See `replace_region_in_text` for documentation of the other options.
#[allow(clippy::expect_fun_call)]
pub fn replace_region_in_file<F>(path: &str, start: &str, end: &str, replace_start: bool, replacements: F) where F: Fn() -> Vec<String> {
pub fn replace_region_in_file<F>(path: &str, start: &str, end: &str, replace_start: bool, write_back: bool, replacements: F) -> FileChange where F: Fn() -> Vec<String> {
let mut f = fs::File::open(path).expect(&format!("File not found: {}", path));
let mut contents = String::new();
f.read_to_string(&mut contents).expect("Something went wrong reading the file");
let replaced = replace_region_in_text(&contents, start, end, replace_start, replacements);
let file_change = replace_region_in_text(&contents, start, end, replace_start, replacements);
if write_back {
let mut f = fs::File::create(path).expect(&format!("File not found: {}", path));
f.write_all(replaced.as_bytes()).expect("Unable to write file");
f.write_all(file_change.new_lines.as_bytes()).expect("Unable to write file");
// Ensure we write the changes with a trailing newline so that
// the file has the proper line endings.
f.write_all(b"\n").expect("Unable to write file");
}
file_change
}
/// Replace a region in a text delimited by two lines matching regexes.
///
@ -213,10 +223,10 @@ pub fn replace_region_in_file<F>(path: &str, start: &str, end: &str, replace_sta
/// || {
/// vec!["a different".to_string(), "text".to_string()]
/// }
/// );
/// ).new_lines;
/// assert_eq!("replace_start\na different\ntext\nreplace_end", result);
/// ```
pub fn replace_region_in_text<F>(text: &str, start: &str, end: &str, replace_start: bool, replacements: F) -> String where F: Fn() -> Vec<String> {
pub fn replace_region_in_text<F>(text: &str, start: &str, end: &str, replace_start: bool, replacements: F) -> FileChange where F: Fn() -> Vec<String> {
let lines = text.lines();
let mut in_old_region = false;
let mut found = false;
@ -224,7 +234,7 @@ pub fn replace_region_in_text<F>(text: &str, start: &str, end: &str, replace_sta
let start = Regex::new(start).unwrap();
let end = Regex::new(end).unwrap();
for line in lines {
for line in lines.clone() {
if in_old_region {
if end.is_match(&line) {
in_old_region = false;
@ -248,7 +258,10 @@ pub fn replace_region_in_text<F>(text: &str, start: &str, end: &str, replace_sta
// is incorrect.
eprintln!("error: regex `{:?}` not found. You may have to update it.", start);
}
new_lines.join("\n")
FileChange {
changed: lines.ne(new_lines.clone()),
new_lines: new_lines.join("\n")
}
}
#[test]
@ -305,7 +318,7 @@ def
ghi"#;
let result = replace_region_in_text(text, r#"^\s*abc$"#, r#"^\s*def"#, false, || {
vec!["hello world".to_string()]
});
}).new_lines;
assert_eq!(expected, result);
}
@ -323,7 +336,7 @@ def
ghi"#;
let result = replace_region_in_text(text, r#"^\s*abc$"#, r#"^\s*def"#, true, || {
vec!["hello world".to_string()]
});
}).new_lines;
assert_eq!(expected, result);
}

55
clippy_dev/src/main.rs
View File

@ -15,6 +15,12 @@ extern crate regex;
use clap::{App, Arg, SubCommand};
use clippy_dev::*;
#[derive(PartialEq)]
enum UpdateMode {
Check,
Change
}
fn main() {
let matches = App::new("Clippy developer tooling")
.subcommand(
@ -28,8 +34,12 @@ fn main() {
.arg(
Arg::with_name("print-only")
.long("print-only")
.short("p")
.help("Print a table of lints to STDOUT. This does not include deprecated and internal lints. (Does not modify any files)"),
.help("Print a table of lints to STDOUT. This does not include deprecated and internal lints. (Does not modify any files)")
)
.arg(
Arg::with_name("check")
.long("check")
.help("Checks that util/dev update_lints has been run. Used on CI."),
)
)
.get_matches();
@ -37,8 +47,10 @@ fn main() {
if let Some(matches) = matches.subcommand_matches("update_lints") {
if matches.is_present("print-only") {
print_lints();
} else if matches.is_present("check") {
update_lints(UpdateMode::Check);
} else {
update_lints();
update_lints(UpdateMode::Change);
}
}
}
@ -63,53 +75,58 @@ fn print_lints() {
println!("there are {} lints", lint_count);
}
fn update_lints() {
fn update_lints(update_mode: UpdateMode) {
let lint_list: Vec<Lint> = gather_all().collect();
let usable_lints: Vec<Lint> = Lint::usable_lints(lint_list.clone().into_iter()).collect();
let lint_count = usable_lints.len();
replace_region_in_file(
let mut file_change = replace_region_in_file(
"../README.md",
r#"\[There are \d+ lints included in this crate!\]\(https://rust-lang-nursery.github.io/rust-clippy/master/index.html\)"#,
"",
true,
update_mode == UpdateMode::Change,
|| {
vec![
format!("[There are {} lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)", lint_count)
]
}
);
).changed;
replace_region_in_file(
file_change |= replace_region_in_file(
"../CHANGELOG.md",
"<!-- begin autogenerated links to lint list -->",
"<!-- end autogenerated links to lint list -->",
false,
update_mode == UpdateMode::Change,
|| { gen_changelog_lint_list(lint_list.clone()) }
);
).changed;
replace_region_in_file(
file_change |= replace_region_in_file(
"../clippy_lints/src/lib.rs",
"begin deprecated lints",
"end deprecated lints",
false,
update_mode == UpdateMode::Change,
|| { gen_deprecated(&lint_list) }
);
).changed;
replace_region_in_file(
file_change |= replace_region_in_file(
"../clippy_lints/src/lib.rs",
"begin lints modules",
"end lints modules",
false,
update_mode == UpdateMode::Change,
|| { gen_modules_list(lint_list.clone()) }
);
).changed;
// Generate lists of lints in the clippy::all lint group
replace_region_in_file(
file_change |= replace_region_in_file(
"../clippy_lints/src/lib.rs",
r#"reg.register_lint_group\("clippy::all""#,
r#"\]\);"#,
false,
update_mode == UpdateMode::Change,
|| {
// clippy::all should only include the following lint groups:
let all_group_lints = usable_lints.clone().into_iter().filter(|l| {
@ -121,16 +138,22 @@ fn update_lints() {
gen_lint_group_list(all_group_lints)
}
);
).changed;
// Generate the list of lints for all other lint groups
for (lint_group, lints) in Lint::by_lint_group(&usable_lints) {
replace_region_in_file(
file_change |= replace_region_in_file(
"../clippy_lints/src/lib.rs",
&format!("reg.register_lint_group\\(\"clippy::{}\"", lint_group),
r#"\]\);"#,
false,
update_mode == UpdateMode::Change,
|| { gen_lint_group_list(lints.clone()) }
);
).changed;
}
if update_mode == UpdateMode::Check && file_change {
println!("Not all lints defined properly. Please run util/dev update_lints to make sure all lints are defined properly.");
std::process::exit(1);
}
}

12
clippy_lints/src/lib.rs
View File

@ -548,8 +548,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
bytecount::NAIVE_BYTECOUNT,
collapsible_if::COLLAPSIBLE_IF,
const_static_lifetime::CONST_STATIC_LIFETIME,
copies::IF_SAME_THEN_ELSE,
copies::IFS_SAME_COND,
copies::IF_SAME_THEN_ELSE,
cyclomatic_complexity::CYCLOMATIC_COMPLEXITY,
derive::DERIVE_HASH_XOR_EQ,
double_comparison::DOUBLE_COMPARISONS,
@ -743,12 +743,12 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
unused_io_amount::UNUSED_IO_AMOUNT,
unused_label::UNUSED_LABEL,
vec::USELESS_VEC,
write::PRINTLN_EMPTY_STRING,
write::PRINT_LITERAL,
write::PRINT_WITH_NEWLINE,
write::PRINTLN_EMPTY_STRING,
write::WRITELN_EMPTY_STRING,
write::WRITE_LITERAL,
write::WRITE_WITH_NEWLINE,
write::WRITELN_EMPTY_STRING,
zero_div_zero::ZERO_DIVIDED_BY_ZERO,
]);
@ -831,12 +831,12 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
types::IMPLICIT_HASHER,
types::LET_UNIT_VALUE,
unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME,
write::PRINTLN_EMPTY_STRING,
write::PRINT_LITERAL,
write::PRINT_WITH_NEWLINE,
write::PRINTLN_EMPTY_STRING,
write::WRITELN_EMPTY_STRING,
write::WRITE_LITERAL,
write::WRITE_WITH_NEWLINE,
write::WRITELN_EMPTY_STRING,
]);
reg.register_lint_group("clippy::complexity", Some("clippy_complexity"), vec![
@ -916,8 +916,8 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
bit_mask::BAD_BIT_MASK,
bit_mask::INEFFECTIVE_BIT_MASK,
booleans::LOGIC_BUG,
copies::IF_SAME_THEN_ELSE,
copies::IFS_SAME_COND,
copies::IF_SAME_THEN_ELSE,
derive::DERIVE_HASH_XOR_EQ,
drop_forget_ref::DROP_COPY,
drop_forget_ref::DROP_REF,

240
util/update_lints.py
View File

@ -10,245 +10,11 @@
# option. This file may not be copied, modified, or distributed
# except according to those terms.
# Generate a Markdown table of all lints, and put it in README.md.
# With -n option, only print the new table to stdout.
# With -c option, print a warning and set exit status to 1 if a file would be
# changed.
import os
import re
import sys
from subprocess import call
declare_deprecated_lint_re = re.compile(r'''
declare_deprecated_lint! \s* [{(] \s*
pub \s+ (?P<name>[A-Z_][A-Z_0-9]*) \s*,\s*
" (?P<desc>(?:[^"\\]+|\\.)*) " \s* [})]
''', re.VERBOSE | re.DOTALL)
declare_clippy_lint_re = re.compile(r'''
declare_clippy_lint! \s* [{(] \s*
pub \s+ (?P<name>[A-Z_][A-Z_0-9]*) \s*,\s*
(?P<cat>[a-z_]+) \s*,\s*
" (?P<desc>(?:[^"\\]+|\\.)*) " \s* [})]
''', re.VERBOSE | re.DOTALL)
nl_escape_re = re.compile(r'\\\n\s*')
docs_link = 'https://rust-lang-nursery.github.io/rust-clippy/master/index.html'
def collect(deprecated_lints, clippy_lints, fn):
"""Collect all lints from a file.
Adds entries to the lints list as `(module, name, level, desc)`.
"""
with open(fn) as fp:
code = fp.read()
for match in declare_deprecated_lint_re.finditer(code):
# remove \-newline escapes from description string
desc = nl_escape_re.sub('', match.group('desc'))
deprecated_lints.append((os.path.splitext(os.path.basename(fn))[0],
match.group('name').lower(),
desc.replace('\\"', '"')))
for match in declare_clippy_lint_re.finditer(code):
# remove \-newline escapes from description string
desc = nl_escape_re.sub('', match.group('desc'))
cat = match.group('cat')
if cat in ('internal', 'internal_warn'):
continue
module_name = os.path.splitext(os.path.basename(fn))[0]
if module_name == 'mod':
module_name = os.path.basename(os.path.dirname(fn))
clippy_lints[cat].append((module_name,
match.group('name').lower(),
"allow",
desc.replace('\\"', '"')))
def gen_group(lints):
"""Write lint group (list of all lints in the form module::NAME)."""
for (module, name, _, _) in sorted(lints):
yield ' %s::%s,\n' % (module, name.upper())
def gen_mods(lints):
"""Declare modules"""
for module in sorted(set(lint[0] for lint in lints)):
yield 'pub mod %s;\n' % module
def gen_deprecated(lints):
"""Declare deprecated lints"""
for lint in lints:
yield ' store.register_removed(\n'
yield ' "%s",\n' % lint[1]
yield ' "%s",\n' % lint[2]
yield ' );\n'
def replace_region(fn, region_start, region_end, callback,
replace_start=True, write_back=True):
"""Replace a region in a file delimited by two lines matching regexes.
A callback is called to write the new region. If `replace_start` is true,
the start delimiter line is replaced as well. The end delimiter line is
never replaced.
"""
# read current content
with open(fn) as fp:
lines = list(fp)
found = False
# replace old region with new region
new_lines = []
in_old_region = False
for line in lines:
if in_old_region:
if re.search(region_end, line):
in_old_region = False
new_lines.extend(callback())
new_lines.append(line)
elif re.search(region_start, line):
if not replace_start:
new_lines.append(line)
# old region starts here
in_old_region = True
found = True
else:
new_lines.append(line)
if not found:
print("regex " + region_start + " not found")
# write back to file
if write_back:
with open(fn, 'w') as fp:
fp.writelines(new_lines)
# if something changed, return true
return lines != new_lines
def main(print_only=False, check=False):
deprecated_lints = []
clippy_lints = {
"correctness": [],
"style": [],
"complexity": [],
"perf": [],
"restriction": [],
"pedantic": [],
"cargo": [],
"nursery": [],
}
# check directory
if not os.path.isfile('clippy_lints/src/lib.rs'):
print('Error: call this script from clippy checkout directory!')
return
# collect all lints from source files
for root, dirs, files in os.walk('clippy_lints/src'):
for fn in files:
if fn.endswith('.rs'):
collect(deprecated_lints, clippy_lints,
os.path.join(root, fn))
# determine version
with open('Cargo.toml') as fp:
for line in fp:
if line.startswith('version ='):
clippy_version = line.split()[2].strip('"')
break
else:
print('Error: version not found in Cargo.toml!')
return
all_lints = []
clippy_lint_groups = [
"correctness",
"style",
"complexity",
"perf",
]
clippy_lint_list = []
for x in clippy_lint_groups:
clippy_lint_list += clippy_lints[x]
for _, value in clippy_lints.iteritems():
all_lints += value
if print_only:
call(["./util/dev", "update_lints", "--print-only"])
return
# update the lint counter in README.md
changed = replace_region(
'README.md',
r'^\[There are \d+ lints included in this crate!\]\(https://rust-lang-nursery.github.io/rust-clippy/master/index.html\)$', "",
lambda: ['[There are %d lints included in this crate!](https://rust-lang-nursery.github.io/rust-clippy/master/index.html)\n' %
(len(all_lints))],
write_back=not check)
# update the links in the CHANGELOG
changed |= replace_region(
'CHANGELOG.md',
"<!-- begin autogenerated links to lint list -->",
"<!-- end autogenerated links to lint list -->",
lambda: ["[`{0}`]: {1}#{0}\n".format(l[1], docs_link) for l in
sorted(all_lints + deprecated_lints,
key=lambda l: l[1])],
replace_start=False, write_back=not check)
# update version of clippy_lints in Cargo.toml
changed |= replace_region(
'Cargo.toml', r'# begin automatic update', '# end automatic update',
lambda: ['clippy_lints = { version = "%s", path = "clippy_lints" }\n' %
clippy_version],
replace_start=False, write_back=not check)
# update version of clippy_lints in Cargo.toml
changed |= replace_region(
'clippy_lints/Cargo.toml', r'# begin automatic update', '# end automatic update',
lambda: ['version = "%s"\n' % clippy_version],
replace_start=False, write_back=not check)
# update the `pub mod` list
changed |= replace_region(
'clippy_lints/src/lib.rs', r'begin lints modules', r'end lints modules',
lambda: gen_mods(all_lints),
replace_start=False, write_back=not check)
# same for "clippy::*" lint collections
changed |= replace_region(
'clippy_lints/src/lib.rs', r'reg.register_lint_group\("clippy::all"', r'\]\);',
lambda: gen_group(clippy_lint_list),
replace_start=False, write_back=not check)
for key, value in clippy_lints.iteritems():
# same for "clippy::*" lint collections
changed |= replace_region(
'clippy_lints/src/lib.rs', r'reg.register_lint_group\("clippy::' + key + r'"', r'\]\);',
lambda: gen_group(value),
replace_start=False, write_back=not check)
# same for "deprecated" lint collection
changed |= replace_region(
'clippy_lints/src/lib.rs', r'begin deprecated lints', r'end deprecated lints',
lambda: gen_deprecated(deprecated_lints),
replace_start=False,
write_back=not check)
if check and changed:
print('Please run util/update_lints.py to regenerate lints lists.')
def main():
print('Error: Please use `util/dev` to update lints')
return 1
if __name__ == '__main__':
sys.exit(main(print_only='-n' in sys.argv, check='-c' in sys.argv))
sys.exit(main())