Auto merge of #4308 - jakubadamw:use-self-variant-scope, r=flip1995

Extend the `use_self` lint to suggest uses of `Self::Variant`

Fixes #4296.

changelog: Extend the `use_self` lint to suggest uses of `Self::Variant`
This commit is contained in:
bors 2019-07-31 14:50:01 +00:00
commit 128647d821
14 changed files with 158 additions and 105 deletions

View file

@ -15,13 +15,13 @@ pub enum CliError {
impl From<io::Error> for CliError {
fn from(error: io::Error) -> Self {
CliError::IoError(error)
Self::IoError(error)
}
}
impl From<walkdir::Error> for CliError {
fn from(error: walkdir::Error) -> Self {
CliError::WalkDirError(error)
Self::WalkDirError(error)
}
}

View file

@ -160,12 +160,12 @@ impl ConversionType {
/// Creates a conversion type if the type is allowed & conversion is valid
fn try_new(from: &str, to: &str) -> Option<Self> {
if UINTS.contains(&from) {
Some(ConversionType::FromUnsigned)
Some(Self::FromUnsigned)
} else if SINTS.contains(&from) {
if UINTS.contains(&to) {
Some(ConversionType::SignedToUnsigned)
Some(Self::SignedToUnsigned)
} else if SINTS.contains(&to) {
Some(ConversionType::SignedToSigned)
Some(Self::SignedToSigned)
} else {
None
}

View file

@ -48,27 +48,27 @@ pub enum Constant {
impl PartialEq for Constant {
fn eq(&self, other: &Self) -> bool {
match (self, other) {
(&Constant::Str(ref ls), &Constant::Str(ref rs)) => ls == rs,
(&Constant::Binary(ref l), &Constant::Binary(ref r)) => l == r,
(&Constant::Char(l), &Constant::Char(r)) => l == r,
(&Constant::Int(l), &Constant::Int(r)) => l == r,
(&Constant::F64(l), &Constant::F64(r)) => {
(&Self::Str(ref ls), &Self::Str(ref rs)) => ls == rs,
(&Self::Binary(ref l), &Self::Binary(ref r)) => l == r,
(&Self::Char(l), &Self::Char(r)) => l == r,
(&Self::Int(l), &Self::Int(r)) => l == r,
(&Self::F64(l), &Self::F64(r)) => {
// We want `Fw32 == FwAny` and `FwAny == Fw64`, and by transitivity we must have
// `Fw32 == Fw64`, so dont compare them.
// `to_bits` is required to catch non-matching 0.0, -0.0, and NaNs.
l.to_bits() == r.to_bits()
},
(&Constant::F32(l), &Constant::F32(r)) => {
(&Self::F32(l), &Self::F32(r)) => {
// We want `Fw32 == FwAny` and `FwAny == Fw64`, and by transitivity we must have
// `Fw32 == Fw64`, so dont compare them.
// `to_bits` is required to catch non-matching 0.0, -0.0, and NaNs.
f64::from(l).to_bits() == f64::from(r).to_bits()
},
(&Constant::Bool(l), &Constant::Bool(r)) => l == r,
(&Constant::Vec(ref l), &Constant::Vec(ref r)) | (&Constant::Tuple(ref l), &Constant::Tuple(ref r)) => {
(&Self::Bool(l), &Self::Bool(r)) => l == r,
(&Self::Vec(ref l), &Self::Vec(ref r)) | (&Self::Tuple(ref l), &Self::Tuple(ref r)) => {
l == r
},
(&Constant::Repeat(ref lv, ref ls), &Constant::Repeat(ref rv, ref rs)) => ls == rs && lv == rv,
(&Self::Repeat(ref lv, ref ls), &Self::Repeat(ref rv, ref rs)) => ls == rs && lv == rv,
// TODO: are there inter-type equalities?
_ => false,
}
@ -82,38 +82,38 @@ impl Hash for Constant {
{
std::mem::discriminant(self).hash(state);
match *self {
Constant::Str(ref s) => {
Self::Str(ref s) => {
s.hash(state);
},
Constant::Binary(ref b) => {
Self::Binary(ref b) => {
b.hash(state);
},
Constant::Char(c) => {
Self::Char(c) => {
c.hash(state);
},
Constant::Int(i) => {
Self::Int(i) => {
i.hash(state);
},
Constant::F32(f) => {
Self::F32(f) => {
f64::from(f).to_bits().hash(state);
},
Constant::F64(f) => {
Self::F64(f) => {
f.to_bits().hash(state);
},
Constant::Bool(b) => {
Self::Bool(b) => {
b.hash(state);
},
Constant::Vec(ref v) | Constant::Tuple(ref v) => {
Self::Vec(ref v) | Self::Tuple(ref v) => {
v.hash(state);
},
Constant::Repeat(ref c, l) => {
Self::Repeat(ref c, l) => {
c.hash(state);
l.hash(state);
},
Constant::RawPtr(u) => {
Self::RawPtr(u) => {
u.hash(state);
},
Constant::Err(ref s) => {
Self::Err(ref s) => {
s.hash(state);
},
}
@ -123,25 +123,25 @@ impl Hash for Constant {
impl Constant {
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)),
(&Constant::Int(l), &Constant::Int(r)) => {
(&Self::Str(ref ls), &Self::Str(ref rs)) => Some(ls.cmp(rs)),
(&Self::Char(ref l), &Self::Char(ref r)) => Some(l.cmp(r)),
(&Self::Int(l), &Self::Int(r)) => {
if let ty::Int(int_ty) = cmp_type.sty {
Some(sext(tcx, l, int_ty).cmp(&sext(tcx, r, int_ty)))
} else {
Some(l.cmp(&r))
}
},
(&Constant::F64(l), &Constant::F64(r)) => l.partial_cmp(&r),
(&Constant::F32(l), &Constant::F32(r)) => l.partial_cmp(&r),
(&Constant::Bool(ref l), &Constant::Bool(ref r)) => Some(l.cmp(r)),
(&Constant::Tuple(ref l), &Constant::Tuple(ref r)) | (&Constant::Vec(ref l), &Constant::Vec(ref r)) => l
(&Self::F64(l), &Self::F64(r)) => l.partial_cmp(&r),
(&Self::F32(l), &Self::F32(r)) => l.partial_cmp(&r),
(&Self::Bool(ref l), &Self::Bool(ref r)) => Some(l.cmp(r)),
(&Self::Tuple(ref l), &Self::Tuple(ref r)) | (&Self::Vec(ref l), &Self::Vec(ref r)) => l
.iter()
.zip(r.iter())
.map(|(li, ri)| Self::partial_cmp(tcx, cmp_type, li, ri))
.find(|r| r.map_or(true, |o| o != Ordering::Equal))
.unwrap_or_else(|| Some(l.len().cmp(&r.len()))),
(&Constant::Repeat(ref lv, ref ls), &Constant::Repeat(ref rv, ref rs)) => {
(&Self::Repeat(ref lv, ref ls), &Self::Repeat(ref rv, ref rs)) => {
match Self::partial_cmp(tcx, cmp_type, lv, rv) {
Some(Equal) => Some(ls.cmp(rs)),
x => x,

View file

@ -143,20 +143,20 @@ impl FloatFormat {
fn new(s: &str) -> Self {
s.chars()
.find_map(|x| match x {
'e' => Some(FloatFormat::LowerExp),
'E' => Some(FloatFormat::UpperExp),
'e' => Some(Self::LowerExp),
'E' => Some(Self::UpperExp),
_ => None,
})
.unwrap_or(FloatFormat::Normal)
.unwrap_or(Self::Normal)
}
fn format<T>(&self, f: T) -> String
where
T: fmt::UpperExp + fmt::LowerExp + fmt::Display,
{
match self {
FloatFormat::LowerExp => format!("{:e}", f),
FloatFormat::UpperExp => format!("{:E}", f),
FloatFormat::Normal => format!("{}", f),
Self::LowerExp => format!("{:e}", f),
Self::UpperExp => format!("{:E}", f),
Self::Normal => format!("{}", f),
}
}
}

View file

@ -115,8 +115,8 @@ impl Radix {
/// Returns a reasonable digit group size for this radix.
crate fn suggest_grouping(&self) -> usize {
match *self {
Radix::Binary | Radix::Hexadecimal => 4,
Radix::Octal | Radix::Decimal => 3,
Self::Binary | Self::Hexadecimal => 4,
Self::Octal | Self::Decimal => 3,
}
}
}
@ -285,7 +285,7 @@ enum WarningType {
impl WarningType {
crate fn display(&self, grouping_hint: &str, cx: &EarlyContext<'_>, span: syntax_pos::Span) {
match self {
WarningType::MistypedLiteralSuffix => span_lint_and_sugg(
Self::MistypedLiteralSuffix => span_lint_and_sugg(
cx,
MISTYPED_LITERAL_SUFFIXES,
span,
@ -294,7 +294,7 @@ impl WarningType {
grouping_hint.to_string(),
Applicability::MaybeIncorrect,
),
WarningType::UnreadableLiteral => span_lint_and_sugg(
Self::UnreadableLiteral => span_lint_and_sugg(
cx,
UNREADABLE_LITERAL,
span,
@ -303,7 +303,7 @@ impl WarningType {
grouping_hint.to_owned(),
Applicability::MachineApplicable,
),
WarningType::LargeDigitGroups => span_lint_and_sugg(
Self::LargeDigitGroups => span_lint_and_sugg(
cx,
LARGE_DIGIT_GROUPS,
span,
@ -312,7 +312,7 @@ impl WarningType {
grouping_hint.to_owned(),
Applicability::MachineApplicable,
),
WarningType::InconsistentDigitGrouping => span_lint_and_sugg(
Self::InconsistentDigitGrouping => span_lint_and_sugg(
cx,
INCONSISTENT_DIGIT_GROUPING,
span,
@ -321,7 +321,7 @@ impl WarningType {
grouping_hint.to_owned(),
Applicability::MachineApplicable,
),
WarningType::DecimalRepresentation => span_lint_and_sugg(
Self::DecimalRepresentation => span_lint_and_sugg(
cx,
DECIMAL_LITERAL_REPRESENTATION,
span,

View file

@ -2505,14 +2505,14 @@ impl SelfKind {
let is_actually_self = |ty| is_self_ty(ty) || SpanlessEq::new(cx).eq_ty(ty, self_ty);
if is_self(arg) {
match self {
SelfKind::Value => is_actually_self(ty),
SelfKind::Ref | SelfKind::RefMut => {
Self::Value => is_actually_self(ty),
Self::Ref | Self::RefMut => {
if allow_value_for_ref && is_actually_self(ty) {
return true;
}
match ty.node {
hir::TyKind::Rptr(_, ref mt_ty) => {
let mutability_match = if self == SelfKind::Ref {
let mutability_match = if self == Self::Ref {
mt_ty.mutbl == hir::MutImmutable
} else {
mt_ty.mutbl == hir::MutMutable
@ -2526,20 +2526,20 @@ impl SelfKind {
}
} else {
match self {
SelfKind::Value => false,
SelfKind::Ref => is_as_ref_or_mut_trait(ty, self_ty, generics, &paths::ASREF_TRAIT),
SelfKind::RefMut => is_as_ref_or_mut_trait(ty, self_ty, generics, &paths::ASMUT_TRAIT),
SelfKind::No => true,
Self::Value => false,
Self::Ref => is_as_ref_or_mut_trait(ty, self_ty, generics, &paths::ASREF_TRAIT),
Self::RefMut => is_as_ref_or_mut_trait(ty, self_ty, generics, &paths::ASMUT_TRAIT),
Self::No => true,
}
}
}
fn description(self) -> &'static str {
match self {
SelfKind::Value => "self by value",
SelfKind::Ref => "self by reference",
SelfKind::RefMut => "self by mutable reference",
SelfKind::No => "no self",
Self::Value => "self by value",
Self::Ref => "self by reference",
Self::RefMut => "self by mutable reference",
Self::No => "no self",
}
}
}
@ -2609,8 +2609,8 @@ fn single_segment_ty(ty: &hir::Ty) -> Option<&hir::PathSegment> {
impl Convention {
fn check(&self, other: &str) -> bool {
match *self {
Convention::Eq(this) => this == other,
Convention::StartsWith(this) => other.starts_with(this) && this != other,
Self::Eq(this) => this == other,
Self::StartsWith(this) => other.starts_with(this) && this != other,
}
}
}
@ -2618,8 +2618,8 @@ impl Convention {
impl fmt::Display for Convention {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
match *self {
Convention::Eq(this) => this.fmt(f),
Convention::StartsWith(this) => this.fmt(f).and_then(|_| '*'.fmt(f)),
Self::Eq(this) => this.fmt(f),
Self::StartsWith(this) => this.fmt(f).and_then(|_| '*'.fmt(f)),
}
}
}
@ -2636,11 +2636,11 @@ impl OutType {
fn matches(self, cx: &LateContext<'_, '_>, ty: &hir::FunctionRetTy) -> bool {
let is_unit = |ty: &hir::Ty| SpanlessEq::new(cx).eq_ty_kind(&ty.node, &hir::TyKind::Tup(vec![].into()));
match (self, ty) {
(OutType::Unit, &hir::DefaultReturn(_)) => true,
(OutType::Unit, &hir::Return(ref ty)) if is_unit(ty) => true,
(OutType::Bool, &hir::Return(ref ty)) if is_bool(ty) => true,
(OutType::Any, &hir::Return(ref ty)) if !is_unit(ty) => true,
(OutType::Ref, &hir::Return(ref ty)) => matches!(ty.node, hir::TyKind::Rptr(_, _)),
(Self::Unit, &hir::DefaultReturn(_)) => true,
(Self::Unit, &hir::Return(ref ty)) if is_unit(ty) => true,
(Self::Bool, &hir::Return(ref ty)) if is_bool(ty) => true,
(Self::Any, &hir::Return(ref ty)) if !is_unit(ty) => true,
(Self::Ref, &hir::Return(ref ty)) => matches!(ty.node, hir::TyKind::Rptr(_, _)),
_ => false,
}
}

View file

@ -84,6 +84,7 @@ declare_clippy_lint! {
"referencing const with interior mutability"
}
#[allow(dead_code)]
#[derive(Copy, Clone)]
enum Source {
Item { item: Span },
@ -94,12 +95,12 @@ enum Source {
impl Source {
fn lint(&self) -> (&'static Lint, &'static str, Span) {
match self {
Source::Item { item } | Source::Assoc { item, .. } => (
Self::Item { item } | Self::Assoc { item, .. } => (
DECLARE_INTERIOR_MUTABLE_CONST,
"a const item should never be interior mutable",
*item,
),
Source::Expr { expr } => (
Self::Expr { expr } => (
BORROW_INTERIOR_MUTABLE_CONST,
"a const item with interior mutability should not be borrowed",
*expr,

View file

@ -133,8 +133,8 @@ enum Method {
impl Method {
fn suggestion(self) -> &'static str {
match self {
Method::Offset => "add",
Method::WrappingOffset => "wrapping_add",
Self::Offset => "add",
Self::WrappingOffset => "wrapping_add",
}
}
}
@ -142,8 +142,8 @@ impl Method {
impl fmt::Display for Method {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Method::Offset => write!(f, "offset"),
Method::WrappingOffset => write!(f, "wrapping_offset"),
Self::Offset => write!(f, "offset"),
Self::WrappingOffset => write!(f, "wrapping_offset"),
}
}
}

View file

@ -1725,10 +1725,10 @@ impl PartialEq for FullInt {
impl PartialOrd for FullInt {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(match (self, other) {
(&FullInt::S(s), &FullInt::S(o)) => s.cmp(&o),
(&FullInt::U(s), &FullInt::U(o)) => s.cmp(&o),
(&FullInt::S(s), &FullInt::U(o)) => Self::cmp_s_u(s, o),
(&FullInt::U(s), &FullInt::S(o)) => Self::cmp_s_u(o, s).reverse(),
(&Self::S(s), &Self::S(o)) => s.cmp(&o),
(&Self::U(s), &Self::U(o)) => s.cmp(&o),
(&Self::S(s), &Self::U(o)) => Self::cmp_s_u(s, o),
(&Self::U(s), &Self::S(o)) => Self::cmp_s_u(o, s).reverse(),
})
}
}

View file

@ -51,9 +51,11 @@ declare_lint_pass!(UseSelf => [USE_SELF]);
const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element";
fn span_use_self_lint(cx: &LateContext<'_, '_>, path: &Path) {
fn span_use_self_lint(cx: &LateContext<'_, '_>, path: &Path, last_segment: Option<&PathSegment>) {
let last_segment = last_segment.unwrap_or_else(|| path.segments.last().expect(SEGMENTS_MSG));
// Path segments only include actual path, no methods or fields.
let last_path_span = path.segments.last().expect(SEGMENTS_MSG).ident.span;
let last_path_span = last_segment.ident.span;
// Only take path up to the end of last_path_span.
let span = path.span.with_hi(last_path_span.hi());
@ -80,22 +82,18 @@ impl<'a, 'tcx> Visitor<'tcx> for TraitImplTyVisitor<'a, 'tcx> {
let trait_ty = self.trait_type_walker.next();
let impl_ty = self.impl_type_walker.next();
if let TyKind::Path(QPath::Resolved(_, path)) = &t.node {
if_chain! {
if let TyKind::Path(QPath::Resolved(_, path)) = &t.node;
// The implementation and trait types don't match which means that
// the concrete type was specified by the implementation
if impl_ty != trait_ty {
if let Some(impl_ty) = impl_ty {
if self.item_type == impl_ty {
let is_self_ty = if let def::Res::SelfTy(..) = path.res {
true
} else {
false
};
if !is_self_ty {
span_use_self_lint(self.cx, path);
}
}
if impl_ty != trait_ty;
if let Some(impl_ty) = impl_ty;
if self.item_type == impl_ty;
then {
match path.res {
def::Res::SelfTy(..) => {},
_ => span_use_self_lint(self.cx, path, None)
}
}
}
@ -220,15 +218,35 @@ struct UseSelfVisitor<'a, 'tcx> {
impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> {
fn visit_path(&mut self, path: &'tcx Path, _id: HirId) {
if path.segments.len() >= 2 {
let last_but_one = &path.segments[path.segments.len() - 2];
if last_but_one.ident.name != kw::SelfUpper {
let enum_def_id = match path.res {
Res::Def(DefKind::Variant, variant_def_id) =>
self.cx.tcx.parent(variant_def_id),
Res::Def(DefKind::Ctor(def::CtorOf::Variant, _), ctor_def_id) => {
let variant_def_id = self.cx.tcx.parent(ctor_def_id);
variant_def_id.and_then(|def_id| self.cx.tcx.parent(def_id))
}
_ => None
};
if self.item_path.res.opt_def_id() == enum_def_id {
span_use_self_lint(self.cx, path, Some(last_but_one));
}
}
}
if path.segments.last().expect(SEGMENTS_MSG).ident.name != kw::SelfUpper {
if self.item_path.res == path.res {
span_use_self_lint(self.cx, path);
} else if let Res::Def(DefKind::Ctor(def::CtorOf::Struct, CtorKind::Fn), ctor_did) = path.res {
if self.item_path.res.opt_def_id() == self.cx.tcx.parent(ctor_did) {
span_use_self_lint(self.cx, path);
span_use_self_lint(self.cx, path, None);
} else if let Res::Def(DefKind::Ctor(def::CtorOf::Struct, CtorKind::Fn), ctor_def_id) = path.res {
if self.item_path.res.opt_def_id() == self.cx.tcx.parent(ctor_def_id) {
span_use_self_lint(self.cx, path, None);
}
}
}
walk_path(self, path);
}

View file

@ -44,15 +44,15 @@ pub enum Error {
impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
match *self {
Error::Io(ref err) => err.fmt(f),
Error::Toml(ref err) => err.fmt(f),
Self::Io(ref err) => err.fmt(f),
Self::Toml(ref err) => err.fmt(f),
}
}
}
impl From<io::Error> for Error {
fn from(e: io::Error) -> Self {
Error::Io(e)
Self::Io(e)
}
}

View file

@ -265,6 +265,8 @@ mod nesting {
enum Enum {
A,
B(u64),
C { field: bool }
}
impl Enum {
fn method() {
@ -272,6 +274,12 @@ mod nesting {
use self::Enum::*; // Issue 3425
static STATIC: Enum = Enum::A; // Can't use Self as type
}
fn method2() {
let _ = Self::B(42);
let _ = Self::C { field: true };
let _ = Self::A;
}
}
}

View file

@ -265,6 +265,8 @@ mod nesting {
enum Enum {
A,
B(u64),
C { field: bool }
}
impl Enum {
fn method() {
@ -272,6 +274,12 @@ mod nesting {
use self::Enum::*; // Issue 3425
static STATIC: Enum = Enum::A; // Can't use Self as type
}
fn method2() {
let _ = Enum::B(42);
let _ = Enum::C { field: true };
let _ = Enum::A;
}
}
}

View file

@ -175,22 +175,40 @@ LL | Bar { foo: Foo {} }
| ^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:304:13
--> $DIR/use_self.rs:279:21
|
LL | let _ = Enum::B(42);
| ^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:280:21
|
LL | let _ = Enum::C { field: true };
| ^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:281:21
|
LL | let _ = Enum::A;
| ^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:312:13
|
LL | nested::A::fun_1();
| ^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:305:13
--> $DIR/use_self.rs:313:13
|
LL | nested::A::A;
| ^^^^^^^^^ help: use the applicable keyword: `Self`
error: unnecessary structure name repetition
--> $DIR/use_self.rs:307:13
--> $DIR/use_self.rs:315:13
|
LL | nested::A {};
| ^^^^^^^^^ help: use the applicable keyword: `Self`
error: aborting due to 31 previous errors
error: aborting due to 34 previous errors