Closes #259: Adds new lint items_before_use

CHANGELOG.md

@@ -5895,6 +5895,7 @@ Released 2018-09-13
 [`is_digit_ascii_radix`]: https://rust-lang.github.io/rust-clippy/master/index.html#is_digit_ascii_radix
 [`items_after_statements`]: https://rust-lang.github.io/rust-clippy/master/index.html#items_after_statements
 [`items_after_test_module`]: https://rust-lang.github.io/rust-clippy/master/index.html#items_after_test_module
+[`items_before_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#items_before_use
 [`iter_cloned_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_cloned_collect
 [`iter_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_count
 [`iter_filter_is_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_filter_is_ok
@@ -6555,6 +6556,7 @@ Released 2018-09-13
 [`source-item-ordering`]: https://doc.rust-lang.org/clippy/lint_configuration.html#source-item-ordering
 [`stack-size-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#stack-size-threshold
 [`standard-macro-braces`]: https://doc.rust-lang.org/clippy/lint_configuration.html#standard-macro-braces
+[`strict-order-of-use`]: https://doc.rust-lang.org/clippy/lint_configuration.html#strict-order-of-use
 [`struct-field-name-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#struct-field-name-threshold
 [`suppress-restriction-lint-in-const`]: https://doc.rust-lang.org/clippy/lint_configuration.html#suppress-restriction-lint-in-const
 [`too-large-for-stack`]: https://doc.rust-lang.org/clippy/lint_configuration.html#too-large-for-stack

book/src/lint_configuration.md

@@ -957,6 +957,16 @@ could be used with a full path two `MacroMatcher`s have to be added one with the
 * [`nonstandard_macro_braces`](https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces)
 
 
+## `strict-order-of-use`
+Makes the lint strict, use statements must precede mod and extern crate statements too. (Stylistic Choice)
+
+**Default Value:** `false`
+
+---
+**Affected lints:**
+* [`items_before_use`](https://rust-lang.github.io/rust-clippy/master/index.html#items_before_use)
+
+
 ## `struct-field-name-threshold`
 The minimum number of struct fields for the lints about field names to trigger
 

clippy_config/src/conf.rs

@@ -804,6 +804,9 @@ define_Conf! {
     /// `crate_name::macro_name` and one with just the macro name.
     #[lints(nonstandard_macro_braces)]
     standard_macro_braces: Vec<MacroMatcher> = Vec::new(),
+    /// Makes the lint strict, use statements must precede mod and extern crate statements too. (Stylistic Choice)
+    #[lints(items_before_use)]
+    strict_order_of_use: bool = false,
     /// The minimum number of struct fields for the lints about field names to trigger
     #[lints(struct_field_names)]
     struct_field_name_threshold: u64 = 3,

clippy_lints/src/booleans.rs

@@ -1,3 +1,4 @@
+#![allow(clippy::items_before_use)]
 use clippy_config::Conf;
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
 use clippy_utils::msrvs::{self, Msrv};

clippy_lints/src/declared_lints.rs

@@ -233,6 +233,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::item_name_repetitions::STRUCT_FIELD_NAMES_INFO,
     crate::items_after_statements::ITEMS_AFTER_STATEMENTS_INFO,
     crate::items_after_test_module::ITEMS_AFTER_TEST_MODULE_INFO,
+    crate::items_before_use::ITEMS_BEFORE_USE_INFO,
     crate::iter_not_returning_iterator::ITER_NOT_RETURNING_ITERATOR_INFO,
     crate::iter_over_hash_type::ITER_OVER_HASH_TYPE_INFO,
     crate::iter_without_into_iter::INTO_ITER_WITHOUT_ITER_INFO,

clippy_lints/src/infinite_iter.rs

@@ -1,3 +1,4 @@
+#![allow(clippy::items_before_use)]
 use clippy_utils::diagnostics::span_lint;
 use clippy_utils::ty::{get_type_diagnostic_name, implements_trait};
 use clippy_utils::{higher, sym};

clippy_lints/src/items_before_use.rs

@@ -0,0 +1,153 @@
+use clippy_config::Conf;
+use clippy_utils::diagnostics::span_lint_and_note;
+use clippy_utils::sym;
+use rustc_hir::{HirId, Item, ItemKind, Mod};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::impl_lint_pass;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks that `use` statements come before all other items (functions, structs, constants, etc.)
+    /// at the module level, exceptions beinng `mod` and `extern crate` statements before `use` (confgiurable).
+    /// Ignores all use statements in cfg blocks as it's a common pattern.
+    ///
+    /// ### Why is this bad?
+    /// Having `use` statements scattered throughout a module makes it harder to see all imports
+    /// at a glance. Keeping imports grouped near the top (with `mod` and `extern crate`)
+    /// improves code organization and readability.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// mod my_module {
+    ///     fn rand() {}
+    /// };
+    ///
+    /// fn foo() {}
+    /// use std::collections::HashMap;
+    ///
+    /// const VALUE: i32 = 32;
+    /// use std::vec::Vec;
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// mod my_module {
+    ///     fn rand2() {}
+    /// };
+    /// use std::collections::HashMap;
+    /// use std::vec::Vec;
+    ///
+    /// fn foo() {}
+    /// const VALUE: i32 = 32;
+    /// ```
+    #[clippy::version = "1.89.0"]
+    pub ITEMS_BEFORE_USE,
+    style,
+    "checks if module level `use` statements precede all other items"
+}
+
+// check function to ignore cfg blocks (allowed in both stylistic and pedantic levels)
+fn is_cfg(cx: &LateContext<'_>, item: &Item<'_>) -> bool {
+    let mut def = item.owner_id.def_id;
+
+    loop {
+        let attrs = cx.tcx.hir_attrs(item.hir_id());
+        if attrs.iter().any(|attr| {
+            attr.has_any_name(&[
+                sym::cfg,
+                sym::cfg_attr,
+                sym::cfg_eval,
+                sym::cfg_hide,
+                sym::cfg_panic,
+                sym::cfg_trace,
+                sym::cfg_doctest,
+                sym::cfg_version,
+                sym::cfg_sanitize,
+                sym::cfg_fmt_debug,
+                sym::cfg_ub_checks,
+                sym::cfg_accessible,
+                sym::cfg_attr_multi,
+                sym::cfg_attr_trace,
+                sym::cfg_target_abi,
+                sym::cfg_sanitizer_cfi,
+                sym::cfg_target_vendor,
+                sym::cfg_target_compact,
+                sym::cfg_target_feature,
+                sym::cfg_contract_checks,
+                sym::cfg_overflow_checks,
+                sym::cfg_boolean_literals,
+                sym::cfg_relocation_model,
+                sym::cfg_target_has_atomic,
+                sym::cfg_emscripten_wasm_eh,
+                sym::cfg_target_thread_local,
+                sym::cfg_target_has_reliable_f16_f128,
+                sym::cfg_target_has_atomic_equal_alignment,
+                sym::doc_cfg,
+                sym::doc_cfg_hide,
+                sym::doc_auto_cfg,
+                sym::link_cfg,
+            ])
+        }) {
+            return true;
+        }
+        match cx.tcx.opt_parent(def.to_def_id()) {
+            Some(parent) => def = parent.expect_local(),
+            None => break false,
+        }
+    }
+}
+
+pub struct ItemsBeforeUse {
+    pub strict_order_of_use: bool,
+}
+
+impl ItemsBeforeUse {
+    pub fn new(conf: &'static Conf) -> Self {
+        Self {
+            strict_order_of_use: conf.strict_order_of_use,
+        }
+    }
+}
+
+impl_lint_pass!(ItemsBeforeUse => [ITEMS_BEFORE_USE]);
+
+impl<'tcx> LateLintPass<'tcx> for ItemsBeforeUse {
+    fn check_mod(&mut self, cx: &LateContext<'tcx>, module: &'tcx Mod<'tcx>, _hir_id: HirId) {
+        let mut saw_non_use = false;
+        let mut saw_mod_or_extern = false;
+        for item_ids in module.item_ids {
+            let item = cx.tcx.hir_item(*item_ids);
+
+            if is_cfg(cx, item) {
+                continue;
+            }
+
+            match item.kind {
+                ItemKind::Use(..) => {
+                    // strict mode (pedantic) will lint for mod and extern crare too
+                    if (saw_mod_or_extern || saw_non_use) && self.strict_order_of_use {
+                        span_lint_and_note(
+                            cx,
+                            ITEMS_BEFORE_USE,
+                            item.span,
+                            "strict_order_of_use enabled: use statements should precede all other items including mod and extern crate statements",
+                            None,
+                            "Move this statement to the top of the module",
+                        );
+                    } else if saw_non_use {
+                        // stylistic (on by default) will only lint for non-mod/extern items
+                        span_lint_and_note(
+                            cx,
+                            ITEMS_BEFORE_USE,
+                            item.span,
+                            "module level use statements should precede all other items",
+                            None,
+                            "consider moving this statement to the top of the module",
+                        );
+                    }
+                },
+                ItemKind::Mod(..) | ItemKind::ExternCrate(..) => saw_mod_or_extern = true,
+                _ => saw_non_use = true,
+            }
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -17,7 +17,8 @@
     clippy::must_use_candidate,
     rustc::diagnostic_outside_of_impl,
     rustc::untranslatable_diagnostic,
-    clippy::literal_string_with_formatting_args
+    clippy::literal_string_with_formatting_args,
+    clippy::items_before_use
 )]
 #![warn(
     trivial_casts,
@@ -184,6 +185,7 @@ mod invalid_upcast_comparisons;
 mod item_name_repetitions;
 mod items_after_statements;
 mod items_after_test_module;
+mod items_before_use;
 mod iter_not_returning_iterator;
 mod iter_over_hash_type;
 mod iter_without_into_iter;
@@ -951,5 +953,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(cloned_ref_to_slice_refs::ClonedRefToSliceRefs::new(conf)));
     store.register_late_pass(|_| Box::new(infallible_try_from::InfallibleTryFrom));
     store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny));
+    store.register_late_pass(move |_| Box::new(items_before_use::ItemsBeforeUse::new(conf)));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

tests/ui-internal/check_clippy_version_attribute.rs

@@ -1,5 +1,6 @@
 #![feature(rustc_private)]
 #![deny(clippy::invalid_clippy_version_attribute, clippy::missing_clippy_version_attribute)]
+#![allow(clippy::items_before_use)]
 
 #[macro_use]
 extern crate rustc_middle;

tests/ui-internal/check_clippy_version_attribute.stderr

@@ -1,5 +1,5 @@
 error: this item has an invalid `clippy::version` attribute
-  --> tests/ui-internal/check_clippy_version_attribute.rs:40:1
+  --> tests/ui-internal/check_clippy_version_attribute.rs:41:1
    |
 LL | / declare_tool_lint! {
 LL | |
@@ -19,7 +19,7 @@ LL | #![deny(clippy::invalid_clippy_version_attribute, clippy::missing_clippy_ve
    = note: this error originates in the macro `$crate::declare_tool_lint` which comes from the expansion of the macro `declare_tool_lint` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: this item has an invalid `clippy::version` attribute
-  --> tests/ui-internal/check_clippy_version_attribute.rs:49:1
+  --> tests/ui-internal/check_clippy_version_attribute.rs:50:1
    |
 LL | / declare_tool_lint! {
 LL | |
@@ -34,7 +34,7 @@ LL | | }
    = note: this error originates in the macro `$crate::declare_tool_lint` which comes from the expansion of the macro `declare_tool_lint` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: this lint is missing the `clippy::version` attribute or version value
-  --> tests/ui-internal/check_clippy_version_attribute.rs:61:1
+  --> tests/ui-internal/check_clippy_version_attribute.rs:62:1
    |
 LL | / declare_tool_lint! {
 LL | |
@@ -54,7 +54,7 @@ LL | #![deny(clippy::invalid_clippy_version_attribute, clippy::missing_clippy_ve
    = note: this error originates in the macro `$crate::declare_tool_lint` which comes from the expansion of the macro `declare_tool_lint` (in Nightly builds, run with -Z macro-backtrace for more info)
 
 error: this lint is missing the `clippy::version` attribute or version value
-  --> tests/ui-internal/check_clippy_version_attribute.rs:70:1
+  --> tests/ui-internal/check_clippy_version_attribute.rs:71:1
    |
 LL | / declare_tool_lint! {
 LL | |

tests/ui-toml/absolute_paths/absolute_paths.allow_crates.stderr

@@ -1,5 +1,5 @@
 error: consider bringing this path into scope with the `use` keyword
-  --> tests/ui-toml/absolute_paths/absolute_paths.rs:14:13
+  --> tests/ui-toml/absolute_paths/absolute_paths.rs:15:13
    |
 LL |     let _ = std::path::is_separator(' ');
    |             ^^^^^^^^^^^^^^^^^^^^^^^
@@ -11,31 +11,31 @@ LL | #![deny(clippy::absolute_paths)]
    |         ^^^^^^^^^^^^^^^^^^^^^^
 
 error: consider bringing this path into scope with the `use` keyword
-  --> tests/ui-toml/absolute_paths/absolute_paths.rs:20:13
+  --> tests/ui-toml/absolute_paths/absolute_paths.rs:21:13
    |
 LL |     let _ = ::std::path::MAIN_SEPARATOR;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: consider bringing this path into scope with the `use` keyword
-  --> tests/ui-toml/absolute_paths/absolute_paths.rs:25:13
+  --> tests/ui-toml/absolute_paths/absolute_paths.rs:26:13
    |
 LL |     let _ = std::collections::hash_map::HashMap::<i32, i32>::new();
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: consider bringing this path into scope with the `use` keyword
-  --> tests/ui-toml/absolute_paths/absolute_paths.rs:28:31
+  --> tests/ui-toml/absolute_paths/absolute_paths.rs:29:31
    |
 LL |     let _: &std::path::Path = std::path::Path::new("");
    |                               ^^^^^^^^^^^^^^^
 
 error: consider bringing this path into scope with the `use` keyword
-  --> tests/ui-toml/absolute_paths/absolute_paths.rs:28:13
+  --> tests/ui-toml/absolute_paths/absolute_paths.rs:29:13
    |
 LL |     let _: &std::path::Path = std::path::Path::new("");
    |             ^^^^^^^^^^^^^^^
 
 error: consider bringing this path into scope with the `use` keyword
-  --> tests/ui-toml/absolute_paths/absolute_paths.rs:43:13
+  --> tests/ui-toml/absolute_paths/absolute_paths.rs:44:13
    |
 LL |     let _ = std::option::Option::None::<i32>;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

tests/ui-toml/absolute_paths/absolute_paths.allow_long.stderr

@@ -1,5 +1,5 @@
 error: consider bringing this path into scope with the `use` keyword
-  --> tests/ui-toml/absolute_paths/absolute_paths.rs:25:13
+  --> tests/ui-toml/absolute_paths/absolute_paths.rs:26:13
    |
 LL |     let _ = std::collections::hash_map::HashMap::<i32, i32>::new();
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^