Merge branch 'master' into dogfood_target_dir
This commit is contained in:
commit
0fbcb30303
@ -15,41 +15,41 @@ High level approach:
|
||||
|
||||
All issues on Clippy are mentored, if you want help with a bug just ask @Manishearth, @llogiq, @mcarton or @oli-obk.
|
||||
|
||||
Some issues are easier than others. The [good first issue](https://github.com/rust-lang-nursery/rust-clippy/labels/good%20first%20issue)
|
||||
Some issues are easier than others. The [`good first issue`](https://github.com/rust-lang-nursery/rust-clippy/labels/good%20first%20issue)
|
||||
label can be used to find the easy issues. If you want to work on an issue, please leave a comment
|
||||
so that we can assign it to you!
|
||||
|
||||
Issues marked [T-AST](https://github.com/rust-lang-nursery/rust-clippy/labels/T-AST) involve simple
|
||||
Issues marked [`T-AST`](https://github.com/rust-lang-nursery/rust-clippy/labels/T-AST) involve simple
|
||||
matching of the syntax tree structure, and are generally easier than
|
||||
[T-middle](https://github.com/rust-lang-nursery/rust-clippy/labels/T-middle) issues, which involve types
|
||||
[`T-middle`](https://github.com/rust-lang-nursery/rust-clippy/labels/T-middle) issues, which involve types
|
||||
and resolved paths.
|
||||
|
||||
Issues marked [E-medium](https://github.com/rust-lang-nursery/rust-clippy/labels/E-medium) are generally
|
||||
pretty easy too, though it's recommended you work on an E-easy issue first. They are mostly classified
|
||||
as `E-medium`, since they might be somewhat involved code wise, but not difficult per-se.
|
||||
|
||||
[Llogiq's blog post on lints](https://llogiq.github.io/2015/06/04/workflows.html) is a nice primer
|
||||
to lint-writing, though it does get into advanced stuff. Most lints consist of an implementation of
|
||||
`LintPass` with one or more of its default methods overridden. See the existing lints for examples
|
||||
of this.
|
||||
|
||||
T-AST issues will generally need you to match against a predefined syntax structure. To figure out
|
||||
[`T-AST`](https://github.com/rust-lang-nursery/rust-clippy/labels/T-AST) issues will generally need you to match against a predefined syntax structure. To figure out
|
||||
how this syntax structure is encoded in the AST, it is recommended to run `rustc -Z ast-json` on an
|
||||
example of the structure and compare with the
|
||||
[nodes in the AST docs](http://manishearth.github.io/rust-internals-docs/syntax/ast/). Usually
|
||||
the lint will end up to be a nested series of matches and ifs,
|
||||
[like so](https://github.com/rust-lang-nursery/rust-clippy/blob/de5ccdfab68a5e37689f3c950ed1532ba9d652a0/src/misc.rs#L34).
|
||||
|
||||
T-middle issues can be more involved and require verifying types. The
|
||||
[`E-medium`](https://github.com/rust-lang-nursery/rust-clippy/labels/E-medium) issues are generally
|
||||
pretty easy too, though it's recommended you work on an E-easy issue first. They are mostly classified
|
||||
as `E-medium`, since they might be somewhat involved code wise, but not difficult per-se.
|
||||
|
||||
[`T-middle`](https://github.com/rust-lang-nursery/rust-clippy/labels/T-middle) issues can
|
||||
be more involved and require verifying types. The
|
||||
[`ty`](http://manishearth.github.io/rust-internals-docs/rustc/ty) module contains a
|
||||
lot of methods that are useful, though one of the most useful would be `expr_ty` (gives the type of
|
||||
an AST expression). `match_def_path()` in Clippy's `utils` module can also be useful.
|
||||
|
||||
### Writing code
|
||||
|
||||
Compiling clippy can take almost a minute or more depending on your machine.
|
||||
You can set the environment flag `CARGO_INCREMENTAL=1` to cut down that time to
|
||||
almost a third on average, depending on the influence your change has.
|
||||
Compiling clippy from scratch can take almost a minute or more depending on your machine.
|
||||
However, since Rust 1.24.0 incremental compilation is enabled by default and compile times for small changes should be quick.
|
||||
|
||||
[Llogiq's blog post on lints](https://llogiq.github.io/2015/06/04/workflows.html) is a nice primer
|
||||
to lint-writing, though it does get into advanced stuff. Most lints consist of an implementation of
|
||||
`LintPass` with one or more of its default methods overridden. See the existing lints for examples
|
||||
of this.
|
||||
|
||||
Please document your lint with a doc comment akin to the following:
|
||||
|
||||
@ -61,8 +61,13 @@ Please document your lint with a doc comment akin to the following:
|
||||
/// **Known problems:** None. (Or describe where it could go wrong.)
|
||||
///
|
||||
/// **Example:**
|
||||
///
|
||||
/// ```rust
|
||||
/// Insert a short example if you have one.
|
||||
/// // Bad
|
||||
/// Insert a short example of code that triggers the lint
|
||||
///
|
||||
/// // Good
|
||||
/// Insert a short example of improved code that doesn't trigger the lint
|
||||
/// ```
|
||||
```
|
||||
|
||||
@ -80,12 +85,6 @@ If you don't want to wait for all tests to finish, you can also execute a single
|
||||
TESTNAME=ui/empty_line_after_outer_attr cargo test --test compile-test
|
||||
```
|
||||
|
||||
And you can also combine this with `CARGO_INCREMENTAL`:
|
||||
|
||||
```bash
|
||||
CARGO_INCREMENTAL=1 TESTNAME=ui/doc cargo test --test compile-test
|
||||
```
|
||||
|
||||
### Testing manually
|
||||
|
||||
Manually testing against an example file is useful if you have added some
|
||||
|
@ -7,7 +7,7 @@ use rustc::ty::{self, TyCtxt};
|
||||
use semver::Version;
|
||||
use syntax::ast::{Attribute, AttrStyle, Lit, LitKind, MetaItemKind, NestedMetaItem, NestedMetaItemKind};
|
||||
use syntax::codemap::Span;
|
||||
use utils::{in_macro, last_line_of_span, match_def_path, opt_def_id, paths, snippet_opt, span_lint, span_lint_and_then};
|
||||
use utils::{in_macro, last_line_of_span, match_def_path, opt_def_id, paths, snippet_opt, span_lint, span_lint_and_then, without_block_comments};
|
||||
|
||||
/// **What it does:** Checks for items annotated with `#[inline(always)]`,
|
||||
/// unless the annotated function is empty or simply panics.
|
||||
@ -85,7 +85,11 @@ declare_clippy_lint! {
|
||||
/// If it was meant to be an outer attribute, then the following item
|
||||
/// should not be separated by empty lines.
|
||||
///
|
||||
/// **Known problems:** None
|
||||
/// **Known problems:** Can cause false positives.
|
||||
///
|
||||
/// From the clippy side it's difficult to detect empty lines between an attributes and the
|
||||
/// following item because empty lines and comments are not part of the AST. The parsing
|
||||
/// currently works for basic cases but is not perfect.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
@ -105,7 +109,7 @@ declare_clippy_lint! {
|
||||
/// ```
|
||||
declare_clippy_lint! {
|
||||
pub EMPTY_LINE_AFTER_OUTER_ATTR,
|
||||
style,
|
||||
nursery,
|
||||
"empty line after outer attribute"
|
||||
}
|
||||
|
||||
@ -276,6 +280,8 @@ fn check_attrs(cx: &LateContext, span: Span, name: &Name, attrs: &[Attribute]) {
|
||||
|
||||
if let Some(snippet) = snippet_opt(cx, end_of_attr_to_item) {
|
||||
let lines = snippet.split('\n').collect::<Vec<_>>();
|
||||
let lines = without_block_comments(lines);
|
||||
|
||||
if lines.iter().filter(|l| l.trim().is_empty()).count() > 2 {
|
||||
span_lint(
|
||||
cx,
|
||||
|
@ -1086,3 +1086,36 @@ pub fn clip(tcx: TyCtxt, u: u128, ity: ast::UintTy) -> u128 {
|
||||
let amt = 128 - bits;
|
||||
(u << amt) >> amt
|
||||
}
|
||||
|
||||
/// Remove block comments from the given Vec of lines
|
||||
///
|
||||
/// # Examples
|
||||
///
|
||||
/// ```rust,ignore
|
||||
/// without_block_comments(vec!["/*", "foo", "*/"]);
|
||||
/// // => vec![]
|
||||
///
|
||||
/// without_block_comments(vec!["bar", "/*", "foo", "*/"]);
|
||||
/// // => vec!["bar"]
|
||||
/// ```
|
||||
pub fn without_block_comments(lines: Vec<&str>) -> Vec<&str> {
|
||||
let mut without = vec![];
|
||||
|
||||
let mut nest_level = 0;
|
||||
|
||||
for line in lines.into_iter() {
|
||||
if line.contains("/*") {
|
||||
nest_level += 1;
|
||||
continue;
|
||||
} else if line.contains("*/") {
|
||||
nest_level -= 1;
|
||||
continue;
|
||||
}
|
||||
|
||||
if nest_level == 0 {
|
||||
without.push(line);
|
||||
}
|
||||
}
|
||||
|
||||
without
|
||||
}
|
||||
|
@ -79,4 +79,16 @@ pub enum FooFighter {
|
||||
Bar4
|
||||
}
|
||||
|
||||
// This should not produce a warning because the empty line is inside a block comment
|
||||
#[crate_type = "lib"]
|
||||
/*
|
||||
|
||||
*/
|
||||
pub struct S;
|
||||
|
||||
// This should not produce a warning
|
||||
#[crate_type = "lib"]
|
||||
/* test */
|
||||
pub struct T;
|
||||
|
||||
fn main() { }
|
||||
|
29
tests/without_block_comments.rs
Normal file
29
tests/without_block_comments.rs
Normal file
@ -0,0 +1,29 @@
|
||||
extern crate clippy_lints;
|
||||
use clippy_lints::utils::without_block_comments;
|
||||
|
||||
#[test]
|
||||
fn test_lines_without_block_comments() {
|
||||
let result = without_block_comments(vec!["/*", "", "*/"]);
|
||||
println!("result: {:?}", result);
|
||||
assert!(result.is_empty());
|
||||
|
||||
let result = without_block_comments(
|
||||
vec!["", "/*", "", "*/", "#[crate_type = \"lib\"]", "/*", "", "*/", ""]
|
||||
);
|
||||
assert_eq!(result, vec!["", "#[crate_type = \"lib\"]", ""]);
|
||||
|
||||
let result = without_block_comments(vec!["/* rust", "", "*/"]);
|
||||
assert!(result.is_empty());
|
||||
|
||||
let result = without_block_comments(vec!["/* one-line comment */"]);
|
||||
assert!(result.is_empty());
|
||||
|
||||
let result = without_block_comments(vec!["/* nested", "/* multi-line", "comment", "*/", "test", "*/"]);
|
||||
assert!(result.is_empty());
|
||||
|
||||
let result = without_block_comments(vec!["/* nested /* inline /* comment */ test */ */"]);
|
||||
assert!(result.is_empty());
|
||||
|
||||
let result = without_block_comments(vec!["foo", "bar", "baz"]);
|
||||
assert_eq!(result, vec!["foo", "bar", "baz"]);
|
||||
}
|
@ -7,12 +7,11 @@ import collections
|
||||
import logging as log
|
||||
log.basicConfig(level=log.INFO, format='%(levelname)s: %(message)s')
|
||||
|
||||
Lint = collections.namedtuple('Lint', 'name level doc sourcefile')
|
||||
Lint = collections.namedtuple('Lint', 'name level doc sourcefile group')
|
||||
Config = collections.namedtuple('Config', 'name ty doc default')
|
||||
|
||||
lintname_re = re.compile(r'''pub\s+([A-Z_][A-Z_0-9]*)''')
|
||||
level_re = re.compile(r'''(Forbid|Deny|Warn|Allow)''')
|
||||
group_re = re.compile(r'''([a-z_][a-z_0-9]+)''')
|
||||
group_re = re.compile(r'''\s*([a-z_][a-z_0-9]+)''')
|
||||
conf_re = re.compile(r'''define_Conf! {\n([^}]*)\n}''', re.MULTILINE)
|
||||
confvar_re = re.compile(
|
||||
r'''/// Lint: (\w+). (.*).*\n\s*\([^,]+,\s+"([^"]+)",\s+([^=\)]+)=>\s+(.*)\),''', re.MULTILINE)
|
||||
@ -27,6 +26,7 @@ lint_levels = {
|
||||
"nursery": 'Allow',
|
||||
}
|
||||
|
||||
|
||||
def parse_lints(lints, filepath):
|
||||
last_comment = []
|
||||
comment = True
|
||||
@ -57,36 +57,30 @@ def parse_lints(lints, filepath):
|
||||
else:
|
||||
last_comment = []
|
||||
if not comment:
|
||||
if name:
|
||||
g = group_re.search(line)
|
||||
if g:
|
||||
group = g.group(1).lower()
|
||||
level = lint_levels[group]
|
||||
log.info("found %s with level %s in %s",
|
||||
name, level, filepath)
|
||||
lints.append(Lint(name, level, last_comment, filepath, group))
|
||||
last_comment = []
|
||||
comment = True
|
||||
else:
|
||||
m = lintname_re.search(line)
|
||||
if m:
|
||||
name = m.group(1).lower()
|
||||
m = lintname_re.search(line)
|
||||
|
||||
if m:
|
||||
name = m.group(1).lower()
|
||||
line = next(fp)
|
||||
|
||||
if deprecated:
|
||||
level = "Deprecated"
|
||||
group = "deprecated"
|
||||
else:
|
||||
while True:
|
||||
g = group_re.search(line)
|
||||
if g:
|
||||
group = g.group(1).lower()
|
||||
level = lint_levels[group]
|
||||
break
|
||||
line = next(fp)
|
||||
|
||||
log.info("found %s with level %s in %s",
|
||||
name, level, filepath)
|
||||
lints.append(Lint(name, level, last_comment, filepath, group))
|
||||
last_comment = []
|
||||
comment = True
|
||||
|
||||
if deprecated:
|
||||
level = "Deprecated"
|
||||
else:
|
||||
while True:
|
||||
m = level_re.search(line)
|
||||
if m:
|
||||
level = m.group(0)
|
||||
break
|
||||
line = next(fp)
|
||||
if not clippy:
|
||||
log.info("found %s with level %s in %s",
|
||||
name, level, filepath)
|
||||
lints.append(Lint(name, level, last_comment, filepath, "deprecated"))
|
||||
last_comment = []
|
||||
comment = True
|
||||
if "}" in line:
|
||||
log.warn("Warning: missing Lint-Name in %s", filepath)
|
||||
comment = True
|
||||
|
Loading…
x
Reference in New Issue
Block a user