Skip to content

Internal lint for usage of ty::TyKind #3493

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

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
9 changes: 2 additions & 7 deletions clippy_lints/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,17 +119,12 @@ impl Hash for Constant {
}

impl Constant {
pub fn partial_cmp(
tcx: TyCtxt<'_, '_, '_>,
cmp_type: &ty::TyKind<'_>,
left: &Self,
right: &Self,
) -> Option<Ordering> {
pub fn partial_cmp(tcx: TyCtxt<'_, '_, '_>, cmp_type: ty::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)) => {
if let ty::Int(int_ty) = *cmp_type {
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))
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/default_trait_access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

use crate::rustc::hir::*;
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use crate::rustc::ty::TyKind;
use crate::rustc::ty;
use crate::rustc::{declare_tool_lint, lint_array};
use crate::rustc_errors::Applicability;
use if_chain::if_chain;
Expand Down Expand Up @@ -71,7 +71,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DefaultTraitAccess {
// TODO: Work out a way to put "whatever the imported way of referencing
// this type in this file" rather than a fully-qualified type.
let expr_ty = cx.tables.expr_ty(expr);
if let TyKind::Adt(..) = expr_ty.sty {
if let ty::Adt(..) = expr_ty.sty {
let replacement = format!("{}::default()", expr_ty);
span_lint_and_sugg(
cx,
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/excessive_precision.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

use crate::rustc::hir;
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use crate::rustc::ty::TyKind;
use crate::rustc::ty;
use crate::rustc::{declare_tool_lint, lint_array};
use crate::rustc_errors::Applicability;
use crate::syntax::ast::*;
Expand Down Expand Up @@ -56,7 +56,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExcessivePrecision {
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) {
if_chain! {
let ty = cx.tables.expr_ty(expr);
if let TyKind::Float(fty) = ty.sty;
if let ty::Float(fty) = ty.sty;
if let hir::ExprKind::Lit(ref lit) = expr.node;
if let LitKind::Float(sym, _) | LitKind::FloatUnsuffixed(sym) = lit.node;
if let Some(sugg) = self.check(sym, fty);
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
reg.register_late_lint_pass(box utils::internal_lints::CompilerLintFunctions::new());
reg.register_early_lint_pass(box utils::internal_lints::DefaultHashTypes::default());
reg.register_late_lint_pass(box utils::internal_lints::LintWithoutLintPass::default());
reg.register_late_lint_pass(box utils::internal_lints::TyKindUsage);
reg.register_late_lint_pass(box utils::inspector::Pass);
reg.register_late_lint_pass(box utils::author::Pass);
reg.register_late_lint_pass(box types::TypePass);
Expand Down Expand Up @@ -552,6 +553,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
utils::internal_lints::COMPILER_LINT_FUNCTIONS,
utils::internal_lints::DEFAULT_HASH_TYPES,
utils::internal_lints::LINT_WITHOUT_LINT_PASS,
utils::internal_lints::USAGE_OF_TY_TYKIND,
]);

reg.register_lint_group("clippy::all", Some("clippy"), vec![
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1252,7 +1252,7 @@ fn is_end_eq_array_len(cx: &LateContext<'_, '_>, end: &Expr, limits: ast::RangeL
if_chain! {
if let ExprKind::Lit(ref lit) = end.node;
if let ast::LitKind::Int(end_int, _) = lit.node;
if let ty::TyKind::Array(_, arr_len_const) = indexed_ty.sty;
if let ty::Array(_, arr_len_const) = indexed_ty.sty;
if let Some(arr_len) = arr_len_const.assert_usize(cx.tcx);
then {
return match limits {
Expand Down Expand Up @@ -1375,7 +1375,7 @@ fn check_for_loop_arg(cx: &LateContext<'_, '_>, pat: &Pat, arg: &Expr, expr: &Ex
match cx.tables.expr_ty(&args[0]).sty {
// If the length is greater than 32 no traits are implemented for array and
// therefore we cannot use `&`.
ty::TyKind::Array(_, size) if size.assert_usize(cx.tcx).expect("array size") > 32 => (),
ty::Array(_, size) if size.assert_usize(cx.tcx).expect("array size") > 32 => (),
_ => lint_iter_method(cx, args, arg, method_name),
};
} else {
Expand Down
10 changes: 5 additions & 5 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use crate::rustc::hir;
use crate::rustc::hir::def::Def;
use crate::rustc::lint::{in_external_macro, LateContext, LateLintPass, Lint, LintArray, LintContext, LintPass};
use crate::rustc::ty::{self, Predicate, Ty, TyKind};
use crate::rustc::ty::{self, Predicate, Ty};
use crate::rustc::{declare_tool_lint, lint_array};
use crate::rustc_errors::Applicability;
use crate::syntax::ast;
Expand Down Expand Up @@ -978,7 +978,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
}

// if return type is impl trait, check the associated types
if let TyKind::Opaque(def_id, _) = ret_ty.sty {
if let ty::Opaque(def_id, _) = ret_ty.sty {
// one of the associated types must be Self
for predicate in &cx.tcx.predicates_of(def_id).predicates {
match predicate {
Expand Down Expand Up @@ -2204,7 +2204,7 @@ fn ty_has_iter_method(
];

let (self_ty, mutbl) = match self_ref_ty.sty {
ty::TyKind::Ref(_, self_ty, mutbl) => (self_ty, mutbl),
ty::Ref(_, self_ty, mutbl) => (self_ty, mutbl),
_ => unreachable!(),
};
let method_name = match mutbl {
Expand All @@ -2213,8 +2213,8 @@ fn ty_has_iter_method(
};

let def_id = match self_ty.sty {
ty::TyKind::Array(..) => return Some((INTO_ITER_ON_ARRAY, "array", method_name)),
ty::TyKind::Slice(..) => return Some((INTO_ITER_ON_REF, "slice", method_name)),
ty::Array(..) => return Some((INTO_ITER_ON_ARRAY, "array", method_name)),
ty::Slice(..) => return Some((INTO_ITER_ON_REF, "slice", method_name)),
ty::Adt(adt, _) => adt.did,
_ => return None,
};
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/minmax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MinMaxPass {
}
match (
outer_max,
Constant::partial_cmp(cx.tcx, &cx.tables.expr_ty(ie).sty, &outer_c, &inner_c),
Constant::partial_cmp(cx.tcx, cx.tables.expr_ty(ie), &outer_c, &inner_c),
) {
(_, None) | (MinMax::Max, Some(Ordering::Less)) | (MinMax::Min, Some(Ordering::Greater)) => (),
_ => {
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/needless_pass_by_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {

// Dereference suggestion
let sugg = |db: &mut DiagnosticBuilder<'_>| {
if let ty::TyKind::Adt(def, ..) = ty.sty {
if let ty::Adt(def, ..) = ty.sty {
if let Some(span) = cx.tcx.hir.span_if_local(def.did) {
if cx.param_env.can_type_implement_copy(cx.tcx, ty).is_ok() {
db.span_help(span, "consider marking this type as Copy");
Expand Down
8 changes: 4 additions & 4 deletions clippy_lints/src/trivially_copy_pass_by_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::rustc::hir::intravisit::FnKind;
use crate::rustc::hir::*;
use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use crate::rustc::session::config::Config as SessionConfig;
use crate::rustc::ty::{FnSig, TyKind};
use crate::rustc::ty::{self, FnSig};
use crate::rustc::{declare_tool_lint, lint_array};
use crate::rustc_errors::Applicability;
use crate::rustc_target::abi::LayoutOf;
Expand Down Expand Up @@ -99,8 +99,8 @@ impl<'a, 'tcx> TriviallyCopyPassByRef {
// argument. In that case we can't switch to pass-by-value as the
// argument will not live long enough.
let output_lts = match sig.output().sty {
TyKind::Ref(output_lt, _, _) => vec![output_lt],
TyKind::Adt(_, substs) => substs.regions().collect(),
ty::Ref(output_lt, _, _) => vec![output_lt],
ty::Adt(_, substs) => substs.regions().collect(),
_ => vec![],
};

Expand All @@ -112,7 +112,7 @@ impl<'a, 'tcx> TriviallyCopyPassByRef {
}

if_chain! {
if let TyKind::Ref(input_lt, ty, Mutability::MutImmutable) = ty.sty;
if let ty::Ref(input_lt, ty, Mutability::MutImmutable) = ty.sty;
if !output_lts.contains(&input_lt);
if is_copy(cx, ty);
if let Some(size) = cx.layout_of(ty).ok().map(|l| l.size.bytes());
Expand Down
103 changes: 103 additions & 0 deletions clippy_lints/src/utils/internal_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,3 +325,106 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CompilerLintFunctions {
}
}
}

/// **What it does:** Checks for matches on `ty::TyKind::<kind>` and for usage of `ty::TyKind`
/// instead of `ty::Ty`. See issue
/// [rust-lang/rust#49509](https://github.com/rust-lang/rust/issues/49509) for details.
///
/// **Why is this bad?** `ty` reexports `TyKind::*`, so the enum variants of `TyKind` can be used
/// directly through `ty`. Also `TyKind` should never be used directly outside of the `sty` module.
/// See
/// [rust-lang/rust-clippy#2077](https://github.com/rust-lang/rust-clippy/pull/2077#discussion_r238553405)
///
/// **Known problems:** None.
///
/// **Example:**
/// Bad:
/// ```rust
/// if let ty::TyKind::Int(int_ty) = ty.sty {}
/// ```
///
/// Good:
/// ```
/// if let ty::Int(int_ty) = ty.sty {}
/// ```
declare_clippy_lint! {
pub USAGE_OF_TY_TYKIND,
internal,
"Using `ty::TyKind::<kind>` instead of just `ty::<kind>`"
}

#[derive(Copy, Clone)]
pub struct TyKindUsage;

impl LintPass for TyKindUsage {
fn get_lints(&self) -> LintArray {
lint_array!(USAGE_OF_TY_TYKIND)
}
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TyKindUsage {
fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &'tcx Expr) {
let qpaths = match &expr.node {
ExprKind::Match(_, arms, _) => {
let mut qpaths = vec![];
for arm in arms {
for pat in &arm.pats {
match &pat.node {
PatKind::Path(qpath) | PatKind::TupleStruct(qpath, ..) => qpaths.push(qpath),
_ => (),
}
}
}
qpaths
},
ExprKind::Path(qpath) => vec![qpath],
_ => vec![],
};
for qpath in qpaths {
if_chain! {
if let QPath::Resolved(_, path) = qpath;
let segments_iter = path.segments.iter().rev().skip(1).rev();
if let Some(last) = segments_iter.clone().last();
if last.ident.as_str() == "TyKind";
let path = Path {
span: path.span.with_hi(last.ident.span.hi()),
def: path.def,
segments: segments_iter.cloned().collect(),
};
if let Some(def) = last.def;
if match_def_path(cx.tcx, def.def_id(), &paths::TY_KIND);
then {
span_lint_and_sugg(
cx,
USAGE_OF_TY_TYKIND,
path.span,
"usage of `ty::TyKind::<kind>`",
"try using ty::<kind> directly",
"ty".to_string(),
Applicability::MaybeIncorrect, // ty maybe needs an import
);
}
}
}
}

fn check_ty(&mut self, cx: &LateContext<'_, '_>, ty: &'tcx Ty) {
if_chain! {
if let TyKind::Path(qpath) = &ty.node;
if let QPath::Resolved(_, path) = qpath;
if let Some(last) = path.segments.iter().last();
if last.ident.as_str() == "TyKind";
if let Some(def) = last.def;
if match_def_path(cx.tcx, def.def_id(), &paths::TY_KIND);
then {
span_help_and_lint(
cx,
USAGE_OF_TY_TYKIND,
path.span,
"usage of `ty::TyKind`",
"try using `ty::Ty` instead",
);
}
}
}
}
1 change: 1 addition & 0 deletions clippy_lints/src/utils/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ pub const TO_STRING: [&str; 3] = ["alloc", "string", "ToString"];
pub const TO_STRING_METHOD: [&str; 4] = ["alloc", "string", "ToString", "to_string"];
pub const TRANSMUTE: [&str; 4] = ["core", "intrinsics", "", "transmute"];
pub const TRY_INTO_RESULT: [&str; 4] = ["std", "ops", "Try", "into_result"];
pub const TY_KIND: [&str; 4] = ["rustc", "ty", "sty", "TyKind"];
pub const UNINIT: [&str; 4] = ["core", "intrinsics", "", "uninit"];
pub const VEC: [&str; 3] = ["alloc", "vec", "Vec"];
pub const VEC_DEQUE: [&str; 4] = ["alloc", "collections", "vec_deque", "VecDeque"];
Expand Down
55 changes: 55 additions & 0 deletions tests/ui/tykind_usage.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright 2014-2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![warn(clippy::usage_of_ty_tykind)]
#![feature(rustc_private)]

extern crate rustc;

use rustc::ty::{self, Ty, TyKind};

fn main() {
let sty = TyKind::Bool;

match sty {
TyKind::Bool => (),
TyKind::Char => (),
TyKind::Int(..) => (),
TyKind::Uint(..) => (),
TyKind::Float(..) => (),
TyKind::Adt(..) => (),
TyKind::Foreign(..) => (),
TyKind::Str => (),
TyKind::Array(..) => (),
TyKind::Slice(..) => (),
TyKind::RawPtr(..) => (),
TyKind::Ref(..) => (),
TyKind::FnDef(..) => (),
TyKind::FnPtr(..) => (),
TyKind::Dynamic(..) => (),
TyKind::Closure(..) => (),
TyKind::Generator(..) => (),
TyKind::GeneratorWitness(..) => (),
TyKind::Never => (),
TyKind::Tuple(..) => (),
TyKind::Projection(..) => (),
TyKind::UnnormalizedProjection(..) => (),
TyKind::Opaque(..) => (),
TyKind::Param(..) => (),
TyKind::Bound(..) => (),
TyKind::Placeholder(..) => (),
TyKind::Infer(..) => (),
TyKind::Error => (),
}

if let ty::Int(int_ty) = sty {}
if let TyKind::Int(int_ty) = sty {}

fn ty_kind(ty_bad: TyKind<'_>, ty_good: Ty<'_>) {}
}
Loading