From: flip1995 Date: Wed, 24 Apr 2019 21:22:54 +0000 (+0200) Subject: Implement internal lints X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=747d288be4a85f3adac67b820df74eef0571ef64;p=rust.git Implement internal lints - USAGE_OF_QUALIFIED_TY - TY_PASS_BY_REFERENCE --- diff --git a/src/librustc/lint/internal.rs b/src/librustc/lint/internal.rs index 91f1bee26de..126a7cd3349 100644 --- a/src/librustc/lint/internal.rs +++ b/src/librustc/lint/internal.rs @@ -1,7 +1,7 @@ //! Some lints that are only useful in the compiler or crates that use compiler internals, such as //! Clippy. -use crate::hir::{HirId, Path, PathSegment, QPath, Ty, TyKind}; +use crate::hir::{GenericArg, HirId, MutTy, Mutability, Path, PathSegment, QPath, Ty, TyKind}; use crate::lint::{ EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintArray, LintContext, LintPass, }; @@ -57,12 +57,28 @@ fn check_ident(&mut self, cx: &EarlyContext<'_>, ident: Ident) { declare_lint! { pub USAGE_OF_TY_TYKIND, Allow, - "Usage of `ty::TyKind` outside of the `ty::sty` module" + "usage of `ty::TyKind` outside of the `ty::sty` module" } -declare_lint_pass!(TyKindUsage => [USAGE_OF_TY_TYKIND]); +declare_lint! { + pub TY_PASS_BY_REFERENCE, + Allow, + "passing `Ty` or `TyCtxt` by reference" +} + +declare_lint! { + pub USAGE_OF_QUALIFIED_TY, + Allow, + "using `ty::{Ty,TyCtxt}` instead of importing it" +} + +declare_lint_pass!(TyTyKind => [ + USAGE_OF_TY_TYKIND, + TY_PASS_BY_REFERENCE, + USAGE_OF_QUALIFIED_TY, +]); -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TyKindUsage { +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TyTyKind { fn check_path(&mut self, cx: &LateContext<'_, '_>, path: &'tcx Path, _: HirId) { let segments = path.segments.iter().rev().skip(1).rev(); @@ -82,16 +98,72 @@ fn check_path(&mut self, cx: &LateContext<'_, '_>, path: &'tcx Path, _: HirId) { } fn check_ty(&mut self, cx: &LateContext<'_, '_>, ty: &'tcx Ty) { - if let TyKind::Path(qpath) = &ty.node { - if let QPath::Resolved(_, path) = qpath { - if let Some(last) = path.segments.iter().last() { - if lint_ty_kind_usage(cx, last) { - cx.struct_span_lint(USAGE_OF_TY_TYKIND, path.span, "usage of `ty::TyKind`") - .help("try using `ty::Ty` instead") + match &ty.node { + TyKind::Path(qpath) => { + if let QPath::Resolved(_, path) = qpath { + if let Some(last) = path.segments.iter().last() { + if lint_ty_kind_usage(cx, last) { + cx.struct_span_lint( + USAGE_OF_TY_TYKIND, + path.span, + "usage of `ty::TyKind`", + ) + .help("try using `Ty` instead") .emit(); + } else { + if ty.span.ctxt().outer().expn_info().is_some() { + return; + } + if let Some(t) = is_ty_or_ty_ctxt(cx, ty) { + if path.segments.len() > 1 { + cx.struct_span_lint( + USAGE_OF_QUALIFIED_TY, + path.span, + &format!("usage of qualified `ty::{}`", t), + ) + .span_suggestion( + path.span, + "try using it unqualified", + t, + // The import probably needs to be changed + Applicability::MaybeIncorrect, + ) + .emit(); + } + } + } + } + } + } + TyKind::Rptr( + _, + MutTy { + ty: inner_ty, + mutbl: Mutability::MutImmutable, + }, + ) => { + if let Some(impl_did) = cx.tcx.impl_of_method(ty.hir_id.owner_def_id()) { + if cx.tcx.impl_trait_ref(impl_did).is_some() { + return; } } + if let Some(t) = is_ty_or_ty_ctxt(cx, &inner_ty) { + cx.struct_span_lint( + TY_PASS_BY_REFERENCE, + ty.span, + &format!("passing `{}` by reference", t), + ) + .span_suggestion( + ty.span, + "try passing by value", + t, + // Changing type of function argument + Applicability::MaybeIncorrect, + ) + .emit(); + } } + _ => {} } } } @@ -107,3 +179,43 @@ fn lint_ty_kind_usage(cx: &LateContext<'_, '_>, segment: &PathSegment) -> bool { false } + +fn is_ty_or_ty_ctxt(cx: &LateContext<'_, '_>, ty: &Ty) -> Option { + match &ty.node { + TyKind::Path(qpath) => { + if let QPath::Resolved(_, path) = qpath { + let did = path.def.opt_def_id()?; + if cx.match_def_path(did, &["rustc", "ty", "Ty"]) { + return Some(format!("Ty{}", gen_args(path.segments.last().unwrap()))); + } else if cx.match_def_path(did, &["rustc", "ty", "context", "TyCtxt"]) { + return Some(format!("TyCtxt{}", gen_args(path.segments.last().unwrap()))); + } + } + } + _ => {} + } + + None +} + +fn gen_args(segment: &PathSegment) -> String { + if let Some(args) = &segment.args { + let lifetimes = args + .args + .iter() + .filter_map(|arg| { + if let GenericArg::Lifetime(lt) = arg { + Some(lt.name.ident().to_string()) + } else { + None + } + }) + .collect::>(); + + if !lifetimes.is_empty() { + return format!("<{}>", lifetimes.join(", ")); + } + } + + String::new() +} diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 7d23da857bb..0bb9d4389dd 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -495,7 +495,7 @@ macro_rules! register_passes { pub fn register_internals(store: &mut lint::LintStore, sess: Option<&Session>) { store.register_early_pass(sess, false, false, box DefaultHashTypes::new()); - store.register_late_pass(sess, false, false, false, box TyKindUsage); + store.register_late_pass(sess, false, false, false, box TyTyKind); store.register_group( sess, false, @@ -504,6 +504,8 @@ pub fn register_internals(store: &mut lint::LintStore, sess: Option<&Session>) { vec![ LintId::of(DEFAULT_HASH_TYPES), LintId::of(USAGE_OF_TY_TYKIND), + LintId::of(TY_PASS_BY_REFERENCE), + LintId::of(USAGE_OF_QUALIFIED_TY), ], ); } diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index c8d6ee9db6f..9581b6b52f7 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -315,7 +315,7 @@ fn insert_as_pending_if_two_phase( start_location, assigned_place, borrow_index, ); - if !allow_two_phase_borrow(&self.tcx, kind) { + if !allow_two_phase_borrow(self.tcx, kind) { debug!(" -> {:?}", start_location); return; }