Skip to content

Make test harness lint about unnnameable tests. #114414

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

Merged
merged 1 commit into from
Aug 4, 2023
Merged
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
2 changes: 2 additions & 0 deletions compiler/rustc_builtin_macros/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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
22 changes: 22 additions & 0 deletions compiler/rustc_builtin_macros/src/test_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>` instead, and only call `assume_init` after initialization is done

Expand Down
78 changes: 1 addition & 77 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<hir::OwnerId>, // 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.
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
4 changes: 0 additions & 4 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
40 changes: 40 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -3403,6 +3442,7 @@ declare_lint_pass! {
UNKNOWN_CRATE_TYPES,
UNKNOWN_DIAGNOSTIC_ATTRIBUTES,
UNKNOWN_LINTS,
UNNAMEABLE_TEST_ITEMS,
UNNAMEABLE_TYPES,
UNREACHABLE_CODE,
UNREACHABLE_PATTERNS,
Expand Down