Auto merge of #9295 - Guilherme-Vasconcelos:manual-empty-string-creation, r=dswij
Add `manual_empty_string_creations` lint Closes #2972 - [x] Followed [lint naming conventions][lint_naming] - [x] Added passing UI tests (including committed `.stderr` file) - [x] `cargo test` passes locally - [x] Executed `cargo dev update_lints` - [x] Added lint documentation - [x] Run `cargo dev fmt` changelog: [`manual_empty_string_creations`]: Add lint for empty String not being created with `String::new()`
This commit is contained in:
commit
868dba9f65
32 changed files with 371 additions and 38 deletions
|
|
@ -124,6 +124,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
|
|||
LintId::of(main_recursion::MAIN_RECURSION),
|
||||
LintId::of(manual_async_fn::MANUAL_ASYNC_FN),
|
||||
LintId::of(manual_bits::MANUAL_BITS),
|
||||
LintId::of(manual_empty_string_creations::MANUAL_EMPTY_STRING_CREATIONS),
|
||||
LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
|
||||
LintId::of(manual_rem_euclid::MANUAL_REM_EUCLID),
|
||||
LintId::of(manual_retain::MANUAL_RETAIN),
|
||||
|
|
|
|||
|
|
@ -244,6 +244,7 @@ store.register_lints(&[
|
|||
manual_assert::MANUAL_ASSERT,
|
||||
manual_async_fn::MANUAL_ASYNC_FN,
|
||||
manual_bits::MANUAL_BITS,
|
||||
manual_empty_string_creations::MANUAL_EMPTY_STRING_CREATIONS,
|
||||
manual_instant_elapsed::MANUAL_INSTANT_ELAPSED,
|
||||
manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
|
||||
manual_ok_or::MANUAL_OK_OR,
|
||||
|
|
|
|||
|
|
@ -44,6 +44,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
|
|||
LintId::of(main_recursion::MAIN_RECURSION),
|
||||
LintId::of(manual_async_fn::MANUAL_ASYNC_FN),
|
||||
LintId::of(manual_bits::MANUAL_BITS),
|
||||
LintId::of(manual_empty_string_creations::MANUAL_EMPTY_STRING_CREATIONS),
|
||||
LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
|
||||
LintId::of(map_clone::MAP_CLONE),
|
||||
LintId::of(match_result_ok::MATCH_RESULT_OK),
|
||||
|
|
|
|||
|
|
@ -273,6 +273,7 @@ mod main_recursion;
|
|||
mod manual_assert;
|
||||
mod manual_async_fn;
|
||||
mod manual_bits;
|
||||
mod manual_empty_string_creations;
|
||||
mod manual_instant_elapsed;
|
||||
mod manual_non_exhaustive;
|
||||
mod manual_ok_or;
|
||||
|
|
@ -933,6 +934,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports::default()));
|
||||
store.register_late_pass(|| Box::new(manual_instant_elapsed::ManualInstantElapsed));
|
||||
store.register_late_pass(|| Box::new(partialeq_to_none::PartialeqToNone));
|
||||
store.register_late_pass(|| Box::new(manual_empty_string_creations::ManualEmptyStringCreations));
|
||||
// add lints here, do not remove this comment, it's used in `new_lint`
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -192,7 +192,7 @@ fn suggested_ret(cx: &LateContext<'_>, output: &Ty<'_>) -> Option<(&'static str,
|
|||
match output.kind {
|
||||
TyKind::Tup(tys) if tys.is_empty() => {
|
||||
let sugg = "remove the return type";
|
||||
Some((sugg, "".into()))
|
||||
Some((sugg, String::new()))
|
||||
},
|
||||
_ => {
|
||||
let sugg = "return the output of the future directly";
|
||||
|
|
|
|||
141
clippy_lints/src/manual_empty_string_creations.rs
Normal file
141
clippy_lints/src/manual_empty_string_creations.rs
Normal file
|
|
@ -0,0 +1,141 @@
|
|||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||
use rustc_ast::LitKind;
|
||||
use rustc_errors::Applicability::MachineApplicable;
|
||||
use rustc_hir::{Expr, ExprKind, PathSegment, QPath, TyKind};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_middle::ty;
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
use rustc_span::{sym, symbol, Span};
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
///
|
||||
/// Checks for usage of `""` to create a `String`, such as `"".to_string()`, `"".to_owned()`,
|
||||
/// `String::from("")` and others.
|
||||
///
|
||||
/// ### Why is this bad?
|
||||
///
|
||||
/// Different ways of creating an empty string makes your code less standardized, which can
|
||||
/// be confusing.
|
||||
///
|
||||
/// ### Example
|
||||
/// ```rust
|
||||
/// let a = "".to_string();
|
||||
/// let b: String = "".into();
|
||||
/// ```
|
||||
/// Use instead:
|
||||
/// ```rust
|
||||
/// let a = String::new();
|
||||
/// let b = String::new();
|
||||
/// ```
|
||||
#[clippy::version = "1.65.0"]
|
||||
pub MANUAL_EMPTY_STRING_CREATIONS,
|
||||
style,
|
||||
"empty String is being created manually"
|
||||
}
|
||||
declare_lint_pass!(ManualEmptyStringCreations => [MANUAL_EMPTY_STRING_CREATIONS]);
|
||||
|
||||
impl LateLintPass<'_> for ManualEmptyStringCreations {
|
||||
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
|
||||
if expr.span.from_expansion() {
|
||||
return;
|
||||
}
|
||||
|
||||
let ty = cx.typeck_results().expr_ty(expr);
|
||||
match ty.kind() {
|
||||
ty::Adt(adt_def, _) if adt_def.is_struct() => {
|
||||
if !cx.tcx.is_diagnostic_item(sym::String, adt_def.did()) {
|
||||
return;
|
||||
}
|
||||
},
|
||||
_ => return,
|
||||
}
|
||||
|
||||
match expr.kind {
|
||||
ExprKind::Call(func, args) => {
|
||||
parse_call(cx, expr.span, func, args);
|
||||
},
|
||||
ExprKind::MethodCall(path_segment, args, _) => {
|
||||
parse_method_call(cx, expr.span, path_segment, args);
|
||||
},
|
||||
_ => (),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Checks if an expression's kind corresponds to an empty &str.
|
||||
fn is_expr_kind_empty_str(expr_kind: &ExprKind<'_>) -> bool {
|
||||
if let ExprKind::Lit(lit) = expr_kind &&
|
||||
let LitKind::Str(value, _) = lit.node &&
|
||||
value == symbol::kw::Empty
|
||||
{
|
||||
return true;
|
||||
}
|
||||
|
||||
false
|
||||
}
|
||||
|
||||
/// Emits the `MANUAL_EMPTY_STRING_CREATION` warning and suggests the appropriate fix.
|
||||
fn warn_then_suggest(cx: &LateContext<'_>, span: Span) {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
MANUAL_EMPTY_STRING_CREATIONS,
|
||||
span,
|
||||
"empty String is being created manually",
|
||||
"consider using",
|
||||
"String::new()".into(),
|
||||
MachineApplicable,
|
||||
);
|
||||
}
|
||||
|
||||
/// Tries to parse an expression as a method call, emiting the warning if necessary.
|
||||
fn parse_method_call(cx: &LateContext<'_>, span: Span, path_segment: &PathSegment<'_>, args: &[Expr<'_>]) {
|
||||
if args.is_empty() {
|
||||
// When parsing TryFrom::try_from(...).expect(...), we will have more than 1 arg.
|
||||
return;
|
||||
}
|
||||
|
||||
let ident = path_segment.ident.as_str();
|
||||
let method_arg_kind = &args[0].kind;
|
||||
if ["to_string", "to_owned", "into"].contains(&ident) && is_expr_kind_empty_str(method_arg_kind) {
|
||||
warn_then_suggest(cx, span);
|
||||
} else if let ExprKind::Call(func, args) = method_arg_kind {
|
||||
// If our first argument is a function call itself, it could be an `unwrap`-like function.
|
||||
// E.g. String::try_from("hello").unwrap(), TryFrom::try_from("").expect("hello"), etc.
|
||||
parse_call(cx, span, func, args);
|
||||
}
|
||||
}
|
||||
|
||||
/// Tries to parse an expression as a function call, emiting the warning if necessary.
|
||||
fn parse_call(cx: &LateContext<'_>, span: Span, func: &Expr<'_>, args: &[Expr<'_>]) {
|
||||
if args.len() != 1 {
|
||||
return;
|
||||
}
|
||||
|
||||
let arg_kind = &args[0].kind;
|
||||
if let ExprKind::Path(qpath) = &func.kind {
|
||||
if let QPath::TypeRelative(_, _) = qpath {
|
||||
// String::from(...) or String::try_from(...)
|
||||
if let QPath::TypeRelative(ty, path_seg) = qpath &&
|
||||
[sym::from, sym::try_from].contains(&path_seg.ident.name) &&
|
||||
let TyKind::Path(qpath) = &ty.kind &&
|
||||
let QPath::Resolved(_, path) = qpath &&
|
||||
let [path_seg] = path.segments &&
|
||||
path_seg.ident.name == sym::String &&
|
||||
is_expr_kind_empty_str(arg_kind)
|
||||
{
|
||||
warn_then_suggest(cx, span);
|
||||
}
|
||||
} else if let QPath::Resolved(_, path) = qpath {
|
||||
// From::from(...) or TryFrom::try_from(...)
|
||||
if let [path_seg1, path_seg2] = path.segments &&
|
||||
is_expr_kind_empty_str(arg_kind) && (
|
||||
(path_seg1.ident.name == sym::From && path_seg2.ident.name == sym::from) ||
|
||||
(path_seg1.ident.name == sym::TryFrom && path_seg2.ident.name == sym::try_from)
|
||||
)
|
||||
{
|
||||
warn_then_suggest(cx, span);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -78,7 +78,7 @@ pub(super) fn check<'tcx>(
|
|||
map_span,
|
||||
String::from(if unwrap_snippet_none { "and_then" } else { "map_or" }),
|
||||
),
|
||||
(expr.span.with_lo(unwrap_recv.span.hi()), String::from("")),
|
||||
(expr.span.with_lo(unwrap_recv.span.hi()), String::new()),
|
||||
];
|
||||
|
||||
if !unwrap_snippet_none {
|
||||
|
|
|
|||
|
|
@ -46,7 +46,7 @@ fn span_lint(cx: &EarlyContext<'_>, span: Span, only_one: bool) {
|
|||
"these patterns are unneeded as the `..` pattern can match those elements"
|
||||
},
|
||||
if only_one { "remove it" } else { "remove them" },
|
||||
"".to_string(),
|
||||
String::new(),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -130,7 +130,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWraps {
|
|||
(
|
||||
ret_expr.span,
|
||||
if inner_type.is_unit() {
|
||||
"".to_string()
|
||||
String::new()
|
||||
} else {
|
||||
snippet(cx, arg.span.source_callsite(), "..").to_string()
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue