Auto merge of #7288 - camsteffen:use-self2, r=phansch
Fix use_self FPs on type params changelog: Fix [`use_self`] false positives on type parameters Fixes #4140 Fixes #7139
This commit is contained in:
bors
commit
6379d260e7
@ -1,23 +1,21 @@
|
||||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||
use clippy_utils::source::snippet_opt;
|
||||
use clippy_utils::ty::same_type_and_consts;
|
||||
use clippy_utils::{in_macro, meets_msrv, msrvs};
|
||||
use if_chain::if_chain;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{
|
||||
self as hir,
|
||||
def::{self, DefKind},
|
||||
def::{CtorOf, DefKind, Res},
|
||||
def_id::LocalDefId,
|
||||
intravisit::{walk_ty, NestedVisitorMap, Visitor},
|
||||
Expr, ExprKind, FnRetTy, FnSig, GenericArg, HirId, Impl, ImplItemKind, Item, ItemKind, Node, Path, PathSegment,
|
||||
QPath, TyKind,
|
||||
Expr, ExprKind, FnRetTy, FnSig, GenericArg, HirId, Impl, ImplItemKind, Item, ItemKind, Node, Path, QPath, TyKind,
|
||||
};
|
||||
use rustc_lint::{LateContext, LateLintPass, LintContext};
|
||||
use rustc_middle::hir::map::Map;
|
||||
use rustc_middle::ty::{AssocKind, Ty};
|
||||
use rustc_semver::RustcVersion;
|
||||
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
||||
use rustc_span::{BytePos, Span};
|
||||
use rustc_span::Span;
|
||||
use rustc_typeck::hir_ty_to_ty;
|
||||
|
||||
declare_clippy_lint! {
|
||||
@ -234,113 +232,60 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
|
||||
}
|
||||
|
||||
fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>) {
|
||||
if in_macro(hir_ty.span)
|
||||
|| in_impl(cx, hir_ty)
|
||||
|| !meets_msrv(self.msrv.as_ref(), &msrvs::TYPE_ALIAS_ENUM_VARIANTS)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
let lint_dependend_on_expr_kind = if let Some(StackItem::Check {
|
||||
if_chain! {
|
||||
if !in_macro(hir_ty.span) && !in_impl(cx, hir_ty);
|
||||
if meets_msrv(self.msrv.as_ref(), &msrvs::TYPE_ALIAS_ENUM_VARIANTS);
|
||||
if let Some(StackItem::Check {
|
||||
hir_id,
|
||||
types_to_lint,
|
||||
types_to_skip,
|
||||
..
|
||||
}) = self.stack.last()
|
||||
{
|
||||
if types_to_skip.contains(&hir_ty.hir_id) {
|
||||
false
|
||||
} else if types_to_lint.contains(&hir_ty.hir_id) {
|
||||
true
|
||||
} else {
|
||||
}) = self.stack.last();
|
||||
if !types_to_skip.contains(&hir_ty.hir_id);
|
||||
if types_to_lint.contains(&hir_ty.hir_id)
|
||||
|| {
|
||||
let self_ty = ty_from_hir_id(cx, *hir_id);
|
||||
should_lint_ty(hir_ty, hir_ty_to_ty(cx.tcx, hir_ty), self_ty)
|
||||
}
|
||||
} else {
|
||||
false
|
||||
};
|
||||
|
||||
if lint_dependend_on_expr_kind {
|
||||
// FIXME: this span manipulation should not be necessary
|
||||
// @flip1995 found an ast lowering issue in
|
||||
// https://github.com/rust-lang/rust/blob/master/src/librustc_ast_lowering/path.rs#l142-l162
|
||||
let hir = cx.tcx.hir();
|
||||
let id = hir.get_parent_node(hir_ty.hir_id);
|
||||
|
||||
if !hir.opt_span(id).map_or(false, in_macro) {
|
||||
match hir.find(id) {
|
||||
Some(Node::Expr(Expr {
|
||||
kind: ExprKind::Path(QPath::TypeRelative(_, segment)),
|
||||
..
|
||||
})) => span_lint_until_last_segment(cx, hir_ty.span, segment),
|
||||
_ => span_lint(cx, hir_ty.span),
|
||||
}
|
||||
if !hir.opt_span(id).map_or(false, in_macro);
|
||||
then {
|
||||
span_lint(cx, hir_ty.span);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
|
||||
fn expr_ty_matches(cx: &LateContext<'_>, expr: &Expr<'_>, self_ty: Ty<'_>) -> bool {
|
||||
let def_id = expr.hir_id.owner;
|
||||
if cx.tcx.has_typeck_results(def_id) {
|
||||
cx.tcx.typeck(def_id).expr_ty_opt(expr) == Some(self_ty)
|
||||
} else {
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
if in_macro(expr.span) || !meets_msrv(self.msrv.as_ref(), &msrvs::TYPE_ALIAS_ENUM_VARIANTS) {
|
||||
return;
|
||||
}
|
||||
|
||||
if let Some(StackItem::Check { hir_id, .. }) = self.stack.last() {
|
||||
let self_ty = ty_from_hir_id(cx, *hir_id);
|
||||
|
||||
match &expr.kind {
|
||||
ExprKind::Struct(QPath::Resolved(_, path), ..) => {
|
||||
if expr_ty_matches(cx, expr, self_ty) {
|
||||
match path.res {
|
||||
def::Res::SelfTy(..) => (),
|
||||
def::Res::Def(DefKind::Variant, _) => span_lint_on_path_until_last_segment(cx, path),
|
||||
_ => {
|
||||
span_lint(cx, path.span);
|
||||
},
|
||||
}
|
||||
if_chain! {
|
||||
if !in_macro(expr.span);
|
||||
if meets_msrv(self.msrv.as_ref(), &msrvs::TYPE_ALIAS_ENUM_VARIANTS);
|
||||
if let Some(StackItem::Check { hir_id, .. }) = self.stack.last();
|
||||
if cx.typeck_results().expr_ty(expr) == ty_from_hir_id(cx, *hir_id);
|
||||
then {} else { return; }
|
||||
}
|
||||
match expr.kind {
|
||||
ExprKind::Struct(QPath::Resolved(_, path), ..) => match path.res {
|
||||
Res::SelfTy(..) => (),
|
||||
Res::Def(DefKind::Variant, _) => lint_path_to_variant(cx, path),
|
||||
_ => span_lint(cx, path.span),
|
||||
},
|
||||
// tuple struct instantiation (`Foo(arg)` or `Enum::Foo(arg)`)
|
||||
ExprKind::Call(fun, _) => {
|
||||
if let Expr {
|
||||
kind: ExprKind::Path(ref qpath),
|
||||
..
|
||||
} = fun
|
||||
{
|
||||
if expr_ty_matches(cx, expr, self_ty) {
|
||||
let res = cx.qpath_res(qpath, fun.hir_id);
|
||||
|
||||
if let def::Res::Def(DefKind::Ctor(ctor_of, _), ..) = res {
|
||||
if let ExprKind::Path(QPath::Resolved(_, path)) = fun.kind {
|
||||
if let Res::Def(DefKind::Ctor(ctor_of, _), ..) = path.res {
|
||||
match ctor_of {
|
||||
def::CtorOf::Variant => {
|
||||
span_lint_on_qpath_resolved(cx, qpath, true);
|
||||
},
|
||||
def::CtorOf::Struct => {
|
||||
span_lint_on_qpath_resolved(cx, qpath, false);
|
||||
},
|
||||
}
|
||||
CtorOf::Variant => lint_path_to_variant(cx, path),
|
||||
CtorOf::Struct => span_lint(cx, path.span),
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
// unit enum variants (`Enum::A`)
|
||||
ExprKind::Path(qpath) => {
|
||||
if expr_ty_matches(cx, expr, self_ty) {
|
||||
span_lint_on_qpath_resolved(cx, qpath, true);
|
||||
}
|
||||
},
|
||||
ExprKind::Path(QPath::Resolved(_, path)) => lint_path_to_variant(cx, path),
|
||||
_ => (),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
extract_msrv_attr!(LateContext);
|
||||
}
|
||||
@ -405,33 +350,12 @@ fn span_lint(cx: &LateContext<'_>, span: Span) {
|
||||
);
|
||||
}
|
||||
|
||||
#[allow(clippy::cast_possible_truncation)]
|
||||
fn span_lint_until_last_segment(cx: &LateContext<'_>, span: Span, segment: &PathSegment<'_>) {
|
||||
let sp = span.with_hi(segment.ident.span.lo());
|
||||
// remove the trailing ::
|
||||
let span_without_last_segment = match snippet_opt(cx, sp) {
|
||||
Some(snippet) => match snippet.rfind("::") {
|
||||
Some(bidx) => sp.with_hi(sp.lo() + BytePos(bidx as u32)),
|
||||
None => sp,
|
||||
},
|
||||
None => sp,
|
||||
};
|
||||
span_lint(cx, span_without_last_segment);
|
||||
}
|
||||
|
||||
fn span_lint_on_path_until_last_segment(cx: &LateContext<'_>, path: &Path<'_>) {
|
||||
if path.segments.len() > 1 {
|
||||
span_lint_until_last_segment(cx, path.span, path.segments.last().unwrap());
|
||||
}
|
||||
}
|
||||
|
||||
fn span_lint_on_qpath_resolved(cx: &LateContext<'_>, qpath: &QPath<'_>, until_last_segment: bool) {
|
||||
if let QPath::Resolved(_, path) = qpath {
|
||||
if until_last_segment {
|
||||
span_lint_on_path_until_last_segment(cx, path);
|
||||
} else {
|
||||
span_lint(cx, path.span);
|
||||
}
|
||||
fn lint_path_to_variant(cx: &LateContext<'_>, path: &Path<'_>) {
|
||||
if let [.., self_seg, _variant] = path.segments {
|
||||
let span = path
|
||||
.span
|
||||
.with_hi(self_seg.args().span_ext().unwrap_or(self_seg.ident.span).hi());
|
||||
span_lint(cx, span);
|
||||
}
|
||||
}
|
||||
|
||||
@ -462,7 +386,7 @@ fn should_lint_ty(hir_ty: &hir::Ty<'_>, ty: Ty<'_>, self_ty: Ty<'_>) -> bool {
|
||||
if same_type_and_consts(ty, self_ty);
|
||||
if let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind;
|
||||
then {
|
||||
!matches!(path.res, def::Res::SelfTy(..))
|
||||
!matches!(path.res, Res::SelfTy(..) | Res::Def(DefKind::TyParam, _))
|
||||
} else {
|
||||
false
|
||||
}
|
||||
|
||||
@ -492,3 +492,26 @@ mod issue7206 {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
mod self_is_ty_param {
|
||||
trait Trait {
|
||||
type Type;
|
||||
type Hi;
|
||||
|
||||
fn test();
|
||||
}
|
||||
|
||||
impl<I> Trait for I
|
||||
where
|
||||
I: Iterator,
|
||||
I::Item: Trait, // changing this to Self would require <Self as Iterator>
|
||||
{
|
||||
type Type = I;
|
||||
type Hi = I::Item;
|
||||
|
||||
fn test() {
|
||||
let _: I::Item;
|
||||
let _: I; // this could lint, but is questionable
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -279,7 +279,7 @@ mod generics {
|
||||
impl<T> Foo<T> {
|
||||
// `Self` is applicable here
|
||||
fn foo(value: T) -> Foo<T> {
|
||||
Foo { value }
|
||||
Foo::<T> { value }
|
||||
}
|
||||
|
||||
// `Cannot` use `Self` as a return type as the generic types are different
|
||||
@ -492,3 +492,26 @@ mod issue7206 {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
mod self_is_ty_param {
|
||||
trait Trait {
|
||||
type Type;
|
||||
type Hi;
|
||||
|
||||
fn test();
|
||||
}
|
||||
|
||||
impl<I> Trait for I
|
||||
where
|
||||
I: Iterator,
|
||||
I::Item: Trait, // changing this to Self would require <Self as Iterator>
|
||||
{
|
||||
type Type = I;
|
||||
type Hi = I::Item;
|
||||
|
||||
fn test() {
|
||||
let _: I::Item;
|
||||
let _: I; // this could lint, but is questionable
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -153,8 +153,8 @@ LL | fn foo(value: T) -> Foo<T> {
|
||||
error: unnecessary structure name repetition
|
||||
--> $DIR/use_self.rs:282:13
|
||||
|
|
||||
LL | Foo { value }
|
||||
| ^^^ help: use the applicable keyword: `Self`
|
||||
LL | Foo::<T> { value }
|
||||
| ^^^^^^^^ help: use the applicable keyword: `Self`
|
||||
|
||||
error: unnecessary structure name repetition
|
||||
--> $DIR/use_self.rs:454:13
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user