From 77b21b644f2072768d24dee331494b082ea133d1 Mon Sep 17 00:00:00 2001 From: Vincent Dal Maso Date: Mon, 17 Jun 2019 17:36:42 +0200 Subject: [PATCH] Move expression check to LateLintPass Changes: - Move from EarlyLintPass - Fix entrypoint check with function path def_id. --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 4 +- clippy_lints/src/main_recursion.rs | 55 +++++++++++-------- src/lintlist/mod.rs | 7 +++ .../entrypoint_recursion.rs | 12 ++++ .../entrypoint_recursion.stderr | 11 ++++ .../no_std_main_recursion.rs | 5 +- .../no_std_main_recursion.stderr | 0 .../crate_level_checks/std_main_recursion.rs | 1 + .../std_main_recursion.stderr | 8 +-- 10 files changed, 72 insertions(+), 32 deletions(-) create mode 100644 tests/ui/crate_level_checks/entrypoint_recursion.rs create mode 100644 tests/ui/crate_level_checks/entrypoint_recursion.stderr delete mode 100644 tests/ui/crate_level_checks/no_std_main_recursion.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 089897811..e4a1a602c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1000,6 +1000,7 @@ Released 2018-09-13 [`let_unit_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#let_unit_value [`linkedlist`]: https://rust-lang.github.io/rust-clippy/master/index.html#linkedlist [`logic_bug`]: https://rust-lang.github.io/rust-clippy/master/index.html#logic_bug +[`main_recursion`]: https://rust-lang.github.io/rust-clippy/master/index.html#main_recursion [`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy [`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap [`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 325195caa..258be38e4 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -474,7 +474,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box types::LetUnitValue); reg.register_late_lint_pass(box types::UnitCmp); reg.register_late_lint_pass(box loops::Loops); - reg.register_early_lint_pass(box main_recursion::MainRecursion::new()); + reg.register_late_lint_pass(box main_recursion::MainRecursion::default()); reg.register_late_lint_pass(box lifetimes::Lifetimes); reg.register_late_lint_pass(box entry::HashMapPass); reg.register_late_lint_pass(box ranges::Ranges); @@ -762,6 +762,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { loops::WHILE_IMMUTABLE_CONDITION, loops::WHILE_LET_LOOP, loops::WHILE_LET_ON_ITERATOR, + main_recursion::MAIN_RECURSION, map_clone::MAP_CLONE, map_unit_fn::OPTION_MAP_UNIT_FN, map_unit_fn::RESULT_MAP_UNIT_FN, @@ -935,6 +936,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { loops::FOR_KV_MAP, loops::NEEDLESS_RANGE_LOOP, loops::WHILE_LET_ON_ITERATOR, + main_recursion::MAIN_RECURSION, map_clone::MAP_CLONE, matches::MATCH_BOOL, matches::MATCH_OVERLAPPING_ARM, diff --git a/clippy_lints/src/main_recursion.rs b/clippy_lints/src/main_recursion.rs index 9ef4de21f..88f1e685c 100644 --- a/clippy_lints/src/main_recursion.rs +++ b/clippy_lints/src/main_recursion.rs @@ -1,53 +1,60 @@ - -use syntax::ast::{Crate, Expr, ExprKind}; -use syntax::symbol::sym; -use rustc::lint::{LintArray, LintPass, EarlyLintPass, EarlyContext}; +use rustc::hir::{Crate, Expr, ExprKind, QPath}; +use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::{declare_tool_lint, impl_lint_pass}; +use syntax::symbol::sym; +use crate::utils::{is_entrypoint_fn, snippet, span_help_and_lint}; use if_chain::if_chain; -use crate::utils::span_help_and_lint; declare_clippy_lint! { + /// **What it does:** Checks for recursion using the entrypoint. + /// + /// **Why is this bad?** Apart from special setups (which we could detect following attributes like #![no_std]), + /// recursing into main() seems like an unintuitive antipattern we should be able to detect. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```no_run + /// fn main() { + /// main(); + /// } + /// ``` pub MAIN_RECURSION, - pedantic, - "function named `foo`, which is not a descriptive name" + style, + "recursion using the entrypoint" } +#[derive(Default)] pub struct MainRecursion { - has_no_std_attr: bool + has_no_std_attr: bool, } impl_lint_pass!(MainRecursion => [MAIN_RECURSION]); -impl MainRecursion { - pub fn new() -> MainRecursion { - MainRecursion { - has_no_std_attr: false - } - } -} - -impl EarlyLintPass for MainRecursion { - fn check_crate(&mut self, _: &EarlyContext<'_>, krate: &Crate) { +impl LateLintPass<'_, '_> for MainRecursion { + fn check_crate(&mut self, _: &LateContext<'_, '_>, krate: &Crate) { self.has_no_std_attr = krate.attrs.iter().any(|attr| attr.path == sym::no_std); } - fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { + fn check_expr_post(&mut self, cx: &LateContext<'_, '_>, expr: &Expr) { if self.has_no_std_attr { return; } if_chain! { if let ExprKind::Call(func, _) = &expr.node; - if let ExprKind::Path(_, path) = &func.node; - if *path == sym::main; + if let ExprKind::Path(path) = &func.node; + if let QPath::Resolved(_, path) = &path; + if let Some(def_id) = path.res.opt_def_id(); + if is_entrypoint_fn(cx, def_id); then { span_help_and_lint( cx, MAIN_RECURSION, - expr.span, - "You are recursing into main()", - "Consider using another function for this recursion" + func.span, + &format!("recursing into entrypoint `{}`", snippet(cx, func.span, "main")), + "consider using another function for this recursion" ) } } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index aa4576640..802ba60b9 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -917,6 +917,13 @@ pub const ALL_LINTS: [Lint; 309] = [ deprecation: None, module: "booleans", }, + Lint { + name: "main_recursion", + group: "style", + desc: "recursion using the entrypoint", + deprecation: None, + module: "main_recursion", + }, Lint { name: "manual_memcpy", group: "perf", diff --git a/tests/ui/crate_level_checks/entrypoint_recursion.rs b/tests/ui/crate_level_checks/entrypoint_recursion.rs new file mode 100644 index 000000000..995787c53 --- /dev/null +++ b/tests/ui/crate_level_checks/entrypoint_recursion.rs @@ -0,0 +1,12 @@ +// ignore-macos +// ignore-windows + +#![feature(main)] + +#[warn(clippy::main_recursion)] +#[allow(unconditional_recursion)] +#[main] +fn a() { + println!("Hello, World!"); + a(); +} diff --git a/tests/ui/crate_level_checks/entrypoint_recursion.stderr b/tests/ui/crate_level_checks/entrypoint_recursion.stderr new file mode 100644 index 000000000..f52fc949f --- /dev/null +++ b/tests/ui/crate_level_checks/entrypoint_recursion.stderr @@ -0,0 +1,11 @@ +error: recursing into entrypoint `a` + --> $DIR/entrypoint_recursion.rs:11:5 + | +LL | a(); + | ^ + | + = note: `-D clippy::main-recursion` implied by `-D warnings` + = help: consider using another function for this recursion + +error: aborting due to previous error + diff --git a/tests/ui/crate_level_checks/no_std_main_recursion.rs b/tests/ui/crate_level_checks/no_std_main_recursion.rs index 857af96a0..4d19f38e2 100644 --- a/tests/ui/crate_level_checks/no_std_main_recursion.rs +++ b/tests/ui/crate_level_checks/no_std_main_recursion.rs @@ -1,5 +1,5 @@ #![feature(lang_items, link_args, start, libc)] -#![link_args="-nostartfiles"] +#![link_args = "-nostartfiles"] #![no_std] use core::panic::PanicInfo; @@ -8,7 +8,6 @@ use core::sync::atomic::{AtomicUsize, Ordering}; static N: AtomicUsize = AtomicUsize::new(0); #[warn(clippy::main_recursion)] -#[allow(unconditional_recursion)] #[start] fn main(argc: isize, argv: *const *const u8) -> isize { let x = N.load(Ordering::Relaxed); @@ -28,4 +27,4 @@ fn panic(_info: &PanicInfo) -> ! { } #[lang = "eh_personality"] -extern fn eh_personality() {} +extern "C" fn eh_personality() {} diff --git a/tests/ui/crate_level_checks/no_std_main_recursion.stderr b/tests/ui/crate_level_checks/no_std_main_recursion.stderr deleted file mode 100644 index e69de29bb..000000000 diff --git a/tests/ui/crate_level_checks/std_main_recursion.rs b/tests/ui/crate_level_checks/std_main_recursion.rs index e7689ffb7..89ff66099 100644 --- a/tests/ui/crate_level_checks/std_main_recursion.rs +++ b/tests/ui/crate_level_checks/std_main_recursion.rs @@ -1,5 +1,6 @@ #[warn(clippy::main_recursion)] #[allow(unconditional_recursion)] fn main() { + println!("Hello, World!"); main(); } diff --git a/tests/ui/crate_level_checks/std_main_recursion.stderr b/tests/ui/crate_level_checks/std_main_recursion.stderr index 7979010ea..0a260f9d2 100644 --- a/tests/ui/crate_level_checks/std_main_recursion.stderr +++ b/tests/ui/crate_level_checks/std_main_recursion.stderr @@ -1,11 +1,11 @@ -error: You are recursing into main() - --> $DIR/std_main_recursion.rs:4:5 +error: recursing into entrypoint `main` + --> $DIR/std_main_recursion.rs:5:5 | LL | main(); - | ^^^^^^ + | ^^^^ | = note: `-D clippy::main-recursion` implied by `-D warnings` - = help: Consider using another function for this recursion + = help: consider using another function for this recursion error: aborting due to previous error