-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Auto merge of #6165 - dvermd:ref_option_ref, r=flip1995
Add lint 'ref_option_ref' #1377 This lint checks for usage of `&Option<&T>` which can be simplified as `Option<&T>` as suggested in #1377. This WIP PR is here to get feedback on the lint as there's more cases to be handled: * statics/consts, * associated types, * type alias, * function/method parameter/return, * ADT definitions (struct/tuple struct fields, enum variants) changelog: Add 'ref_option_ref' lint
- Loading branch information
Showing
9 changed files
with
209 additions
and
12 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| use crate::utils::{last_path_segment, snippet, span_lint_and_sugg}; | ||
| use rustc_hir::{GenericArg, Mutability, Ty, TyKind}; | ||
| use rustc_lint::{LateContext, LateLintPass}; | ||
| use rustc_session::{declare_lint_pass, declare_tool_lint}; | ||
|
|
||
| use if_chain::if_chain; | ||
| use rustc_errors::Applicability; | ||
|
|
||
| declare_clippy_lint! { | ||
| /// **What it does:** Checks for usage of `&Option<&T>`. | ||
| /// | ||
| /// **Why is this bad?** Since `&` is Copy, it's useless to have a | ||
| /// reference on `Option<&T>`. | ||
| /// | ||
| /// **Known problems:** It may be irrevelent to use this lint on | ||
| /// public API code as it will make a breaking change to apply it. | ||
| /// | ||
| /// **Example:** | ||
| /// | ||
| /// ```rust,ignore | ||
| /// let x: &Option<&u32> = &Some(&0u32); | ||
| /// ``` | ||
| /// Use instead: | ||
| /// ```rust,ignore | ||
| /// let x: Option<&u32> = Some(&0u32); | ||
| /// ``` | ||
| pub REF_OPTION_REF, | ||
| pedantic, | ||
| "use `Option<&T>` instead of `&Option<&T>`" | ||
| } | ||
|
|
||
| declare_lint_pass!(RefOptionRef => [REF_OPTION_REF]); | ||
|
|
||
| impl<'tcx> LateLintPass<'tcx> for RefOptionRef { | ||
| fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx Ty<'tcx>) { | ||
| if_chain! { | ||
| if let TyKind::Rptr(_, ref mut_ty) = ty.kind; | ||
| if mut_ty.mutbl == Mutability::Not; | ||
| if let TyKind::Path(ref qpath) = &mut_ty.ty.kind; | ||
| let last = last_path_segment(qpath); | ||
| if let Some(res) = last.res; | ||
| if let Some(def_id) = res.opt_def_id(); | ||
|
|
||
| if cx.tcx.is_diagnostic_item(sym!(option_type), def_id); | ||
| if let Some(ref params) = last_path_segment(qpath).args ; | ||
| if !params.parenthesized; | ||
| if let Some(inner_ty) = params.args.iter().find_map(|arg| match arg { | ||
| GenericArg::Type(inner_ty) => Some(inner_ty), | ||
| _ => None, | ||
| }); | ||
| if let TyKind::Rptr(_, _) = inner_ty.kind; | ||
|
|
||
| then { | ||
| span_lint_and_sugg( | ||
| cx, | ||
| REF_OPTION_REF, | ||
| ty.span, | ||
| "since `&` implements the `Copy` trait, `&Option<&T>` can be simplified to `Option<&T>`", | ||
| "try", | ||
| format!("Option<{}>", &snippet(cx, inner_ty.span, "..")), | ||
| Applicability::MaybeIncorrect, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| #![allow(unused)] | ||
| #![warn(clippy::ref_option_ref)] | ||
|
|
||
| // This lint is not tagged as run-rustfix because automatically | ||
| // changing the type of a variable would also means changing | ||
| // all usages of this variable to match and This is not handled | ||
| // by this lint. | ||
|
|
||
| static THRESHOLD: i32 = 10; | ||
| static REF_THRESHOLD: &Option<&i32> = &Some(&THRESHOLD); | ||
| const CONST_THRESHOLD: &i32 = &10; | ||
| const REF_CONST: &Option<&i32> = &Some(&CONST_THRESHOLD); | ||
|
|
||
| type RefOptRefU32<'a> = &'a Option<&'a u32>; | ||
| type RefOptRef<'a, T> = &'a Option<&'a T>; | ||
|
|
||
| fn foo(data: &Option<&u32>) {} | ||
|
|
||
| fn bar(data: &u32) -> &Option<&u32> { | ||
| &None | ||
| } | ||
|
|
||
| struct StructRef<'a> { | ||
| data: &'a Option<&'a u32>, | ||
| } | ||
|
|
||
| struct StructTupleRef<'a>(u32, &'a Option<&'a u32>); | ||
|
|
||
| enum EnumRef<'a> { | ||
| Variant1(u32), | ||
| Variant2(&'a Option<&'a u32>), | ||
| } | ||
|
|
||
| trait RefOptTrait { | ||
| type A; | ||
| fn foo(&self, _: Self::A); | ||
| } | ||
|
|
||
| impl RefOptTrait for u32 { | ||
| type A = &'static Option<&'static Self>; | ||
|
|
||
| fn foo(&self, _: Self::A) {} | ||
| } | ||
|
|
||
| fn main() { | ||
| let x: &Option<&u32> = &None; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| error: since `&` implements the `Copy` trait, `&Option<&T>` can be simplified to `Option<&T>` | ||
| --> $DIR/ref_option_ref.rs:10:23 | ||
| | | ||
| LL | static REF_THRESHOLD: &Option<&i32> = &Some(&THRESHOLD); | ||
| | ^^^^^^^^^^^^^ help: try: `Option<&i32>` | ||
| | | ||
| = note: `-D clippy::ref-option-ref` implied by `-D warnings` | ||
|
|
||
| error: since `&` implements the `Copy` trait, `&Option<&T>` can be simplified to `Option<&T>` | ||
| --> $DIR/ref_option_ref.rs:12:18 | ||
| | | ||
| LL | const REF_CONST: &Option<&i32> = &Some(&CONST_THRESHOLD); | ||
| | ^^^^^^^^^^^^^ help: try: `Option<&i32>` | ||
|
|
||
| error: since `&` implements the `Copy` trait, `&Option<&T>` can be simplified to `Option<&T>` | ||
| --> $DIR/ref_option_ref.rs:14:25 | ||
| | | ||
| LL | type RefOptRefU32<'a> = &'a Option<&'a u32>; | ||
| | ^^^^^^^^^^^^^^^^^^^ help: try: `Option<&'a u32>` | ||
|
|
||
| error: since `&` implements the `Copy` trait, `&Option<&T>` can be simplified to `Option<&T>` | ||
| --> $DIR/ref_option_ref.rs:15:25 | ||
| | | ||
| LL | type RefOptRef<'a, T> = &'a Option<&'a T>; | ||
| | ^^^^^^^^^^^^^^^^^ help: try: `Option<&'a T>` | ||
|
|
||
| error: since `&` implements the `Copy` trait, `&Option<&T>` can be simplified to `Option<&T>` | ||
| --> $DIR/ref_option_ref.rs:17:14 | ||
| | | ||
| LL | fn foo(data: &Option<&u32>) {} | ||
| | ^^^^^^^^^^^^^ help: try: `Option<&u32>` | ||
|
|
||
| error: since `&` implements the `Copy` trait, `&Option<&T>` can be simplified to `Option<&T>` | ||
| --> $DIR/ref_option_ref.rs:19:23 | ||
| | | ||
| LL | fn bar(data: &u32) -> &Option<&u32> { | ||
| | ^^^^^^^^^^^^^ help: try: `Option<&u32>` | ||
|
|
||
| error: since `&` implements the `Copy` trait, `&Option<&T>` can be simplified to `Option<&T>` | ||
| --> $DIR/ref_option_ref.rs:24:11 | ||
| | | ||
| LL | data: &'a Option<&'a u32>, | ||
| | ^^^^^^^^^^^^^^^^^^^ help: try: `Option<&'a u32>` | ||
|
|
||
| error: since `&` implements the `Copy` trait, `&Option<&T>` can be simplified to `Option<&T>` | ||
| --> $DIR/ref_option_ref.rs:27:32 | ||
| | | ||
| LL | struct StructTupleRef<'a>(u32, &'a Option<&'a u32>); | ||
| | ^^^^^^^^^^^^^^^^^^^ help: try: `Option<&'a u32>` | ||
|
|
||
| error: since `&` implements the `Copy` trait, `&Option<&T>` can be simplified to `Option<&T>` | ||
| --> $DIR/ref_option_ref.rs:31:14 | ||
| | | ||
| LL | Variant2(&'a Option<&'a u32>), | ||
| | ^^^^^^^^^^^^^^^^^^^ help: try: `Option<&'a u32>` | ||
|
|
||
| error: since `&` implements the `Copy` trait, `&Option<&T>` can be simplified to `Option<&T>` | ||
| --> $DIR/ref_option_ref.rs:40:14 | ||
| | | ||
| LL | type A = &'static Option<&'static Self>; | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Option<&'static Self>` | ||
|
|
||
| error: since `&` implements the `Copy` trait, `&Option<&T>` can be simplified to `Option<&T>` | ||
| --> $DIR/ref_option_ref.rs:46:12 | ||
| | | ||
| LL | let x: &Option<&u32> = &None; | ||
| | ^^^^^^^^^^^^^ help: try: `Option<&u32>` | ||
|
|
||
| error: aborting due to 11 previous errors | ||
|
|