-
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.
Suggest
Option<&T> instead of &Option<T>
- Loading branch information
Showing
7 changed files
with
208 additions
and
0 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
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,60 @@ | ||
| use crate::functions::hir::GenericArg; | ||
| use crate::functions::REF_OPTION_SIG; | ||
| use clippy_utils::diagnostics::span_lint_and_then; | ||
| use clippy_utils::source::snippet; | ||
| use rustc_errors::Applicability; | ||
| use rustc_hir as hir; | ||
| use rustc_hir::{GenericArgs, ImplItem, MutTy, QPath, TraitItem, TyKind}; | ||
| use rustc_lint::LateContext; | ||
| use rustc_span::{sym, Span}; | ||
|
|
||
| fn check_fn_sig(cx: &LateContext<'_>, decl: &hir::FnDecl<'_>, sig_span: Span) { | ||
| let mut fixes = Vec::new(); | ||
| for param in decl.inputs { | ||
| if let TyKind::Ref(_, MutTy { ty, .. }) = param.kind | ||
| // TODO: is this the right way to check if ty is an Option? | ||
| && let TyKind::Path(QPath::Resolved(_, path)) = ty.kind | ||
| && path.segments.len() == 1 | ||
| && let seg = &path.segments[0] | ||
| && seg.ident.name == sym::Option | ||
| // check if option contains a regular type, not a reference | ||
| // TODO: Should this instead just check that opt_ty is a TyKind::Path? | ||
| && let Some(GenericArgs { args: [GenericArg::Type(opt_ty)], .. }) = seg.args | ||
| && !matches!(opt_ty.kind, TyKind::Ref(..)) | ||
| { | ||
| // FIXME: Should this use the Option path from the original type? | ||
| // FIXME: Should reference be added in some other way to the snippet? | ||
| fixes.push((param.span, format!("Option<&{}>", snippet(cx, opt_ty.span, "..")))); | ||
| } | ||
| } | ||
|
|
||
| if !fixes.is_empty() { | ||
| span_lint_and_then( | ||
| cx, | ||
| REF_OPTION_SIG, | ||
| sig_span, | ||
| "it is more idiomatic to use `Option<&T>` instead of `&Option<T>`", | ||
| |diag| { | ||
| diag.multipart_suggestion("change this to", fixes, Applicability::Unspecified); | ||
| }, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) { | ||
| if let hir::ItemKind::Fn(ref sig, _, _) = item.kind { | ||
| check_fn_sig(cx, sig.decl, sig.span); | ||
| } | ||
| } | ||
|
|
||
| pub(super) fn check_impl_item(cx: &LateContext<'_>, impl_item: &ImplItem<'_>) { | ||
| if let hir::ImplItemKind::Fn(ref sig, _) = impl_item.kind { | ||
| check_fn_sig(cx, sig.decl, sig.span); | ||
| } | ||
| } | ||
|
|
||
| pub(super) fn check_trait_item(cx: &LateContext<'_>, trait_item: &TraitItem<'_>) { | ||
| if let hir::TraitItemKind::Fn(ref sig, _) = trait_item.kind { | ||
| check_fn_sig(cx, sig.decl, sig.span); | ||
| } | ||
| } |
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,25 @@ | ||
| #![allow(unused, clippy::all)] | ||
| #![warn(clippy::ref_option_sig)] | ||
|
|
||
| fn main() {} | ||
|
|
||
| fn opt_u8(a: Option<&u8>) {} | ||
| fn opt_gen<T>(a: Option<&T>) {} | ||
| fn opt_string(a: Option<&String>) {} | ||
| fn mult_string(a: Option<&String>, b: Option<&Vec<u8>>) {} | ||
|
|
||
| pub fn pub_opt_string(a: Option<&String>) {} | ||
|
|
||
| pub trait HasOpt { | ||
| fn trait_opt(&self, a: Option<&Vec<u8>>); | ||
| } | ||
|
|
||
| trait PrivateHasOpt { | ||
| fn private_trait_opt(&self, a: Option<&Option<String>>); | ||
| } | ||
|
|
||
| pub struct UnitOpt; | ||
|
|
||
| impl UnitOpt { | ||
| pub fn opt_params(&self, a: Option<&()>) {} | ||
| } |
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,25 @@ | ||
| #![allow(unused, clippy::all)] | ||
| #![warn(clippy::ref_option_sig)] | ||
|
|
||
| fn main() {} | ||
|
|
||
| fn opt_u8(a: &Option<u8>) {} | ||
| fn opt_gen<T>(a: &Option<T>) {} | ||
| fn opt_string(a: &Option<String>) {} | ||
| fn mult_string(a: &Option<String>, b: &Option<Vec<u8>>) {} | ||
|
|
||
| pub fn pub_opt_string(a: &Option<String>) {} | ||
|
|
||
| pub trait HasOpt { | ||
| fn trait_opt(&self, a: &Option<Vec<u8>>); | ||
| } | ||
|
|
||
| trait PrivateHasOpt { | ||
| fn private_trait_opt(&self, a: &Option<Option<String>>); | ||
| } | ||
|
|
||
| pub struct UnitOpt; | ||
|
|
||
| impl UnitOpt { | ||
| pub fn opt_params(&self, a: &Option<()>) {} | ||
| } |
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,72 @@ | ||
| error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>` | ||
| --> tests/ui/ref_option_sig.rs:6:1 | ||
| | | ||
| LL | fn opt_u8(a: &Option<u8>) {} | ||
| | ^^^^^^^^^^^^^-----------^ | ||
| | | | ||
| | help: change this to: `Option<&u8>` | ||
| | | ||
| = note: `-D clippy::ref-option-sig` implied by `-D warnings` | ||
| = help: to override `-D warnings` add `#[allow(clippy::ref_option_sig)]` | ||
|
|
||
| error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>` | ||
| --> tests/ui/ref_option_sig.rs:7:1 | ||
| | | ||
| LL | fn opt_gen<T>(a: &Option<T>) {} | ||
| | ^^^^^^^^^^^^^^^^^----------^ | ||
| | | | ||
| | help: change this to: `Option<&T>` | ||
|
|
||
| error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>` | ||
| --> tests/ui/ref_option_sig.rs:8:1 | ||
| | | ||
| LL | fn opt_string(a: &Option<String>) {} | ||
| | ^^^^^^^^^^^^^^^^^---------------^ | ||
| | | | ||
| | help: change this to: `Option<&String>` | ||
|
|
||
| error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>` | ||
| --> tests/ui/ref_option_sig.rs:9:1 | ||
| | | ||
| LL | fn mult_string(a: &Option<String>, b: &Option<Vec<u8>>) {} | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | | ||
| help: change this to | ||
| | | ||
| LL | fn mult_string(a: Option<&String>, b: Option<&Vec<u8>>) {} | ||
| | ~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~ | ||
|
|
||
| error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>` | ||
| --> tests/ui/ref_option_sig.rs:11:1 | ||
| | | ||
| LL | pub fn pub_opt_string(a: &Option<String>) {} | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^---------------^ | ||
| | | | ||
| | help: change this to: `Option<&String>` | ||
|
|
||
| error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>` | ||
| --> tests/ui/ref_option_sig.rs:14:5 | ||
| | | ||
| LL | fn trait_opt(&self, a: &Option<Vec<u8>>); | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^----------------^^ | ||
| | | | ||
| | help: change this to: `Option<&Vec<u8>>` | ||
|
|
||
| error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>` | ||
| --> tests/ui/ref_option_sig.rs:18:5 | ||
| | | ||
| LL | fn private_trait_opt(&self, a: &Option<Option<String>>); | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-----------------------^^ | ||
| | | | ||
| | help: change this to: `Option<&Option<String>>` | ||
|
|
||
| error: it is more idiomatic to use `Option<&T>` instead of `&Option<T>` | ||
| --> tests/ui/ref_option_sig.rs:24:5 | ||
| | | ||
| LL | pub fn opt_params(&self, a: &Option<()>) {} | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^-----------^ | ||
| | | | ||
| | help: change this to: `Option<&()>` | ||
|
|
||
| error: aborting due to 8 previous errors | ||
|
|