diff --git a/CHANGELOG.md b/CHANGELOG.md index b773422b01507..c0679d280c266 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -878,6 +878,7 @@ All notable changes to this project will be documented in this file. [`min_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_max [`misaligned_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#misaligned_transmute [`misrefactored_assign_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#misrefactored_assign_op +[`missing_const_for_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn [`missing_docs_in_private_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items [`missing_inline_in_public_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_inline_in_public_items [`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes diff --git a/README.md b/README.md index 6473b8efc5480..dad18ef756901 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 292 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 293 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 4683d353ccf87..3483aae0ca3bc 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -23,6 +23,8 @@ extern crate rustc_data_structures; #[allow(unused_extern_crates)] extern crate rustc_errors; #[allow(unused_extern_crates)] +extern crate rustc_mir; +#[allow(unused_extern_crates)] extern crate rustc_plugin; #[allow(unused_extern_crates)] extern crate rustc_target; @@ -144,6 +146,7 @@ pub mod methods; pub mod minmax; pub mod misc; pub mod misc_early; +pub mod missing_const_for_fn; pub mod missing_doc; pub mod missing_inline; pub mod multiple_crate_versions; @@ -486,6 +489,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box slow_vector_initialization::Pass); reg.register_late_lint_pass(box types::RefToMut); reg.register_late_lint_pass(box assertions_on_constants::AssertionsOnConstants); + reg.register_late_lint_pass(box missing_const_for_fn::MissingConstForFn); reg.register_lint_group("clippy::restriction", Some("clippy_restriction"), vec![ arithmetic::FLOAT_ARITHMETIC, @@ -1027,6 +1031,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_lint_group("clippy::nursery", Some("clippy_nursery"), vec![ attrs::EMPTY_LINE_AFTER_OUTER_ATTR, fallible_impl_from::FALLIBLE_IMPL_FROM, + missing_const_for_fn::MISSING_CONST_FOR_FN, mutex_atomic::MUTEX_INTEGER, needless_borrow::NEEDLESS_BORROW, redundant_clone::REDUNDANT_CLONE, diff --git a/clippy_lints/src/missing_const_for_fn.rs b/clippy_lints/src/missing_const_for_fn.rs new file mode 100644 index 0000000000000..9228c586bbfd5 --- /dev/null +++ b/clippy_lints/src/missing_const_for_fn.rs @@ -0,0 +1,121 @@ +use crate::utils::{is_entrypoint_fn, span_lint}; +use rustc::hir; +use rustc::hir::intravisit::FnKind; +use rustc::hir::{Body, Constness, FnDecl}; +use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; +use rustc::{declare_tool_lint, lint_array}; +use rustc_mir::transform::qualify_min_const_fn::is_min_const_fn; +use syntax::ast::NodeId; +use syntax_pos::Span; + +/// **What it does:** +/// +/// Suggests the use of `const` in functions and methods where possible. +/// +/// **Why is this bad?** +/// +/// Not having the function const prevents callers of the function from being const as well. +/// +/// **Known problems:** +/// +/// Const functions are currently still being worked on, with some features only being available +/// on nightly. This lint does not consider all edge cases currently and the suggestions may be +/// incorrect if you are using this lint on stable. +/// +/// Also, the lint only runs one pass over the code. Consider these two non-const functions: +/// +/// ```rust +/// fn a() -> i32 { +/// 0 +/// } +/// fn b() -> i32 { +/// a() +/// } +/// ``` +/// +/// When running Clippy, the lint will only suggest to make `a` const, because `b` at this time +/// can't be const as it calls a non-const function. Making `a` const and running Clippy again, +/// will suggest to make `b` const, too. +/// +/// **Example:** +/// +/// ```rust +/// fn new() -> Self { +/// Self { random_number: 42 } +/// } +/// ``` +/// +/// Could be a const fn: +/// +/// ```rust +/// const fn new() -> Self { +/// Self { random_number: 42 } +/// } +/// ``` +declare_clippy_lint! { + pub MISSING_CONST_FOR_FN, + nursery, + "Lint functions definitions that could be made `const fn`" +} + +#[derive(Clone)] +pub struct MissingConstForFn; + +impl LintPass for MissingConstForFn { + fn get_lints(&self) -> LintArray { + lint_array!(MISSING_CONST_FOR_FN) + } + + fn name(&self) -> &'static str { + "MissingConstForFn" + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingConstForFn { + fn check_fn( + &mut self, + cx: &LateContext<'_, '_>, + kind: FnKind<'_>, + _: &FnDecl, + _: &Body, + span: Span, + node_id: NodeId, + ) { + let def_id = cx.tcx.hir().local_def_id(node_id); + + if is_entrypoint_fn(cx, def_id) { + return; + } + + // Perform some preliminary checks that rule out constness on the Clippy side. This way we + // can skip the actual const check and return early. + match kind { + FnKind::ItemFn(_, _, header, ..) => { + if already_const(header) { + return; + } + }, + FnKind::Method(_, sig, ..) => { + if already_const(sig.header) { + return; + } + }, + _ => return, + } + + let mir = cx.tcx.optimized_mir(def_id); + + if let Err((span, err)) = is_min_const_fn(cx.tcx, def_id, &mir) { + if cx.tcx.is_min_const_fn(def_id) { + cx.tcx.sess.span_err(span, &err); + } + } else { + span_lint(cx, MISSING_CONST_FOR_FN, span, "this could be a const_fn"); + } + } +} + +// We don't have to lint on something that's already `const` +fn already_const(header: hir::FnHeader) -> bool { + header.constness == Constness::Const +} diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 2b2b3e8b2f6a5..ee3356fdc8234 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -3,7 +3,7 @@ use if_chain::if_chain; use matches::matches; use rustc::hir; use rustc::hir::def::Def; -use rustc::hir::def_id::{DefId, CRATE_DEF_INDEX}; +use rustc::hir::def_id::{DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; use rustc::hir::intravisit::{NestedVisitorMap, Visitor}; use rustc::hir::Node; use rustc::hir::*; @@ -350,6 +350,14 @@ pub fn method_chain_args<'a>(expr: &'a Expr, methods: &[&str]) -> Option, def_id: DefId) -> bool { + if let Some((entry_fn_def_id, _)) = cx.tcx.entry_fn(LOCAL_CRATE) { + return def_id == entry_fn_def_id; + } + false +} + /// Get the name of the item the expression is in, if available. pub fn get_item_name(cx: &LateContext<'_, '_>, expr: &Expr) -> Option { let parent_id = cx.tcx.hir().get_parent(expr.id); diff --git a/tests/ui/missing_const_for_fn/cant_be_const.rs b/tests/ui/missing_const_for_fn/cant_be_const.rs new file mode 100644 index 0000000000000..36efe16b84f0f --- /dev/null +++ b/tests/ui/missing_const_for_fn/cant_be_const.rs @@ -0,0 +1,57 @@ +//! False-positive tests to ensure we don't suggest `const` for things where it would cause a +//! compilation error. +//! The .stderr output of this test should be empty. Otherwise it's a bug somewhere. + +#![warn(clippy::missing_const_for_fn)] +#![feature(start)] + +struct Game; + +// This should not be linted because it's already const +const fn already_const() -> i32 { + 32 +} + +impl Game { + // This should not be linted because it's already const + pub const fn already_const() -> i32 { + 32 + } +} + +// Allowing on this function, because it would lint, which we don't want in this case. +#[allow(clippy::missing_const_for_fn)] +fn random() -> u32 { + 42 +} + +// We should not suggest to make this function `const` because `random()` is non-const +fn random_caller() -> u32 { + random() +} + +static Y: u32 = 0; + +// We should not suggest to make this function `const` because const functions are not allowed to +// refer to a static variable +fn get_y() -> u32 { + Y + //~^ ERROR E0013 +} + +// Don't lint entrypoint functions +#[start] +fn init(num: isize, something: *const *const u8) -> isize { + 1 +} + +trait Foo { + // This should not be suggested to be made const + // (rustc doesn't allow const trait methods) + fn f() -> u32; + + // This should not be suggested to be made const either + fn g() -> u32 { + 33 + } +} diff --git a/tests/ui/missing_const_for_fn/cant_be_const.stderr b/tests/ui/missing_const_for_fn/cant_be_const.stderr new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/tests/ui/missing_const_for_fn/could_be_const.rs b/tests/ui/missing_const_for_fn/could_be_const.rs new file mode 100644 index 0000000000000..139e64de1ff5d --- /dev/null +++ b/tests/ui/missing_const_for_fn/could_be_const.rs @@ -0,0 +1,57 @@ +#![warn(clippy::missing_const_for_fn)] +#![allow(clippy::let_and_return)] + +use std::mem::transmute; + +struct Game { + guess: i32, +} + +impl Game { + // Could be const + pub fn new() -> Self { + Self { guess: 42 } + } +} + +// Could be const +fn one() -> i32 { + 1 +} + +// Could also be const +fn two() -> i32 { + let abc = 2; + abc +} + +// FIXME: This is a false positive in the `is_min_const_fn` function. +// At least until the `const_string_new` feature is stabilzed. +fn string() -> String { + String::new() +} + +// Could be const +unsafe fn four() -> i32 { + 4 +} + +// Could also be const +fn generic(t: T) -> T { + t +} + +// FIXME: Depends on the `const_transmute` and `const_fn` feature gates. +// In the future Clippy should be able to suggest this as const, too. +fn sub(x: u32) -> usize { + unsafe { transmute(&x) } +} + +// NOTE: This is currently not yet allowed to be const +// Once implemented, Clippy should be able to suggest this as const, too. +fn generic_arr(t: [T; 1]) -> T { + t[0] +} + +// Should not be const +fn main() {} diff --git a/tests/ui/missing_const_for_fn/could_be_const.stderr b/tests/ui/missing_const_for_fn/could_be_const.stderr new file mode 100644 index 0000000000000..22ea852905dac --- /dev/null +++ b/tests/ui/missing_const_for_fn/could_be_const.stderr @@ -0,0 +1,53 @@ +error: this could be a const_fn + --> $DIR/could_be_const.rs:12:5 + | +LL | / pub fn new() -> Self { +LL | | Self { guess: 42 } +LL | | } + | |_____^ + | + = note: `-D clippy::missing-const-for-fn` implied by `-D warnings` + +error: this could be a const_fn + --> $DIR/could_be_const.rs:18:1 + | +LL | / fn one() -> i32 { +LL | | 1 +LL | | } + | |_^ + +error: this could be a const_fn + --> $DIR/could_be_const.rs:23:1 + | +LL | / fn two() -> i32 { +LL | | let abc = 2; +LL | | abc +LL | | } + | |_^ + +error: this could be a const_fn + --> $DIR/could_be_const.rs:30:1 + | +LL | / fn string() -> String { +LL | | String::new() +LL | | } + | |_^ + +error: this could be a const_fn + --> $DIR/could_be_const.rs:35:1 + | +LL | / unsafe fn four() -> i32 { +LL | | 4 +LL | | } + | |_^ + +error: this could be a const_fn + --> $DIR/could_be_const.rs:40:1 + | +LL | / fn generic(t: T) -> T { +LL | | t +LL | | } + | |_^ + +error: aborting due to 6 previous errors +