Auto merge of #4425 - mikerite:4375, r=flip1995

Fix `temporary_cstring_as_ptr` false negative

Fixes #4375.

Changes the check to test when `.unwrap().as_ptr()` is called on any
`Result<CString, _>` as suggested by @flip1995
(https://github.com/rust-lang/rust-clippy/issues/4375#issuecomment-520724123).

changelog: Fix `temporary_cstring_as_ptr` false negative

clippy_lints/src/methods/mod.rs

@@ -8,7 +8,6 @@ use std::iter;
 use if_chain::if_chain;
 use matches::matches;
 use rustc::hir;
-use rustc::hir::def::{DefKind, Res};
 use rustc::hir::intravisit::{self, Visitor};
 use rustc::lint::{in_external_macro, LateContext, LateLintPass, Lint, LintArray, LintContext, LintPass};
 use rustc::ty::{self, Predicate, Ty};
@@ -1668,13 +1667,12 @@ fn lint_extend(cx: &LateContext<'_, '_>, expr: &hir::Expr, args: &[hir::Expr]) {
     }
 }
 
-fn lint_cstring_as_ptr(cx: &LateContext<'_, '_>, expr: &hir::Expr, new: &hir::Expr, unwrap: &hir::Expr) {
+fn lint_cstring_as_ptr(cx: &LateContext<'_, '_>, expr: &hir::Expr, source: &hir::Expr, unwrap: &hir::Expr) {
     if_chain! {
-        if let hir::ExprKind::Call(ref fun, ref args) = new.node;
-        if args.len() == 1;
-        if let hir::ExprKind::Path(ref path) = fun.node;
-        if let Res::Def(DefKind::Method, did) = cx.tables.qpath_res(path, fun.hir_id);
-        if match_def_path(cx, did, &paths::CSTRING_NEW);
+        let source_type = cx.tables.expr_ty(source);
+        if let ty::Adt(def, substs) = source_type.sty;
+        if match_def_path(cx, def.did, &paths::RESULT);
+        if match_type(cx, substs.type_at(0), &paths::CSTRING);
         then {
             span_lint_and_then(
                 cx,

clippy_lints/src/utils/paths.rs

@@ -17,7 +17,7 @@ pub const CLONE_TRAIT_METHOD: [&str; 4] = ["core", "clone", "Clone", "clone"];
 pub const CMP_MAX: [&str; 3] = ["core", "cmp", "max"];
 pub const CMP_MIN: [&str; 3] = ["core", "cmp", "min"];
 pub const COW: [&str; 3] = ["alloc", "borrow", "Cow"];
-pub const CSTRING_NEW: [&str; 5] = ["std", "ffi", "c_str", "CString", "new"];
+pub const CSTRING: [&str; 4] = ["std", "ffi", "c_str", "CString"];
 pub const DEFAULT_TRAIT: [&str; 3] = ["core", "default", "Default"];
 pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "default"];
 pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"];

tests/ui/cstring.rs

@@ -1,9 +1,24 @@
+#![deny(clippy::temporary_cstring_as_ptr)]
+
 fn main() {}
 
-#[allow(clippy::result_unwrap_used)]
 fn temporary_cstring() {
     use std::ffi::CString;
 
     CString::new("foo").unwrap().as_ptr();
     CString::new("foo").expect("dummy").as_ptr();
 }
+
+mod issue4375 {
+    use std::ffi::CString;
+    use std::os::raw::c_char;
+
+    extern "C" {
+        fn foo(data: *const c_char);
+    }
+
+    pub fn bar(v: &[u8]) {
+        let cstr = CString::new(v);
+        unsafe { foo(cstr.unwrap().as_ptr()) }
+    }
+}

tests/ui/cstring.stderr

@@ -1,29 +1,46 @@
 error: you are getting the inner pointer of a temporary `CString`
-  --> $DIR/cstring.rs:7:5
+  --> $DIR/cstring.rs:8:5
    |
 LL |     CString::new("foo").unwrap().as_ptr();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
-   = note: `#[deny(clippy::temporary_cstring_as_ptr)]` on by default
+note: lint level defined here
+  --> $DIR/cstring.rs:1:9
+   |
+LL | #![deny(clippy::temporary_cstring_as_ptr)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    = note: that pointer will be invalid outside this expression
 help: assign the `CString` to a variable to extend its lifetime
-  --> $DIR/cstring.rs:7:5
+  --> $DIR/cstring.rs:8:5
    |
 LL |     CString::new("foo").unwrap().as_ptr();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: you are getting the inner pointer of a temporary `CString`
-  --> $DIR/cstring.rs:8:5
+  --> $DIR/cstring.rs:9:5
    |
 LL |     CString::new("foo").expect("dummy").as_ptr();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: that pointer will be invalid outside this expression
 help: assign the `CString` to a variable to extend its lifetime
-  --> $DIR/cstring.rs:8:5
+  --> $DIR/cstring.rs:9:5
    |
 LL |     CString::new("foo").expect("dummy").as_ptr();
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 2 previous errors
+error: you are getting the inner pointer of a temporary `CString`
+  --> $DIR/cstring.rs:22:22
+   |
+LL |         unsafe { foo(cstr.unwrap().as_ptr()) }
+   |                      ^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: that pointer will be invalid outside this expression
+help: assign the `CString` to a variable to extend its lifetime
+  --> $DIR/cstring.rs:22:22
+   |
+LL |         unsafe { foo(cstr.unwrap().as_ptr()) }
+   |                      ^^^^^^^^^^^^^
+
+error: aborting due to 3 previous errors