Skip to content

Commit

Permalink
Auto merge of rust-lang#3893 - mati865:rustup, r=oli-obk
Browse files Browse the repository at this point in the history 
Rustup

Supersedes rust-lang/rust-clippy#3889

Addresses some review comments from previous PR and rustups to rust-lang#58899
  • Loading branch information
@bors
bors committed Mar 18, 2019
2 parents 54e2051 + 6cb0605 commit 92612c9
Show file tree
Hide file tree
Showing 22 changed files with 233 additions and 152 deletions.
160 changes: 83 additions & 77 deletions clippy_lints/src/attrs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc::ty::{self, TyCtxt};
use rustc::{declare_tool_lint, lint_array};
use rustc_errors::Applicability;
use semver::Version;
use syntax::ast::{AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem, NestedMetaItemKind};
use syntax::ast::{AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
use syntax::source_map::Span;

declare_clippy_lint! {
Expand Down Expand Up @@ -208,22 +208,24 @@ impl LintPass for AttrPass {
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AttrPass {
fn check_attribute(&mut self, cx: &LateContext<'a, 'tcx>, attr: &'tcx Attribute) {
if let Some(items) = &attr.meta_item_list() {
match &*attr.name().as_str() {
"allow" | "warn" | "deny" | "forbid" => {
check_clippy_lint_names(cx, items);
},
_ => {},
}
if items.is_empty() || attr.name() != "deprecated" {
return;
}
for item in items {
if_chain! {
if let NestedMetaItemKind::MetaItem(mi) = &item.node;
if let MetaItemKind::NameValue(lit) = &mi.node;
if mi.name() == "since";
then {
check_semver(cx, item.span, lit);
if let Some(ident) = attr.ident_str() {
match ident {
"allow" | "warn" | "deny" | "forbid" => {
check_clippy_lint_names(cx, items);
},
_ => {},
}
if items.is_empty() || !attr.check_name("deprecated") {
return;
}
for item in items {
if_chain! {
if let NestedMetaItem::MetaItem(mi) = &item;
if let MetaItemKind::NameValue(lit) = &mi.node;
if mi.check_name("since");
then {
check_semver(cx, item.span(), lit);
}
}
}
}
Expand All @@ -236,55 +238,57 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AttrPass {
}
match item.node {
ItemKind::ExternCrate(..) | ItemKind::Use(..) => {
let skip_unused_imports = item.attrs.iter().any(|attr| attr.name() == "macro_use");
let skip_unused_imports = item.attrs.iter().any(|attr| attr.check_name("macro_use"));

for attr in &item.attrs {
if let Some(lint_list) = &attr.meta_item_list() {
match &*attr.name().as_str() {
"allow" | "warn" | "deny" | "forbid" => {
// whitelist `unused_imports` and `deprecated` for `use` items
// and `unused_imports` for `extern crate` items with `macro_use`
for lint in lint_list {
match item.node {
ItemKind::Use(..) => {
if is_word(lint, "unused_imports") || is_word(lint, "deprecated") {
return;
}
},
ItemKind::ExternCrate(..) => {
if is_word(lint, "unused_imports") && skip_unused_imports {
return;
}
if is_word(lint, "unused_extern_crates") {
return;
}
},
_ => {},
}
}
let line_span = last_line_of_span(cx, attr.span);

if let Some(mut sugg) = snippet_opt(cx, line_span) {
if sugg.contains("#[") {
span_lint_and_then(
cx,
USELESS_ATTRIBUTE,
line_span,
"useless lint attribute",
|db| {
sugg = sugg.replacen("#[", "#![", 1);
db.span_suggestion(
line_span,
"if you just forgot a `!`, use",
sugg,
Applicability::MachineApplicable,
);
if let Some(ident) = attr.ident_str() {
match ident {
"allow" | "warn" | "deny" | "forbid" => {
// whitelist `unused_imports` and `deprecated` for `use` items
// and `unused_imports` for `extern crate` items with `macro_use`
for lint in lint_list {
match item.node {
ItemKind::Use(..) => {
if is_word(lint, "unused_imports") || is_word(lint, "deprecated") {
return;
}
},
ItemKind::ExternCrate(..) => {
if is_word(lint, "unused_imports") && skip_unused_imports {
return;
}
if is_word(lint, "unused_extern_crates") {
return;
}
},
);
_ => {},
}
}
}
},
_ => {},
let line_span = last_line_of_span(cx, attr.span);

if let Some(mut sugg) = snippet_opt(cx, line_span) {
if sugg.contains("#[") {
span_lint_and_then(
cx,
USELESS_ATTRIBUTE,
line_span,
"useless lint attribute",
|db| {
sugg = sugg.replacen("#[", "#![", 1);
db.span_suggestion(
line_span,
"if you just forgot a `!`, use",
sugg,
Applicability::MachineApplicable,
);
},
);
}
}
},
_ => {},
}
}
}
}
Expand All @@ -311,10 +315,11 @@ fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
let lint_store = cx.lints();
for lint in items {
if_chain! {
if let Some(word) = lint.word();
if let Some(tool_name) = word.is_scoped();
if let Some(meta_item) = lint.meta_item();
if meta_item.path.segments.len() > 1;
if let tool_name = meta_item.path.segments[0].ident;
if tool_name.as_str() == "clippy";
let name = word.name();
let name = meta_item.path.segments.last().unwrap().ident.name;
if let CheckLintNameResult::Tool(Err((None, _))) = lint_store.check_lint_name(
&name.as_str(),
Some(tool_name.as_str()),
Expand All @@ -323,7 +328,7 @@ fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
span_lint_and_then(
cx,
UNKNOWN_CLIPPY_LINTS,
lint.span,
lint.span(),
&format!("unknown clippy lint: clippy::{}", name),
|db| {
if name.as_str().chars().any(char::is_uppercase) {
Expand All @@ -337,7 +342,7 @@ fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
CheckLintNameResult::NoLint(None) => (),
_ => {
db.span_suggestion(
lint.span,
lint.span(),
"lowercase the lint name",
name_lower,
Applicability::MaybeIncorrect,
Expand All @@ -352,22 +357,22 @@ fn check_clippy_lint_names(cx: &LateContext<'_, '_>, items: &[NestedMetaItem]) {
}
}

fn is_relevant_item(tcx: TyCtxt<'_, '_, '_>, item: &Item) -> bool {
fn is_relevant_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item: &Item) -> bool {
if let ItemKind::Fn(_, _, _, eid) = item.node {
is_relevant_expr(tcx, tcx.body_tables(eid), &tcx.hir().body(eid).value)
} else {
true
}
}

fn is_relevant_impl(tcx: TyCtxt<'_, '_, '_>, item: &ImplItem) -> bool {
fn is_relevant_impl<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item: &ImplItem) -> bool {
match item.node {
ImplItemKind::Method(_, eid) => is_relevant_expr(tcx, tcx.body_tables(eid), &tcx.hir().body(eid).value),
_ => false,
}
}

fn is_relevant_trait(tcx: TyCtxt<'_, '_, '_>, item: &TraitItem) -> bool {
fn is_relevant_trait<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, item: &TraitItem) -> bool {
match item.node {
TraitItemKind::Method(_, TraitMethod::Required(_)) => true,
TraitItemKind::Method(_, TraitMethod::Provided(eid)) => {
Expand All @@ -377,7 +382,7 @@ fn is_relevant_trait(tcx: TyCtxt<'_, '_, '_>, item: &TraitItem) -> bool {
}
}

fn is_relevant_block(tcx: TyCtxt<'_, '_, '_>, tables: &ty::TypeckTables<'_>, block: &Block) -> bool {
fn is_relevant_block<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, tables: &ty::TypeckTables<'_>, block: &Block) -> bool {
if let Some(stmt) = block.stmts.first() {
match &stmt.node {
StmtKind::Local(_) => true,
Expand All @@ -389,7 +394,7 @@ fn is_relevant_block(tcx: TyCtxt<'_, '_, '_>, tables: &ty::TypeckTables<'_>, blo
}
}

fn is_relevant_expr(tcx: TyCtxt<'_, '_, '_>, tables: &ty::TypeckTables<'_>, expr: &Expr) -> bool {
fn is_relevant_expr<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, tables: &ty::TypeckTables<'_>, expr: &Expr) -> bool {
match &expr.node {
ExprKind::Block(block, _) => is_relevant_block(tcx, tables, block),
ExprKind::Ret(Some(e)) => is_relevant_expr(tcx, tables, e),
Expand Down Expand Up @@ -443,7 +448,7 @@ fn check_attrs(cx: &LateContext<'_, '_>, span: Span, name: Name, attrs: &[Attrib
}

if let Some(values) = attr.meta_item_list() {
if values.len() != 1 || attr.name() != "inline" {
if values.len() != 1 || !attr.check_name("inline") {
continue;
}
if is_word(&values[0], "always") {
Expand Down Expand Up @@ -476,8 +481,8 @@ fn check_semver(cx: &LateContext<'_, '_>, span: Span, lit: &Lit) {
}

fn is_word(nmi: &NestedMetaItem, expected: &str) -> bool {
if let NestedMetaItemKind::MetaItem(mi) = &nmi.node {
mi.is_word() && mi.name() == expected
if let NestedMetaItem::MetaItem(mi) = &nmi {
mi.is_word() && mi.check_name(expected)
} else {
false
}
Expand Down Expand Up @@ -514,15 +519,16 @@ impl EarlyLintPass for CfgAttrPass {
fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
if_chain! {
// check cfg_attr
if attr.name() == "cfg_attr";
if attr.check_name("cfg_attr");
if let Some(items) = attr.meta_item_list();
if items.len() == 2;
// check for `rustfmt`
if let Some(feature_item) = items[0].meta_item();
if feature_item.name() == "rustfmt";
if feature_item.check_name("rustfmt");
// check for `rustfmt_skip` and `rustfmt::skip`
if let Some(skip_item) = &items[1].meta_item();
if skip_item.name() == "rustfmt_skip" || skip_item.name() == "skip";
if skip_item.check_name("rustfmt_skip") ||
skip_item.path.segments.last().expect("empty path in attribute").ident.name == "skip";
// Only lint outer attributes, because custom inner attributes are unstable
// Tracking issue: https://github.com/rust-lang/rust/issues/54726
if let AttrStyle::Outer = attr.style;
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/consts.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl Hash for Constant {
}

impl Constant {
pub fn partial_cmp(tcx: TyCtxt<'_, '_, '_>, cmp_type: ty::Ty<'_>, left: &Self, right: &Self) -> Option<Ordering> {
pub fn partial_cmp(tcx: TyCtxt<'_, '_, '_>, cmp_type: Ty<'_>, left: &Self, right: &Self) -> Option<Ordering> {
match (left, right) {
(&Constant::Str(ref ls), &Constant::Str(ref rs)) => Some(ls.cmp(rs)),
(&Constant::Char(ref l), &Constant::Char(ref r)) => Some(l.cmp(r)),
Expand Down Expand Up @@ -268,7 +268,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
}

#[allow(clippy::cast_possible_wrap)]
fn constant_not(&self, o: &Constant, ty: ty::Ty<'_>) -> Option<Constant> {
fn constant_not(&self, o: &Constant, ty: Ty<'_>) -> Option<Constant> {
use self::Constant::*;
match *o {
Bool(b) => Some(Bool(!b)),
Expand All @@ -284,7 +284,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> {
}
}

fn constant_negate(&self, o: &Constant, ty: ty::Ty<'_>) -> Option<Constant> {
fn constant_negate(&self, o: &Constant, ty: Ty<'_>) -> Option<Constant> {
use self::Constant::*;
match *o {
Int(value) => {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/doc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ pub fn check_attrs<'a>(cx: &EarlyContext<'_>, valid_idents: &FxHashSet<String>,
spans.extend_from_slice(&current_spans);
doc.push_str(&current);
}
} else if attr.name() == "doc" {
} else if attr.check_name("doc") {
// ignore mix of sugared and non-sugared doc
return;
}
Expand Down
34 changes: 17 additions & 17 deletions clippy_lints/src/eta_reduction.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use if_chain::if_chain;
use rustc::hir::*;
use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass};
use rustc::ty;
use rustc::ty::{self, Ty};
use rustc::{declare_tool_lint, lint_array};
use rustc_errors::Applicability;

Expand Down Expand Up @@ -129,54 +129,54 @@ fn get_ufcs_type_name(
method_def_id: def_id::DefId,
self_arg: &Expr,
) -> std::option::Option<String> {
let expected_type_of_self = &cx.tcx.fn_sig(method_def_id).inputs_and_output().skip_binder()[0].sty;
let actual_type_of_self = &cx.tables.node_type(self_arg.hir_id).sty;
let expected_type_of_self = &cx.tcx.fn_sig(method_def_id).inputs_and_output().skip_binder()[0];
let actual_type_of_self = &cx.tables.node_type(self_arg.hir_id);

if let Some(trait_id) = cx.tcx.trait_of_item(method_def_id) {
if match_borrow_depth(expected_type_of_self, actual_type_of_self) {
return Some(cx.tcx.item_path_str(trait_id));
if match_borrow_depth(expected_type_of_self, &actual_type_of_self) {
return Some(cx.tcx.def_path_str(trait_id));
}
}

cx.tcx.impl_of_method(method_def_id).and_then(|_| {
//a type may implicitly implement other type's methods (e.g. Deref)
if match_types(expected_type_of_self, actual_type_of_self) {
if match_types(expected_type_of_self, &actual_type_of_self) {
return Some(get_type_name(cx, &actual_type_of_self));
}
None
})
}

fn match_borrow_depth(lhs: &ty::TyKind<'_>, rhs: &ty::TyKind<'_>) -> bool {
match (lhs, rhs) {
(ty::Ref(_, t1, _), ty::Ref(_, t2, _)) => match_borrow_depth(&t1.sty, &t2.sty),
fn match_borrow_depth(lhs: Ty<'_>, rhs: Ty<'_>) -> bool {
match (&lhs.sty, &rhs.sty) {
(ty::Ref(_, t1, _), ty::Ref(_, t2, _)) => match_borrow_depth(&t1, &t2),
(l, r) => match (l, r) {
(ty::Ref(_, _, _), _) | (_, ty::Ref(_, _, _)) => false,
(_, _) => true,
},
}
}

fn match_types(lhs: &ty::TyKind<'_>, rhs: &ty::TyKind<'_>) -> bool {
match (lhs, rhs) {
fn match_types(lhs: Ty<'_>, rhs: Ty<'_>) -> bool {
match (&lhs.sty, &rhs.sty) {
(ty::Bool, ty::Bool)
| (ty::Char, ty::Char)
| (ty::Int(_), ty::Int(_))
| (ty::Uint(_), ty::Uint(_))
| (ty::Str, ty::Str) => true,
(ty::Ref(_, t1, _), ty::Ref(_, t2, _))
| (ty::Array(t1, _), ty::Array(t2, _))
| (ty::Slice(t1), ty::Slice(t2)) => match_types(&t1.sty, &t2.sty),
| (ty::Slice(t1), ty::Slice(t2)) => match_types(t1, t2),
(ty::Adt(def1, _), ty::Adt(def2, _)) => def1 == def2,
(_, _) => false,
}
}

fn get_type_name(cx: &LateContext<'_, '_>, kind: &ty::TyKind<'_>) -> String {
match kind {
ty::Adt(t, _) => cx.tcx.item_path_str(t.did),
ty::Ref(_, r, _) => get_type_name(cx, &r.sty),
_ => kind.to_string(),
fn get_type_name(cx: &LateContext<'_, '_>, ty: Ty<'_>) -> String {
match ty.sty {
ty::Adt(t, _) => cx.tcx.def_path_str(t.did),
ty::Ref(_, r, _) => get_type_name(cx, &r),
_ => ty.to_string(),
}
}

Expand Down
Loading

0 comments on commit 92612c9

Please sign in to comment.