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

Fixes #9677 - Lint against mod lib; #9708

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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 @@ -4055,6 +4055,7 @@ Released 2018-09-13
[`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes
[`mixed_case_hex_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_case_hex_literals
[`mixed_read_write_in_expression`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_read_write_in_expression
[`mod_lib`]: https://rust-lang.github.io/rust-clippy/master/index.html#mod_lib
[`mod_module_files`]: https://rust-lang.github.io/rust-clippy/master/index.html#mod_module_files
[`module_inception`]: https://rust-lang.github.io/rust-clippy/master/index.html#module_inception
[`module_name_repetitions`]: https://rust-lang.github.io/rust-clippy/master/index.html#module_name_repetitions
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ mod missing_enforced_import_rename;
mod missing_inline;
mod missing_trait_methods;
mod mixed_read_write_in_expression;
mod mod_lib;
mod module_style;
mod multi_assignments;
mod mut_key;
Expand Down Expand Up @@ -912,6 +913,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_early_pass(|| Box::new(partial_pub_fields::PartialPubFields));
store.register_late_pass(|_| Box::new(missing_trait_methods::MissingTraitMethods));
store.register_late_pass(|_| Box::new(from_raw_with_void_ptr::FromRawWithVoidPtr));
store.register_early_pass(|| Box::new(mod_lib::ModLib));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
52 changes: 52 additions & 0 deletions clippy_lints/src/mod_lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
use clippy_utils::diagnostics::span_lint_and_help;
use rustc_ast::{ptr::P, Crate, Item, ItemKind, ModKind};
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;

declare_clippy_lint! {
/// ### What it does
///
/// ### Why is this bad?
///
/// ### Example
/// ```rust
/// // example code where clippy issues a warning
/// ```
/// Use instead:
/// ```rust
/// // example code which does not raise clippy warning
/// ```
#[clippy::version = "1.66.0"]
pub MOD_LIB,
pedantic,
"default lint description"
}
declare_lint_pass!(ModLib => [MOD_LIB]);

impl EarlyLintPass for ModLib {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for working on this! I know this PR isn’t ready for review, just wanted to say we’re trying to make all new lints to use ‘LateLintPass’, cheers! Feel free to share any of your issues

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kraktus I've updated the code to implement LateLintPass instead. Am I going in the right direction here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does! Looks like your lint is not registered yet, using cargo new_lint (don’t remember the exact name of the subcommand, find it with cargo —help while in clippy repo, will create the skeleton of the lint for you

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually cargo dev update_lint should suffice in the current situation. The lint exists, but it isn't yet registered.

fn check_crate(&mut self, cx: &EarlyContext<'_>, krate: &Crate) {
// println!("MOO Checking crate {:#?}", krate);
check_mod(cx, &krate.items);
}
}

fn check_mod(cx: &EarlyContext<'_>, items: &[P<Item>]) {
for item in items {
if let ItemKind::Mod(_, ModKind::Loaded(..)) = item.kind {
// println!("MOO Got a Mod: {:#?}", items);
// println!("MOO Got a Mod: {:#?}", item.ident.name);

if item.ident.name == sym::lib {
span_lint_and_help(
cx,
MOD_LIB,
item.span,
"uncommon use of mod::lib",
None,
"you probably meant use::package instead",
);
}
}
}
}
12 changes: 12 additions & 0 deletions src/docs/mod_lib.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
### What it does

### Why is this bad?

### Example
```
// example code where clippy issues a warning
```
Use instead:
```
// example code which does not raise clippy warning
```
3 changes: 3 additions & 0 deletions tests/ui/mod_lib/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pub fn something() -> i32 {
42
}
9 changes: 9 additions & 0 deletions tests/ui/mod_lib/lib.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error[E0601]: `main` function not found in crate `lib`
--> $DIR/lib.rs:3:2
|
LL | }
| ^ consider adding a `main` function to `$DIR/lib.rs`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0601`.
8 changes: 8 additions & 0 deletions tests/ui/mod_lib/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#![allow(unused)]
#![warn(clippy::mod_lib)]

mod lib;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a mod lib { } should suffice here, which obviates the need for a secondary file.


fn main() {
println!("{}", lib::something());
}
21 changes: 21 additions & 0 deletions tests/ui/mod_lib/main.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
error: found module declaration for lib.rs
--> $DIR/main.rs:4:1
|
LL | mod lib;
| ^^^^^^^^
|
= note: lib.rs is the root of this crate's library target
= help: to refer to it from other targets, use the library's name as the path
= note: `-D special-module-name` implied by `-D warnings`

error: uncommon use of mod::lib
--> $DIR/main.rs:4:1
|
LL | mod lib;
| ^^^^^^^^
|
= help: you probably meant use::package instead
= note: `-D clippy::mod-lib` implied by `-D warnings`

error: aborting due to 2 previous errors