Move expression check to LateLintPass

Changes:
- Move from EarlyLintPass
- Fix entrypoint check with function path def_id.

Author: Vincent Dal Maso

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

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,

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"
                 )
             }
         }

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

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();
+}

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
+

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() {}

tests/ui/crate_level_checks/no_std_main_recursion.stderr

@@ -1,5 +1,6 @@
 #[warn(clippy::main_recursion)]
 #[allow(unconditional_recursion)]
 fn main() {
+    println!("Hello, World!");
     main();
 }

tests/ui/crate_level_checks/std_main_recursion.rs

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