Auto merge of #114414 - cjgillot:early-unnameable-test, r=petrochenkov

Make test harness lint about unnnameable tests.

Implementation of https://github.com/rust-lang/rust/pull/113734#discussion_r1283073418

About the options suggested in https://github.com/rust-lang/rust/issues/36629#issuecomment-404753945: adding this case to unused_attribute was just more complicated. I'll try to understand a bit more what you had in mind in https://github.com/rust-lang/rfcs/pull/2471#issuecomment-397241123

This was just simpler to do in a standalone PR. I'll remove the corresponding changes from https://github.com/rust-lang/rust/pull/113734 later.

r? `@petrochenkov`
This commit is contained in:
bors 2023-08-04 14:13:11 +00:00
commit ec5b882c2f
7 changed files with 65 additions and 85 deletions

View File

@ -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

View File

@ -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 {

View File

@ -130,8 +130,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

View File

@ -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<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.

View File

@ -189,8 +189,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`.

View File

@ -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 {

View File

@ -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,