ret type impl (not yet working)

rust-lang · Sep 3, 2024 · ff653ef · ff653ef
1 parent 320c0b9
commit ff653ef
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 20 deletions.
diff --git a/clippy_lints/src/functions/ref_option.rs b/clippy_lints/src/functions/ref_option.rs
@@ -4,29 +4,39 @@ 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_hir::{GenericArgs, ImplItem, MutTy, QPath, TraitItem, Ty, TyKind};
 use rustc_lint::LateContext;
 use rustc_span::{sym, Span};
 
+fn check_ty(cx: &LateContext<'_>, ty: &Ty<'_>, param_span: Span, fixes: &mut Vec<(Span, String)>) {
+    // TODO: is this the right way to check if ty is an Option?
+    if 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, ".."))));
+    }
+}
+
 fn check_fn_sig(cx: &LateContext<'_>, decl: &hir::FnDecl<'_>, sig_span: Span) {
     let mut fixes = Vec::new();
+    // Check function params
     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 let TyKind::Ref(_, MutTy { ty, .. }) = param.kind {
+            check_ty(cx, ty, param.span, &mut fixes);
         }
     }
+    // Check output type
+    if let hir::FnRetTy::Return(ty) = &decl.output {
+        check_ty(cx, ty, ty.span, &mut fixes);
+    }
 
     if !fixes.is_empty() {
         span_lint_and_then(

diff --git a/tests/ui/ref_option.fixed b/tests/ui/ref_option.fixed
@@ -6,6 +6,12 @@ fn main() {}
 fn opt_u8(a: Option<&u8>) {}
 fn opt_gen<T>(a: Option<&T>) {}
 fn opt_string(a: Option<&String>) {}
+fn ret_string<'a>(p: &'a String) -> &'a Option<u8> {
+    unimplemented!()
+}
+fn ret_string_static() -> &'static Option<u8> {
+    unimplemented!()
+}
 fn mult_string(a: Option<&String>, b: Option<&Vec<u8>>) {}
 
 pub fn pub_opt_string(a: Option<&String>) {}

diff --git a/tests/ui/ref_option.rs b/tests/ui/ref_option.rs
@@ -6,6 +6,12 @@ fn main() {}
 fn opt_u8(a: &Option<u8>) {}
 fn opt_gen<T>(a: &Option<T>) {}
 fn opt_string(a: &Option<String>) {}
+fn ret_string<'a>(p: &'a String) -> &'a Option<u8> {
+    unimplemented!()
+}
+fn ret_string_static() -> &'static Option<u8> {
+    unimplemented!()
+}
 fn mult_string(a: &Option<String>, b: &Option<Vec<u8>>) {}
 
 pub fn pub_opt_string(a: &Option<String>) {}

diff --git a/tests/ui/ref_option.stderr b/tests/ui/ref_option.stderr
@@ -26,7 +26,7 @@ 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.rs:9:1
+  --> tests/ui/ref_option.rs:15:1
    |
 LL | fn mult_string(a: &Option<String>, b: &Option<Vec<u8>>) {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -37,31 +37,31 @@ 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.rs:11:1
+  --> tests/ui/ref_option.rs:17: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.rs:14:5
+  --> tests/ui/ref_option.rs:20: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.rs:18:5
+  --> tests/ui/ref_option.rs:24: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.rs:24:5
+  --> tests/ui/ref_option.rs:30:5
    |
 LL |     pub fn opt_params(&self, a: &Option<()>) {}
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^-----------^