Skip to content

Commit

Permalink
Auto merge of #5083 - Areredify:issue-4399, r=flip1995
Browse files Browse the repository at this point in the history
dont fire `possible_missing_comma` if intendation is present

Closes #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.
  • Loading branch information
bors committed Jan 25, 2020
2 parents 6b54194 + a234aef commit fd6f609
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 17 deletions.
38 changes: 22 additions & 16 deletions clippy_lints/src/formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use if_chain::if_chain;
use rustc::lint::in_external_macro;
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
use syntax::ast::*;

declare_clippy_lint! {
Expand Down Expand Up @@ -242,26 +243,31 @@ fn has_unary_equivalent(bin_op: BinOpKind) -> bool {
bin_op == BinOpKind::And || bin_op == BinOpKind::Mul || bin_op == BinOpKind::Sub
}

fn indentation(cx: &EarlyContext<'_>, span: Span) -> usize {
cx.sess.source_map().lookup_char_pos(span.lo()).col.0
}

/// Implementation of the `POSSIBLE_MISSING_COMMA` lint for array
fn check_array(cx: &EarlyContext<'_>, expr: &Expr) {
if let ExprKind::Array(ref array) = expr.kind {
for element in array {
if let ExprKind::Binary(ref op, ref lhs, _) = element.kind {
if has_unary_equivalent(op.node) && !differing_macro_contexts(lhs.span, op.span) {
let space_span = lhs.span.between(op.span);
if let Some(space_snippet) = snippet_opt(cx, space_span) {
let lint_span = lhs.span.with_lo(lhs.span.hi());
if space_snippet.contains('\n') {
span_note_and_lint(
cx,
POSSIBLE_MISSING_COMMA,
lint_span,
"possibly missing a comma here",
lint_span,
"to remove this lint, add a comma or write the expr in a single line",
);
}
}
if_chain! {
if let ExprKind::Binary(ref op, ref lhs, _) = element.kind;
if has_unary_equivalent(op.node) && !differing_macro_contexts(lhs.span, op.span);
let space_span = lhs.span.between(op.span);
if let Some(space_snippet) = snippet_opt(cx, space_span);
let lint_span = lhs.span.with_lo(lhs.span.hi());
if space_snippet.contains('\n');
if indentation(cx, op.span) <= indentation(cx, lhs.span);
then {
span_note_and_lint(
cx,
POSSIBLE_MISSING_COMMA,
lint_span,
"possibly missing a comma here",
lint_span,
"to remove this lint, add a comma or write the expr in a single line",
);
}
}
}
Expand Down
11 changes: 11 additions & 0 deletions tests/ui/formatting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,4 +143,15 @@ fn main() {
true
| false,
];

// don't lint if the indentation suggests not to
let _ = &[
1 + 2, 3
- 4, 5
];
// lint if it doesnt
let _ = &[
-1
-4,
];
}
10 changes: 9 additions & 1 deletion tests/ui/formatting.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -115,5 +115,13 @@ LL | -1, -2, -3 // <= no comma here
|
= note: to remove this lint, add a comma or write the expr in a single line

error: aborting due to 13 previous errors
error: possibly missing a comma here
--> $DIR/formatting.rs:154:11
|
LL | -1
| ^
|
= note: to remove this lint, add a comma or write the expr in a single line

error: aborting due to 14 previous errors

0 comments on commit fd6f609

Please sign in to comment.