Skip to content

add suppress_restriction_lint_in_const config #9920

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

Merged
merged 7 commits into from
Dec 6, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
45 changes: 32 additions & 13 deletions clippy_lints/src/indexing_slicing.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
//! lint on indexing and slicing operations

use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
use clippy_utils::higher;
use rustc_ast::ast::RangeLimits;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_session::{declare_tool_lint, impl_lint_pass};

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -82,15 +82,29 @@ declare_clippy_lint! {
"indexing/slicing usage"
}

declare_lint_pass!(IndexingSlicing => [INDEXING_SLICING, OUT_OF_BOUNDS_INDEXING]);
impl_lint_pass!(IndexingSlicing => [INDEXING_SLICING, OUT_OF_BOUNDS_INDEXING]);

#[derive(Copy, Clone)]
pub struct IndexingSlicing {
suppress_restriction_lint_in_const: bool,
}

impl IndexingSlicing {
pub fn new(suppress_restriction_lint_in_const: bool) -> Self {
Self {
suppress_restriction_lint_in_const,
}
}
}

impl<'tcx> LateLintPass<'tcx> for IndexingSlicing {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if cx.tcx.hir().is_inside_const_context(expr.hir_id) {
if self.suppress_restriction_lint_in_const && cx.tcx.hir().is_inside_const_context(expr.hir_id) {
return;
}

if let ExprKind::Index(array, index) = &expr.kind {
let note = "the suggestion might not be applicable in constant blocks";
let ty = cx.typeck_results().expr_ty(array).peel_refs();
if let Some(range) = higher::Range::hir(index) {
// Ranged indexes, i.e., &x[n..m], &x[n..], &x[..n] and &x[..]
Expand Down Expand Up @@ -141,7 +155,13 @@ impl<'tcx> LateLintPass<'tcx> for IndexingSlicing {
(None, None) => return, // [..] is ok.
};

span_lint_and_help(cx, INDEXING_SLICING, expr.span, "slicing may panic", None, help_msg);
span_lint_and_then(cx, INDEXING_SLICING, expr.span, "slicing may panic", |diag| {
diag.help(help_msg);

if cx.tcx.hir().is_inside_const_context(expr.hir_id) {
diag.note(note);
}
});
} else {
// Catchall non-range index, i.e., [n] or [n << m]
if let ty::Array(..) = ty.kind() {
Expand All @@ -156,14 +176,13 @@ impl<'tcx> LateLintPass<'tcx> for IndexingSlicing {
}
}

span_lint_and_help(
cx,
INDEXING_SLICING,
expr.span,
"indexing may panic",
None,
"consider using `.get(n)` or `.get_mut(n)` instead",
);
span_lint_and_then(cx, INDEXING_SLICING, expr.span, "indexing may panic", |diag| {
diag.help("consider using `.get(n)` or `.get_mut(n)` instead");

if cx.tcx.hir().is_inside_const_context(expr.hir_id) {
diag.note(note);
}
});
}
}
}
Expand Down
7 changes: 6 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
let avoid_breaking_exported_api = conf.avoid_breaking_exported_api;
let allow_expect_in_tests = conf.allow_expect_in_tests;
let allow_unwrap_in_tests = conf.allow_unwrap_in_tests;
let suppress_restriction_lint_in_const = conf.suppress_restriction_lint_in_const;
store.register_late_pass(move |_| Box::new(approx_const::ApproxConstant::new(msrv())));
store.register_late_pass(move |_| {
Box::new(methods::Methods::new(
Expand Down Expand Up @@ -683,7 +684,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|_| Box::new(inherent_impl::MultipleInherentImpl));
store.register_late_pass(|_| Box::new(neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd));
store.register_late_pass(|_| Box::new(unwrap::Unwrap));
store.register_late_pass(|_| Box::new(indexing_slicing::IndexingSlicing));
store.register_late_pass(move |_| {
Box::new(indexing_slicing::IndexingSlicing::new(
suppress_restriction_lint_in_const,
))
});
store.register_late_pass(|_| Box::new(non_copy_const::NonCopyConst));
store.register_late_pass(|_| Box::new(ptr_offset_with_cast::PtrOffsetWithCast));
store.register_late_pass(|_| Box::new(redundant_clone::RedundantClone));
Expand Down
8 changes: 8 additions & 0 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,14 @@ define_Conf! {
///
/// Whether to allow mixed uninlined format args, e.g. `format!("{} {}", a, foo.bar)`
(allow_mixed_uninlined_format_args: bool = true),
/// Lint: INDEXING_SLICING
///
/// Whether to suppress a restriction lint in constant code. In same
/// cases the restructured operation might not be unavoidable, as the
/// suggested counterparts are unavailable in constant code. This
/// configuration will cause restriction lints to trigger even
/// if no suggestion can be made.
(suppress_restriction_lint_in_const: bool = false),
Copy link
Member

Choose a reason for hiding this comment

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

The description sounds perfect :)

}

/// Search for the configuration file.
Expand Down
1 change: 1 addition & 0 deletions tests/ui-toml/suppress_lint_in_const/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
suppress-restriction-lint-in-const = true
60 changes: 60 additions & 0 deletions tests/ui-toml/suppress_lint_in_const/test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
#![feature(inline_const)]
#![warn(clippy::indexing_slicing)]
// We also check the out_of_bounds_indexing lint here, because it lints similar things and
// we want to avoid false positives.
#![warn(clippy::out_of_bounds_indexing)]
#![allow(unconditional_panic, clippy::no_effect, clippy::unnecessary_operation)]

const ARR: [i32; 2] = [1, 2];
const REF: &i32 = &ARR[idx()]; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true.
const REF_ERR: &i32 = &ARR[idx4()]; // Ok, let rustc handle const contexts.

const fn idx() -> usize {
1
}
const fn idx4() -> usize {
4
}

fn main() {
let x = [1, 2, 3, 4];
let index: usize = 1;
x[index];
x[4]; // Ok, let rustc's `unconditional_panic` lint handle `usize` indexing on arrays.
x[1 << 3]; // Ok, let rustc's `unconditional_panic` lint handle `usize` indexing on arrays.

x[0]; // Ok, should not produce stderr.
x[3]; // Ok, should not produce stderr.
x[const { idx() }]; // Ok, should not produce stderr.
x[const { idx4() }]; // Ok, let rustc's `unconditional_panic` lint handle `usize` indexing on arrays.
const { &ARR[idx()] }; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true.
const { &ARR[idx4()] }; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true.

let y = &x;
y[0]; // Ok, referencing shouldn't affect this lint. See the issue 6021
y[4]; // Ok, rustc will handle references too.

let v = vec![0; 5];
v[0];
v[10];
v[1 << 3];

const N: usize = 15; // Out of bounds
const M: usize = 3; // In bounds
x[N]; // Ok, let rustc's `unconditional_panic` lint handle `usize` indexing on arrays.
x[M]; // Ok, should not produce stderr.
v[N];
v[M];
}

/// An opaque integer representation
pub struct Integer<'a> {
/// The underlying data
value: &'a [u8],
}
impl<'a> Integer<'a> {
// Check whether `self` holds a negative number or not
pub const fn is_negative(&self) -> bool {
self.value[0] & 0b1000_0000 != 0
}
}
70 changes: 70 additions & 0 deletions tests/ui-toml/suppress_lint_in_const/test.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
error[E0080]: evaluation of `main::{constant#3}` failed
--> $DIR/test.rs:31:14
|
LL | const { &ARR[idx4()] }; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true.
| ^^^^^^^^^^^ index out of bounds: the length is 2 but the index is 4

note: erroneous constant used
--> $DIR/test.rs:31:5
|
LL | const { &ARR[idx4()] }; // Ok, should not produce stderr, since `suppress-restriction-lint-in-const` is set true.
| ^^^^^^^^^^^^^^^^^^^^^^

error: indexing may panic
--> $DIR/test.rs:22:5
|
LL | x[index];
| ^^^^^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead
= note: `-D clippy::indexing-slicing` implied by `-D warnings`

error: indexing may panic
--> $DIR/test.rs:38:5
|
LL | v[0];
| ^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead

error: indexing may panic
--> $DIR/test.rs:39:5
|
LL | v[10];
| ^^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead

error: indexing may panic
--> $DIR/test.rs:40:5
|
LL | v[1 << 3];
| ^^^^^^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead

error: indexing may panic
--> $DIR/test.rs:46:5
|
LL | v[N];
| ^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead

error: indexing may panic
--> $DIR/test.rs:47:5
|
LL | v[M];
| ^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead

error[E0080]: evaluation of constant value failed
--> $DIR/test.rs:10:24
|
LL | const REF_ERR: &i32 = &ARR[idx4()]; // Ok, let rustc handle const contexts.
| ^^^^^^^^^^^ index out of bounds: the length is 2 but the index is 4

error: aborting due to 8 previous errors

For more information about this error, try `rustc --explain E0080`.
1 change: 1 addition & 0 deletions tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown fie
pass-by-value-size-limit
single-char-binding-names-threshold
standard-macro-braces
suppress-restriction-lint-in-const
third-party
too-large-for-stack
too-many-arguments-threshold
Expand Down
6 changes: 3 additions & 3 deletions tests/ui/indexing_slicing_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#![allow(unconditional_panic, clippy::no_effect, clippy::unnecessary_operation)]

const ARR: [i32; 2] = [1, 2];
const REF: &i32 = &ARR[idx()]; // Ok, should not produce stderr.
const REF: &i32 = &ARR[idx()]; // This should be linted, since `suppress-restriction-lint-in-const` default is false.
const REF_ERR: &i32 = &ARR[idx4()]; // Ok, let rustc handle const contexts.

const fn idx() -> usize {
Expand All @@ -27,8 +27,8 @@ fn main() {
x[3]; // Ok, should not produce stderr.
x[const { idx() }]; // Ok, should not produce stderr.
x[const { idx4() }]; // Ok, let rustc's `unconditional_panic` lint handle `usize` indexing on arrays.
const { &ARR[idx()] }; // Ok, should not produce stderr.
const { &ARR[idx4()] }; // Ok, let rustc handle const contexts.
const { &ARR[idx()] }; // This should be linted, since `suppress-restriction-lint-in-const` default is false.
const { &ARR[idx4()] }; // This should be linted, since `suppress-restriction-lint-in-const` default is false.

let y = &x;
y[0]; // Ok, referencing shouldn't affect this lint. See the issue 6021
Expand Down
44 changes: 40 additions & 4 deletions tests/ui/indexing_slicing_index.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,32 @@
error: indexing may panic
--> $DIR/indexing_slicing_index.rs:9:20
|
LL | const REF: &i32 = &ARR[idx()]; // This should be linted, since `suppress-restriction-lint-in-const` default is false.
| ^^^^^^^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead
= note: the suggestion might not be applicable in constant blocks
= note: `-D clippy::indexing-slicing` implied by `-D warnings`

error: indexing may panic
--> $DIR/indexing_slicing_index.rs:10:24
|
LL | const REF_ERR: &i32 = &ARR[idx4()]; // Ok, let rustc handle const contexts.
| ^^^^^^^^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead
= note: the suggestion might not be applicable in constant blocks

error[E0080]: evaluation of `main::{constant#3}` failed
--> $DIR/indexing_slicing_index.rs:31:14
|
LL | const { &ARR[idx4()] }; // Ok, let rustc handle const contexts.
LL | const { &ARR[idx4()] }; // This should be linted, since `suppress-restriction-lint-in-const` default is false.
| ^^^^^^^^^^^ index out of bounds: the length is 2 but the index is 4

note: erroneous constant used
--> $DIR/indexing_slicing_index.rs:31:5
|
LL | const { &ARR[idx4()] }; // Ok, let rustc handle const contexts.
LL | const { &ARR[idx4()] }; // This should be linted, since `suppress-restriction-lint-in-const` default is false.
| ^^^^^^^^^^^^^^^^^^^^^^

error: indexing may panic
Expand All @@ -17,7 +36,24 @@ LL | x[index];
| ^^^^^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead
= note: `-D clippy::indexing-slicing` implied by `-D warnings`

error: indexing may panic
--> $DIR/indexing_slicing_index.rs:30:14
|
LL | const { &ARR[idx()] }; // This should be linted, since `suppress-restriction-lint-in-const` default is false.
| ^^^^^^^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead
= note: the suggestion might not be applicable in constant blocks

error: indexing may panic
--> $DIR/indexing_slicing_index.rs:31:14
|
LL | const { &ARR[idx4()] }; // This should be linted, since `suppress-restriction-lint-in-const` default is false.
| ^^^^^^^^^^^
|
= help: consider using `.get(n)` or `.get_mut(n)` instead
= note: the suggestion might not be applicable in constant blocks

error: indexing may panic
--> $DIR/indexing_slicing_index.rs:38:5
Expand Down Expand Up @@ -65,6 +101,6 @@ error[E0080]: evaluation of constant value failed
LL | const REF_ERR: &i32 = &ARR[idx4()]; // Ok, let rustc handle const contexts.
| ^^^^^^^^^^^ index out of bounds: the length is 2 but the index is 4

error: aborting due to 8 previous errors
error: aborting due to 12 previous errors

For more information about this error, try `rustc --explain E0080`.