diff --git a/CHANGELOG.md b/CHANGELOG.md
index 285a2ff8060..a6fafdf5357 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1672,6 +1672,7 @@ Released 2018-09-13
[`manual_memcpy`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_memcpy
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
[`manual_saturating_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_saturating_arithmetic
+[`manual_strip`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_strip
[`manual_swap`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_swap
[`many_single_char_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#many_single_char_names
[`map_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_clone
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index c017c5cb5d0..38ddc69c8cb 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -230,6 +230,7 @@ mod macro_use;
mod main_recursion;
mod manual_async_fn;
mod manual_non_exhaustive;
+mod manual_strip;
mod map_clone;
mod map_identity;
mod map_unit_fn;
@@ -626,6 +627,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&main_recursion::MAIN_RECURSION,
&manual_async_fn::MANUAL_ASYNC_FN,
&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE,
+ &manual_strip::MANUAL_STRIP,
&map_clone::MAP_CLONE,
&map_identity::MAP_IDENTITY,
&map_unit_fn::OPTION_MAP_UNIT_FN,
@@ -1109,6 +1111,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box self_assignment::SelfAssignment);
store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs);
store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync);
+ store.register_late_pass(|| box manual_strip::ManualStrip);
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1335,6 +1338,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&main_recursion::MAIN_RECURSION),
LintId::of(&manual_async_fn::MANUAL_ASYNC_FN),
LintId::of(&manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE),
+ LintId::of(&manual_strip::MANUAL_STRIP),
LintId::of(&map_clone::MAP_CLONE),
LintId::of(&map_identity::MAP_IDENTITY),
LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
@@ -1626,6 +1630,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&loops::EXPLICIT_COUNTER_LOOP),
LintId::of(&loops::MUT_RANGE_BOUND),
LintId::of(&loops::WHILE_LET_LOOP),
+ LintId::of(&manual_strip::MANUAL_STRIP),
LintId::of(&map_identity::MAP_IDENTITY),
LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN),
diff --git a/clippy_lints/src/manual_strip.rs b/clippy_lints/src/manual_strip.rs
new file mode 100644
index 00000000000..127938aecd6
--- /dev/null
+++ b/clippy_lints/src/manual_strip.rs
@@ -0,0 +1,246 @@
+use crate::consts::{constant, Constant};
+use crate::utils::usage::mutated_variables;
+use crate::utils::{
+ eq_expr_value, higher, match_def_path, multispan_sugg, paths, qpath_res, snippet, span_lint_and_then,
+};
+
+use if_chain::if_chain;
+use rustc_ast::ast::LitKind;
+use rustc_hir::def::Res;
+use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
+use rustc_hir::BinOpKind;
+use rustc_hir::{BorrowKind, Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::hir::map::Map;
+use rustc_middle::ty;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::source_map::Spanned;
+use rustc_span::Span;
+
+declare_clippy_lint! {
+ /// **What it does:**
+ /// Suggests using `strip_{prefix,suffix}` over `str::{starts,ends}_with` and slicing using
+ /// the pattern's length.
+ ///
+ /// **Why is this bad?**
+ /// Using `str:strip_{prefix,suffix}` is safer and may have better performance as there is no
+ /// slicing which may panic and the compiler does not need to insert this panic code. It is
+ /// also sometimes more readable as it removes the need for duplicating or storing the pattern
+ /// used by `str::{starts,ends}_with` and in the slicing.
+ ///
+ /// **Known problems:**
+ /// None.
+ ///
+ /// **Example:**
+ ///
+ /// ```rust
+ /// let s = "hello, world!";
+ /// if s.starts_with("hello, ") {
+ /// assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!");
+ /// }
+ /// ```
+ /// Use instead:
+ /// ```rust
+ /// let s = "hello, world!";
+ /// if let Some(end) = s.strip_prefix("hello, ") {
+ /// assert_eq!(end.to_uppercase(), "WORLD!");
+ /// }
+ /// ```
+ pub MANUAL_STRIP,
+ complexity,
+ "suggests using `strip_{prefix,suffix}` over `str::{starts,ends}_with` and slicing"
+}
+
+declare_lint_pass!(ManualStrip => [MANUAL_STRIP]);
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+enum StripKind {
+ Prefix,
+ Suffix,
+}
+
+impl<'tcx> LateLintPass<'tcx> for ManualStrip {
+ fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+ if_chain! {
+ if let Some((cond, then, _)) = higher::if_block(&expr);
+ if let ExprKind::MethodCall(_, _, [target_arg, pattern], _) = cond.kind;
+ if let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(cond.hir_id);
+ if let ExprKind::Path(target_path) = &target_arg.kind;
+ then {
+ let strip_kind = if match_def_path(cx, method_def_id, &paths::STR_STARTS_WITH) {
+ StripKind::Prefix
+ } else if match_def_path(cx, method_def_id, &paths::STR_ENDS_WITH) {
+ StripKind::Suffix
+ } else {
+ return;
+ };
+ let target_res = qpath_res(cx, &target_path, target_arg.hir_id);
+ if target_res == Res::Err {
+ return;
+ };
+
+ if_chain! {
+ if let Res::Local(hir_id) = target_res;
+ if let Some(used_mutably) = mutated_variables(then, cx);
+ if used_mutably.contains(&hir_id);
+ then {
+ return;
+ }
+ }
+
+ let strippings = find_stripping(cx, strip_kind, target_res, pattern, then);
+ if !strippings.is_empty() {
+
+ let kind_word = match strip_kind {
+ StripKind::Prefix => "prefix",
+ StripKind::Suffix => "suffix",
+ };
+
+ let test_span = expr.span.until(then.span);
+ span_lint_and_then(cx, MANUAL_STRIP, strippings[0], &format!("stripping a {} manually", kind_word), |diag| {
+ diag.span_note(test_span, &format!("the {} was tested here", kind_word));
+ multispan_sugg(
+ diag,
+ &format!("try using the `strip_{}` method", kind_word),
+ vec![(test_span,
+ format!("if let Some(<stripped>) = {}.strip_{}({}) ",
+ snippet(cx, target_arg.span, ".."),
+ kind_word,
+ snippet(cx, pattern.span, "..")))]
+ .into_iter().chain(strippings.into_iter().map(|span| (span, "<stripped>".into()))),
+ )
+ });
+ }
+ }
+ }
+ }
+}
+
+// Returns `Some(arg)` if `expr` matches `arg.len()` and `None` otherwise.
+fn len_arg<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> {
+ if_chain! {
+ if let ExprKind::MethodCall(_, _, [arg], _) = expr.kind;
+ if let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
+ if match_def_path(cx, method_def_id, &paths::STR_LEN);
+ then {
+ Some(arg)
+ }
+ else {
+ None
+ }
+ }
+}
+
+// Returns the length of the `expr` if it's a constant string or char.
+fn constant_length(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<u128> {
+ let (value, _) = constant(cx, cx.typeck_results(), expr)?;
+ match value {
+ Constant::Str(value) => Some(value.len() as u128),
+ Constant::Char(value) => Some(value.len_utf8() as u128),
+ _ => None,
+ }
+}
+
+// Tests if `expr` equals the length of the pattern.
+fn eq_pattern_length<'tcx>(cx: &LateContext<'tcx>, pattern: &Expr<'_>, expr: &'tcx Expr<'_>) -> bool {
+ if let ExprKind::Lit(Spanned {
+ node: LitKind::Int(n, _),
+ ..
+ }) = expr.kind
+ {
+ constant_length(cx, pattern).map_or(false, |length| length == n)
+ } else {
+ len_arg(cx, expr).map_or(false, |arg| eq_expr_value(cx, pattern, arg))
+ }
+}
+
+// Tests if `expr` is a `&str`.
+fn is_ref_str(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
+ match cx.typeck_results().expr_ty_adjusted(&expr).kind() {
+ ty::Ref(_, ty, _) => ty.is_str(),
+ _ => false,
+ }
+}
+
+// Removes the outer `AddrOf` expression if needed.
+fn peel_ref<'a>(expr: &'a Expr<'_>) -> &'a Expr<'a> {
+ if let ExprKind::AddrOf(BorrowKind::Ref, _, unref) = &expr.kind {
+ unref
+ } else {
+ expr
+ }
+}
+
+// Find expressions where `target` is stripped using the length of `pattern`.
+// We'll suggest replacing these expressions with the result of the `strip_{prefix,suffix}`
+// method.
+fn find_stripping<'tcx>(
+ cx: &LateContext<'tcx>,
+ strip_kind: StripKind,
+ target: Res,
+ pattern: &'tcx Expr<'_>,
+ expr: &'tcx Expr<'_>,
+) -> Vec<Span> {
+ struct StrippingFinder<'a, 'tcx> {
+ cx: &'a LateContext<'tcx>,
+ strip_kind: StripKind,
+ target: Res,
+ pattern: &'tcx Expr<'tcx>,
+ results: Vec<Span>,
+ }
+
+ impl<'a, 'tcx> Visitor<'tcx> for StrippingFinder<'a, 'tcx> {
+ type Map = Map<'tcx>;
+ fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
+ NestedVisitorMap::None
+ }
+
+ fn visit_expr(&mut self, ex: &'tcx Expr<'_>) {
+ if_chain! {
+ if is_ref_str(self.cx, ex);
+ let unref = peel_ref(ex);
+ if let ExprKind::Index(indexed, index) = &unref.kind;
+ if let Some(range) = higher::range(index);
+ if let higher::Range { start, end, .. } = range;
+ if let ExprKind::Path(path) = &indexed.kind;
+ if qpath_res(self.cx, path, ex.hir_id) == self.target;
+ then {
+ match (self.strip_kind, start, end) {
+ (StripKind::Prefix, Some(start), None) => {
+ if eq_pattern_length(self.cx, self.pattern, start) {
+ self.results.push(ex.span);
+ return;
+ }
+ },
+ (StripKind::Suffix, None, Some(end)) => {
+ if_chain! {
+ if let ExprKind::Binary(Spanned { node: BinOpKind::Sub, .. }, left, right) = end.kind;
+ if let Some(left_arg) = len_arg(self.cx, left);
+ if let ExprKind::Path(left_path) = &left_arg.kind;
+ if qpath_res(self.cx, left_path, left_arg.hir_id) == self.target;
+ if eq_pattern_length(self.cx, self.pattern, right);
+ then {
+ self.results.push(ex.span);
+ return;
+ }
+ }
+ },
+ _ => {}
+ }
+ }
+ }
+
+ walk_expr(self, ex);
+ }
+ }
+
+ let mut finder = StrippingFinder {
+ cx,
+ strip_kind,
+ target,
+ pattern,
+ results: vec![],
+ };
+ walk_expr(&mut finder, expr);
+ finder.results
+}
diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs
index 65320d6a0e0..f0f7719e2fd 100644
--- a/clippy_lints/src/utils/paths.rs
+++ b/clippy_lints/src/utils/paths.rs
@@ -115,6 +115,9 @@ pub const STD_MEM_TRANSMUTE: [&str; 3] = ["std", "mem", "transmute"];
pub const STD_PTR_NULL: [&str; 3] = ["std", "ptr", "null"];
pub const STRING_AS_MUT_STR: [&str; 4] = ["alloc", "string", "String", "as_mut_str"];
pub const STRING_AS_STR: [&str; 4] = ["alloc", "string", "String", "as_str"];
+pub const STR_ENDS_WITH: [&str; 4] = ["core", "str", "<impl str>", "ends_with"];
+pub const STR_LEN: [&str; 4] = ["core", "str", "<impl str>", "len"];
+pub const STR_STARTS_WITH: [&str; 4] = ["core", "str", "<impl str>", "starts_with"];
pub const SYNTAX_CONTEXT: [&str; 3] = ["rustc_span", "hygiene", "SyntaxContext"];
pub const TO_OWNED: [&str; 3] = ["alloc", "borrow", "ToOwned"];
pub const TO_OWNED_METHOD: [&str; 4] = ["alloc", "borrow", "ToOwned", "to_owned"];
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index a7d38c93433..8bceef80abf 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -1144,6 +1144,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
deprecation: None,
module: "methods",
},
+ Lint {
+ name: "manual_strip",
+ group: "complexity",
+ desc: "suggests using `strip_{prefix,suffix}` over `str::{starts,ends}_with` and slicing",
+ deprecation: None,
+ module: "manual_strip",
+ },
Lint {
name: "manual_swap",
group: "complexity",
diff --git a/tests/ui/manual_strip.rs b/tests/ui/manual_strip.rs
new file mode 100644
index 00000000000..d1b4772c7de
--- /dev/null
+++ b/tests/ui/manual_strip.rs
@@ -0,0 +1,59 @@
+#![warn(clippy::manual_strip)]
+
+fn main() {
+ let s = "abc";
+
+ if s.starts_with("ab") {
+ str::to_string(&s["ab".len()..]);
+ s["ab".len()..].to_string();
+
+ str::to_string(&s[2..]);
+ s[2..].to_string();
+ }
+
+ if s.ends_with("bc") {
+ str::to_string(&s[..s.len() - "bc".len()]);
+ s[..s.len() - "bc".len()].to_string();
+
+ str::to_string(&s[..s.len() - 2]);
+ s[..s.len() - 2].to_string();
+ }
+
+ // Character patterns
+ if s.starts_with('a') {
+ str::to_string(&s[1..]);
+ s[1..].to_string();
+ }
+
+ // Variable prefix
+ let prefix = "ab";
+ if s.starts_with(prefix) {
+ str::to_string(&s[prefix.len()..]);
+ }
+
+ // Constant prefix
+ const PREFIX: &str = "ab";
+ if s.starts_with(PREFIX) {
+ str::to_string(&s[PREFIX.len()..]);
+ str::to_string(&s[2..]);
+ }
+
+ // Constant target
+ const TARGET: &str = "abc";
+ if TARGET.starts_with(prefix) {
+ str::to_string(&TARGET[prefix.len()..]);
+ }
+
+ // String target - not mutated.
+ let s1: String = "abc".into();
+ if s1.starts_with("ab") {
+ s1[2..].to_uppercase();
+ }
+
+ // String target - mutated. (Don't lint.)
+ let mut s2: String = "abc".into();
+ if s2.starts_with("ab") {
+ s2.push('d');
+ s2[2..].to_uppercase();
+ }
+}
diff --git a/tests/ui/manual_strip.stderr b/tests/ui/manual_strip.stderr
new file mode 100644
index 00000000000..1352a8713d4
--- /dev/null
+++ b/tests/ui/manual_strip.stderr
@@ -0,0 +1,132 @@
+error: stripping a prefix manually
+ --> $DIR/manual_strip.rs:7:24
+ |
+LL | str::to_string(&s["ab".len()..]);
+ | ^^^^^^^^^^^^^^^^
+ |
+ = note: `-D clippy::manual-strip` implied by `-D warnings`
+note: the prefix was tested here
+ --> $DIR/manual_strip.rs:6:5
+ |
+LL | if s.starts_with("ab") {
+ | ^^^^^^^^^^^^^^^^^^^^^^^
+help: try using the `strip_prefix` method
+ |
+LL | if let Some(<stripped>) = s.strip_prefix("ab") {
+LL | str::to_string(<stripped>);
+LL | <stripped>.to_string();
+LL |
+LL | str::to_string(<stripped>);
+LL | <stripped>.to_string();
+ |
+
+error: stripping a suffix manually
+ --> $DIR/manual_strip.rs:15:24
+ |
+LL | str::to_string(&s[..s.len() - "bc".len()]);
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^
+ |
+note: the suffix was tested here
+ --> $DIR/manual_strip.rs:14:5
+ |
+LL | if s.ends_with("bc") {
+ | ^^^^^^^^^^^^^^^^^^^^^
+help: try using the `strip_suffix` method
+ |
+LL | if let Some(<stripped>) = s.strip_suffix("bc") {
+LL | str::to_string(<stripped>);
+LL | <stripped>.to_string();
+LL |
+LL | str::to_string(<stripped>);
+LL | <stripped>.to_string();
+ |
+
+error: stripping a prefix manually
+ --> $DIR/manual_strip.rs:24:24
+ |
+LL | str::to_string(&s[1..]);
+ | ^^^^^^^
+ |
+note: the prefix was tested here
+ --> $DIR/manual_strip.rs:23:5
+ |
+LL | if s.starts_with('a') {
+ | ^^^^^^^^^^^^^^^^^^^^^^
+help: try using the `strip_prefix` method
+ |
+LL | if let Some(<stripped>) = s.strip_prefix('a') {
+LL | str::to_string(<stripped>);
+LL | <stripped>.to_string();
+ |
+
+error: stripping a prefix manually
+ --> $DIR/manual_strip.rs:31:24
+ |
+LL | str::to_string(&s[prefix.len()..]);
+ | ^^^^^^^^^^^^^^^^^^
+ |
+note: the prefix was tested here
+ --> $DIR/manual_strip.rs:30:5
+ |
+LL | if s.starts_with(prefix) {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^
+help: try using the `strip_prefix` method
+ |
+LL | if let Some(<stripped>) = s.strip_prefix(prefix) {
+LL | str::to_string(<stripped>);
+ |
+
+error: stripping a prefix manually
+ --> $DIR/manual_strip.rs:37:24
+ |
+LL | str::to_string(&s[PREFIX.len()..]);
+ | ^^^^^^^^^^^^^^^^^^
+ |
+note: the prefix was tested here
+ --> $DIR/manual_strip.rs:36:5
+ |
+LL | if s.starts_with(PREFIX) {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^
+help: try using the `strip_prefix` method
+ |
+LL | if let Some(<stripped>) = s.strip_prefix(PREFIX) {
+LL | str::to_string(<stripped>);
+LL | str::to_string(<stripped>);
+ |
+
+error: stripping a prefix manually
+ --> $DIR/manual_strip.rs:44:24
+ |
+LL | str::to_string(&TARGET[prefix.len()..]);
+ | ^^^^^^^^^^^^^^^^^^^^^^^
+ |
+note: the prefix was tested here
+ --> $DIR/manual_strip.rs:43:5
+ |
+LL | if TARGET.starts_with(prefix) {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: try using the `strip_prefix` method
+ |
+LL | if let Some(<stripped>) = TARGET.strip_prefix(prefix) {
+LL | str::to_string(<stripped>);
+ |
+
+error: stripping a prefix manually
+ --> $DIR/manual_strip.rs:50:9
+ |
+LL | s1[2..].to_uppercase();
+ | ^^^^^^^
+ |
+note: the prefix was tested here
+ --> $DIR/manual_strip.rs:49:5
+ |
+LL | if s1.starts_with("ab") {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^
+help: try using the `strip_prefix` method
+ |
+LL | if let Some(<stripped>) = s1.strip_prefix("ab") {
+LL | <stripped>.to_uppercase();
+ |
+
+error: aborting due to 7 previous errors
+