Skip to content

WIP: Add Applicability to suggestion lints #2943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion clippy_lints/src/bytecount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use rustc::lint::*;
use rustc::{declare_lint, lint_array};
use if_chain::if_chain;
use rustc::ty;
use rustc_errors::Applicability;
use syntax::ast::{Name, UintTy};
use crate::utils::{contains_name, get_pat_name, match_type, paths, single_segment_path, snippet, span_lint_and_sugg,
walk_ptrs_ty};
Expand Down Expand Up @@ -86,7 +87,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ByteCount {
"Consider using the bytecount crate",
format!("bytecount::count({}, {})",
snippet(cx, haystack.span, ".."),
snippet(cx, needle.span, "..")));
snippet(cx, needle.span, "..")),
Applicability::HasPlaceholders);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only "maybe" has placeholders. We should make the snippet function return a tuple of Applicability and snippet-String

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best thing to do here would be to have it accept &mut Applicability IMO

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like

let mut applicability = span.into(); // if macro-ish, makes it MaybeIncorrect
let foo = span_to_snippet(...., &mut applicability) // if Applicable, makes it HasPlaceholders in the macro case

}
};
}
Expand Down
5 changes: 4 additions & 1 deletion clippy_lints/src/collapsible_if.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

use rustc::lint::*;
use rustc::{declare_lint, lint_array};
use rustc_errors::Applicability;
use if_chain::if_chain;
use syntax::ast;

Expand Down Expand Up @@ -114,7 +115,9 @@ fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {
block.span,
"this `else { if .. }` block can be collapsed",
"try",
snippet_block(cx, else_.span, "..").into_owned());
snippet_block(cx, else_.span, "..").into_owned(),
Applicability::HasPlaceholders,
);
}
_ => (),
}
Expand Down
4 changes: 3 additions & 1 deletion clippy_lints/src/default_trait_access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use rustc::lint::*;
use rustc::{declare_lint, lint_array};
use if_chain::if_chain;
use rustc::ty::TypeVariants;
use rustc_errors::Applicability;

use crate::utils::{any_parent_is_automatically_derived, match_def_path, opt_def_id, paths, span_lint_and_sugg};

Expand Down Expand Up @@ -59,7 +60,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DefaultTraitAccess {
expr.span,
&format!("Calling {} is more clear than this expression", replacement),
"try",
replacement);
replacement,
Applicability::MaybeIncorrect);
}
},
QPath::TypeRelative(..) => {},
Expand Down
3 changes: 2 additions & 1 deletion clippy_lints/src/double_comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use rustc::hir::*;
use rustc::lint::*;
use rustc::{declare_lint, lint_array};
use rustc_errors::Applicability;
use syntax::codemap::Span;

use crate::utils::{snippet, span_lint_and_sugg, SpanlessEq};
Expand Down Expand Up @@ -64,7 +65,7 @@ impl<'a, 'tcx> DoubleComparisonPass {
let sugg = format!("{} {} {}", lhs_str, stringify!($op), rhs_str);
span_lint_and_sugg(cx, DOUBLE_COMPARISONS, span,
"This binary expression can be simplified",
"try", sugg);
"try", sugg, Applicability::HasPlaceholders);
}}
}
match (op, lkind, rkind) {
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/duration_subsec.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use rustc::hir::*;
use rustc::lint::*;
use rustc::{declare_lint, lint_array};
use rustc_errors::Applicability;
use if_chain::if_chain;
use syntax::codemap::Spanned;

Expand Down Expand Up @@ -58,6 +59,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DurationSubsec {
&format!("Calling `{}()` is more concise than this calculation", suggested_fn),
"try",
format!("{}.{}()", snippet(cx, args[0].span, "_"), suggested_fn),
Applicability::HasPlaceholders,
);
}
}
Expand Down
5 changes: 2 additions & 3 deletions clippy_lints/src/else_if_without_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc::lint::*;
use rustc::{declare_lint, lint_array};
use syntax::ast::*;

use crate::utils::span_lint_and_sugg;
use crate::utils::span_help_and_lint;

/// **What it does:** Checks for usage of if expressions with an `else if` branch,
/// but without a final `else` branch.
Expand Down Expand Up @@ -56,13 +56,12 @@ impl EarlyLintPass for ElseIfWithoutElse {

while let ExprKind::If(_, _, Some(ref els)) = item.node {
if let ExprKind::If(_, _, None) = els.node {
span_lint_and_sugg(
span_help_and_lint(
cx,
ELSE_IF_WITHOUT_ELSE,
els.span,
"if expression with an `else if`, but without a final `else`",
"add an `else` block here",
"".to_string()
);
}

Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/eq_op.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use rustc::hir::*;
use rustc::lint::*;
use rustc::{declare_lint, lint_array};
use rustc_errors::Applicability;
use crate::utils::{in_macro, implements_trait, is_copy, multispan_sugg, snippet, span_lint, span_lint_and_then, SpanlessEq};

/// **What it does:** Checks for equal operands to comparison, logical and
Expand Down Expand Up @@ -107,6 +108,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EqOp {
db,
"use the values directly".to_string(),
vec![(left.span, lsnip), (right.span, rsnip)],
Applicability::HasPlaceholders,
);
},
)
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/excessive_precision.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use rustc::lint::*;
use rustc::{declare_lint, lint_array};
use if_chain::if_chain;
use rustc::ty::TypeVariants;
use rustc_errors::Applicability;
use std::f32;
use std::f64;
use std::fmt;
Expand Down Expand Up @@ -58,6 +59,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExcessivePrecision {
"float has excessive precision",
"consider changing the type or truncating it to",
sugg,
Applicability::MachineApplicable,
);
}
}
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/infallible_destructuring_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use super::utils::{get_arg_name, match_var, remove_blocks, snippet, span_lint_an
use rustc::hir::*;
use rustc::lint::*;
use rustc::{declare_lint, lint_array};
use rustc_errors::Applicability;
use if_chain::if_chain;

/// **What it does:** Checks for matches being used to destructure a single-variant enum
Expand Down Expand Up @@ -74,6 +75,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
snippet(cx, local.pat.span, ".."),
snippet(cx, target.span, ".."),
),
Applicability::HasPlaceholders,
);
}
}
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/len_zero.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use rustc::hir::*;
use rustc::lint::*;
use rustc::{declare_lint, lint_array};
use rustc::ty;
use rustc_errors::Applicability;
use std::collections::HashSet;
use syntax::ast::{Lit, LitKind, Name};
use syntax::codemap::{Span, Spanned};
Expand Down Expand Up @@ -226,6 +227,7 @@ fn check_len(cx: &LateContext<'_, '_>, span: Span, method_name: Name, args: &[Ex
&format!("length comparison to {}", if compare_to == 0 { "zero" } else { "one" }),
"using `is_empty` is more concise",
format!("{}{}.is_empty()", op, snippet(cx, args[0].span, "_")),
Applicability::HasPlaceholders,
);
}
}
Expand Down
5 changes: 5 additions & 0 deletions clippy_lints/src/literal_representation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

use rustc::lint::*;
use rustc::{declare_lint, lint_array};
use rustc_errors::Applicability;
use if_chain::if_chain;
use syntax::ast::*;
use syntax_pos;
Expand Down Expand Up @@ -238,6 +239,7 @@ impl WarningType {
"long literal lacking separators",
"consider",
grouping_hint.to_owned(),
Applicability::MachineApplicable,
),
WarningType::LargeDigitGroups => span_lint_and_sugg(
cx,
Expand All @@ -246,6 +248,7 @@ impl WarningType {
"digit groups should be smaller",
"consider",
grouping_hint.to_owned(),
Applicability::MachineApplicable,
),
WarningType::InconsistentDigitGrouping => span_lint_and_sugg(
cx,
Expand All @@ -254,6 +257,7 @@ impl WarningType {
"digits grouped inconsistently by underscores",
"consider",
grouping_hint.to_owned(),
Applicability::MachineApplicable,
),
WarningType::DecimalRepresentation => span_lint_and_sugg(
cx,
Expand All @@ -262,6 +266,7 @@ impl WarningType {
"integer literal has a better hexadecimal representation",
"consider",
grouping_hint.to_owned(),
Applicability::MachineApplicable,
),
};
}
Expand Down
9 changes: 9 additions & 0 deletions clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use rustc::middle::mem_categorization::Categorization;
use rustc::middle::mem_categorization::cmt_;
use rustc::ty::{self, Ty};
use rustc::ty::subst::Subst;
use rustc_errors::Applicability;
use std::collections::{HashMap, HashSet};
use std::iter::{once, Iterator};
use syntax::ast;
Expand Down Expand Up @@ -470,6 +471,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
snippet(cx, arms[0].pats[0].span, ".."),
snippet(cx, matchexpr.span, "..")
),
Applicability::HasPlaceholders,
);
}
},
Expand Down Expand Up @@ -501,6 +503,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
"this loop could be written as a `for` loop",
"try",
format!("for {} in {} {{ .. }}", loop_var, iterator),
Applicability::HasPlaceholders,
);
}
}
Expand Down Expand Up @@ -965,6 +968,7 @@ fn detect_manual_memcpy<'a, 'tcx>(
"it looks like you're manually copying between slices",
"try replacing the loop by",
big_sugg,
Applicability::MachineApplicable,
);
}
}
Expand Down Expand Up @@ -1069,6 +1073,7 @@ fn check_for_loop_range<'a, 'tcx>(
(pat.span, format!("({}, <item>)", ident.name)),
(arg.span, format!("{}.{}().enumerate(){}{}", indexed, method, take, skip)),
],
Applicability::HasPlaceholders,
);
},
);
Expand All @@ -1089,6 +1094,7 @@ fn check_for_loop_range<'a, 'tcx>(
db,
"consider using an iterator".to_string(),
vec![(pat.span, "<item>".to_string()), (arg.span, repl)],
Applicability::HasPlaceholders,
);
},
);
Expand Down Expand Up @@ -1200,6 +1206,7 @@ fn lint_iter_method(cx: &LateContext<'_, '_>, args: &[Expr], arg: &Expr, method_
iteration methods",
"to write this more concisely, try",
format!("&{}{}", muta, object),
Applicability::HasPlaceholders,
)
}

Expand Down Expand Up @@ -1238,6 +1245,7 @@ fn check_for_loop_arg(cx: &LateContext<'_, '_>, pat: &Pat, arg: &Expr, expr: &Ex
iteration methods`",
"to write this more concisely, try",
object.to_string(),
Applicability::HasPlaceholders,
);
}
} else if method_name == "next" && match_trait_method(cx, arg, &paths::ITERATOR) {
Expand Down Expand Up @@ -1398,6 +1406,7 @@ fn check_for_loop_over_map_kv<'a, 'tcx>(
(pat_span, snippet(cx, new_pat_span, kind).into_owned()),
(arg_span, format!("{}.{}s{}()", map.maybe_par(), kind, mutbl)),
],
Applicability::HasPlaceholders,
);
},
);
Expand Down
7 changes: 5 additions & 2 deletions clippy_lints/src/matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use rustc::lint::*;
use rustc::{declare_lint, lint_array};
use if_chain::if_chain;
use rustc::ty::{self, Ty};
use rustc_errors::Applicability;
use std::cmp::Ordering;
use std::collections::Bound;
use syntax::ast::LitKind;
Expand Down Expand Up @@ -249,6 +250,7 @@ fn report_single_match_single_pattern(cx: &LateContext<'_, '_>, ex: &Expr, arms:
expr_block(cx, &arms[0].body, None, ".."),
els_str
),
Applicability::HasPlaceholders,
);
}

Expand Down Expand Up @@ -431,7 +433,7 @@ fn check_match_ref_pats(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr:
}));

span_lint_and_then(cx, MATCH_REF_PATS, expr.span, title, |db| {
multispan_sugg(db, msg.to_owned(), suggs);
multispan_sugg(db, msg.to_owned(), suggs, Applicability::HasPlaceholders);
});
}
}
Expand All @@ -455,7 +457,8 @@ fn check_match_as_ref(cx: &LateContext<'_, '_>, ex: &Expr, arms: &[Arm], expr: &
expr.span,
&format!("use {}() instead", suggestion),
"try this",
format!("{}.{}()", snippet(cx, ex.span, "_"), suggestion)
format!("{}.{}()", snippet(cx, ex.span, "_"), suggestion),
Applicability::HasPlaceholders,
)
}
}
Expand Down
Loading