Skip to content
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

Add recursion check on main function #4203

Merged
merged 4 commits into from
Aug 5, 2019
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 309 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 310 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
4 changes: 4 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ pub mod let_if_seq;
pub mod lifetimes;
pub mod literal_representation;
pub mod loops;
pub mod main_recursion;
pub mod map_clone;
pub mod map_unit_fn;
pub mod matches;
Expand Down Expand Up @@ -473,6 +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_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);
Expand Down Expand Up @@ -760,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,
Expand Down Expand Up @@ -933,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,
Expand Down
62 changes: 62 additions & 0 deletions clippy_lints/src/main_recursion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
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;

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,
style,
"recursion using the entrypoint"
}

#[derive(Default)]
pub struct MainRecursion {
has_no_std_attr: bool,
}

impl_lint_pass!(MainRecursion => [MAIN_RECURSION]);

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_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 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,
func.span,
&format!("recursing into entrypoint `{}`", snippet(cx, func.span, "main")),
"consider using another function for this recursion"
)
}
}
}
}
9 changes: 8 additions & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;

// begin lint list, do not remove this comment, it’s used in `update_lints`
pub const ALL_LINTS: [Lint; 309] = [
pub const ALL_LINTS: [Lint; 310] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
Expand Down Expand Up @@ -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",
Expand Down
12 changes: 12 additions & 0 deletions tests/ui/crate_level_checks/entrypoint_recursion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// ignore-macos
// ignore-windows

#![feature(main)]

#[warn(clippy::main_recursion)]
#[allow(unconditional_recursion)]
#[main]
fn a() {
println!("Hello, World!");
a();
}
11 changes: 11 additions & 0 deletions tests/ui/crate_level_checks/entrypoint_recursion.stderr
Original file line number Diff line number Diff line change
@@ -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

33 changes: 33 additions & 0 deletions tests/ui/crate_level_checks/no_std_main_recursion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// ignore-macos
// ignore-windows

#![feature(lang_items, link_args, start, libc)]
#![link_args = "-nostartfiles"]
#![no_std]

use core::panic::PanicInfo;
use core::sync::atomic::{AtomicUsize, Ordering};

static N: AtomicUsize = AtomicUsize::new(0);

#[warn(clippy::main_recursion)]
#[start]
fn main(argc: isize, argv: *const *const u8) -> isize {
let x = N.load(Ordering::Relaxed);
N.store(x + 1, Ordering::Relaxed);

if x < 3 {
main(argc, argv);
}

0
}

#[allow(clippy::empty_loop)]
#[panic_handler]
fn panic(_info: &PanicInfo) -> ! {
loop {}
}

#[lang = "eh_personality"]
extern "C" fn eh_personality() {}
6 changes: 6 additions & 0 deletions tests/ui/crate_level_checks/std_main_recursion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#[warn(clippy::main_recursion)]
#[allow(unconditional_recursion)]
fn main() {
println!("Hello, World!");
main();
}
11 changes: 11 additions & 0 deletions tests/ui/crate_level_checks/std_main_recursion.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
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

error: aborting due to previous error