Rollup merge of #116787 - a-lafrance:span-internal-lint, r=oli-obk

Implement an internal lint encouraging use of `Span::eq_ctxt`

Adds a new Rustc internal lint that forbids use of `.ctxt() == .ctxt()` for spans, encouraging use of `.eq_ctxt()` instead (see #49509).

Also fixed a few violations of the lint in the Rustc codebase (a fun additional way I could test my code). Edit: MIR opt folks I believe that's why you're CC'ed, just a heads up.

Two things I'm not sure about:
1. The way I chose to detect calls to `Span::ctxt`. I know adding diagnostic items to methods is generally discouraged, but after some searching and experimenting I couldn't find anything else that worked, so I went with it. That said, I'm happy to implement something different if there's a better way out there. (For what it's worth, if there is a better way, it might be worth documenting in the rustc-dev-guide, which I'm happy to take care of)
2. The error message for the lint. Ideally it would probably be good to give some context as to why the suggestion is made (e.g. `rustc::default_hash_types` tells the user that it's because of performance), but I don't have that context so I couldn't put it in the error message. Happy to iterate on the error message based on feedback during review.

r? ``@oli-obk``

Author: matthiaskrgr

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(),

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()` instead of `.ctxt() == .ctxt()`
+
 lint_supertrait_as_deref_target = `{$t}` implements `Deref` with supertrait `{$target_principal}` as target
     .label = target type is set here
 

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,33 @@ impl LateLintPass<'_> for BadOptAccess {
         }
     }
 }
+
+declare_tool_lint! {
+    pub rustc::SPAN_USE_EQ_CTXT,
+    Allow,
+    "forbid uses of `==` with `Span::ctxt`, suggest `Span::eq_ctxt` instead",
+    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);
+            }
+        }
+    }
+}
+
+fn is_span_ctxt_call(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+    match &expr.kind {
+        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,
+    }
+}

compiler/rustc_lint/src/lib.rs

@@ -531,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
@@ -548,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),
         ],
     );
 }

compiler/rustc_lint/src/lints.rs

@@ -900,6 +900,10 @@ pub struct QueryInstability {
     pub query: Symbol,
 }
 
+#[derive(LintDiagnostic)]
+#[diag(lint_span_use_eq_ctxt)]
+pub struct SpanUseEqCtxtDiag;
+
 #[derive(LintDiagnostic)]
 #[diag(lint_tykind_kind)]
 pub struct TykindKind {

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;
         }

compiler/rustc_passes/src/loops.rs

@@ -231,7 +231,7 @@ impl<'a, 'hir> CheckLoopVisitor<'a, 'hir> {
             AsyncClosure(closure_span) => {
                 self.sess.emit_err(BreakInsideAsyncBlock { span, closure_span, name });
             }
-            UnlabeledBlock(block_span) if is_break && block_span.ctxt() == break_span.ctxt() => {
+            UnlabeledBlock(block_span) if is_break && block_span.eq_ctxt(break_span) => {
                 let suggestion = Some(OutsideLoopSuggestion { block_span, break_span });
                 self.sess.emit_err(OutsideLoop { span, name, is_break, suggestion });
             }

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`.
+    #[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 {

compiler/rustc_span/src/symbol.rs

@@ -303,6 +303,7 @@ symbols! {
         SliceIndex,
         SliceIter,
         Some,
+        SpanCtxt,
         String,
         StructuralEq,
         StructuralPartialEq,

src/tools/clippy/clippy_lints/src/dereference.rs

@@ -701,7 +701,7 @@ fn deref_method_same_type<'tcx>(result_ty: Ty<'tcx>, arg_ty: Ty<'tcx>) -> bool {
 
 fn in_postfix_position<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'tcx>) -> bool {
     if let Some(parent) = get_parent_expr(cx, e)
-        && parent.span.ctxt() == e.span.ctxt()
+        && parent.span.eq_ctxt(e.span)
     {
         match parent.kind {
             ExprKind::Call(child, _) | ExprKind::MethodCall(_, child, _, _) | ExprKind::Index(child, _, _)

src/tools/clippy/clippy_lints/src/entry.rs

@@ -241,7 +241,7 @@ fn try_parse_contains<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Optio
                 },
             ],
             _,
-        ) if key_span.ctxt() == expr.span.ctxt() => {
+        ) if key_span.eq_ctxt(expr.span) => {
             let id = cx.typeck_results().type_dependent_def_id(expr.hir_id)?;
             let expr = ContainsExpr {
                 negated,

src/tools/clippy/clippy_lints/src/formatting.rs

@@ -274,7 +274,7 @@ fn check_array(cx: &EarlyContext<'_>, expr: &Expr) {
         for element in array {
             if_chain! {
                 if let ExprKind::Binary(ref op, ref lhs, _) = element.kind;
-                if has_unary_equivalent(op.node) && lhs.span.ctxt() == op.span.ctxt();
+                if has_unary_equivalent(op.node) && lhs.span.eq_ctxt(op.span);
                 let space_span = lhs.span.between(op.span);
                 if let Some(space_snippet) = snippet_opt(cx, space_span);
                 let lint_span = lhs.span.with_lo(lhs.span.hi());

src/tools/clippy/clippy_lints/src/let_with_type_underscore.rs

@@ -31,7 +31,7 @@ impl LateLintPass<'_> for UnderscoreTyped {
             if !in_external_macro(cx.tcx.sess, local.span);
             if let Some(ty) = local.ty; // Ensure that it has a type defined
             if let TyKind::Infer = &ty.kind; // that type is '_'
-            if local.span.ctxt() == ty.span.ctxt();
+            if local.span.eq_ctxt(ty.span);
             then {
                 // NOTE: Using `is_from_proc_macro` on `init` will require that it's initialized,
                 // this doesn't. Alternatively, `WithSearchPat` can be implemented for `Ty`

src/tools/clippy/clippy_lints/src/manual_let_else.rs

@@ -59,7 +59,7 @@ impl<'tcx> QuestionMark {
             let Some(init) = local.init &&
             local.els.is_none() &&
             local.ty.is_none() &&
-            init.span.ctxt() == stmt.span.ctxt() &&
+            init.span.eq_ctxt(stmt.span) &&
             let Some(if_let_or_match) = IfLetOrMatch::parse(cx, init)
         {
             match if_let_or_match {

src/tools/clippy/clippy_lints/src/matches/collapsible_match.rs

@@ -57,7 +57,7 @@ fn check_arm<'tcx>(
                 }
             },
         };
-        if outer_pat.span.ctxt() == inner_scrutinee.span.ctxt();
+        if outer_pat.span.eq_ctxt(inner_scrutinee.span);
         // match expression must be a local binding
         // match <local> { .. }
         if let Some(binding_id) = path_to_local(peel_ref_operators(cx, inner_scrutinee));

src/tools/clippy/clippy_lints/src/matches/manual_utils.rs

@@ -119,7 +119,7 @@ where
     // it's being passed by value.
     let scrutinee = peel_hir_expr_refs(scrutinee).0;
     let (scrutinee_str, _) = snippet_with_context(cx, scrutinee.span, expr_ctxt, "..", &mut app);
-    let scrutinee_str = if scrutinee.span.ctxt() == expr.span.ctxt() && scrutinee.precedence().order() < PREC_POSTFIX {
+    let scrutinee_str = if scrutinee.span.eq_ctxt(expr.span) && scrutinee.precedence().order() < PREC_POSTFIX {
         format!("({scrutinee_str})")
     } else {
         scrutinee_str.into()
@@ -130,7 +130,7 @@ where
         if_chain! {
             if !some_expr.needs_unsafe_block;
             if let Some(func) = can_pass_as_func(cx, id, some_expr.expr);
-            if func.span.ctxt() == some_expr.expr.span.ctxt();
+            if func.span.eq_ctxt(some_expr.expr.span);
             then {
                 snippet_with_applicability(cx, func.span, "..", &mut app).into_owned()
             } else {

src/tools/clippy/clippy_lints/src/methods/map_unwrap_or.rs

@@ -56,7 +56,7 @@ pub(super) fn check<'tcx>(
         // lint, with note if neither arg is > 1 line and both map() and
         // unwrap_or_else() have the same span
         let multiline = map_snippet.lines().count() > 1 || unwrap_snippet.lines().count() > 1;
-        let same_span = map_arg.span.ctxt() == unwrap_arg.span.ctxt();
+        let same_span = map_arg.span.eq_ctxt(unwrap_arg.span);
         if same_span && !multiline {
             let var_snippet = snippet(cx, recv.span, "..");
             span_lint_and_sugg(

src/tools/clippy/clippy_lints/src/needless_question_mark.rs

@@ -125,7 +125,7 @@ fn check(cx: &LateContext<'_>, expr: &Expr<'_>) {
         if let ExprKind::Match(inner_expr_with_q, _, MatchSource::TryDesugar(_)) = &arg.kind;
         if let ExprKind::Call(called, [inner_expr]) = &inner_expr_with_q.kind;
         if let ExprKind::Path(QPath::LangItem(LangItem::TryTraitBranch, ..)) = &called.kind;
-        if expr.span.ctxt() == inner_expr.span.ctxt();
+        if expr.span.eq_ctxt(inner_expr.span);
         let expr_ty = cx.typeck_results().expr_ty(expr);
         let inner_ty = cx.typeck_results().expr_ty(inner_expr);
         if expr_ty == inner_ty;

src/tools/clippy/clippy_lints/src/non_octal_unix_permissions.rs

@@ -51,7 +51,7 @@ impl<'tcx> LateLintPass<'tcx> for NonOctalUnixPermissions {
                         || (path.ident.name == sym!(set_mode)
                             && cx.tcx.is_diagnostic_item(sym::FsPermissions, adt.did()));
                     if let ExprKind::Lit(_) = param.kind;
-                    if param.span.ctxt() == expr.span.ctxt();
+                    if param.span.eq_ctxt(expr.span);
 
                     then {
                         let Some(snip) = snippet_opt(cx, param.span) else {
@@ -70,7 +70,7 @@ impl<'tcx> LateLintPass<'tcx> for NonOctalUnixPermissions {
                     if let Some(def_id) = cx.qpath_res(path, func.hir_id).opt_def_id();
                     if match_def_path(cx, def_id, &paths::PERMISSIONS_FROM_MODE);
                     if let ExprKind::Lit(_) = param.kind;
-                    if param.span.ctxt() == expr.span.ctxt();
+                    if param.span.eq_ctxt(expr.span);
                     if let Some(snip) = snippet_opt(cx, param.span);
                     if !snip.starts_with("0o");
                     then {

src/tools/clippy/clippy_lints/src/redundant_async_block.rs

@@ -48,7 +48,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantAsyncBlock {
             let Some(body_expr) = desugar_async_block(cx, expr) &&
             let Some(expr) = desugar_await(peel_blocks(body_expr)) &&
             // The await prefix must not come from a macro as its content could change in the future.
-            expr.span.ctxt() == body_expr.span.ctxt() &&
+            expr.span.eq_ctxt(body_expr.span) &&
             // An async block does not have immediate side-effects from a `.await` point-of-view.
             (!expr.can_have_side_effects() || desugar_async_block(cx, expr).is_some()) &&
             let Some(shortened_span) = walk_span_to_context(expr.span, span.ctxt())

src/tools/clippy/clippy_lints/src/reference.rs

@@ -50,7 +50,7 @@ impl EarlyLintPass for DerefAddrOf {
         if_chain! {
             if let ExprKind::Unary(UnOp::Deref, ref deref_target) = e.kind;
             if let ExprKind::AddrOf(_, ref mutability, ref addrof_target) = without_parens(deref_target).kind;
-            if deref_target.span.ctxt() == e.span.ctxt();
+            if deref_target.span.eq_ctxt(e.span);
             if !addrof_target.span.from_expansion();
             then {
                 let mut applicability = Applicability::MachineApplicable;

src/tools/clippy/clippy_lints/src/suspicious_xor_used_as_pow.rs

@@ -33,7 +33,7 @@ impl LateLintPass<'_> for ConfusingXorAndPow {
         if !in_external_macro(cx.sess(), expr.span)
             && let ExprKind::Binary(op, left, right) = &expr.kind
             && op.node == BinOpKind::BitXor
-            && left.span.ctxt() == right.span.ctxt()
+            && left.span.eq_ctxt(right.span)
             && let ExprKind::Lit(lit_left) = &left.kind
             && let ExprKind::Lit(lit_right) = &right.kind
             && matches!(lit_right.node, LitKind::Int(..) | LitKind::Float(..))

src/tools/clippy/clippy_utils/src/macros.rs

@@ -245,7 +245,7 @@ impl<'a> PanicExpn<'a> {
             return None;
         };
         let result = match name {
-            "panic" if arg.span.ctxt() == expr.span.ctxt() => Self::Empty,
+            "panic" if arg.span.eq_ctxt(expr.span) => Self::Empty,
             "panic" | "panic_str" => Self::Str(arg),
             "panic_display" | "panic_cold_display" => {
                 let ExprKind::AddrOf(_, _, e) = &arg.kind else {

tests/ui-fulldeps/internal-lints/span_use_eq_ctxt.rs

@@ -0,0 +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 `.eq_ctxt()` instead of `.ctxt() == .ctxt()`
+}

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
+