Skip to content

Commit

Permalink
Add internal lint against Ty == Ty
Browse files Browse the repository at this point in the history
This operation does not do what people usually want. Also check against
other comparison operators since they are just as sketchy.

Allow the `ty_eq_operator` lint for now until the next bootstrap bump
  • Loading branch information
Noratrieb committed Oct 15, 2023
1 parent 4331c15 commit 307610c
Show file tree
Hide file tree
Showing 7 changed files with 165 additions and 10 deletions.
7 changes: 7 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,13 @@ lint_suspicious_double_ref_deref =
lint_trivial_untranslatable_diag = diagnostic with static strings only
lint_ty_compare_operator_used = using the a comparison operator on `Ty`
.note = this does probably not what you want as it does not handle inference variables and more
.help = for more information, see https://rustc-dev-guide.rust-lang.org/ty.html#comparing-types
lint_ty_no_compare_operator_for_diagnostics = it's also not recommended to use it for diagnostics
lint_ty_qualified = usage of qualified `ty::{$ty}`
.suggestion = try importing it and using it unqualified
Expand Down
45 changes: 36 additions & 9 deletions compiler/rustc_lint/src/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
use crate::lints::{
BadOptAccessDiag, DefaultHashTypesDiag, DiagOutOfImpl, LintPassByHand, NonExistentDocKeyword,
QueryInstability, TyQualified, TykindDiag, TykindKind, UntranslatableDiag,
UntranslatableDiagnosticTrivial,
QueryInstability, TyCompareOperatorUsed, TyQualified, TykindDiag, TykindKind,
UntranslatableDiag, UntranslatableDiagnosticTrivial,
};
use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext};
use rustc_ast as ast;
use rustc_hir as hir;
use rustc_hir::def::Res;
use rustc_hir::{def_id::DefId, Expr, ExprKind, GenericArg, PatKind, Path, PathSegment, QPath};
use rustc_hir::{HirId, Impl, Item, ItemKind, Node, Pat, Ty, TyKind};
Expand Down Expand Up @@ -127,12 +128,7 @@ declare_lint_pass!(TyTyKind => [
]);

impl<'tcx> LateLintPass<'tcx> for TyTyKind {
fn check_path(
&mut self,
cx: &LateContext<'tcx>,
path: &rustc_hir::Path<'tcx>,
_: rustc_hir::HirId,
) {
fn check_path(&mut self, cx: &LateContext<'tcx>, path: &hir::Path<'tcx>, _: hir::HirId) {
if let Some(segment) = path.segments.iter().nth_back(1)
&& lint_ty_kind_usage(cx, &segment.res)
{
Expand Down Expand Up @@ -269,6 +265,37 @@ fn gen_args(segment: &PathSegment<'_>) -> String {
String::new()
}

declare_tool_lint! {
pub rustc::TY_COMPARE_OPERATOR,
Allow,
"using the a comparison operator on `Ty`"
}

declare_lint_pass!(TyCompareOperator => [TY_COMPARE_OPERATOR]);

impl<'tcx> LateLintPass<'tcx> for TyCompareOperator {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
use hir::BinOpKind::*;

if let hir::ExprKind::Binary(
hir::BinOp { node:Eq | Ne | Gt | Lt | Le | Ge, span},
lhs,
rhs
) = expr.kind
&& let ty::Adt(lhs_def, _) = cx.typeck_results().node_type(lhs.hir_id).peel_refs().kind()
&& let ty::Adt(rhs_def, _) = cx.typeck_results().node_type(rhs.hir_id).peel_refs().kind()
&& cx.tcx.is_diagnostic_item(sym::Ty, lhs_def.did())
&& cx.tcx.is_diagnostic_item(sym::Ty, rhs_def.did())
{
cx.emit_spanned_lint(
TY_COMPARE_OPERATOR,
span,
TyCompareOperatorUsed,
)
}
}
}

declare_tool_lint! {
/// The `lint_pass_impl_without_macro` detects manual implementations of a lint
/// pass, without using [`declare_lint_pass`] or [`impl_lint_pass`].
Expand Down Expand Up @@ -318,7 +345,7 @@ fn is_doc_keyword(s: Symbol) -> bool {
}

impl<'tcx> LateLintPass<'tcx> for ExistingDocKeyword {
fn check_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::Item<'_>) {
fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) {
for attr in cx.tcx.hir().attrs(item.hir_id()) {
if !attr.has_name(sym::doc) {
continue;
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,8 @@ fn register_internals(store: &mut LintStore) {
store.register_late_mod_pass(|_| Box::new(ExistingDocKeyword));
store.register_lints(&TyTyKind::get_lints());
store.register_late_mod_pass(|_| Box::new(TyTyKind));
store.register_lints(&TyCompareOperator::get_lints());
store.register_late_mod_pass(|_| Box::new(TyCompareOperator));
store.register_lints(&Diagnostics::get_lints());
store.register_early_pass(|| Box::new(Diagnostics));
store.register_late_mod_pass(|_| Box::new(Diagnostics));
Expand All @@ -543,6 +545,7 @@ fn register_internals(store: &mut LintStore) {
LintId::of(DEFAULT_HASH_TYPES),
LintId::of(POTENTIAL_QUERY_INSTABILITY),
LintId::of(USAGE_OF_TY_TYKIND),
LintId::of(TY_COMPARE_OPERATOR),
LintId::of(PASS_BY_VALUE),
LintId::of(LINT_PASS_IMPL_WITHOUT_MACRO),
LintId::of(USAGE_OF_QUALIFIED_TY),
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,13 @@ pub struct TyQualified {
pub suggestion: Span,
}

#[derive(LintDiagnostic)]
#[diag(lint_ty_compare_operator_used)]
#[note]
#[note(lint_ty_no_compare_operator_for_diagnostics)]
#[help]
pub struct TyCompareOperatorUsed;

#[derive(LintDiagnostic)]
#[diag(lint_lintpass_by_hand)]
#[help]
Expand Down
9 changes: 8 additions & 1 deletion src/bootstrap/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1878,9 +1878,16 @@ impl<'a> Builder<'a> {
rustdocflags.arg("-Wrustdoc::invalid_codeblock_attributes");
}

if mode == Mode::Rustc {
if mode == Mode::Rustc || mode == Mode::ToolRustc {
rustflags.arg("-Zunstable-options");
rustflags.arg("-Wrustc::internal");

// #[cfg(not(bootstrap))]
if stage != 0 {
// FIXME(Nilstrieb): Allow this lint for now.
// After it's in beta, we can start work on fixing it.
rustflags.arg("-Arustc::ty-compare-operator");
}
}

// Throughout the build Cargo can execute a number of build scripts
Expand Down
37 changes: 37 additions & 0 deletions tests/ui/lint/internal/ty_compare_operator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// compile-flags: -Z unstable-options

#![feature(rustc_attrs)]
#![forbid(rustc::ty_compare_operator)]

mod ty1 {
#[derive(PartialEq, PartialOrd)]
#[rustc_diagnostic_item = "Ty"]
pub struct Ty;
}

mod ty2 {
#[derive(PartialEq, PartialOrd)]
pub struct Ty;
}

fn main() {
let _ = ty1::Ty == ty1::Ty;
//~^ ERROR using the a comparison operator on `Ty`
let _ = ty1::Ty != ty1::Ty;
//~^ ERROR using the a comparison operator on `Ty`
let _ = ty1::Ty < ty1::Ty;
//~^ ERROR using the a comparison operator on `Ty`
let _ = ty1::Ty <= ty1::Ty;
//~^ ERROR using the a comparison operator on `Ty`
let _ = ty1::Ty > ty1::Ty;
//~^ ERROR using the a comparison operator on `Ty`
let _ = ty1::Ty >= ty1::Ty;
//~^ ERROR using the a comparison operator on `Ty`

let _ = ty2::Ty == ty2::Ty;
let _ = ty2::Ty != ty2::Ty;
let _ = ty2::Ty < ty2::Ty;
let _ = ty2::Ty <= ty2::Ty;
let _ = ty2::Ty > ty2::Ty;
let _ = ty2::Ty >= ty2::Ty;
}
67 changes: 67 additions & 0 deletions tests/ui/lint/internal/ty_compare_operator.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
error: using the a comparison operator on `Ty`
--> $DIR/ty_compare_operator.rs:18:21
|
LL | let _ = ty1::Ty == ty1::Ty;
| ^^
|
= note: this does probably not what you want as it does not handle inference variables and more
= note: it's also not recommended to use it for diagnostics
= help: for more information, see https://rustc-dev-guide.rust-lang.org/ty.html#comparing-types
note: the lint level is defined here
--> $DIR/ty_compare_operator.rs:4:11
|
LL | #![forbid(rustc::ty_compare_operator)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

error: using the a comparison operator on `Ty`
--> $DIR/ty_compare_operator.rs:20:21
|
LL | let _ = ty1::Ty != ty1::Ty;
| ^^
|
= note: this does probably not what you want as it does not handle inference variables and more
= note: it's also not recommended to use it for diagnostics
= help: for more information, see https://rustc-dev-guide.rust-lang.org/ty.html#comparing-types

error: using the a comparison operator on `Ty`
--> $DIR/ty_compare_operator.rs:22:21
|
LL | let _ = ty1::Ty < ty1::Ty;
| ^
|
= note: this does probably not what you want as it does not handle inference variables and more
= note: it's also not recommended to use it for diagnostics
= help: for more information, see https://rustc-dev-guide.rust-lang.org/ty.html#comparing-types

error: using the a comparison operator on `Ty`
--> $DIR/ty_compare_operator.rs:24:21
|
LL | let _ = ty1::Ty <= ty1::Ty;
| ^^
|
= note: this does probably not what you want as it does not handle inference variables and more
= note: it's also not recommended to use it for diagnostics
= help: for more information, see https://rustc-dev-guide.rust-lang.org/ty.html#comparing-types

error: using the a comparison operator on `Ty`
--> $DIR/ty_compare_operator.rs:26:21
|
LL | let _ = ty1::Ty > ty1::Ty;
| ^
|
= note: this does probably not what you want as it does not handle inference variables and more
= note: it's also not recommended to use it for diagnostics
= help: for more information, see https://rustc-dev-guide.rust-lang.org/ty.html#comparing-types

error: using the a comparison operator on `Ty`
--> $DIR/ty_compare_operator.rs:28:21
|
LL | let _ = ty1::Ty >= ty1::Ty;
| ^^
|
= note: this does probably not what you want as it does not handle inference variables and more
= note: it's also not recommended to use it for diagnostics
= help: for more information, see https://rustc-dev-guide.rust-lang.org/ty.html#comparing-types

error: aborting due to 6 previous errors

0 comments on commit 307610c

Please sign in to comment.