Skip to content

Commit

Permalink
submodules: update clippy from a8d90f6 to fd0428f
Browse files Browse the repository at this point in the history
Changes:
````
Treat more strange pattern
Split up `if_same_then_else` ui test
Apply review comments
Run `update_lints`
Reduce span range
Rename `ok_if_let` to `if_let_some_result`
Apply review comments
Add suggestion in `if_let_some_result`
rustup rust-lang/rust#67712
Allow `unused_self` lint at the function level
Downgrade range_plus_one to pedantic
Rustup to rust-lang/rust#68204
Add lifetimes to `LateLintPass`
Fix rustc lint import paths generated by `new_lint`
Add lint for default lint description
Update documentation for adding new lints
Generate new lints easily
Split up `booleans` ui test
Fix the ordering on `nonminimal_bool`
````
  • Loading branch information
matthiaskrgr committed Jan 19, 2020
1 parent e0b22d4 commit e656531
Show file tree
Hide file tree
Showing 54 changed files with 1,236 additions and 673 deletions.
52 changes: 52 additions & 0 deletions clippy_dev/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use clap::{App, Arg, SubCommand};
use clippy_dev::*;

mod fmt;
mod new_lint;
mod stderr_length_check;

#[derive(PartialEq)]
Expand Down Expand Up @@ -51,6 +52,47 @@ fn main() {
.help("Checks that util/dev update_lints has been run. Used on CI."),
),
)
.subcommand(
SubCommand::with_name("new_lint")
.about("Create new lint and run util/dev update_lints")
.arg(
Arg::with_name("pass")
.short("p")
.long("pass")
.help("Specify whether the lint runs during the early or late pass")
.takes_value(true)
.possible_values(&["early", "late"])
.required(true),
)
.arg(
Arg::with_name("name")
.short("n")
.long("name")
.help("Name of the new lint in snake case, ex: fn_too_long")
.takes_value(true)
.required(true),
)
.arg(
Arg::with_name("category")
.short("c")
.long("category")
.help("What category the lint belongs to")
.default_value("nursery")
.possible_values(&[
"style",
"correctness",
"complexity",
"perf",
"pedantic",
"restriction",
"cargo",
"nursery",
"internal",
"internal_warn",
])
.takes_value(true),
),
)
.arg(
Arg::with_name("limit-stderr-length")
.long("limit-stderr-length")
Expand All @@ -75,6 +117,16 @@ fn main() {
update_lints(&UpdateMode::Change);
}
},
("new_lint", Some(matches)) => {
match new_lint::create(
matches.value_of("pass"),
matches.value_of("name"),
matches.value_of("category"),
) {
Ok(_) => update_lints(&UpdateMode::Change),
Err(e) => eprintln!("Unable to create lint: {}", e),
}
},
_ => {},
}
}
Expand Down
185 changes: 185 additions & 0 deletions clippy_dev/src/new_lint.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
use std::fs::{File, OpenOptions};
use std::io;
use std::io::prelude::*;
use std::io::ErrorKind;
use std::path::{Path, PathBuf};

pub fn create(pass: Option<&str>, lint_name: Option<&str>, category: Option<&str>) -> Result<(), io::Error> {
let pass = pass.expect("`pass` argument is validated by clap");
let lint_name = lint_name.expect("`name` argument is validated by clap");
let category = category.expect("`category` argument is validated by clap");

match open_files(lint_name) {
Ok((mut test_file, mut lint_file)) => {
let (pass_type, pass_lifetimes, pass_import, context_import) = match pass {
"early" => ("EarlyLintPass", "", "use syntax::ast::*;", "EarlyContext"),
"late" => ("LateLintPass", "<'_, '_>", "use rustc_hir::*;", "LateContext"),
_ => {
unreachable!("`pass_type` should only ever be `early` or `late`!");
},
};

let camel_case_name = to_camel_case(lint_name);

if let Err(e) = test_file.write_all(get_test_file_contents(lint_name).as_bytes()) {
return Err(io::Error::new(
ErrorKind::Other,
format!("Could not write to test file: {}", e),
));
};

if let Err(e) = lint_file.write_all(
get_lint_file_contents(
pass_type,
pass_lifetimes,
lint_name,
&camel_case_name,
category,
pass_import,
context_import,
)
.as_bytes(),
) {
return Err(io::Error::new(
ErrorKind::Other,
format!("Could not write to lint file: {}", e),
));
}
Ok(())
},
Err(e) => Err(io::Error::new(
ErrorKind::Other,
format!("Unable to create lint: {}", e),
)),
}
}

fn open_files(lint_name: &str) -> Result<(File, File), io::Error> {
let project_root = project_root()?;

let test_file_path = project_root.join("tests").join("ui").join(format!("{}.rs", lint_name));
let lint_file_path = project_root
.join("clippy_lints")
.join("src")
.join(format!("{}.rs", lint_name));

if Path::new(&test_file_path).exists() {
return Err(io::Error::new(
ErrorKind::AlreadyExists,
format!("test file {:?} already exists", test_file_path),
));
}
if Path::new(&lint_file_path).exists() {
return Err(io::Error::new(
ErrorKind::AlreadyExists,
format!("lint file {:?} already exists", lint_file_path),
));
}

let test_file = OpenOptions::new().write(true).create_new(true).open(test_file_path)?;
let lint_file = OpenOptions::new().write(true).create_new(true).open(lint_file_path)?;

Ok((test_file, lint_file))
}

fn project_root() -> Result<PathBuf, io::Error> {
let current_dir = std::env::current_dir()?;
for path in current_dir.ancestors() {
let result = std::fs::read_to_string(path.join("Cargo.toml"));
if let Err(err) = &result {
if err.kind() == io::ErrorKind::NotFound {
continue;
}
}

let content = result?;
if content.contains("[package]\nname = \"clippy\"") {
return Ok(path.to_path_buf());
}
}
Err(io::Error::new(ErrorKind::Other, "Unable to find project root"))
}

fn to_camel_case(name: &str) -> String {
name.split('_')
.map(|s| {
if s.is_empty() {
String::from("")
} else {
[&s[0..1].to_uppercase(), &s[1..]].concat()
}
})
.collect()
}

fn get_test_file_contents(lint_name: &str) -> String {
format!(
"#![warn(clippy::{})]
fn main() {{
// test code goes here
}}
",
lint_name
)
}

fn get_lint_file_contents(
pass_type: &str,
pass_lifetimes: &str,
lint_name: &str,
camel_case_name: &str,
category: &str,
pass_import: &str,
context_import: &str,
) -> String {
format!(
"use rustc_lint::{{LintArray, LintPass, {type}, {context_import}}};
use rustc_session::{{declare_lint_pass, declare_tool_lint}};
{pass_import}
declare_clippy_lint! {{
/// **What it does:**
///
/// **Why is this bad?**
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// // example code
/// ```
pub {name_upper},
{category},
\"default lint description\"
}}
declare_lint_pass!({name_camel} => [{name_upper}]);
impl {type}{lifetimes} for {name_camel} {{}}
",
type=pass_type,
lifetimes=pass_lifetimes,
name_upper=lint_name.to_uppercase(),
name_camel=camel_case_name,
category=category,
pass_import=pass_import,
context_import=context_import
)
}

#[test]
fn test_camel_case() {
let s = "a_lint";
let s2 = to_camel_case(s);
assert_eq!(s2, "ALint");

let name = "a_really_long_new_lint";
let name2 = to_camel_case(name);
assert_eq!(name2, "AReallyLongNewLint");

let name3 = "lint__name";
let name4 = to_camel_case(name3);
assert_eq!(name4, "LintName");
}
2 changes: 1 addition & 1 deletion clippy_lints/src/booleans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ impl<'a, 'tcx, 'v> SuggestContext<'a, 'tcx, 'v> {
}
},
Or(v) => {
for (index, inner) in v.iter().enumerate() {
for (index, inner) in v.iter().rev().enumerate() {
if index > 0 {
self.output.push_str(" || ");
}
Expand Down
6 changes: 5 additions & 1 deletion clippy_lints/src/copy_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ declare_lint_pass!(CopyIterator => [COPY_ITERATOR]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CopyIterator {
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) {
if let ItemKind::Impl(_, _, _, _, Some(ref trait_ref), _, _) = item.kind {
if let ItemKind::Impl {
of_trait: Some(ref trait_ref),
..
} = item.kind
{
let ty = cx.tcx.type_of(cx.tcx.hir().local_def_id(item.hir_id));

if is_copy(cx, ty) && match_path(&trait_ref.path, &paths::ITERATOR) {
Expand Down
6 changes: 5 additions & 1 deletion clippy_lints/src/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ declare_lint_pass!(Derive => [EXPL_IMPL_CLONE_ON_COPY, DERIVE_HASH_XOR_EQ]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Derive {
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) {
if let ItemKind::Impl(_, _, _, _, Some(ref trait_ref), _, _) = item.kind {
if let ItemKind::Impl {
of_trait: Some(ref trait_ref),
..
} = item.kind
{
let ty = cx.tcx.type_of(cx.tcx.hir().local_def_id(item.hir_id));
let is_automatically_derived = is_automatically_derived(&*item.attrs);

Expand Down
7 changes: 5 additions & 2 deletions clippy_lints/src/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,15 +159,18 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DocMarkdown {
lint_for_missing_headers(cx, item.hir_id, item.span, sig, headers);
}
},
hir::ItemKind::Impl(_, _, _, _, ref trait_ref, ..) => {
hir::ItemKind::Impl {
of_trait: ref trait_ref,
..
} => {
self.in_trait_impl = trait_ref.is_some();
},
_ => {},
}
}

fn check_item_post(&mut self, _cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item<'_>) {
if let hir::ItemKind::Impl(..) = item.kind {
if let hir::ItemKind::Impl { .. } = item.kind {
self.in_trait_impl = false;
}
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/escape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for BoxedLocal {
let parent_node = cx.tcx.hir().find(parent_id);

if let Some(Node::Item(item)) = parent_node {
if let ItemKind::Impl(_, _, _, _, Some(..), _, _) = item.kind {
if let ItemKind::Impl { of_trait: Some(_), .. } = item.kind {
return;
}
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/fallible_impl_from.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for FallibleImplFrom {
// check for `impl From<???> for ..`
let impl_def_id = cx.tcx.hir().local_def_id(item.hir_id);
if_chain! {
if let hir::ItemKind::Impl(.., impl_items) = item.kind;
if let hir::ItemKind::Impl{ items: impl_items, .. } = item.kind;
if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(impl_def_id);
if match_def_path(cx, impl_trait_ref.def_id, &FROM_TRAIT);
then {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions {
hir_id: hir::HirId,
) {
let is_impl = if let Some(hir::Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) {
matches!(item.kind, hir::ItemKind::Impl(_, _, _, _, Some(_), _, _))
matches!(item.kind, hir::ItemKind::Impl{ of_trait: Some(_), .. })
} else {
false
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::utils::{match_type, method_chain_args, paths, snippet, span_help_and_lint};
use crate::utils::{match_type, method_chain_args, paths, snippet_with_applicability, span_lint_and_sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::*;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
Expand Down Expand Up @@ -39,20 +40,32 @@ declare_lint_pass!(OkIfLet => [IF_LET_SOME_RESULT]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for OkIfLet {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr<'_>) {
if_chain! { //begin checking variables
if let ExprKind::Match(ref op, ref body, ref source) = expr.kind; //test if expr is a match
if let MatchSource::IfLetDesugar { .. } = *source; //test if it is an If Let
if let ExprKind::MethodCall(_, _, ref result_types) = op.kind; //check is expr.ok() has type Result<T,E>.ok()
if let ExprKind::Match(ref op, ref body, source) = expr.kind; //test if expr is a match
if let MatchSource::IfLetDesugar { .. } = source; //test if it is an If Let
if let ExprKind::MethodCall(_, ok_span, ref result_types) = op.kind; //check is expr.ok() has type Result<T,E>.ok()
if let PatKind::TupleStruct(QPath::Resolved(_, ref x), ref y, _) = body[0].pat.kind; //get operation
if method_chain_args(op, &["ok"]).is_some(); //test to see if using ok() methoduse std::marker::Sized;
let is_result_type = match_type(cx, cx.tables.expr_ty(&result_types[0]), &paths::RESULT);
if print::to_string(print::NO_ANN, |s| s.print_path(x, false)) == "Some" && is_result_type;

then {
let is_result_type = match_type(cx, cx.tables.expr_ty(&result_types[0]), &paths::RESULT);
let some_expr_string = snippet(cx, y[0].span, "");
if print::to_string(print::NO_ANN, |s| s.print_path(x, false)) == "Some" && is_result_type {
span_help_and_lint(cx, IF_LET_SOME_RESULT, expr.span,
let mut applicability = Applicability::MachineApplicable;
let some_expr_string = snippet_with_applicability(cx, y[0].span, "", &mut applicability);
let trimmed_ok = snippet_with_applicability(cx, op.span.until(ok_span), "", &mut applicability);
let sugg = format!(
"if let Ok({}) = {}",
some_expr_string,
trimmed_ok.trim().trim_end_matches('.'),
);
span_lint_and_sugg(
cx,
IF_LET_SOME_RESULT,
expr.span.with_hi(op.span.hi()),
"Matching on `Some` with `ok()` is redundant",
&format!("Consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string));
}
&format!("Consider matching on `Ok({})` and removing the call to `ok` instead", some_expr_string),
sugg,
applicability,
);
}
}
}
Expand Down
Loading

0 comments on commit e656531

Please sign in to comment.