Skip to content

Commit

Permalink
Move expression check to LateLintPass
Browse files Browse the repository at this point in the history
Changes:
- Move from EarlyLintPass
- Fix entrypoint check with function path def_id.
  • Loading branch information
Vincent Dal Maso authored and flip1995 committed Aug 5, 2019
1 parent 4eab691 commit 77b21b6
Show file tree
Hide file tree
Showing 10 changed files with 72 additions and 32 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
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
4 changes: 3 additions & 1 deletion clippy_lints/src/lib.rs
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
55 changes: 31 additions & 24 deletions 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"
)
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/lintlist/mod.rs
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
@@ -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
@@ -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

5 changes: 2 additions & 3 deletions 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;
Expand All @@ -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);
Expand All @@ -28,4 +27,4 @@ fn panic(_info: &PanicInfo) -> ! {
}

#[lang = "eh_personality"]
extern fn eh_personality() {}
extern "C" fn eh_personality() {}
Empty file.
1 change: 1 addition & 0 deletions 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();
}
8 changes: 4 additions & 4 deletions 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

0 comments on commit 77b21b6

Please sign in to comment.