diff options
31 files changed, 722 insertions, 109 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index c7e02aaf4e1..adb4a5c8261 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2024,6 +2024,7 @@ Released 2018-09-13 [`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure [`redundant_closure_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_call [`redundant_closure_for_method_calls`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_for_method_calls +[`redundant_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_else [`redundant_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names [`redundant_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern [`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index edecba57e44..55e4755c278 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -480,7 +480,7 @@ fn check_code(cx: &LateContext<'_>, text: &str, span: Span) { | ItemKind::ForeignMod(..) => return false, // We found a main function ... ItemKind::Fn(_, sig, _, Some(block)) if item.ident.name == sym::main => { - let is_async = matches!(sig.header.asyncness, Async::Yes{..}); + let is_async = matches!(sig.header.asyncness, Async::Yes { .. }); let returns_nothing = match &sig.decl.output { FnRetTy::Default(..) => true, FnRetTy::Ty(ty) if ty.kind.is_unit() => true, diff --git a/clippy_lints/src/functions.rs b/clippy_lints/src/functions.rs index 8b58d1f2601..fd93548b55c 100644 --- a/clippy_lints/src/functions.rs +++ b/clippy_lints/src/functions.rs @@ -405,13 +405,10 @@ impl<'tcx> Functions { break; } if in_comment { - match line.find("*/") { - Some(i) => { - line = &line[i + 2..]; - in_comment = false; - continue; - }, - None => break, + if let Some(i) = line.find("*/") { + line = &line[i + 2..]; + in_comment = false; + continue; } } else { let multi_idx = line.find("/*").unwrap_or_else(|| line.len()); @@ -423,8 +420,8 @@ impl<'tcx> Functions { in_comment = true; continue; } - break; } + break; } if code_in_line { line_count += 1; diff --git a/clippy_lints/src/future_not_send.rs b/clippy_lints/src/future_not_send.rs index f9697afe405..eb5857348fd 100644 --- a/clippy_lints/src/future_not_send.rs +++ b/clippy_lints/src/future_not_send.rs @@ -92,13 +92,8 @@ impl<'tcx> LateLintPass<'tcx> for FutureNotSend { |db| { cx.tcx.infer_ctxt().enter(|infcx| { for FulfillmentError { obligation, .. } in send_errors { - infcx.maybe_note_obligation_cause_for_async_await( - db, - &obligation, - ); - if let Trait(trait_pred, _) = - obligation.predicate.skip_binders() - { + infcx.maybe_note_obligation_cause_for_async_await(db, &obligation); + if let Trait(trait_pred, _) = obligation.predicate.skip_binders() { db.note(&format!( "`{}` doesn't implement `{}`", trait_pred.self_ty(), diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index 8842901d90b..6fe53351090 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -222,9 +222,8 @@ fn check_impl_items(cx: &LateContext<'_>, item: &Item<'_>, impl_items: &[ImplIte let is_empty = if let Some(is_empty) = impl_items.iter().find(|i| is_named_self(cx, i, "is_empty")) { if cx.access_levels.is_exported(is_empty.id.hir_id) { return; - } else { - "a private" } + "a private" } else { "no corresponding" }; diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 2b99ed570b1..ac5a45ccfd2 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -294,6 +294,7 @@ mod question_mark; mod ranges; mod redundant_clone; mod redundant_closure_call; +mod redundant_else; mod redundant_field_names; mod redundant_pub_crate; mod redundant_static_lifetimes; @@ -831,6 +832,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &ranges::REVERSED_EMPTY_RANGES, &redundant_clone::REDUNDANT_CLONE, &redundant_closure_call::REDUNDANT_CLOSURE_CALL, + &redundant_else::REDUNDANT_ELSE, &redundant_field_names::REDUNDANT_FIELD_NAMES, &redundant_pub_crate::REDUNDANT_PUB_CRATE, &redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES, @@ -1132,6 +1134,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| box items_after_statements::ItemsAfterStatements); store.register_early_pass(|| box precedence::Precedence); store.register_early_pass(|| box needless_continue::NeedlessContinue); + store.register_early_pass(|| box redundant_else::RedundantElse); store.register_late_pass(|| box create_dir::CreateDir); store.register_early_pass(|| box needless_arbitrary_self_type::NeedlessArbitrarySelfType); store.register_early_pass(|| box redundant_static_lifetimes::RedundantStaticLifetimes); @@ -1308,6 +1311,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF), LintId::of(&ranges::RANGE_MINUS_ONE), LintId::of(&ranges::RANGE_PLUS_ONE), + LintId::of(&redundant_else::REDUNDANT_ELSE), LintId::of(&ref_option_ref::REF_OPTION_REF), LintId::of(&shadow::SHADOW_UNRELATED), LintId::of(&strings::STRING_ADD_ASSIGN), diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 274d20cfa80..2a1a73f98ee 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -689,10 +689,9 @@ fn check_single_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], exp if stmts.len() == 1 && block_expr.is_none() || stmts.is_empty() && block_expr.is_some() { // single statement/expr "else" block, don't lint return; - } else { - // block with 2+ statements or 1 expr and 1+ statement - Some(els) } + // block with 2+ statements or 1 expr and 1+ statement + Some(els) } else { // not a block, don't lint return; diff --git a/clippy_lints/src/methods/unnecessary_filter_map.rs b/clippy_lints/src/methods/unnecessary_filter_map.rs index 75e123eb593..d082a88cd2d 100644 --- a/clippy_lints/src/methods/unnecessary_filter_map.rs +++ b/clippy_lints/src/methods/unnecessary_filter_map.rs @@ -69,10 +69,9 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc } } return (true, false); - } else { - // We don't know. It might do anything. - return (true, true); } + // We don't know. It might do anything. + return (true, true); } } (true, true) diff --git a/clippy_lints/src/missing_const_for_fn.rs b/clippy_lints/src/missing_const_for_fn.rs index 25245b3dbf0..38e2ce563ee 100644 --- a/clippy_lints/src/missing_const_for_fn.rs +++ b/clippy_lints/src/missing_const_for_fn.rs @@ -99,7 +99,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingConstForFn { let has_const_generic_params = generics .params .iter() - .any(|param| matches!(param.kind, GenericParamKind::Const{ .. })); + .any(|param| matches!(param.kind, GenericParamKind::Const { .. })); if already_const(header) || has_const_generic_params { return; diff --git a/clippy_lints/src/needless_pass_by_value.rs b/clippy_lints/src/needless_pass_by_value.rs index 532c0266946..5043b7b1fc3 100644 --- a/clippy_lints/src/needless_pass_by_value.rs +++ b/clippy_lints/src/needless_pass_by_value.rs @@ -90,9 +90,10 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessPassByValue { // Exclude non-inherent impls if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { - if matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), .. } | - ItemKind::Trait(..)) - { + if matches!( + item.kind, + ItemKind::Impl { of_trait: Some(_), .. } | ItemKind::Trait(..) + ) { return; } } diff --git a/clippy_lints/src/non_expressive_names.rs b/clippy_lints/src/non_expressive_names.rs index 5b42b61fcde..446426b3e61 100644 --- a/clippy_lints/src/non_expressive_names.rs +++ b/clippy_lints/src/non_expressive_names.rs @@ -409,11 +409,10 @@ fn levenstein_not_1(a_name: &str, b_name: &str) -> bool { if let Some(b2) = b_chars.next() { // check if there's just one character inserted return a != b2 || a_chars.ne(b_chars); - } else { - // tuple - // ntuple - return true; } + // tuple + // ntuple + return true; } // for item in items true diff --git a/clippy_lints/src/panic_in_result_fn.rs b/clippy_lints/src/panic_in_result_fn.rs index 72dfccc1089..37e2b50def1 100644 --- a/clippy_lints/src/panic_in_result_fn.rs +++ b/clippy_lints/src/panic_in_result_fn.rs @@ -1,18 +1,16 @@ -use crate::utils::{is_expn_of, is_type_diagnostic_item, return_ty, span_lint_and_then}; +use crate::utils::{find_macro_calls, is_type_diagnostic_item, return_ty, span_lint_and_then}; use rustc_hir as hir; -use rustc_hir::intravisit::{self, FnKind, NestedVisitorMap, Visitor}; -use rustc_hir::Expr; +use rustc_hir::intravisit::FnKind; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::hir::map::Map; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{sym, Span}; declare_clippy_lint! { - /// **What it does:** Checks for usage of `panic!`, `unimplemented!`, `todo!` or `unreachable!` in a function of type result. + /// **What it does:** Checks for usage of `panic!`, `unimplemented!`, `todo!`, `unreachable!` or assertions in a function of type result. /// - /// **Why is this bad?** For some codebases, it is desirable for functions of type result to return an error instead of crashing. Hence unimplemented, panic and unreachable should be avoided. + /// **Why is this bad?** For some codebases, it is desirable for functions of type result to return an error instead of crashing. Hence panicking macros should be avoided. /// - /// **Known problems:** None. + /// **Known problems:** Functions called from a function returning a `Result` may invoke a panicking macro. This is not checked. /// /// **Example:** /// @@ -22,9 +20,15 @@ declare_clippy_lint! { /// panic!("error"); /// } /// ``` + /// Use instead: + /// ```rust + /// fn result_without_panic() -> Result<bool, String> { + /// Err(String::from("error")) + /// } + /// ``` pub PANIC_IN_RESULT_FN, restriction, - "functions of type `Result<..>` that contain `panic!()`, `todo!()` or `unreachable()` or `unimplemented()` " + "functions of type `Result<..>` that contain `panic!()`, `todo!()`, `unreachable()`, `unimplemented()` or assertion" } declare_lint_pass!(PanicInResultFn => [PANIC_IN_RESULT_FN]); @@ -47,43 +51,33 @@ impl<'tcx> LateLintPass<'tcx> for PanicInResultFn { } } -struct FindPanicUnimplementedUnreachable { - result: Vec<Span>, -} - -impl<'tcx> Visitor<'tcx> for FindPanicUnimplementedUnreachable { - type Map = Map<'tcx>; - - fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { - if ["unimplemented", "unreachable", "panic", "todo"] - .iter() - .any(|fun| is_expn_of(expr.span, fun).is_some()) - { - self.result.push(expr.span); - } - // and check sub-expressions - intravisit::walk_expr(self, expr); - } - - fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { - NestedVisitorMap::None - } -} - fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir::Body<'tcx>) { - let mut panics = FindPanicUnimplementedUnreachable { result: Vec::new() }; - panics.visit_expr(&body.value); - if !panics.result.is_empty() { + let panics = find_macro_calls( + &[ + "unimplemented", + "unreachable", + "panic", + "todo", + "assert", + "assert_eq", + "assert_ne", + "debug_assert", + "debug_assert_eq", + "debug_assert_ne", + ], + body, + ); + if !panics.is_empty() { span_lint_and_then( cx, PANIC_IN_RESULT_FN, impl_span, - "used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result`", + "used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result`", move |diag| { diag.help( - "`unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing", + "`unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing", ); - diag.span_note(panics.result, "return Err() instead of panicking"); + diag.span_note(panics, "return Err() instead of panicking"); }, ); } diff --git a/clippy_lints/src/pass_by_ref_or_value.rs b/clippy_lints/src/pass_by_ref_or_value.rs index f03facc235e..6a17d654ac9 100644 --- a/clippy_lints/src/pass_by_ref_or_value.rs +++ b/clippy_lints/src/pass_by_ref_or_value.rs @@ -244,9 +244,10 @@ impl<'tcx> LateLintPass<'tcx> for PassByRefOrValue { // Exclude non-inherent impls if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { - if matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), .. } | - ItemKind::Trait(..)) - { + if matches!( + item.kind, + ItemKind::Impl { of_trait: Some(_), .. } | ItemKind::Trait(..) + ) { return; } } diff --git a/clippy_lints/src/redundant_clone.rs b/clippy_lints/src/redundant_clone.rs index f0e507105a6..06adbb523d7 100644 --- a/clippy_lints/src/redundant_clone.rs +++ b/clippy_lints/src/redundant_clone.rs @@ -390,7 +390,10 @@ impl<'tcx> mir::visit::Visitor<'tcx> for LocalUseVisitor { let local = place.local; if local == self.used.0 - && !matches!(ctx, PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(_)) + && !matches!( + ctx, + PlaceContext::MutatingUse(MutatingUseContext::Drop) | PlaceContext::NonUse(_) + ) { self.used.1 = true; } diff --git a/clippy_lints/src/redundant_else.rs b/clippy_lints/src/redundant_else.rs new file mode 100644 index 00000000000..3d585cd27a3 --- /dev/null +++ b/clippy_lints/src/redundant_else.rs @@ -0,0 +1,135 @@ +use crate::utils::span_lint_and_help; +use rustc_ast::ast::{Block, Expr, ExprKind, Stmt, StmtKind}; +use rustc_ast::visit::{walk_expr, Visitor}; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_middle::lint::in_external_macro; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// **What it does:** Checks for `else` blocks that can be removed without changing semantics. + /// + /// **Why is this bad?** The `else` block adds unnecessary indentation and verbosity. + /// + /// **Known problems:** Some may prefer to keep the `else` block for clarity. + /// + /// **Example:** + /// + /// ```rust + /// fn my_func(count: u32) { + /// if count == 0 { + /// print!("Nothing to do"); + /// return; + /// } else { + /// print!("Moving on..."); + /// } + /// } + /// ``` + /// Use instead: + /// ```rust + /// fn my_func(count: u32) { + /// if count == 0 { + /// print!("Nothing to do"); + /// return; + /// } + /// print!("Moving on..."); + /// } + /// ``` + pub REDUNDANT_ELSE, + pedantic, + "`else` branch that can be removed without changing semantics" +} + +declare_lint_pass!(RedundantElse => [REDUNDANT_ELSE]); + +impl EarlyLintPass for RedundantElse { + fn check_stmt(&mut self, cx: &EarlyContext<'_>, stmt: &Stmt) { + if in_external_macro(cx.sess, stmt.span) { + return; + } + // Only look at expressions that are a whole statement + let expr: &Expr = match &stmt.kind { + StmtKind::Expr(expr) | StmtKind::Semi(expr) => expr, + _ => return, + }; + // if else + let (mut then, mut els): (&Block, &Expr) = match &expr.kind { + ExprKind::If(_, then, Some(els)) => (then, els), + _ => return, + }; + loop { + if !BreakVisitor::default().check_block(then) { + // then block does not always break + return; + } + match &els.kind { + // else if else + ExprKind::If(_, next_then, Some(next_els)) => { + then = next_then; + els = next_els; + continue; + }, + // else if without else + ExprKind::If(..) => return, + // done + _ => break, + } + } + span_lint_and_help( + cx, + REDUNDANT_ELSE, + els.span, + "redundant else block", + None, + "remove the `else` block and move the contents out", + ); + } +} + +/// Call `check` functions to check if an expression always breaks control flow +#[derive(Default)] +struct BreakVisitor { + is_break: bool, +} + +impl<'ast> Visitor<'ast> for BreakVisitor { + fn visit_block(&mut self, block: &'ast Block) { + self.is_break = match block.stmts.as_slice() { + [.., last] => self.check_stmt(last), + _ => false, + }; + } + + fn visit_expr(&mut self, expr: &'ast Expr) { + self.is_break = match expr.kind { + ExprKind::Break(..) | ExprKind::Continue(..) | ExprKind::Ret(..) => true, + ExprKind::Match(_, ref arms) => arms.iter().all(|arm| self.check_expr(&arm.body)), + ExprKind::If(_, ref then, Some(ref els)) => self.check_block(then) && self.check_expr(els), + ExprKind::If(_, _, None) + // ignore loops for simplicity + | ExprKind::While(..) | ExprKind::ForLoop(..) | ExprKind::Loop(..) => false, + _ => { + walk_expr(self, expr); + return; + }, + }; + } +} + +impl BreakVisitor { + fn check<T>(&mut self, item: T, visit: fn(&mut Self, T)) -> bool { + visit(self, item); + std::mem::replace(&mut self.is_break, false) + } + + fn check_block(&mut self, block: &Block) -> bool { + self.check(block, Self::visit_block) + } + + fn check_expr(&mut self, expr: &Expr) -> bool { + self.check(expr, Self::visit_expr) + } + + fn check_stmt(&mut self, stmt: &Stmt) -> bool { + self.check(stmt, Self::visit_stmt) + } +} diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 74ba53e6a9a..fd74783335d 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -1104,7 +1104,9 @@ fn is_empty_block(expr: &Expr<'_>) -> bool { expr.kind, ExprKind::Block( Block { - stmts: &[], expr: None, .. + stmts: &[], + expr: None, + .. }, _, ) diff --git a/clippy_lints/src/unnecessary_wraps.rs b/clippy_lints/src/unnecessary_wraps.rs index 5d801511a0b..e763da593d4 100644 --- a/clippy_lints/src/unnecessary_wraps.rs +++ b/clippy_lints/src/unnecessary_wraps.rs @@ -74,7 +74,10 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWraps { } if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { - if matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), ..} | ItemKind::Trait(..)) { + if matches!( + item.kind, + ItemKind::Impl { of_trait: Some(_), .. } | ItemKind::Trait(..) + ) { return; } } diff --git a/clippy_lints/src/utils/ast_utils.rs b/clippy_lints/src/utils/ast_utils.rs index 31b4e25411b..f0267e4c792 100644 --- a/clippy_lints/src/utils/ast_utils.rs +++ b/clippy_lints/src/utils/ast_utils.rs @@ -408,7 +408,10 @@ pub fn eq_use_tree_kind(l: &UseTreeKind, r: &UseTreeKind) -> bool { } pub fn eq_defaultness(l: Defaultness, r: Defaultness) -> bool { - matches!((l, r), (Defaultness::Final, Defaultness::Final) | (Defaultness::Default(_), Defaultness::Default(_))) + matches!( + (l, r), + (Defaultness::Final, Defaultness::Final) | (Defaultness::Default(_), Defaultness::Default(_)) + ) } pub fn eq_vis(l: &Visibility, r: &Visibility) -> bool { diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 3a6b64c90e8..e83371f8b99 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -41,7 +41,7 @@ use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, CRATE_DEF_INDEX, LOCAL_CRATE}; -use rustc_hir::intravisit::{NestedVisitorMap, Visitor}; +use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_hir::Node; use rustc_hir::{ def, Arm, Block, Body, Constness, Crate, Expr, ExprKind, FnDecl, HirId, ImplItem, ImplItemKind, Item, ItemKind, @@ -603,6 +603,37 @@ pub fn contains_return(expr: &hir::Expr<'_>) -> bool { visitor.found } +struct FindMacroCalls<'a, 'b> { + names: &'a [&'b str], + result: Vec<Span>, +} + +impl<'a, 'b, 'tcx> Visitor<'tcx> for FindMacroCalls<'a, 'b> { + type Map = Map<'tcx>; + + fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { + if self.names.iter().any(|fun| is_expn_of(expr.span, fun).is_some()) { + self.result.push(expr.span); + } + // and check sub-expressions + intravisit::walk_expr(self, expr); + } + + fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> { + NestedVisitorMap::None + } +} + +/// Finds calls of the specified macros in a function body. +pub fn find_macro_calls(names: &[&str], body: &Body<'_>) -> Vec<Span> { + let mut fmc = FindMacroCalls { + names, + result: Vec::new(), + }; + fmc.visit_expr(&body.value); + fmc.result +} + /// Converts a span to a code snippet if available, otherwise use default. /// /// This is useful if you want to provide suggestions for your lint or more generally, if you want @@ -1500,7 +1531,7 @@ pub fn is_no_std_crate(krate: &Crate<'_>) -> bool { /// ``` pub fn is_trait_impl_item(cx: &LateContext<'_>, hir_id: HirId) -> bool { if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { - matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), .. }) + matches!(item.kind, ItemKind::Impl { of_trait: Some(_), .. }) } else { false } diff --git a/clippy_lints/src/utils/usage.rs b/clippy_lints/src/utils/usage.rs index a7d0ea6ccfb..fc0db7f64ec 100644 --- a/clippy_lints/src/utils/usage.rs +++ b/clippy_lints/src/utils/usage.rs @@ -116,20 +116,27 @@ pub struct ParamBindingIdCollector { } impl<'tcx> ParamBindingIdCollector { fn collect_binding_hir_ids(body: &'tcx hir::Body<'tcx>) -> Vec<hir::HirId> { - let mut finder = ParamBindingIdCollector { - binding_hir_ids: Vec::new(), - }; - finder.visit_body(body); - finder.binding_hir_ids + let mut hir_ids: Vec<hir::HirId> = Vec::new(); + for param in body.params.iter() { + let mut finder = ParamBindingIdCollector { + binding_hir_ids: Vec::new(), + }; + finder.visit_param(param); + for hir_id in &finder.binding_hir_ids { + hir_ids.push(*hir_id); + } + } + hir_ids } } impl<'tcx> intravisit::Visitor<'tcx> for ParamBindingIdCollector { type Map = Map<'tcx>; - fn visit_param(&mut self, param: &'tcx hir::Param<'tcx>) { - if let hir::PatKind::Binding(_, hir_id, ..) = param.pat.kind { + fn visit_pat(&mut self, pat: &'tcx hir::Pat<'tcx>) { + if let hir::PatKind::Binding(_, hir_id, ..) = pat.kind { self.binding_hir_ids.push(hir_id); } + intravisit::walk_pat(self, pat); } fn nested_visit_map(&mut self) -> intravisit::NestedVisitorMap<Self::Map> { diff --git a/tests/ui/panic_in_result_fn.stderr b/tests/ui/panic_in_result_fn.stderr index ca73ac5a411..eb744b0c198 100644 --- a/tests/ui/panic_in_result_fn.stderr +++ b/tests/ui/panic_in_result_fn.stderr @@ -1,4 +1,4 @@ -error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:7:5 | LL | / fn result_with_panic() -> Result<bool, String> // should emit lint @@ -8,7 +8,7 @@ LL | | } | |_____^ | = note: `-D clippy::panic-in-result-fn` implied by `-D warnings` - = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking --> $DIR/panic_in_result_fn.rs:9:9 | @@ -16,7 +16,7 @@ LL | panic!("error"); | ^^^^^^^^^^^^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) -error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:12:5 | LL | / fn result_with_unimplemented() -> Result<bool, String> // should emit lint @@ -25,7 +25,7 @@ LL | | unimplemented!(); LL | | } | |_____^ | - = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking --> $DIR/panic_in_result_fn.rs:14:9 | @@ -33,7 +33,7 @@ LL | unimplemented!(); | ^^^^^^^^^^^^^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) -error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:17:5 | LL | / fn result_with_unreachable() -> Result<bool, String> // should emit lint @@ -42,7 +42,7 @@ LL | | unreachable!(); LL | | } | |_____^ | - = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking --> $DIR/panic_in_result_fn.rs:19:9 | @@ -50,7 +50,7 @@ LL | unreachable!(); | ^^^^^^^^^^^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) -error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:22:5 | LL | / fn result_with_todo() -> Result<bool, String> // should emit lint @@ -59,7 +59,7 @@ LL | | todo!("Finish this"); LL | | } | |_____^ | - = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking --> $DIR/panic_in_result_fn.rs:24:9 | @@ -67,7 +67,7 @@ LL | todo!("Finish this"); | ^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) -error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:53:1 | LL | / fn function_result_with_panic() -> Result<bool, String> // should emit lint @@ -76,7 +76,7 @@ LL | | panic!("error"); LL | | } | |_^ | - = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking --> $DIR/panic_in_result_fn.rs:55:5 | @@ -84,7 +84,7 @@ LL | panic!("error"); | ^^^^^^^^^^^^^^^^ = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) -error: used `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` in a function that returns `Result` +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:68:1 | LL | / fn main() -> Result<(), String> { @@ -93,7 +93,7 @@ LL | | Ok(()) LL | | } | |_^ | - = help: `unimplemented!()`, `unreachable!()`, `todo!()` or `panic!()` should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing note: return Err() instead of panicking --> $DIR/panic_in_result_fn.rs:69:5 | diff --git a/tests/ui/panic_in_result_fn_assertions.rs b/tests/ui/panic_in_result_fn_assertions.rs new file mode 100644 index 00000000000..ffdf8288adc --- /dev/null +++ b/tests/ui/panic_in_result_fn_assertions.rs @@ -0,0 +1,48 @@ +#![warn(clippy::panic_in_result_fn)] +#![allow(clippy::unnecessary_wraps)] + +struct A; + +impl A { + fn result_with_assert_with_message(x: i32) -> Result<bool, String> // should emit lint + { + assert!(x == 5, "wrong argument"); + Ok(true) + } + + fn result_with_assert_eq(x: i32) -> Result<bool, String> // should emit lint + { + assert_eq!(x, 5); + Ok(true) + } + + fn result_with_assert_ne(x: i32) -> Result<bool, String> // should emit lint + { + assert_ne!(x, 1); + Ok(true) + } + + fn other_with_assert_with_message(x: i32) // should not emit lint + { + assert!(x == 5, "wrong argument"); + } + + fn other_with_assert_eq(x: i32) // should not emit lint + { + assert_eq!(x, 5); + } + + fn other_with_assert_ne(x: i32) // should not emit lint + { + assert_ne!(x, 1); + } + + fn result_without_banned_functions() -> Result<bool, String> // should not emit lint + { + let assert = "assert!"; + println!("No {}", assert); + Ok(true) + } +} + +fn main() {} diff --git a/tests/ui/panic_in_result_fn_assertions.stderr b/tests/ui/panic_in_result_fn_assertions.stderr new file mode 100644 index 00000000000..86f61ad718a --- /dev/null +++ b/tests/ui/panic_in_result_fn_assertions.stderr @@ -0,0 +1,57 @@ +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` + --> $DIR/panic_in_result_fn_assertions.rs:7:5 + | +LL | / fn result_with_assert_with_message(x: i32) -> Result<bool, String> // should emit lint +LL | | { +LL | | assert!(x == 5, "wrong argument"); +LL | | Ok(true) +LL | | } + | |_____^ + | + = note: `-D clippy::panic-in-result-fn` implied by `-D warnings` + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing +note: return Err() instead of panicking + --> $DIR/panic_in_result_fn_assertions.rs:9:9 + | +LL | assert!(x == 5, "wrong argument"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` + --> $DIR/panic_in_result_fn_assertions.rs:13:5 + | +LL | / fn result_with_assert_eq(x: i32) -> Result<bool, String> // should emit lint +LL | | { +LL | | assert_eq!(x, 5); +LL | | Ok(true) +LL | | } + | |_____^ + | + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing +note: return Err() instead of panicking + --> $DIR/panic_in_result_fn_assertions.rs:15:9 + | +LL | assert_eq!(x, 5); + | ^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` + --> $DIR/panic_in_result_fn_assertions.rs:19:5 + | +LL | / fn result_with_assert_ne(x: i32) -> Result<bool, String> // should emit lint +LL | | { +LL | | assert_ne!(x, 1); +LL | | Ok(true) +LL | | } + | |_____^ + | + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing +note: return Err() instead of panicking + --> $DIR/panic_in_result_fn_assertions.rs:21:9 + | +LL | assert_ne!(x, 1); + | ^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 3 previous errors + diff --git a/tests/ui/panic_in_result_fn_debug_assertions.rs b/tests/ui/panic_in_result_fn_debug_assertions.rs new file mode 100644 index 00000000000..b60c79f97c8 --- /dev/null +++ b/tests/ui/panic_in_result_fn_debug_assertions.rs @@ -0,0 +1,48 @@ +#![warn(clippy::panic_in_result_fn)] +#![allow(clippy::unnecessary_wraps)] + +struct A; + +impl A { + fn result_with_debug_assert_with_message(x: i32) -> Result<bool, String> // should emit lint + { + debug_assert!(x == 5, "wrong argument"); + Ok(true) + } + + fn result_with_debug_assert_eq(x: i32) -> Result<bool, String> // should emit lint + { + debug_assert_eq!(x, 5); + Ok(true) + } + + fn result_with_debug_assert_ne(x: i32) -> Result<bool, String> // should emit lint + { + debug_assert_ne!(x, 1); + Ok(true) + } + + fn other_with_debug_assert_with_message(x: i32) // should not emit lint + { + debug_assert!(x == 5, "wrong argument"); + } + + fn other_with_debug_assert_eq(x: i32) // should not emit lint + { + debug_assert_eq!(x, 5); + } + + fn other_with_debug_assert_ne(x: i32) // should not emit lint + { + debug_assert_ne!(x, 1); + } + + fn result_without_banned_functions() -> Result<bool, String> // should not emit lint + { + let debug_assert = "debug_assert!"; + println!("No {}", debug_assert); + Ok(true) + } +} + +fn main() {} diff --git a/tests/ui/panic_in_result_fn_debug_assertions.stderr b/tests/ui/panic_in_result_fn_debug_assertions.stderr new file mode 100644 index 00000000000..ec18e89698c --- /dev/null +++ b/tests/ui/panic_in_result_fn_debug_assertions.stderr @@ -0,0 +1,57 @@ +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` + --> $DIR/panic_in_result_fn_debug_assertions.rs:7:5 + | +LL | / fn result_with_debug_assert_with_message(x: i32) -> Result<bool, String> // should emit lint +LL | | { +LL | | debug_assert!(x == 5, "wrong argument"); +LL | | Ok(true) +LL | | } + | |_____^ + | + = note: `-D clippy::panic-in-result-fn` implied by `-D warnings` + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing +note: return Err() instead of panicking + --> $DIR/panic_in_result_fn_debug_assertions.rs:9:9 + | +LL | debug_assert!(x == 5, "wrong argument"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` + --> $DIR/panic_in_result_fn_debug_assertions.rs:13:5 + | +LL | / fn result_with_debug_assert_eq(x: i32) -> Result<bool, String> // should emit lint +LL | | { +LL | | debug_assert_eq!(x, 5); +LL | | Ok(true) +LL | | } + | |_____^ + | + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing +note: return Err() instead of panicking + --> $DIR/panic_in_result_fn_debug_assertions.rs:15:9 + | +LL | debug_assert_eq!(x, 5); + | ^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` + --> $DIR/panic_in_result_fn_debug_assertions.rs:19:5 + | +LL | / fn result_with_debug_assert_ne(x: i32) -> Result<bool, String> // should emit lint +LL | | { +LL | | debug_assert_ne!(x, 1); +LL | | Ok(true) +LL | | } + | |_____^ + | + = help: `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertions should not be used in a function that returns `Result` as `Result` is expected to return an error instead of crashing +note: return Err() instead of panicking + --> $DIR/panic_in_result_fn_debug_assertions.rs:21:9 + | +LL | debug_assert_ne!(x, 1); + | ^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 3 previous errors + diff --git a/tests/ui/redundant_else.rs b/tests/ui/redundant_else.rs new file mode 100644 index 00000000000..737c8a9f8db --- /dev/null +++ b/tests/ui/redundant_else.rs @@ -0,0 +1,154 @@ +#![warn(clippy::redundant_else)] +#![allow(clippy::needless_return)] + +fn main() { + loop { + // break + if foo() { + println!("Love your neighbor;"); + break; + } else { + println!("yet don't pull down your hedge."); + } + // continue + if foo() { + println!("He that lies down with Dogs,"); + continue; + } else { + println!("shall rise up with fleas."); + } + // match block + if foo() { + match foo() { + 1 => break, + _ => return, + } + } else { + println!("You may delay, but time will not."); + } + } + // else if + if foo() { + return; + } else if foo() { + return; + } else { + println!("A fat kitchen makes a lean will."); + } + // let binding outside of block + let _ = { + if foo() { + return; + } else { + 1 + } + }; + // else if with let binding outside of block + let _ = { + if foo() { + return; + } else if foo() { + return; + } else { + 2 + } + }; + // inside if let + let _ = if let Some(1) = foo() { + let _ = 1; + if foo() { + return; + } else { + 1 + } + } else { + 1 + }; + + // + // non-lint cases + // + + // sanity check + if foo() { + let _ = 1; + } else { + println!("Who is wise? He that learns from every one."); + } + // else if without else + if foo() { + return; + } else if foo() { + foo() + }; + // nested if return + if foo() { + if foo() { + return; + } + } else { + foo() + }; + // match with non-breaking branch + if foo() { + match foo() { + 1 => foo(), + _ => return, + } + } else { + println!("Three may keep a secret, if two of them are dead."); + } + // let binding + let _ = if foo() { + return; + } else { + 1 + }; + // assign + let a; + a = if foo() { + return; + } else { + 1 + }; + // assign-op + a += if foo() { + return; + } else { + 1 + }; + // if return else if else + if foo() { + return; + } else if foo() { + 1 + } else { + 2 + }; + // if else if return else + if foo() { + 1 + } else if foo() { + return; + } else { + 2 + }; + // else if with let binding + let _ = if foo() { + return; + } else if foo() { + return; + } else { + 2 + }; + // inside function call + Box::new(if foo() { + return; + } else { + 1 + }); +} + +fn foo<T>() -> T { + unimplemented!("I'm not Santa Claus") +} diff --git a/tests/ui/redundant_else.stderr b/tests/ui/redundant_else.stderr new file mode 100644 index 00000000000..9000cdc814b --- /dev/null +++ b/tests/ui/redundant_else.stderr @@ -0,0 +1,80 @@ +error: redundant else block + --> $DIR/redundant_else.rs:10:16 + | +LL | } else { + | ________________^ +LL | | println!("yet don't pull down your hedge."); +LL | | } + | |_________^ + | + = note: `-D clippy::redundant-else` implied by `-D warnings` + = help: remove the `else` block and move the contents out + +error: redundant else block + --> $DIR/redundant_else.rs:17:16 + | +LL | } else { + | ________________^ +LL | | println!("shall rise up with fleas."); +LL | | } + | |_________^ + | + = help: remove the `else` block and move the contents out + +error: redundant else block + --> $DIR/redundant_else.rs:26:16 + | +LL | } else { + | ________________^ +LL | | println!("You may delay, but time will not."); +LL | | } + | |_________^ + | + = help: remove the `else` block and move the contents out + +error: redundant else block + --> $DIR/redundant_else.rs:35:12 + | +LL | } else { + | ____________^ +LL | | println!("A fat kitchen makes a lean will."); +LL | | } + | |_____^ + | + = help: remove the `else` block and move the contents out + +error: redundant else block + --> $DIR/redundant_else.rs:42:16 + | +LL | } else { + | ________________^ +LL | | 1 +LL | | } + | |_________^ + | + = help: remove the `else` block and move the contents out + +error: redundant else block + --> $DIR/redundant_else.rs:52:16 + | +LL | } else { + | ________________^ +LL | | 2 +LL | | } + | |_________^ + | + = help: remove the `else` block and move the contents out + +error: redundant else block + --> $DIR/redundant_else.rs:61:16 + | +LL | } else { + | ________________^ +LL | | 1 +LL | | } + | |_________^ + | + = help: remove the `else` block and move the contents out + +error: aborting due to 7 previous errors + diff --git a/tests/ui/temporary_assignment.rs b/tests/ui/temporary_assignment.rs index b4a931043b0..ac4c1bc6597 100644 --- a/tests/ui/temporary_assignment.rs +++ b/tests/ui/temporary_assignment.rs @@ -1,5 +1,4 @@ #![warn(clippy::temporary_assignment)] -#![allow(const_item_mutation)] use std::ops::{Deref, DerefMut}; diff --git a/tests/ui/temporary_assignment.stderr b/tests/ui/temporary_assignment.stderr index 4cc32c79f05..7d79901a28d 100644 --- a/tests/ui/temporary_assignment.stderr +++ b/tests/ui/temporary_assignment.stderr @@ -1,5 +1,5 @@ error: assignment to temporary - --> $DIR/temporary_assignment.rs:48:5 + --> $DIR/temporary_assignment.rs:47:5 | LL | Struct { field: 0 }.field = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -7,7 +7,7 @@ LL | Struct { field: 0 }.field = 1; = note: `-D clippy::temporary-assignment` implied by `-D warnings` error: assignment to temporary - --> $DIR/temporary_assignment.rs:49:5 + --> $DIR/temporary_assignment.rs:48:5 | LL | / MultiStruct { LL | | structure: Struct { field: 0 }, @@ -17,13 +17,13 @@ LL | | .field = 1; | |______________^ error: assignment to temporary - --> $DIR/temporary_assignment.rs:54:5 + --> $DIR/temporary_assignment.rs:53:5 | LL | ArrayStruct { array: [0] }.array[0] = 1; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: assignment to temporary - --> $DIR/temporary_assignment.rs:55:5 + --> $DIR/temporary_assignment.rs:54:5 | LL | (0, 0).0 = 1; | ^^^^^^^^^^^^ diff --git a/tests/ui/unnecessary_lazy_eval_unfixable.rs b/tests/ui/unnecessary_lazy_eval_unfixable.rs index 2e923bc97a2..b05dd143bfd 100644 --- a/tests/ui/unnecessary_lazy_eval_unfixable.rs +++ b/tests/ui/unnecessary_lazy_eval_unfixable.rs @@ -15,4 +15,8 @@ fn main() { } let _ = Ok(1).unwrap_or_else(|e::E| 2); let _ = Ok(1).unwrap_or_else(|SomeStruct { .. }| 2); + + // Fix #6343 + let arr = [(Some(1),)]; + Some(&0).and_then(|&i| arr[i].0); } diff --git a/util/dev b/util/dev deleted file mode 100755 index 319de217e0d..00000000000 --- a/util/dev +++ /dev/null @@ -1,7 +0,0 @@ -#!/bin/sh -CARGO_TARGET_DIR=$(pwd)/target/ -export CARGO_TARGET_DIR - -echo 'Deprecated! `util/dev` usage is deprecated, please use `cargo dev` instead.' - -cd clippy_dev && cargo run -- "$@" |