From 2a0a1f918dabe3a1206d40ab9e40a3dafe44c1f0 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Thu, 3 Aug 2023 12:43:42 +0000 Subject: [PATCH] Make test harness lint about unnnameable tests. --- compiler/rustc_builtin_macros/messages.ftl | 2 + .../rustc_builtin_macros/src/test_harness.rs | 22 ++++++ compiler/rustc_lint/messages.ftl | 2 - compiler/rustc_lint/src/builtin.rs | 78 +------------------ compiler/rustc_lint/src/lib.rs | 2 - compiler/rustc_lint/src/lints.rs | 4 - compiler/rustc_lint_defs/src/builtin.rs | 40 ++++++++++ 7 files changed, 65 insertions(+), 85 deletions(-) diff --git a/compiler/rustc_builtin_macros/messages.ftl b/compiler/rustc_builtin_macros/messages.ftl index 79fae7f92f1..8d8db4c13fa 100644 --- a/compiler/rustc_builtin_macros/messages.ftl +++ b/compiler/rustc_builtin_macros/messages.ftl @@ -227,3 +227,5 @@ builtin_macros_unexpected_lit = expected path to a trait, found literal .label = not a trait .str_lit = try using `#[derive({$sym})]` .other = for example, write `#[derive(Debug)]` for `Debug` + +builtin_macros_unnameable_test_items = cannot test inner items diff --git a/compiler/rustc_builtin_macros/src/test_harness.rs b/compiler/rustc_builtin_macros/src/test_harness.rs index 81b618548da..507b74c2437 100644 --- a/compiler/rustc_builtin_macros/src/test_harness.rs +++ b/compiler/rustc_builtin_macros/src/test_harness.rs @@ -4,10 +4,12 @@ use rustc_ast as ast; use rustc_ast::entry::EntryPointType; use rustc_ast::mut_visit::{ExpectOne, *}; use rustc_ast::ptr::P; +use rustc_ast::visit::{walk_item, Visitor}; use rustc_ast::{attr, ModKind}; use rustc_expand::base::{ExtCtxt, ResolverExpand}; use rustc_expand::expand::{AstFragment, ExpansionConfig}; use rustc_feature::Features; +use rustc_session::lint::builtin::UNNAMEABLE_TEST_ITEMS; use rustc_session::Session; use rustc_span::hygiene::{AstPass, SyntaxContext, Transparency}; use rustc_span::symbol::{sym, Ident, Symbol}; @@ -137,11 +139,31 @@ impl<'a> MutVisitor for TestHarnessGenerator<'a> { let prev_tests = mem::take(&mut self.tests); noop_visit_item_kind(&mut item.kind, self); self.add_test_cases(item.id, span, prev_tests); + } else { + // But in those cases, we emit a lint to warn the user of these missing tests. + walk_item(&mut InnerItemLinter { sess: self.cx.ext_cx.sess }, &item); } smallvec![P(item)] } } +struct InnerItemLinter<'a> { + sess: &'a Session, +} + +impl<'a> Visitor<'a> for InnerItemLinter<'_> { + fn visit_item(&mut self, i: &'a ast::Item) { + if let Some(attr) = attr::find_by_name(&i.attrs, sym::rustc_test_marker) { + self.sess.parse_sess.buffer_lint( + UNNAMEABLE_TEST_ITEMS, + attr.span, + i.id, + crate::fluent_generated::builtin_macros_unnameable_test_items, + ); + } + } +} + // Beware, this is duplicated in librustc_passes/entry.rs (with // `rustc_hir::Item`), so make sure to keep them in sync. fn entry_point_type(item: &ast::Item, depth: usize) -> EntryPointType { diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index f482e3d7c12..2ad91cdb444 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -127,8 +127,6 @@ lint_builtin_unexpected_cli_config_name = unexpected `{$name}` as condition name lint_builtin_unexpected_cli_config_value = unexpected condition value `{$value}` for condition name `{$name}` .help = was set with `--cfg` but isn't in the `--check-cfg` expected values -lint_builtin_unnameable_test_items = cannot test inner items - lint_builtin_unpermitted_type_init_label = this code causes undefined behavior when executed lint_builtin_unpermitted_type_init_label_suggestion = help: use `MaybeUninit` instead, and only call `assume_init` after initialization is done diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index e6917f4b2d3..cabc6bd96e9 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -35,7 +35,7 @@ use crate::{ BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds, BuiltinTypeAliasGenericBounds, BuiltinTypeAliasGenericBoundsSuggestion, BuiltinTypeAliasWhereClause, BuiltinUnexpectedCliConfigName, BuiltinUnexpectedCliConfigValue, - BuiltinUngatedAsyncFnTrackCaller, BuiltinUnnameableTestItems, BuiltinUnpermittedTypeInit, + BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub, BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub, BuiltinWhileTrue, SuggestChangingAssocTypes, @@ -1770,82 +1770,6 @@ impl EarlyLintPass for EllipsisInclusiveRangePatterns { } } -declare_lint! { - /// The `unnameable_test_items` lint detects [`#[test]`][test] functions - /// that are not able to be run by the test harness because they are in a - /// position where they are not nameable. - /// - /// [test]: https://doc.rust-lang.org/reference/attributes/testing.html#the-test-attribute - /// - /// ### Example - /// - /// ```rust,test - /// fn main() { - /// #[test] - /// fn foo() { - /// // This test will not fail because it does not run. - /// assert_eq!(1, 2); - /// } - /// } - /// ``` - /// - /// {{produces}} - /// - /// ### Explanation - /// - /// In order for the test harness to run a test, the test function must be - /// located in a position where it can be accessed from the crate root. - /// This generally means it must be defined in a module, and not anywhere - /// else such as inside another function. The compiler previously allowed - /// this without an error, so a lint was added as an alert that a test is - /// not being used. Whether or not this should be allowed has not yet been - /// decided, see [RFC 2471] and [issue #36629]. - /// - /// [RFC 2471]: https://github.com/rust-lang/rfcs/pull/2471#issuecomment-397414443 - /// [issue #36629]: https://github.com/rust-lang/rust/issues/36629 - UNNAMEABLE_TEST_ITEMS, - Warn, - "detects an item that cannot be named being marked as `#[test_case]`", - report_in_external_macro -} - -pub struct UnnameableTestItems { - boundary: Option, // Id of the item under which things are not nameable - items_nameable: bool, -} - -impl_lint_pass!(UnnameableTestItems => [UNNAMEABLE_TEST_ITEMS]); - -impl UnnameableTestItems { - pub fn new() -> Self { - Self { boundary: None, items_nameable: true } - } -} - -impl<'tcx> LateLintPass<'tcx> for UnnameableTestItems { - fn check_item(&mut self, cx: &LateContext<'_>, it: &hir::Item<'_>) { - if self.items_nameable { - if let hir::ItemKind::Mod(..) = it.kind { - } else { - self.items_nameable = false; - self.boundary = Some(it.owner_id); - } - return; - } - - let attrs = cx.tcx.hir().attrs(it.hir_id()); - if let Some(attr) = attr::find_by_name(attrs, sym::rustc_test_marker) { - cx.emit_spanned_lint(UNNAMEABLE_TEST_ITEMS, attr.span, BuiltinUnnameableTestItems); - } - } - - fn check_item_post(&mut self, _cx: &LateContext<'_>, it: &hir::Item<'_>) { - if !self.items_nameable && self.boundary == Some(it.owner_id) { - self.items_nameable = true; - } - } -} - declare_lint! { /// The `keyword_idents` lint detects edition keywords being used as an /// identifier. diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 96fd3ccf774..d86c90efe86 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -188,8 +188,6 @@ late_lint_methods!( [ pub BuiltinCombinedLateLintPass, [ - // Tracks state across modules - UnnameableTestItems: UnnameableTestItems::new(), // Tracks attributes of parents MissingDoc: MissingDoc::new(), // Builds a global list of all impls of `Debug`. diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index a6a48bf4ffa..1c08c0e44f8 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -370,10 +370,6 @@ pub enum BuiltinEllipsisInclusiveRangePatternsLint { }, } -#[derive(LintDiagnostic)] -#[diag(lint_builtin_unnameable_test_items)] -pub struct BuiltinUnnameableTestItems; - #[derive(LintDiagnostic)] #[diag(lint_builtin_keyword_idents)] pub struct BuiltinKeywordIdents { diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 7e3b6e9e218..1920a11ced4 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -2846,6 +2846,45 @@ declare_lint! { }; } +declare_lint! { + /// The `unnameable_test_items` lint detects [`#[test]`][test] functions + /// that are not able to be run by the test harness because they are in a + /// position where they are not nameable. + /// + /// [test]: https://doc.rust-lang.org/reference/attributes/testing.html#the-test-attribute + /// + /// ### Example + /// + /// ```rust,test + /// fn main() { + /// #[test] + /// fn foo() { + /// // This test will not fail because it does not run. + /// assert_eq!(1, 2); + /// } + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// In order for the test harness to run a test, the test function must be + /// located in a position where it can be accessed from the crate root. + /// This generally means it must be defined in a module, and not anywhere + /// else such as inside another function. The compiler previously allowed + /// this without an error, so a lint was added as an alert that a test is + /// not being used. Whether or not this should be allowed has not yet been + /// decided, see [RFC 2471] and [issue #36629]. + /// + /// [RFC 2471]: https://github.com/rust-lang/rfcs/pull/2471#issuecomment-397414443 + /// [issue #36629]: https://github.com/rust-lang/rust/issues/36629 + pub UNNAMEABLE_TEST_ITEMS, + Warn, + "detects an item that cannot be named being marked as `#[test_case]`", + report_in_external_macro +} + declare_lint! { /// The `useless_deprecated` lint detects deprecation attributes with no effect. /// @@ -3403,6 +3442,7 @@ declare_lint_pass! { UNKNOWN_CRATE_TYPES, UNKNOWN_DIAGNOSTIC_ATTRIBUTES, UNKNOWN_LINTS, + UNNAMEABLE_TEST_ITEMS, UNNAMEABLE_TYPES, UNREACHABLE_CODE, UNREACHABLE_PATTERNS,