From 8769e02d0b17e18e0a52b521a82da4bcae2e6385 Mon Sep 17 00:00:00 2001 From: Arthur Lafrance Date: Sat, 23 Sep 2023 23:50:47 -0700 Subject: implement the basics of the lint static analysis --- compiler/rustc_span/src/span_encoding.rs | 1 + 1 file changed, 1 insertion(+) (limited to 'compiler/rustc_span/src/span_encoding.rs') diff --git a/compiler/rustc_span/src/span_encoding.rs b/compiler/rustc_span/src/span_encoding.rs index bfc9e125362..69ad11e23f0 100644 --- a/compiler/rustc_span/src/span_encoding.rs +++ b/compiler/rustc_span/src/span_encoding.rs @@ -212,6 +212,7 @@ impl Span { /// This function is used as a fast path when decoding the full `SpanData` is not necessary. /// It's a cut-down version of `data_untracked`. + #[rustc_diagnostic_item = "SpanCtxt"] #[inline] pub fn ctxt(self) -> SyntaxContext { if self.len_with_tag_or_marker != BASE_LEN_INTERNED_MARKER { -- cgit 1.4.1-3-g733a5 From f77dea89e183b638841d42d4d7bea48058a98e76 Mon Sep 17 00:00:00 2001 From: Arthur Lafrance Date: Mon, 25 Sep 2023 00:15:00 -0700 Subject: basic lint v2 implemented --- compiler/rustc_lint/messages.ftl | 2 + compiler/rustc_lint/src/internal.rs | 45 +++++++++++++++++++++- compiler/rustc_lint/src/lib.rs | 4 +- compiler/rustc_lint/src/lints.rs | 6 +++ compiler/rustc_lint/src/span_use_eq_ctxt.rs | 38 ------------------ compiler/rustc_span/src/span_encoding.rs | 2 +- compiler/rustc_span/src/symbol.rs | 2 +- .../ui-fulldeps/internal-lints/span_use_eq_ctxt.rs | 13 +++++++ 8 files changed, 69 insertions(+), 43 deletions(-) delete mode 100644 compiler/rustc_lint/src/span_use_eq_ctxt.rs create mode 100644 tests/ui-fulldeps/internal-lints/span_use_eq_ctxt.rs (limited to 'compiler/rustc_span/src/span_encoding.rs') diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 197fe6552d7..261451e530e 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -494,6 +494,8 @@ lint_renamed_lint = lint `{$name}` has been renamed to `{$replace}` lint_requested_level = requested on the command line with `{$level} {$lint_name}` +lint_span_use_eq_ctxt = use `eq_ctxt()` not `ctxt() == ctxt()` + lint_supertrait_as_deref_target = `{$t}` implements `Deref` with supertrait `{$target_principal}` as target .label = target type is set here diff --git a/compiler/rustc_lint/src/internal.rs b/compiler/rustc_lint/src/internal.rs index fc2d3d0a254..c2aa768e945 100644 --- a/compiler/rustc_lint/src/internal.rs +++ b/compiler/rustc_lint/src/internal.rs @@ -3,14 +3,14 @@ use crate::lints::{ BadOptAccessDiag, DefaultHashTypesDiag, DiagOutOfImpl, LintPassByHand, NonExistentDocKeyword, - QueryInstability, TyQualified, TykindDiag, TykindKind, UntranslatableDiag, + QueryInstability, SpanUseEqCtxtDiag, TyQualified, TykindDiag, TykindKind, UntranslatableDiag, UntranslatableDiagnosticTrivial, }; use crate::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext}; use rustc_ast as ast; 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}; +use rustc_hir::{BinOp, BinOpKind, HirId, Impl, Item, ItemKind, Node, Pat, Ty, TyKind}; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::hygiene::{ExpnKind, MacroKind}; @@ -537,3 +537,44 @@ impl LateLintPass<'_> for BadOptAccess { } } } + +// some things i'm not sure about: +// * is Warn the right level? +// * the way i verify that the right method is being called (path + diag item check) + +declare_tool_lint! { + pub rustc::SPAN_USE_EQ_CTXT, + Warn, // is this the right level? + "Use of `==` with `Span::ctxt` rather than `Span::eq_ctxt`", + report_in_external_macro: true +} + +declare_lint_pass!(SpanUseEqCtxt => [SPAN_USE_EQ_CTXT]); + +impl<'tcx> LateLintPass<'tcx> for SpanUseEqCtxt { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'_>) { + if let ExprKind::Binary(BinOp { node: BinOpKind::Eq, .. }, lhs, rhs) = expr.kind { + if is_span_ctxt_call(cx, lhs) && is_span_ctxt_call(cx, rhs) { + cx.emit_spanned_lint( + SPAN_USE_EQ_CTXT, + expr.span, + SpanUseEqCtxtDiag { msg: "fail" }, + ); + } + } + } +} + +fn is_span_ctxt_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + match &expr.kind { + ExprKind::MethodCall(path, receiver, _, _) => { + path.ident.name.as_str() == "ctxt" + && cx + .typeck_results() + .type_dependent_def_id(receiver.hir_id) + .is_some_and(|did| cx.tcx.is_diagnostic_item(sym::Span, did)) + } + + _ => false, + } +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index ca1f620b7c7..d61c59af1e0 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -83,7 +83,6 @@ mod passes; mod ptr_nulls; mod redundant_semicolon; mod reference_casting; -mod span_use_eq_ctxt; mod traits; mod types; mod unused; @@ -532,6 +531,8 @@ fn register_internals(store: &mut LintStore) { store.register_late_mod_pass(|_| Box::new(BadOptAccess)); store.register_lints(&PassByValue::get_lints()); store.register_late_mod_pass(|_| Box::new(PassByValue)); + store.register_lints(&SpanUseEqCtxt::get_lints()); + store.register_late_mod_pass(|_| Box::new(SpanUseEqCtxt)); // FIXME(davidtwco): deliberately do not include `UNTRANSLATABLE_DIAGNOSTIC` and // `DIAGNOSTIC_OUTSIDE_OF_IMPL` here because `-Wrustc::internal` is provided to every crate and // these lints will trigger all of the time - change this once migration to diagnostic structs @@ -549,6 +550,7 @@ fn register_internals(store: &mut LintStore) { LintId::of(USAGE_OF_QUALIFIED_TY), LintId::of(EXISTING_DOC_KEYWORD), LintId::of(BAD_OPT_ACCESS), + LintId::of(SPAN_USE_EQ_CTXT), ], ); } diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index 594ef97b3ff..a02bee506df 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -900,6 +900,12 @@ pub struct QueryInstability { pub query: Symbol, } +#[derive(LintDiagnostic)] +#[diag(lint_span_use_eq_ctxt)] +pub struct SpanUseEqCtxtDiag<'a> { + pub msg: &'a str, +} + #[derive(LintDiagnostic)] #[diag(lint_tykind_kind)] pub struct TykindKind { diff --git a/compiler/rustc_lint/src/span_use_eq_ctxt.rs b/compiler/rustc_lint/src/span_use_eq_ctxt.rs deleted file mode 100644 index 19fea01bf4f..00000000000 --- a/compiler/rustc_lint/src/span_use_eq_ctxt.rs +++ /dev/null @@ -1,38 +0,0 @@ -use crate::{LateContext, LateLintPass}; -use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind}; -use rustc_span::sym; - -declare_lint! { - pub SPAN_USE_EQ_CTXT, - Warn, // is this the right level? - "Use of `==` with `Span::ctxt` rather than `Span::eq_ctxt`" -} - -declare_lint_pass!(SpanUseEqCtxt => [SPAN_USE_EQ_CTXT]); - -impl<'tcx> LateLintPass<'tcx> for SpanUseEqCtxt { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'_>) { - if let ExprKind::Binary(BinOp { node: BinOpKind::Eq, .. }, lhs, rhs) = expr.kind { - if is_span_ctxt_call(cx, lhs) && is_span_ctxt_call(cx, rhs) { - todo!(); // emit lint - } - } - } -} - -fn is_span_ctxt_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - match &expr.kind { - ExprKind::MethodCall(..) => { - // i gave a method a diagnostic item -- FIXME: switch to a diagnostic - // item for the Span type and check: - // * method call path == "ctxt" - // * receiver type matches Span diag item - // also FIXME(todo) remove old SpanCtxt diagnostic item - cx.typeck_results() - .type_dependent_def_id(expr.hir_id) - .is_some_and(|did| cx.tcx.is_diagnostic_item(sym::SpanCtxt, did)) - } - - _ => false, - } -} diff --git a/compiler/rustc_span/src/span_encoding.rs b/compiler/rustc_span/src/span_encoding.rs index 69ad11e23f0..7c7f8448c97 100644 --- a/compiler/rustc_span/src/span_encoding.rs +++ b/compiler/rustc_span/src/span_encoding.rs @@ -75,6 +75,7 @@ use rustc_data_structures::fx::FxIndexSet; /// the dependency to the parent definition's span. This is performed /// using the callback `SPAN_TRACK` to access the query engine. /// +#[cfg_attr(not(test), rustc_diagnostic_item = "Span")] #[derive(Clone, Copy, Eq, PartialEq, Hash)] #[rustc_pass_by_value] pub struct Span { @@ -212,7 +213,6 @@ impl Span { /// This function is used as a fast path when decoding the full `SpanData` is not necessary. /// It's a cut-down version of `data_untracked`. - #[rustc_diagnostic_item = "SpanCtxt"] #[inline] pub fn ctxt(self) -> SyntaxContext { if self.len_with_tag_or_marker != BASE_LEN_INTERNED_MARKER { diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index be8c65862dc..9598b2d0310 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -303,7 +303,7 @@ symbols! { SliceIndex, SliceIter, Some, - SpanCtxt, + Span, String, StructuralEq, StructuralPartialEq, diff --git a/tests/ui-fulldeps/internal-lints/span_use_eq_ctxt.rs b/tests/ui-fulldeps/internal-lints/span_use_eq_ctxt.rs new file mode 100644 index 00000000000..5b4c59a2e8a --- /dev/null +++ b/tests/ui-fulldeps/internal-lints/span_use_eq_ctxt.rs @@ -0,0 +1,13 @@ +// compile-flags: -Z unstable-options + +#![feature(rustc_private)] +#![deny(rustc::span_use_eq_ctxt)] + +extern crate rustc_span; +use rustc_span::Span; + +pub fn f(s: Span, t: Span) -> bool { + s.ctxt() == t.ctxt() //~ ERROR use of span ctxt +} + +fn main() {} -- cgit 1.4.1-3-g733a5 From 5895102c4dab67e7962bd76e1204bcf0fab467c5 Mon Sep 17 00:00:00 2001 From: Arthur Lafrance Date: Mon, 16 Oct 2023 01:05:11 -0700 Subject: debug Span::ctxt() call detection --- compiler/rustc_hir_typeck/src/callee.rs | 2 +- compiler/rustc_lint/messages.ftl | 2 +- compiler/rustc_lint/src/internal.rs | 23 ++++++---------------- compiler/rustc_lint/src/lints.rs | 4 +--- compiler/rustc_mir_transform/src/coverage/spans.rs | 2 +- compiler/rustc_span/src/span_encoding.rs | 2 +- compiler/rustc_span/src/symbol.rs | 2 +- .../ui-fulldeps/internal-lints/span_use_eq_ctxt.rs | 6 +++--- .../internal-lints/span_use_eq_ctxt.stderr | 14 +++++++++++++ 9 files changed, 29 insertions(+), 28 deletions(-) create mode 100644 tests/ui-fulldeps/internal-lints/span_use_eq_ctxt.stderr (limited to 'compiler/rustc_span/src/span_encoding.rs') diff --git a/compiler/rustc_hir_typeck/src/callee.rs b/compiler/rustc_hir_typeck/src/callee.rs index 512d73fc103..1c23ccd1579 100644 --- a/compiler/rustc_hir_typeck/src/callee.rs +++ b/compiler/rustc_hir_typeck/src/callee.rs @@ -650,7 +650,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { .sess .source_map() .is_multiline(call_expr.span.with_lo(callee_expr.span.hi())) - && call_expr.span.ctxt() == callee_expr.span.ctxt(); + && call_expr.span.eq_ctxt(callee_expr.span); if call_is_multiline { err.span_suggestion( callee_expr.span.shrink_to_hi(), diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 261451e530e..4c4d2933bf4 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -494,7 +494,7 @@ lint_renamed_lint = lint `{$name}` has been renamed to `{$replace}` lint_requested_level = requested on the command line with `{$level} {$lint_name}` -lint_span_use_eq_ctxt = use `eq_ctxt()` not `ctxt() == ctxt()` +lint_span_use_eq_ctxt = use `.eq_ctxt()` instead of `.ctxt() == .ctxt()` lint_supertrait_as_deref_target = `{$t}` implements `Deref` with supertrait `{$target_principal}` as target .label = target type is set here diff --git a/compiler/rustc_lint/src/internal.rs b/compiler/rustc_lint/src/internal.rs index c2aa768e945..34f241e8c8d 100644 --- a/compiler/rustc_lint/src/internal.rs +++ b/compiler/rustc_lint/src/internal.rs @@ -538,13 +538,9 @@ impl LateLintPass<'_> for BadOptAccess { } } -// some things i'm not sure about: -// * is Warn the right level? -// * the way i verify that the right method is being called (path + diag item check) - declare_tool_lint! { pub rustc::SPAN_USE_EQ_CTXT, - Warn, // is this the right level? + Allow, "Use of `==` with `Span::ctxt` rather than `Span::eq_ctxt`", report_in_external_macro: true } @@ -555,11 +551,7 @@ impl<'tcx> LateLintPass<'tcx> for SpanUseEqCtxt { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'_>) { if let ExprKind::Binary(BinOp { node: BinOpKind::Eq, .. }, lhs, rhs) = expr.kind { if is_span_ctxt_call(cx, lhs) && is_span_ctxt_call(cx, rhs) { - cx.emit_spanned_lint( - SPAN_USE_EQ_CTXT, - expr.span, - SpanUseEqCtxtDiag { msg: "fail" }, - ); + cx.emit_spanned_lint(SPAN_USE_EQ_CTXT, expr.span, SpanUseEqCtxtDiag); } } } @@ -567,13 +559,10 @@ impl<'tcx> LateLintPass<'tcx> for SpanUseEqCtxt { fn is_span_ctxt_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { match &expr.kind { - ExprKind::MethodCall(path, receiver, _, _) => { - path.ident.name.as_str() == "ctxt" - && cx - .typeck_results() - .type_dependent_def_id(receiver.hir_id) - .is_some_and(|did| cx.tcx.is_diagnostic_item(sym::Span, did)) - } + ExprKind::MethodCall(..) => cx + .typeck_results() + .type_dependent_def_id(expr.hir_id) + .is_some_and(|call_did| cx.tcx.is_diagnostic_item(sym::SpanCtxt, call_did)), _ => false, } diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index a02bee506df..4eaf8bbf5de 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -902,9 +902,7 @@ pub struct QueryInstability { #[derive(LintDiagnostic)] #[diag(lint_span_use_eq_ctxt)] -pub struct SpanUseEqCtxtDiag<'a> { - pub msg: &'a str, -} +pub struct SpanUseEqCtxtDiag; #[derive(LintDiagnostic)] #[diag(lint_tykind_kind)] diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 1d1be8f2492..f1a0f762041 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -404,7 +404,7 @@ impl<'a> CoverageSpansGenerator<'a> { let Some(visible_macro) = curr.visible_macro(self.body_span) else { return }; if let Some(prev) = &self.some_prev - && prev.expn_span.ctxt() == curr.expn_span.ctxt() + && prev.expn_span.eq_ctxt(curr.expn_span) { return; } diff --git a/compiler/rustc_span/src/span_encoding.rs b/compiler/rustc_span/src/span_encoding.rs index 7c7f8448c97..f7d17a267d6 100644 --- a/compiler/rustc_span/src/span_encoding.rs +++ b/compiler/rustc_span/src/span_encoding.rs @@ -75,7 +75,6 @@ use rustc_data_structures::fx::FxIndexSet; /// the dependency to the parent definition's span. This is performed /// using the callback `SPAN_TRACK` to access the query engine. /// -#[cfg_attr(not(test), rustc_diagnostic_item = "Span")] #[derive(Clone, Copy, Eq, PartialEq, Hash)] #[rustc_pass_by_value] pub struct Span { @@ -213,6 +212,7 @@ impl Span { /// This function is used as a fast path when decoding the full `SpanData` is not necessary. /// It's a cut-down version of `data_untracked`. + #[cfg_attr(not(test), rustc_diagnostic_item = "SpanCtxt")] #[inline] pub fn ctxt(self) -> SyntaxContext { if self.len_with_tag_or_marker != BASE_LEN_INTERNED_MARKER { diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 9598b2d0310..be8c65862dc 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -303,7 +303,7 @@ symbols! { SliceIndex, SliceIter, Some, - Span, + SpanCtxt, String, StructuralEq, StructuralPartialEq, diff --git a/tests/ui-fulldeps/internal-lints/span_use_eq_ctxt.rs b/tests/ui-fulldeps/internal-lints/span_use_eq_ctxt.rs index 5b4c59a2e8a..39980ee7c67 100644 --- a/tests/ui-fulldeps/internal-lints/span_use_eq_ctxt.rs +++ b/tests/ui-fulldeps/internal-lints/span_use_eq_ctxt.rs @@ -1,13 +1,13 @@ +// Test the `rustc::span_use_eq_ctxt` internal lint // compile-flags: -Z unstable-options #![feature(rustc_private)] #![deny(rustc::span_use_eq_ctxt)] +#![crate_type = "lib"] extern crate rustc_span; use rustc_span::Span; pub fn f(s: Span, t: Span) -> bool { - s.ctxt() == t.ctxt() //~ ERROR use of span ctxt + s.ctxt() == t.ctxt() //~ ERROR use `.eq_ctxt()` instead of `.ctxt() == .ctxt()` } - -fn main() {} diff --git a/tests/ui-fulldeps/internal-lints/span_use_eq_ctxt.stderr b/tests/ui-fulldeps/internal-lints/span_use_eq_ctxt.stderr new file mode 100644 index 00000000000..b33f6212545 --- /dev/null +++ b/tests/ui-fulldeps/internal-lints/span_use_eq_ctxt.stderr @@ -0,0 +1,14 @@ +error: use `.eq_ctxt()` instead of `.ctxt() == .ctxt()` + --> $DIR/span_use_eq_ctxt.rs:12:5 + | +LL | s.ctxt() == t.ctxt() + | ^^^^^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/span_use_eq_ctxt.rs:5:9 + | +LL | #![deny(rustc::span_use_eq_ctxt)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to previous error + -- cgit 1.4.1-3-g733a5