From 56068b1b671f6490c1be9ea3834784e89a0a4ba7 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 21 Aug 2017 12:57:33 +0200 Subject: [PATCH 1/3] Fix ICE #1969 --- clippy_lints/src/functions.rs | 8 ++++++-- src/main.rs | 2 +- tests/run-pass/ice-1969.rs | 13 +++++++++++++ 3 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 tests/run-pass/ice-1969.rs diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index 1bb8780ea4af..9d9bf38c3974 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -1,6 +1,7 @@ use rustc::hir::intravisit; use rustc::hir; use rustc::lint::*; +use rustc::ty; use std::collections::HashSet; use syntax::ast; use syntax::abi::Abi; @@ -150,9 +151,11 @@ impl<'a, 'tcx> Functions { .collect::>(); if !raw_ptrs.is_empty() { + let tables = cx.tcx.body_tables(body.id()); let mut v = DerefVisitor { cx: cx, ptrs: raw_ptrs, + tables, }; hir::intravisit::walk_expr(&mut v, expr); @@ -172,13 +175,14 @@ fn raw_ptr_arg(arg: &hir::Arg, ty: &hir::Ty) -> Option { struct DerefVisitor<'a, 'tcx: 'a> { cx: &'a LateContext<'a, 'tcx>, ptrs: HashSet, + tables: &'a ty::TypeckTables<'tcx>, } impl<'a, 'tcx> hir::intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> { fn visit_expr(&mut self, expr: &'tcx hir::Expr) { match expr.node { hir::ExprCall(ref f, ref args) => { - let ty = self.cx.tables.expr_ty(f); + let ty = self.tables.expr_ty(f); if type_is_unsafe_function(self.cx, ty) { for arg in args { @@ -187,7 +191,7 @@ impl<'a, 'tcx> hir::intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> { } }, hir::ExprMethodCall(_, _, ref args) => { - let def_id = self.cx.tables.type_dependent_defs()[expr.hir_id].def_id(); + let def_id = self.tables.type_dependent_defs()[expr.hir_id].def_id(); let base_type = self.cx.tcx.type_of(def_id); if type_is_unsafe_function(self.cx, base_type) { diff --git a/src/main.rs b/src/main.rs index 8e92eb55ff09..8e8ed032c1fe 100644 --- a/src/main.rs +++ b/src/main.rs @@ -30,7 +30,7 @@ struct ClippyCompilerCalls { impl ClippyCompilerCalls { fn new(run_lints: bool) -> Self { - ClippyCompilerCalls { + Self { default: RustcDefaultCalls, run_lints: run_lints, } diff --git a/tests/run-pass/ice-1969.rs b/tests/run-pass/ice-1969.rs new file mode 100644 index 000000000000..23a002a5cdee --- /dev/null +++ b/tests/run-pass/ice-1969.rs @@ -0,0 +1,13 @@ +#![feature(plugin)] +#![plugin(clippy)] +#![allow(clippy)] + +fn main() { } + +pub trait Convert { + type Action: From<*const f64>; + + fn convert(val: *const f64) -> Self::Action { + val.into() + } +} From 3eab44acb1a5f8a4a8964d3d5f0566c4672e4b05 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 21 Aug 2017 12:58:06 +0200 Subject: [PATCH 2/3] Don't trigger `Self` suggestion inside derives --- clippy_lints/src/use_self.rs | 5 ++++- tests/ui/methods.stderr | 28 +--------------------------- 2 files changed, 5 insertions(+), 28 deletions(-) diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs index 2a94d8fe9cc5..b1e46e17f13e 100644 --- a/clippy_lints/src/use_self.rs +++ b/clippy_lints/src/use_self.rs @@ -1,7 +1,7 @@ use rustc::lint::{LintArray, LateLintPass, LateContext, LintPass}; use rustc::hir::*; use rustc::hir::intravisit::{Visitor, walk_path, NestedVisitorMap}; -use utils::span_lint_and_then; +use utils::{span_lint_and_then, in_macro}; use syntax::ast::NodeId; use syntax_pos::symbol::keywords::SelfType; @@ -48,6 +48,9 @@ impl LintPass for UseSelf { impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseSelf { fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) { + if in_macro(item.span) { + return; + } if_let_chain!([ let ItemImpl(.., ref item_type, ref refs) = item.node, let Ty_::TyPath(QPath::Resolved(_, ref item_path)) = item_type.node, diff --git a/tests/ui/methods.stderr b/tests/ui/methods.stderr index 99d4ee9d6634..65975c5177c4 100644 --- a/tests/ui/methods.stderr +++ b/tests/ui/methods.stderr @@ -96,12 +96,6 @@ error: unnecessary structure name repetition 58 | pub fn new() -> Lt3<'static> { unimplemented!() } | ^^^^^^^^^^^^ help: use the applicable keyword: `Self` -error: unnecessary structure name repetition - --> $DIR/methods.rs:61:10 - | -61 | #[derive(Clone,Copy)] - | ^^^^^ help: use the applicable keyword: `Self` - error: unnecessary structure name repetition --> $DIR/methods.rs:74:24 | @@ -190,18 +184,6 @@ error: called `map(f).unwrap_or_else(g)` on an Option value. This can be done mo 125 | | ); | |_________________^ -error: unnecessary structure name repetition - --> $DIR/methods.rs:131:16 - | -131 | #[derive(Copy, Clone)] - | ^^^^^ help: use the applicable keyword: `Self` - -error: unnecessary structure name repetition - --> $DIR/methods.rs:145:16 - | -145 | #[derive(Copy, Clone)] - | ^^^^^ help: use the applicable keyword: `Self` - error: unnecessary structure name repetition --> $DIR/methods.rs:151:24 | @@ -220,12 +202,6 @@ error: unnecessary structure name repetition 175 | fn skip(self, _: usize) -> IteratorFalsePositives { | ^^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self` -error: unnecessary structure name repetition - --> $DIR/methods.rs:180:16 - | -180 | #[derive(Copy, Clone)] - | ^^^^^ help: use the applicable keyword: `Self` - error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead. --> $DIR/methods.rs:194:13 | @@ -560,8 +536,6 @@ error: called `ok().expect()` on a Result value. You can call `expect` directly 413 | res6.ok().expect("meh"); | ^^^^^^^^^^^^^^^^^^^^^^^ -error: unnecessary structure name repetition - error: you should use the `starts_with` method --> $DIR/methods.rs:425:5 | @@ -756,5 +730,5 @@ error: called `cloned().collect()` on a slice to create a `Vec`. Calling `to_vec | = note: `-D iter-cloned-collect` implied by `-D warnings` -error: aborting due to 111 previous errors +error: aborting due to 106 previous errors From 2430e06a60ca23960bb746af30742607ae41bdb0 Mon Sep 17 00:00:00 2001 From: Oliver Schneider Date: Mon, 21 Aug 2017 13:32:12 +0200 Subject: [PATCH 3/3] Run Dogfood for `use_self` --- CHANGELOG.md | 3 +++ clippy_lints/src/blacklisted_name.rs | 4 ++-- clippy_lints/src/consts.rs | 8 ++++---- clippy_lints/src/cyclomatic_complexity.rs | 2 +- clippy_lints/src/doc.rs | 4 ++-- clippy_lints/src/enum_variants.rs | 4 ++-- clippy_lints/src/functions.rs | 4 ++-- clippy_lints/src/large_enum_variant.rs | 2 +- clippy_lints/src/lifetimes.rs | 4 ++-- clippy_lints/src/literal_digit_grouping.rs | 14 +++++++------- clippy_lints/src/missing_doc.rs | 8 ++++---- clippy_lints/src/needless_pass_by_value.rs | 2 +- clippy_lints/src/types.rs | 2 +- clippy_lints/src/utils/author.rs | 2 +- clippy_lints/src/utils/hir_utils.rs | 6 +++--- clippy_lints/src/utils/mod.rs | 4 ++-- clippy_lints/src/utils/sugg.rs | 10 ++++++---- 17 files changed, 44 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bb1c606d7368..6bf262bb72f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ # Change Log All notable changes to this project will be documented in this file. +## 0.0.154 +* Fix [`use_self`] triggering inside derives + ## 0.0.153 * Update to *rustc 1.21.0-nightly (8c303ed87 2017-08-20)* * New lint: [`use_self`] diff --git a/clippy_lints/src/blacklisted_name.rs b/clippy_lints/src/blacklisted_name.rs index e88e4108d3d9..e46d8e4855f6 100644 --- a/clippy_lints/src/blacklisted_name.rs +++ b/clippy_lints/src/blacklisted_name.rs @@ -26,8 +26,8 @@ pub struct BlackListedName { } impl BlackListedName { - pub fn new(blacklist: Vec) -> BlackListedName { - BlackListedName { blacklist: blacklist } + pub fn new(blacklist: Vec) -> Self { + Self { blacklist: blacklist } } } diff --git a/clippy_lints/src/consts.rs b/clippy_lints/src/consts.rs index 2a6fa051d2ff..0a76b95931de 100644 --- a/clippy_lints/src/consts.rs +++ b/clippy_lints/src/consts.rs @@ -23,7 +23,7 @@ pub enum FloatWidth { } impl From for FloatWidth { - fn from(ty: FloatTy) -> FloatWidth { + fn from(ty: FloatTy) -> Self { match ty { FloatTy::F32 => FloatWidth::F32, FloatTy::F64 => FloatWidth::F64, @@ -55,7 +55,7 @@ pub enum Constant { } impl PartialEq for Constant { - fn eq(&self, other: &Constant) -> bool { + fn eq(&self, other: &Self) -> bool { match (self, other) { (&Constant::Str(ref ls, ref l_sty), &Constant::Str(ref rs, ref r_sty)) => ls == rs && l_sty == r_sty, (&Constant::Binary(ref l), &Constant::Binary(ref r)) => l == r, @@ -123,7 +123,7 @@ impl Hash for Constant { } impl PartialOrd for Constant { - fn partial_cmp(&self, other: &Constant) -> Option { + fn partial_cmp(&self, other: &Self) -> Option { match (self, other) { (&Constant::Str(ref ls, ref l_sty), &Constant::Str(ref rs, ref r_sty)) => { if l_sty == r_sty { @@ -297,7 +297,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { }; let param_env = self.param_env.and((def_id, substs)); if let Some((def_id, substs)) = lookup_const_by_id(self.tcx, param_env) { - let mut cx = ConstEvalLateContext { + let mut cx = Self { tcx: self.tcx, tables: self.tcx.typeck_tables_of(def_id), needed_resolution: false, diff --git a/clippy_lints/src/cyclomatic_complexity.rs b/clippy_lints/src/cyclomatic_complexity.rs index c7593a195ff5..9596e9812adb 100644 --- a/clippy_lints/src/cyclomatic_complexity.rs +++ b/clippy_lints/src/cyclomatic_complexity.rs @@ -31,7 +31,7 @@ pub struct CyclomaticComplexity { impl CyclomaticComplexity { pub fn new(limit: u64) -> Self { - CyclomaticComplexity { limit: LimitStack::new(limit) } + Self { limit: LimitStack::new(limit) } } } diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index 3ca71694bbd3..c3eba53e035d 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -37,7 +37,7 @@ pub struct Doc { impl Doc { pub fn new(valid_idents: Vec) -> Self { - Doc { valid_idents: valid_idents } + Self { valid_idents: valid_idents } } } @@ -62,7 +62,7 @@ struct Parser<'a> { } impl<'a> Parser<'a> { - fn new(parser: pulldown_cmark::Parser<'a>) -> Parser<'a> { + fn new(parser: pulldown_cmark::Parser<'a>) -> Self { Self { parser: parser } } } diff --git a/clippy_lints/src/enum_variants.rs b/clippy_lints/src/enum_variants.rs index 1227de69db22..eee4e2a7ee16 100644 --- a/clippy_lints/src/enum_variants.rs +++ b/clippy_lints/src/enum_variants.rs @@ -104,8 +104,8 @@ pub struct EnumVariantNames { } impl EnumVariantNames { - pub fn new(threshold: u64) -> EnumVariantNames { - EnumVariantNames { + pub fn new(threshold: u64) -> Self { + Self { modules: Vec::new(), threshold: threshold, } diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index 9d9bf38c3974..70ff96d36d43 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -59,8 +59,8 @@ pub struct Functions { } impl Functions { - pub fn new(threshold: u64) -> Functions { - Functions { threshold: threshold } + pub fn new(threshold: u64) -> Self { + Self { threshold: threshold } } } diff --git a/clippy_lints/src/large_enum_variant.rs b/clippy_lints/src/large_enum_variant.rs index 73a62c97bc84..917e0b773707 100644 --- a/clippy_lints/src/large_enum_variant.rs +++ b/clippy_lints/src/large_enum_variant.rs @@ -34,7 +34,7 @@ pub struct LargeEnumVariant { impl LargeEnumVariant { pub fn new(maximum_size_difference_allowed: u64) -> Self { - LargeEnumVariant { maximum_size_difference_allowed: maximum_size_difference_allowed } + Self { maximum_size_difference_allowed: maximum_size_difference_allowed } } } diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index 507373fede56..47cc41c472cb 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -254,8 +254,8 @@ struct RefVisitor<'a, 'tcx: 'a> { } impl<'v, 't> RefVisitor<'v, 't> { - fn new(cx: &'v LateContext<'v, 't>) -> RefVisitor<'v, 't> { - RefVisitor { + fn new(cx: &'v LateContext<'v, 't>) -> Self { + Self { cx: cx, lts: Vec::new(), abort: false, diff --git a/clippy_lints/src/literal_digit_grouping.rs b/clippy_lints/src/literal_digit_grouping.rs index 0da8ffd0a302..dbb1a7a9b867 100644 --- a/clippy_lints/src/literal_digit_grouping.rs +++ b/clippy_lints/src/literal_digit_grouping.rs @@ -95,7 +95,7 @@ struct DigitInfo<'a> { } impl<'a> DigitInfo<'a> { - pub fn new(lit: &str, float: bool) -> DigitInfo { + pub fn new(lit: &'a str, float: bool) -> Self { // Determine delimiter for radix prefix, if present, and radix. let radix = if lit.starts_with("0x") { Radix::Hexadecimal @@ -120,7 +120,7 @@ impl<'a> DigitInfo<'a> { if !float && (d == 'i' || d == 'u') || float && d == 'f' { let suffix_start = if last_d == '_' { d_idx - 1 } else { d_idx }; let (digits, suffix) = sans_prefix.split_at(suffix_start); - return DigitInfo { + return Self { digits: digits, radix: radix, prefix: prefix, @@ -132,7 +132,7 @@ impl<'a> DigitInfo<'a> { } // No suffix found - DigitInfo { + Self { digits: sans_prefix, radix: radix, prefix: prefix, @@ -257,7 +257,7 @@ impl LiteralDigitGrouping { char::to_digit(firstch, 10).is_some() ], { let digit_info = DigitInfo::new(&src, false); - let _ = LiteralDigitGrouping::do_lint(digit_info.digits).map_err(|warning_type| { + let _ = Self::do_lint(digit_info.digits).map_err(|warning_type| { warning_type.display(&digit_info.grouping_hint(), cx, &lit.span) }); }} @@ -278,14 +278,14 @@ impl LiteralDigitGrouping { // Lint integral and fractional parts separately, and then check consistency of digit // groups if both pass. - let _ = LiteralDigitGrouping::do_lint(parts[0]) + let _ = Self::do_lint(parts[0]) .map(|integral_group_size| { if parts.len() > 1 { // Lint the fractional part of literal just like integral part, but reversed. let fractional_part = &parts[1].chars().rev().collect::(); - let _ = LiteralDigitGrouping::do_lint(fractional_part) + let _ = Self::do_lint(fractional_part) .map(|fractional_group_size| { - let consistent = LiteralDigitGrouping::parts_consistent(integral_group_size, fractional_group_size, parts[0].len(), parts[1].len()); + let consistent = Self::parts_consistent(integral_group_size, fractional_group_size, parts[0].len(), parts[1].len()); if !consistent { WarningType::InconsistentDigitGrouping.display(&digit_info.grouping_hint(), cx, &lit.span); } diff --git a/clippy_lints/src/missing_doc.rs b/clippy_lints/src/missing_doc.rs index f0c417f46460..21e19a3af849 100644 --- a/clippy_lints/src/missing_doc.rs +++ b/clippy_lints/src/missing_doc.rs @@ -56,14 +56,14 @@ pub struct MissingDoc { } impl ::std::default::Default for MissingDoc { - fn default() -> MissingDoc { - MissingDoc::new() + fn default() -> Self { + Self::new() } } impl MissingDoc { - pub fn new() -> MissingDoc { - MissingDoc { doc_hidden_stack: vec![false] } + pub fn new() -> Self { + Self { doc_hidden_stack: vec![false] } } fn doc_hidden(&self) -> bool { diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index e4efaf495a89..b7de586ed80c 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/clippy_lints/src/needless_pass_by_value.rs @@ -197,7 +197,7 @@ struct MovedVariablesCtxt<'a, 'tcx: 'a> { impl<'a, 'tcx> MovedVariablesCtxt<'a, 'tcx> { fn new(cx: &'a LateContext<'a, 'tcx>) -> Self { - MovedVariablesCtxt { + Self { cx: cx, moved_vars: HashSet::new(), spans_need_deref: HashMap::new(), diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 9d6167cc0414..ea3db0e46900 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -730,7 +730,7 @@ pub struct TypeComplexityPass { impl TypeComplexityPass { pub fn new(threshold: u64) -> Self { - TypeComplexityPass { threshold: threshold } + Self { threshold: threshold } } } diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index 7466ee9080b2..a7fdc3d281ba 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -147,7 +147,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { impl PrintVisitor { fn new(s: &'static str) -> Self { - PrintVisitor { + Self { ids: HashMap::new(), current: s.to_owned(), } diff --git a/clippy_lints/src/utils/hir_utils.rs b/clippy_lints/src/utils/hir_utils.rs index 658a07ca1468..b4caad0845a9 100644 --- a/clippy_lints/src/utils/hir_utils.rs +++ b/clippy_lints/src/utils/hir_utils.rs @@ -23,14 +23,14 @@ pub struct SpanlessEq<'a, 'tcx: 'a> { impl<'a, 'tcx: 'a> SpanlessEq<'a, 'tcx> { pub fn new(cx: &'a LateContext<'a, 'tcx>) -> Self { - SpanlessEq { + Self { cx: cx, ignore_fn: false, } } pub fn ignore_fn(self) -> Self { - SpanlessEq { + Self { cx: self.cx, ignore_fn: true, } @@ -283,7 +283,7 @@ pub struct SpanlessHash<'a, 'tcx: 'a> { impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> { pub fn new(cx: &'a LateContext<'a, 'tcx>) -> Self { - SpanlessHash { + Self { cx: cx, s: DefaultHasher::new(), } diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 01788edfc7e9..7f8603f0f274 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -670,8 +670,8 @@ impl Drop for LimitStack { } impl LimitStack { - pub fn new(limit: u64) -> LimitStack { - LimitStack { stack: vec![limit] } + pub fn new(limit: u64) -> Self { + Self { stack: vec![limit] } } pub fn limit(&self) -> u64 { *self.stack.last().expect( diff --git a/clippy_lints/src/utils/sugg.rs b/clippy_lints/src/utils/sugg.rs index d46a65263958..6cfbe8c935e0 100644 --- a/clippy_lints/src/utils/sugg.rs +++ b/clippy_lints/src/utils/sugg.rs @@ -1,5 +1,7 @@ //! Contains utility functions to generate suggestions. #![deny(missing_docs_in_private_items)] +// currently ignores lifetimes and generics +#![allow(use_self)] use rustc::hir; use rustc::lint::{EarlyContext, LateContext, LintContext}; @@ -41,7 +43,7 @@ impl<'a> Display for Sugg<'a> { #[allow(wrong_self_convention)] // ok, because of the function `as_ty` method impl<'a> Sugg<'a> { /// Prepare a suggestion from an expression. - pub fn hir_opt(cx: &LateContext, expr: &hir::Expr) -> Option> { + pub fn hir_opt(cx: &LateContext, expr: &hir::Expr) -> Option { snippet_opt(cx, expr.span).map(|snippet| { let snippet = Cow::Owned(snippet); match expr.node { @@ -80,12 +82,12 @@ impl<'a> Sugg<'a> { /// Convenience function around `hir_opt` for suggestions with a default /// text. - pub fn hir(cx: &LateContext, expr: &hir::Expr, default: &'a str) -> Sugg<'a> { + pub fn hir(cx: &LateContext, expr: &hir::Expr, default: &'a str) -> Self { Self::hir_opt(cx, expr).unwrap_or_else(|| Sugg::NonParen(Cow::Borrowed(default))) } /// Prepare a suggestion from an expression. - pub fn ast(cx: &EarlyContext, expr: &ast::Expr, default: &'a str) -> Sugg<'a> { + pub fn ast(cx: &EarlyContext, expr: &ast::Expr, default: &'a str) -> Self { use syntax::ast::RangeLimits; let snippet = snippet(cx, expr.span, default); @@ -218,7 +220,7 @@ struct ParenHelper { impl ParenHelper { /// Build a `ParenHelper`. fn new(paren: bool, wrapped: T) -> Self { - ParenHelper { + Self { paren: paren, wrapped: wrapped, }