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

Adds a new lint that checks if there is a semicolon on the last block statement if it returns nothing #6681

Merged
merged 8 commits into from Feb 7, 2021
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -2185,6 +2185,7 @@ Released 2018-09-13
[`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push
[`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some
[`self_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_assignment
[`semicolon_if_nothing_returned`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_if_nothing_returned
[`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse
[`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse
[`shadow_same`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_same
Expand Down
4 changes: 4 additions & 0 deletions clippy_lints/src/lib.rs
Expand Up @@ -310,6 +310,7 @@ mod regex;
mod repeat_once;
mod returns;
mod self_assignment;
mod semicolon_if_nothing_returned;
mod serde_api;
mod shadow;
mod single_component_path_imports;
Expand Down Expand Up @@ -876,6 +877,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&returns::LET_AND_RETURN,
&returns::NEEDLESS_RETURN,
&self_assignment::SELF_ASSIGNMENT,
&semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED,
&serde_api::SERDE_API_MISUSE,
&shadow::SHADOW_REUSE,
&shadow::SHADOW_SAME,
Expand Down Expand Up @@ -1237,6 +1239,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box manual_unwrap_or::ManualUnwrapOr);
store.register_late_pass(|| box manual_ok_or::ManualOkOr);
store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs);
store.register_late_pass(|| box semicolon_if_nothing_returned::SemicolonIfNothingReturned);
store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync);
let disallowed_methods = conf.disallowed_methods.iter().cloned().collect::<FxHashSet<_>>();
store.register_late_pass(move || box disallowed_method::DisallowedMethod::new(&disallowed_methods));
Expand Down Expand Up @@ -1291,6 +1294,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&panic_unimplemented::UNIMPLEMENTED),
LintId::of(&panic_unimplemented::UNREACHABLE),
LintId::of(&pattern_type_mismatch::PATTERN_TYPE_MISMATCH),
LintId::of(&semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED),
LintId::of(&shadow::SHADOW_REUSE),
LintId::of(&shadow::SHADOW_SAME),
LintId::of(&strings::STRING_ADD),
Expand Down
66 changes: 66 additions & 0 deletions clippy_lints/src/semicolon_if_nothing_returned.rs
@@ -0,0 +1,66 @@
use crate::utils::{in_macro, snippet_with_macro_callsite, span_lint_and_sugg, sugg};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Block, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
/// **What it does:** Looks for blocks of expressions and fires if the last expression returns `()`
/// but is not followed by a semicolon.
///
/// **Why is this bad?** The semicolon might be optional but when
/// extending the block with new code, it doesn't require a change in previous last line.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// fn main() {
/// println!("Hello world")
/// }
/// ```
/// Use instead:
/// ```rust
/// fn main() {
/// println!("Hello world");
/// }
/// ```
pub SEMICOLON_IF_NOTHING_RETURNED,
restriction,
"add a semicolon if nothing is returned"
}

declare_lint_pass!(SemicolonIfNothingReturned => [SEMICOLON_IF_NOTHING_RETURNED]);

impl LateLintPass<'_> for SemicolonIfNothingReturned {
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) {
if_chain! {
if !in_macro(block.span);
if let Some(expr) = block.expr;
let t_expr = cx.typeck_results().expr_ty(expr);
if t_expr.is_unit();
if let snippet = snippet_with_macro_callsite(cx, expr.span, "}");
if !snippet.ends_with('}');
then {
// filter out the desugared `for` loop
if let ExprKind::DropTemps(..) = &expr.kind {
return;
}

let sugg = sugg::Sugg::hir_with_macro_callsite(cx, &expr, "..");
let suggestion = format!("{0};", sugg);
span_lint_and_sugg(
cx,
SEMICOLON_IF_NOTHING_RETURNED,
expr.span.source_callsite(),
"consider adding a `;` to the last statement for consistent formatting",
"add a `;` here",
suggestion,
Applicability::MaybeIncorrect,
);
}
}
}
}
55 changes: 55 additions & 0 deletions tests/ui/semicolon_if_nothing_returned.rs
@@ -0,0 +1,55 @@
#![warn(clippy::semicolon_if_nothing_returned)]
#![feature(label_break_value)]

fn get_unit() {}

// the functions below trigger the lint
fn main() {
println!("Hello")
}

fn hello() {
get_unit()
}

fn basic101(x: i32) {
let y: i32;
y = x + 1
}

// this is fine
fn print_sum(a: i32, b: i32) {
println!("{}", a + b);
assert_eq!(true, false);
}

fn foo(x: i32) {
let y: i32;
if x < 1 {
y = 4;
} else {
y = 5;
}
}

fn bar(x: i32) {
let y: i32;
match x {
1 => y = 4,
_ => y = 32,
}
}

fn foobar(x: i32) {
let y: i32;
'label: {
y = x + 1;
}
}

fn loop_test(x: i32) {
let y: i32;
for &ext in &["stdout", "stderr", "fixed"] {
println!("{}", ext);
}
}
22 changes: 22 additions & 0 deletions tests/ui/semicolon_if_nothing_returned.stderr
@@ -0,0 +1,22 @@
error: consider adding a `;` to the last statement for consistent formatting
--> $DIR/semicolon_if_nothing_returned.rs:8:5
|
LL | println!("Hello")
| ^^^^^^^^^^^^^^^^^ help: add a `;` here: `println!("Hello");`
|
= note: `-D clippy::semicolon-if-nothing-returned` implied by `-D warnings`

error: consider adding a `;` to the last statement for consistent formatting
--> $DIR/semicolon_if_nothing_returned.rs:12:5
|
LL | get_unit()
| ^^^^^^^^^^ help: add a `;` here: `get_unit();`

error: consider adding a `;` to the last statement for consistent formatting
--> $DIR/semicolon_if_nothing_returned.rs:17:5
|
LL | y = x + 1
| ^^^^^^^^^ help: add a `;` here: `y = x + 1;`

error: aborting due to 3 previous errors