Skip to content

Commit fd6f609

Browse files
committed
Auto merge of rust-lang#5083 - Areredify:issue-4399, r=flip1995
dont fire `possible_missing_comma` if intendation is present Closes rust-lang#4399 changelog: dont fire `possible_missing_comma` if intendation is present I suspect there is already a utils function for indentation (but searching indent didn't yield a function for that), and my solution is certainly not universal, but it's probably the best we can do.
2 parents 6b54194 + a234aef commit fd6f609

File tree

3 files changed

+42
-17
lines changed

3 files changed

+42
-17
lines changed

clippy_lints/src/formatting.rs

+22-16
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use if_chain::if_chain;
33
use rustc::lint::in_external_macro;
44
use rustc_lint::{EarlyContext, EarlyLintPass};
55
use rustc_session::{declare_lint_pass, declare_tool_lint};
6+
use rustc_span::source_map::Span;
67
use syntax::ast::*;
78

89
declare_clippy_lint! {
@@ -242,26 +243,31 @@ fn has_unary_equivalent(bin_op: BinOpKind) -> bool {
242243
bin_op == BinOpKind::And || bin_op == BinOpKind::Mul || bin_op == BinOpKind::Sub
243244
}
244245

246+
fn indentation(cx: &EarlyContext<'_>, span: Span) -> usize {
247+
cx.sess.source_map().lookup_char_pos(span.lo()).col.0
248+
}
249+
245250
/// Implementation of the `POSSIBLE_MISSING_COMMA` lint for array
246251
fn check_array(cx: &EarlyContext<'_>, expr: &Expr) {
247252
if let ExprKind::Array(ref array) = expr.kind {
248253
for element in array {
249-
if let ExprKind::Binary(ref op, ref lhs, _) = element.kind {
250-
if has_unary_equivalent(op.node) && !differing_macro_contexts(lhs.span, op.span) {
251-
let space_span = lhs.span.between(op.span);
252-
if let Some(space_snippet) = snippet_opt(cx, space_span) {
253-
let lint_span = lhs.span.with_lo(lhs.span.hi());
254-
if space_snippet.contains('\n') {
255-
span_note_and_lint(
256-
cx,
257-
POSSIBLE_MISSING_COMMA,
258-
lint_span,
259-
"possibly missing a comma here",
260-
lint_span,
261-
"to remove this lint, add a comma or write the expr in a single line",
262-
);
263-
}
264-
}
254+
if_chain! {
255+
if let ExprKind::Binary(ref op, ref lhs, _) = element.kind;
256+
if has_unary_equivalent(op.node) && !differing_macro_contexts(lhs.span, op.span);
257+
let space_span = lhs.span.between(op.span);
258+
if let Some(space_snippet) = snippet_opt(cx, space_span);
259+
let lint_span = lhs.span.with_lo(lhs.span.hi());
260+
if space_snippet.contains('\n');
261+
if indentation(cx, op.span) <= indentation(cx, lhs.span);
262+
then {
263+
span_note_and_lint(
264+
cx,
265+
POSSIBLE_MISSING_COMMA,
266+
lint_span,
267+
"possibly missing a comma here",
268+
lint_span,
269+
"to remove this lint, add a comma or write the expr in a single line",
270+
);
265271
}
266272
}
267273
}

tests/ui/formatting.rs

+11
Original file line numberDiff line numberDiff line change
@@ -143,4 +143,15 @@ fn main() {
143143
true
144144
| false,
145145
];
146+
147+
// don't lint if the indentation suggests not to
148+
let _ = &[
149+
1 + 2, 3
150+
- 4, 5
151+
];
152+
// lint if it doesnt
153+
let _ = &[
154+
-1
155+
-4,
156+
];
146157
}

tests/ui/formatting.stderr

+9-1
Original file line numberDiff line numberDiff line change
@@ -115,5 +115,13 @@ LL | -1, -2, -3 // <= no comma here
115115
|
116116
= note: to remove this lint, add a comma or write the expr in a single line
117117

118-
error: aborting due to 13 previous errors
118+
error: possibly missing a comma here
119+
--> $DIR/formatting.rs:154:11
120+
|
121+
LL | -1
122+
| ^
123+
|
124+
= note: to remove this lint, add a comma or write the expr in a single line
125+
126+
error: aborting due to 14 previous errors
119127

0 commit comments

Comments
 (0)