Fix false negative for Strings

`String` is not a diagnostic item and was thus not picked up by
`is_type_diagnostic_item`, leading to a false negative for
`collection_is_never_read`

Author: bluthej

clippy_lints/src/collection_is_never_read.rs

@@ -1,9 +1,9 @@
 use clippy_utils::diagnostics::span_lint;
-use clippy_utils::ty::is_type_diagnostic_item;
+use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item};
 use clippy_utils::visitors::for_each_expr_with_closures;
 use clippy_utils::{get_enclosing_block, get_parent_node, path_to_local_id};
 use core::ops::ControlFlow;
-use rustc_hir::{Block, ExprKind, HirId, Local, Node, PatKind};
+use rustc_hir::{Block, ExprKind, HirId, LangItem, Local, Node, PatKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::symbol::sym;
@@ -44,24 +44,23 @@ declare_clippy_lint! {
 }
 declare_lint_pass!(CollectionIsNeverRead => [COLLECTION_IS_NEVER_READ]);
 
-static COLLECTIONS: [Symbol; 10] = [
+// Add `String` here when it is added to diagnostic items
+static COLLECTIONS: [Symbol; 9] = [
     sym::BTreeMap,
     sym::BTreeSet,
     sym::BinaryHeap,
     sym::HashMap,
     sym::HashSet,
     sym::LinkedList,
     sym::Option,
-    sym::String,
     sym::Vec,
     sym::VecDeque,
 ];
 
 impl<'tcx> LateLintPass<'tcx> for CollectionIsNeverRead {
     fn check_local(&mut self, cx: &LateContext<'tcx>, local: &'tcx Local<'tcx>) {
         // Look for local variables whose type is a container. Search surrounding bock for read access.
-        let ty = cx.typeck_results().pat_ty(local.pat);
-        if COLLECTIONS.iter().any(|&sym| is_type_diagnostic_item(cx, ty, sym))
+        if match_acceptable_type(cx, local, &COLLECTIONS)
             && let PatKind::Binding(_, local_id, _, _) = local.pat.kind
             && let Some(enclosing_block) = get_enclosing_block(cx, local.hir_id)
             && has_no_read_access(cx, local_id, enclosing_block)
@@ -71,6 +70,13 @@ impl<'tcx> LateLintPass<'tcx> for CollectionIsNeverRead {
     }
 }
 
+fn match_acceptable_type(cx: &LateContext<'_>, local: &Local<'_>, collections: &[rustc_span::Symbol]) -> bool {
+    let ty = cx.typeck_results().pat_ty(local.pat);
+    collections.iter().any(|&sym| is_type_diagnostic_item(cx, ty, sym))
+    // String type is a lang item but not a diagnostic item for now so we need a separate check
+        || is_type_lang_item(cx, ty, LangItem::String)
+}
+
 fn has_no_read_access<'tcx>(cx: &LateContext<'tcx>, id: HirId, block: &'tcx Block<'tcx>) -> bool {
     let mut has_access = false;
     let mut has_read_access = false;

tests/ui/collection_is_never_read.rs

@@ -163,3 +163,23 @@ fn function_argument() {
     let x = vec![1, 2, 3]; // Ok
     foo(&x);
 }
+
+fn string() {
+    // Do lint (write without read)
+    let mut s = String::new();
+    s.push_str("Hello, World!");
+
+    // Do not lint (read without write)
+    let mut s = String::from("Hello, World!");
+    let _ = s.len();
+
+    // Do not lint (write and read)
+    let mut s = String::from("Hello, World!");
+    s.push_str("foo, bar");
+    let _ = s.len();
+
+    // Do lint the first line, but not the second
+    let mut s = String::from("Hello, World!");
+    let t = String::from("foo, bar");
+    s = t;
+}

tests/ui/collection_is_never_read.stderr

@@ -48,5 +48,17 @@ error: collection is never read
 LL |     let x = vec![1, 2, 3]; // WARNING
    |     ^^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 8 previous errors
+error: collection is never read
+  --> $DIR/collection_is_never_read.rs:169:5
+   |
+LL |     let mut s = String::new();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: collection is never read
+  --> $DIR/collection_is_never_read.rs:182:5
+   |
+LL |     let mut s = String::from("Hello, World!");
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 10 previous errors