New lint: single_field_patterns

CHANGELOG.md

@@ -3255,6 +3255,7 @@ Released 2018-09-13
 [`single_char_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern
 [`single_component_path_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports
 [`single_element_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_element_loop
+[`single_field_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_field_patterns
 [`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match
 [`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else
 [`size_of_in_element_count`]: https://rust-lang.github.io/rust-clippy/master/index.html#size_of_in_element_count

clippy_lints/src/lib.register_all.rs

@@ -253,6 +253,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(self_named_constructors::SELF_NAMED_CONSTRUCTORS),
     LintId::of(serde_api::SERDE_API_MISUSE),
     LintId::of(single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
+    LintId::of(single_field_patterns::SINGLE_FIELD_PATTERNS),
     LintId::of(size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT),
     LintId::of(slow_vector_initialization::SLOW_VECTOR_INITIALIZATION),
     LintId::of(stable_sort_primitive::STABLE_SORT_PRIMITIVE),

clippy_lints/src/lib.register_lints.rs

@@ -435,6 +435,7 @@ store.register_lints(&[
     shadow::SHADOW_SAME,
     shadow::SHADOW_UNRELATED,
     single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS,
+    single_field_patterns::SINGLE_FIELD_PATTERNS,
     size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT,
     slow_vector_initialization::SLOW_VECTOR_INITIALIZATION,
     stable_sort_primitive::STABLE_SORT_PRIMITIVE,

clippy_lints/src/lib.register_style.rs

@@ -99,6 +99,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(returns::NEEDLESS_RETURN),
     LintId::of(self_named_constructors::SELF_NAMED_CONSTRUCTORS),
     LintId::of(single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
+    LintId::of(single_field_patterns::SINGLE_FIELD_PATTERNS),
     LintId::of(tabs_in_doc_comments::TABS_IN_DOC_COMMENTS),
     LintId::of(to_digit_is_some::TO_DIGIT_IS_SOME),
     LintId::of(try_err::TRY_ERR),

clippy_lints/src/lib.rs

@@ -349,6 +349,7 @@ mod semicolon_if_nothing_returned;
 mod serde_api;
 mod shadow;
 mod single_component_path_imports;
+mod single_field_patterns;
 mod size_of_in_element_count;
 mod slow_vector_initialization;
 mod stable_sort_primitive;
@@ -853,6 +854,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(trailing_empty_array::TrailingEmptyArray));
     store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes));
     store.register_late_pass(|| Box::new(needless_late_init::NeedlessLateInit));
+    store.register_late_pass(|| Box::new(single_field_patterns::SingleFieldPatterns));
     store.register_late_pass(|| Box::new(return_self_not_must_use::ReturnSelfNotMustUse));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }

clippy_lints/src/open_options.rs

@@ -5,7 +5,7 @@ use rustc_ast::ast::LitKind;
 use rustc_hir::{Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::source_map::{Span, Spanned};
+use rustc_span::source_map::Span;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -67,11 +67,7 @@ fn get_open_options(cx: &LateContext<'_>, argument: &Expr<'_>, options: &mut Vec
         if match_type(cx, obj_ty, &paths::OPEN_OPTIONS) && arguments.len() >= 2 {
             let argument_option = match arguments[1].kind {
                 ExprKind::Lit(ref span) => {
-                    if let Spanned {
-                        node: LitKind::Bool(lit),
-                        ..
-                    } = *span
-                    {
+                    if let LitKind::Bool(lit) = span.node {
                         if lit { Argument::True } else { Argument::False }
                     } else {
                         // The function is called with a literal which is not a boolean literal.

clippy_lints/src/single_field_patterns.rs

@@ -0,0 +1,263 @@
+#![allow(rustc::usage_of_ty_tykind)]
+
+use clippy_utils::{
+    diagnostics::{multispan_sugg_with_applicability, span_lint_and_then},
+    higher::IfLetOrMatch,
+    higher::WhileLet,
+    source::snippet_opt,
+};
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind, Local, MatchSource, Pat, PatKind, Stmt, StmtKind, UnOp};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::{symbol::Ident, Span};
+use std::iter;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///  Checks for patterns that only match a single field of a struct when they could directly access the field.
+    ///
+    /// ### Why is this bad?
+    ///  It requires more text and more information than directly accessing the field.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # struct Struct {
+    /// #     field1: Option<i32>,
+    /// #     field2: Option<i32>,
+    /// # }
+    /// # fn foo(struct1: Struct) {
+    /// // bad:
+    /// match struct1 {
+    ///     Struct { field1: Some(n), .. } if n >= 50 => {},
+    ///     Struct { field1: None, .. } => {},
+    ///     _ => {},
+    /// }
+    /// // better:
+    /// match struct1.field1 {
+    ///     Some(n) if n >= 50 => {},
+    ///     None => {},
+    ///     _ => {},
+    /// }
+    /// # }
+    /// ```
+    #[clippy::version = "1.59.0"]
+    pub SINGLE_FIELD_PATTERNS,
+    style,
+    "single-field patterns"
+}
+declare_lint_pass!(SingleFieldPatterns => [SINGLE_FIELD_PATTERNS]);
+
+/// This represents 0 or 1 fields being used. Where more may be used, `Option<SingleField>` is used
+///   where `None` represents the absence of a lint
+#[derive(Debug, Clone, Copy)]
+enum SingleField {
+    Id { id: Ident, pattern: Span },
+    Unused, // The name "SingleField" is a lie but idk what's better. "AtMostOneField"?
+}
+
+impl PartialEq for SingleField {
+    fn eq(&self, other: &Self) -> bool {
+        match (self, other) {
+            (SingleField::Id { id: id1, .. }, SingleField::Id { id: id2, .. }) => id1 == id2,
+            (SingleField::Unused, SingleField::Unused) => true,
+            _ => false,
+        }
+    }
+}
+
+impl SingleField {
+    fn new<'a>(mut iter: impl Iterator<Item = (Ident, &'a Pat<'a>)>) -> Option<SingleField> {
+        let one = iter.by_ref().find(|(_, pat)| !matches!(pat.kind, PatKind::Wild));
+        match one {
+            Some((id, pat)) => {
+                if pat.span.from_expansion() {
+                    return None;
+                }
+                if iter.all(|(_, other)| matches!(other.kind, PatKind::Wild)) {
+                    Some(SingleField::Id { id, pattern: pat.span })
+                } else {
+                    None
+                }
+            },
+            None => Some(SingleField::Unused),
+        }
+    }
+}
+
+/// This handles recursive patterns and flattens them out lazily
+/// e.g. 1 | (2 | 9) | 3..5
+struct FlatPatterns<'hir, I>
+where
+    I: Iterator<Item = &'hir Pat<'hir>>,
+{
+    patterns: I,
+    stack: Vec<&'hir Pat<'hir>>,
+}
+
+impl<I: Iterator<Item = &'hir Pat<'hir>>> FlatPatterns<'hir, I> {
+    fn new(patterns: I) -> Self {
+        Self {
+            patterns,
+            stack: Vec::new(),
+        }
+    }
+}
+
+impl<I: Iterator<Item = &'hir Pat<'hir>>> Iterator for FlatPatterns<'hir, I> {
+    type Item = &'hir Pat<'hir>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        if self.stack.is_empty() {
+            if let Some(pat) = self.patterns.next() {
+                self.stack.push(pat);
+            } else {
+                return None;
+            }
+        }
+        while let Some(pat) = self.stack.pop() {
+            match pat.kind {
+                PatKind::Or(pats) => self.stack.extend(pats),
+                _ => return Some(pat),
+            }
+        }
+        unreachable!("Or always has 2 patterns, so one of the prior returns must return");
+    }
+}
+
+fn find_sf_lint<'hir>(
+    cx: &LateContext<'_>,
+    patterns: impl Iterator<Item = &'hir Pat<'hir>>,
+) -> Option<(SingleField, Vec<(Span, String)>)> {
+    let fields = FlatPatterns::new(patterns).map(|p| {
+        (
+            p.span,
+            match p.kind {
+                PatKind::Wild => Some(SingleField::Unused),
+                PatKind::Struct(_, pats, _) => SingleField::new(pats.iter().map(|field| (field.ident, field.pat))),
+                _ => None,
+            },
+        )
+    });
+    let mut spans = Vec::<(Span, String)>::new();
+    let mut the_one: Option<SingleField> = None;
+    for (target, sf) in fields {
+        if target.from_expansion() {
+            return None;
+        }
+        if let Some(sf) = sf {
+            match sf {
+                SingleField::Unused => {
+                    spans.push((target, String::from("_")));
+                },
+                SingleField::Id { pattern, .. } => {
+                    if let Some(str) = snippet_opt(cx, pattern) {
+                        spans.push((target, str));
+                    } else {
+                        return None;
+                    }
+                    if let Some(one) = the_one {
+                        if sf != one {
+                            return None;
+                        }
+                    } else {
+                        the_one = Some(sf);
+                    }
+                },
+            }
+        } else {
+            return None;
+        }
+    }
+    the_one.map(|one| (one, spans))
+}
+
+fn apply_lint_sf(cx: &LateContext<'_>, span: Span, sugg: impl IntoIterator<Item = (Span, String)>) {
+    span_lint_and_then(
+        cx,
+        SINGLE_FIELD_PATTERNS,
+        span,
+        "this single-variant pattern only matches one field",
+        |diag| {
+            multispan_sugg_with_applicability(
+                diag,
+                "try accessing this field directly",
+                Applicability::MaybeIncorrect,
+                sugg,
+            );
+        },
+    );
+}
+
+fn remove_deref<'a>(mut scrutinee: &'a Expr<'a>) -> &'a Expr<'a> {
+    // it would be wrong to convert something like `if let (x, _) = *a` into `if let x = *a.0`
+    while let ExprKind::Unary(UnOp::Deref, expr) = scrutinee.kind {
+        scrutinee = expr;
+    }
+    scrutinee
+}
+
+fn lint_sf<'hir>(
+    cx: &LateContext<'_>,
+    overall_span: Span,
+    scrutinee: &Expr<'_>,
+    patterns: impl Iterator<Item = &'hir Pat<'hir>>,
+) {
+    if scrutinee.span.from_expansion() {
+        return;
+    }
+    let scrutinee_name = if let Some(name) = snippet_opt(cx, remove_deref(scrutinee).span) {
+        name
+    } else {
+        return;
+    };
+    match cx.typeck_results().expr_ty(scrutinee).kind() {
+        ty::Adt(def, ..) if def.is_struct() => {
+            if let Some((field, mut spans)) = find_sf_lint(cx, patterns) {
+                spans.push((
+                    scrutinee.span,
+                    match field {
+                        SingleField::Id { id, .. } => format!("{}.{}", scrutinee_name, id.as_str()),
+                        SingleField::Unused => return,
+                    },
+                ));
+                apply_lint_sf(cx, overall_span, spans);
+            }
+        },
+        _ => (),
+    };
+}
+
+impl LateLintPass<'_> for SingleFieldPatterns {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
+        if expr.span.from_expansion() {
+            return;
+        }
+        match IfLetOrMatch::parse(cx, expr) {
+            Some(IfLetOrMatch::Match(scrutinee, arms, MatchSource::Normal)) => {
+                lint_sf(cx, expr.span, scrutinee, arms.iter().map(|arm| arm.pat));
+            },
+            Some(IfLetOrMatch::IfLet(scrutinee, pat, ..)) => lint_sf(cx, expr.span, scrutinee, iter::once(pat)),
+            _ => {
+                if let Some(WhileLet { let_pat, let_expr, .. }) = WhileLet::hir(expr) {
+                    lint_sf(cx, expr.span, let_expr, iter::once(let_pat));
+                }
+            },
+        };
+    }
+
+    fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &Stmt<'tcx>) {
+        if stmt.span.from_expansion() {
+            return;
+        }
+        if let StmtKind::Local(Local {
+            pat,
+            init: Some(scrutinee),
+            ..
+        }) = stmt.kind
+        {
+            lint_sf(cx, stmt.span, scrutinee, iter::once(*pat));
+        }
+    }
+}

tests/ui/author/struct.rs

@@ -1,4 +1,4 @@
-#[allow(clippy::unnecessary_operation, clippy::single_match)]
+#[allow(clippy::unnecessary_operation, clippy::single_match, clippy::single_field_patterns)]
 fn main() {
     struct Test {
         field: u32,