Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustup: update for the new Ty::walk interface. #5434

Merged
merged 4 commits into from
Apr 7, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions clippy_lints/src/let_underscore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use if_chain::if_chain;
use rustc_hir::{Local, PatKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_session::{declare_lint_pass, declare_tool_lint};

use crate::utils::{is_must_use_func_call, is_must_use_ty, match_type, paths, span_lint_and_help};
Expand Down Expand Up @@ -75,8 +76,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for LetUnderscore {
if let PatKind::Wild = local.pat.kind;
if let Some(ref init) = local.init;
then {
let check_ty = |ty| SYNC_GUARD_PATHS.iter().any(|path| match_type(cx, ty, path));
if cx.tables.expr_ty(init).walk().any(check_ty) {
let init_ty = cx.tables.expr_ty(init);
let contains_sync_guard = init_ty.walk().any(|inner| match inner.unpack() {
GenericArgKind::Type(inner_ty) => SYNC_GUARD_PATHS.iter().any(|path| match_type(cx, inner_ty, path)),
flip1995 marked this conversation as resolved.
Show resolved Hide resolved

GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
});
if contains_sync_guard {
span_lint_and_help(
cx,
LET_UNDERSCORE_LOCK,
Expand Down
25 changes: 16 additions & 9 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use rustc_hir::intravisit::{self, Visitor};
use rustc_lint::{LateContext, LateLintPass, Lint, LintContext};
use rustc_middle::hir::map::Map;
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_middle::ty::{self, Predicate, Ty};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
Expand Down Expand Up @@ -1407,7 +1408,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
let parent = cx.tcx.hir().get_parent_item(impl_item.hir_id);
let item = cx.tcx.hir().expect_item(parent);
let def_id = cx.tcx.hir().local_def_id(item.hir_id);
let ty = cx.tcx.type_of(def_id);
let self_ty = cx.tcx.type_of(def_id);
if_chain! {
if let hir::ImplItemKind::Fn(ref sig, id) = impl_item.kind;
if let Some(first_arg) = iter_input_pats(&sig.decl, cx.tcx.hir().body(id)).next();
Expand All @@ -1429,7 +1430,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
if name == method_name &&
sig.decl.inputs.len() == n_args &&
out_type.matches(cx, &sig.decl.output) &&
self_kind.matches(cx, ty, first_arg_ty) {
self_kind.matches(cx, self_ty, first_arg_ty) {
span_lint(cx, SHOULD_IMPLEMENT_TRAIT, impl_item.span, &format!(
"defining a method called `{}` on this type; consider implementing \
the `{}` trait or choosing a less ambiguous name", name, trait_name));
Expand All @@ -1441,7 +1442,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
.iter()
.find(|(ref conv, _)| conv.check(&name))
{
if !self_kinds.iter().any(|k| k.matches(cx, ty, first_arg_ty)) {
if !self_kinds.iter().any(|k| k.matches(cx, self_ty, first_arg_ty)) {
let lint = if item.vis.node.is_pub() {
WRONG_PUB_SELF_CONVENTION
} else {
Expand Down Expand Up @@ -1471,8 +1472,16 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
if let hir::ImplItemKind::Fn(_, _) = impl_item.kind {
let ret_ty = return_ty(cx, impl_item.hir_id);

let contains_self_ty = |ty: Ty<'tcx>| {
ty.walk().any(|inner| match inner.unpack() {
GenericArgKind::Type(inner_ty) => same_tys(cx, self_ty, inner_ty),

GenericArgKind::Lifetime(_) | GenericArgKind::Const(_) => false,
})
};

// walk the return type and check for Self (this does not check associated types)
if ret_ty.walk().any(|inner_type| same_tys(cx, ty, inner_type)) {
if contains_self_ty(ret_ty) {
return;
}

Expand All @@ -1486,18 +1495,16 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
let associated_type = binder.skip_binder();

// walk the associated type and check for Self
for inner_type in associated_type.walk() {
if same_tys(cx, ty, inner_type) {
return;
}
if contains_self_ty(associated_type) {
return;
}
},
(_, _) => {},
}
}
}

if name == "new" && !same_tys(cx, ret_ty, ty) {
if name == "new" && !same_tys(cx, ret_ty, self_ty) {
span_lint(
cx,
NEW_RET_NO_SELF,
Expand Down
91 changes: 41 additions & 50 deletions clippy_lints/src/use_self.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use rustc_middle::ty;
use rustc_middle::ty::{DefIdTree, Ty};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::kw;
use rustc_typeck::hir_ty_to_ty;

use crate::utils::{differing_macro_contexts, span_lint_and_sugg};

Expand Down Expand Up @@ -80,37 +81,28 @@ fn span_use_self_lint(cx: &LateContext<'_, '_>, path: &Path<'_>, last_segment: O
);
}

struct TraitImplTyVisitor<'a, 'tcx> {
item_type: Ty<'tcx>,
// FIXME: always use this (more correct) visitor, not just in method signatures.
struct SemanticUseSelfVisitor<'a, 'tcx> {
cx: &'a LateContext<'a, 'tcx>,
trait_type_walker: ty::walk::TypeWalker<'tcx>,
impl_type_walker: ty::walk::TypeWalker<'tcx>,
self_ty: Ty<'tcx>,
}

impl<'a, 'tcx> Visitor<'tcx> for TraitImplTyVisitor<'a, 'tcx> {
impl<'a, 'tcx> Visitor<'tcx> for SemanticUseSelfVisitor<'a, 'tcx> {
type Map = Map<'tcx>;

fn visit_ty(&mut self, t: &'tcx hir::Ty<'_>) {
let trait_ty = self.trait_type_walker.next();
let impl_ty = self.impl_type_walker.next();

if_chain! {
if let TyKind::Path(QPath::Resolved(_, path)) = &t.kind;

// 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;
then {
match path.res {
def::Res::SelfTy(..) => {},
_ => span_use_self_lint(self.cx, path, None)
}
fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty<'_>) {
if let TyKind::Path(QPath::Resolved(_, path)) = &hir_ty.kind {
match path.res {
def::Res::SelfTy(..) => {},
_ => {
if hir_ty_to_ty(self.cx.tcx, hir_ty) == self.self_ty {
span_use_self_lint(self.cx, path, None);
}
},
}
}

walk_ty(self, t)
walk_ty(self, hir_ty)
}

fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
Expand All @@ -120,10 +112,9 @@ impl<'a, 'tcx> Visitor<'tcx> for TraitImplTyVisitor<'a, 'tcx> {

fn check_trait_method_impl_decl<'a, 'tcx>(
cx: &'a LateContext<'a, 'tcx>,
item_type: Ty<'tcx>,
impl_item: &ImplItem<'_>,
impl_decl: &'tcx FnDecl<'_>,
impl_trait_ref: &ty::TraitRef<'_>,
impl_trait_ref: ty::TraitRef<'tcx>,
) {
let trait_method = cx
.tcx
Expand All @@ -134,34 +125,35 @@ fn check_trait_method_impl_decl<'a, 'tcx>(
let trait_method_sig = cx.tcx.fn_sig(trait_method.def_id);
let trait_method_sig = cx.tcx.erase_late_bound_regions(&trait_method_sig);

let impl_method_def_id = cx.tcx.hir().local_def_id(impl_item.hir_id);
let impl_method_sig = cx.tcx.fn_sig(impl_method_def_id);
let impl_method_sig = cx.tcx.erase_late_bound_regions(&impl_method_sig);

let output_ty = if let FnRetTy::Return(ty) = &impl_decl.output {
let output_hir_ty = if let FnRetTy::Return(ty) = &impl_decl.output {
Some(&**ty)
} else {
None
};

// `impl_decl_ty` (of type `hir::Ty`) represents the type declared in the signature.
// `impl_ty` (of type `ty:TyS`) is the concrete type that the compiler has determined for
// that declaration. We use `impl_decl_ty` to see if the type was declared as `Self`
// and use `impl_ty` to check its concrete type.
for (impl_decl_ty, (impl_ty, trait_ty)) in impl_decl.inputs.iter().chain(output_ty).zip(
impl_method_sig
.inputs_and_output
.iter()
.zip(trait_method_sig.inputs_and_output),
) {
let mut visitor = TraitImplTyVisitor {
cx,
item_type,
trait_type_walker: trait_ty.walk(),
impl_type_walker: impl_ty.walk(),
};

visitor.visit_ty(&impl_decl_ty);
// `impl_hir_ty` (of type `hir::Ty`) represents the type written in the signature.
// `trait_ty` (of type `ty::Ty`) is the semantic type for the signature in the trait.
// We use `impl_hir_ty` to see if the type was written as `Self`,
// `hir_ty_to_ty(...)` to check semantic types of paths, and
// `trait_ty` to determine which parts of the signature in the trait, mention
// the type being implemented verbatim (as opposed to `Self`).
for (impl_hir_ty, trait_ty) in impl_decl
.inputs
.iter()
.chain(output_hir_ty)
.zip(trait_method_sig.inputs_and_output)
{
// Check if the input/output type in the trait method specifies the implemented
// type verbatim, and only suggest `Self` if that isn't the case.
// This avoids suggestions to e.g. replace `Vec<u8>` with `Vec<Self>`,
// in an `impl Trait for u8`, when the trait always uses `Vec<u8>`.
// See also https://github.com/rust-lang/rust-clippy/issues/2894.
let self_ty = impl_trait_ref.self_ty();
if !trait_ty.walk().any(|inner| inner == self_ty.into()) {
let mut visitor = SemanticUseSelfVisitor { cx, self_ty };

visitor.visit_ty(&impl_hir_ty);
}
}
}

Expand Down Expand Up @@ -197,8 +189,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseSelf {
let impl_item = cx.tcx.hir().impl_item(impl_item_ref.id);
if let ImplItemKind::Fn(FnSig{ decl: impl_decl, .. }, impl_body_id)
= &impl_item.kind {
let item_type = cx.tcx.type_of(impl_def_id);
check_trait_method_impl_decl(cx, item_type, impl_item, impl_decl, &impl_trait_ref);
check_trait_method_impl_decl(cx, impl_item, impl_decl, impl_trait_ref);

let body = cx.tcx.hir().body(*impl_body_id);
visitor.visit_body(body);
Expand Down
2 changes: 1 addition & 1 deletion setup-toolchain.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/bin/bash
#!/usr/bin/env bash
# Set up the appropriate rustc toolchain

set -e
Expand Down