Improved shared_code_in_if_blocks output readability and added tests

This commit is contained in:
xFrednet 2021-01-16 14:04:14 +01:00
parent 469ff96db3
commit 8efc6acc05
3 changed files with 160 additions and 40 deletions

View File

@ -1,16 +1,16 @@
use crate::utils::{both, count_eq, eq_expr_value, in_macro, search_same, SpanlessEq, SpanlessHash};
use crate::utils::{
first_line_of_span, get_parent_expr, higher, if_sequence, indent_of, parent_node_is_if_expr, reindent_multiline,
snippet, span_lint_and_note, span_lint_and_sugg, span_lint_and_then,
first_line_of_span, get_parent_expr, if_sequence, indent_of, parent_node_is_if_expr, reindent_multiline, snippet,
snippet_opt, span_lint_and_note, span_lint_and_sugg, span_lint_and_then,
};
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::Applicability;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::{Block, Expr, HirId};
use rustc_hir::{Block, Expr, ExprKind, HirId};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::map::Map;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::source_map::Span;
use rustc_span::{source_map::Span, BytePos};
use std::borrow::Cow;
declare_clippy_lint! {
@ -218,14 +218,6 @@ fn lint_same_then_else<'tcx>(
);
return;
} else {
println!(
"{:?}\n - expr_eq: {:10}, l_stmts.len(): {:10}, r_stmts.len(): {:10}",
win[0].span,
block_expr_eq,
l_stmts.len(),
r_stmts.len()
)
}
start_eq = start_eq.min(current_start_eq);
@ -328,7 +320,7 @@ fn emit_shared_code_in_if_blocks_lint(
let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, cond_indent);
let span = span_start.to(span_end);
suggestions.push(("START HELP", span, suggestion.to_string()));
suggestions.push(("start", span, suggestion.to_string()));
}
if lint_end {
@ -354,31 +346,56 @@ fn emit_shared_code_in_if_blocks_lint(
let suggestion = "}\n".to_string() + &moved_snipped;
let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, indent);
let span = moved_start.to(span_end);
suggestions.push(("END_RANGE", span, suggestion.to_string()));
let mut span = moved_start.to(span_end);
// Improve formatting if the inner block has indention (i.e. normal Rust formatting)
let test_span = Span::new(span.lo() - BytePos(4), span.lo(), span.ctxt());
if snippet_opt(cx, test_span)
.map(|snip| snip == " ")
.unwrap_or_default()
{
span = span.with_lo(test_span.lo());
}
suggestions.push(("end", span, suggestion.to_string()));
}
if suggestions.len() == 1 {
let (_, span, sugg) = &suggestions[0];
let (place_str, span, sugg) = suggestions.pop().unwrap();
let msg = format!("All if blocks contain the same code at the {}", place_str);
let help = format!("Consider moving the {} statements out like this", place_str);
span_lint_and_sugg(
cx,
SHARED_CODE_IN_IF_BLOCKS,
*span,
"All code blocks contain the same code",
"Consider moving the statements out like this",
sugg.clone(),
span,
msg.as_str(),
help.as_str(),
sugg,
Applicability::Unspecified,
);
} else {
} else if suggestions.len() == 2 {
let (_, end_span, end_sugg) = suggestions.pop().unwrap();
let (_, start_span, start_sugg) = suggestions.pop().unwrap();
span_lint_and_then(
cx,
SHARED_CODE_IN_IF_BLOCKS,
if_expr.span,
"All if blocks contain the same code",
start_span,
"All if blocks contain the same code at the start and the end. Here at the start:",
move |diag| {
for (help, span, sugg) in suggestions {
diag.span_suggestion(span, help, sugg, Applicability::Unspecified);
}
diag.span_note(end_span, "And here at the end:");
diag.span_suggestion(
start_span,
"Consider moving the start statements out like this:",
start_sugg,
Applicability::Unspecified,
);
diag.span_suggestion(
end_span,
"And consider moving the end statements out like this:",
end_sugg,
Applicability::Unspecified,
);
},
);
}

View File

@ -33,6 +33,7 @@ fn simple_examples() {
result
};
// Else if block
if x == 9 {
println!("The index is: 6");
@ -40,9 +41,32 @@ fn simple_examples() {
} else if x == 8 {
println!("The index is: 4");
// We should only get a lint trigger for the last statement
println!("This is also eq with the else block");
println!("Same end of block");
} else {
println!("Same end of block");
println!("This is also eq with the else block");
}
// Use of outer scope value
let outer_scope_value = "I'm outside the if block";
if x < 99 {
let z = "How are you";
println!("I'm a local because I use the value `z`: `{}`", z);
println!(
"I'm moveable because I know: `outer_scope_value`: '{}'",
outer_scope_value
);
} else {
let z = 45678000;
println!("I'm a local because I use the value `z`: `{}`", z);
println!(
"I'm moveable because I know: `outer_scope_value`: '{}'",
outer_scope_value
);
}
// TODO xFrednet 2021-01.13: Fix lint for `if let`
@ -147,4 +171,15 @@ fn not_moveable_due_to_value_scope() {
};
}
#[rustfmt::skip]
fn test_suggestion_with_weird_formatting() {
let x = 9;
let mut a = 0;
let mut b = 0;
// The error message still looks weird tbh but this is the best I can do
// for weird formatting
if x == 17 { b = 1; a = 0x99; } else { a = 0x99; }
}
fn main() {}

View File

@ -3,20 +3,88 @@
// shared_code_in_if_blocks at the top and bottom of the if blocks
fn main() {
// TODO xFrednet 2021-01-12: This
struct DataPack {
id: u32,
name: String,
some_data: Vec<u8>,
}
// General TODOs By xFrednet:
fn overlapping_eq_regions() {
let x = 9;
//
// * Make a test with overlapping eq regions (else ifs)
// * Test if as function parameter, tuple constructor, index, while loop condition
// * Test where only the expression is the same
// * Test where the block only has an expression
// * Test with let on a previous line let _ = \n if...
// * Tests with unreadable formatting (Inline if, Not indented)
// * Test multiline condition if x == 9 \n x == 8 {}
// * Test if for return/break (Only move to front)
// * Test in inline closures
// * Test with structs and tuples
// Overlap with separator
if x == 7 {
let t = 7;
let _overlap_start = t * 2;
let _overlap_end = 2 * t;
let _u = 9;
} else {
let t = 7;
let _overlap_start = t * 2;
let _overlap_end = 2 * t;
println!("Overlap separator");
let _overlap_start = t * 2;
let _overlap_end = 2 * t;
let _u = 9;
}
// Overlap with separator
if x == 99 {
let r = 7;
let _overlap_start = r;
let _overlap_middle = r * r;
let _overlap_end = r * r * r;
let z = "end";
} else {
let r = 7;
let _overlap_start = r;
let _overlap_middle = r * r;
let _overlap_middle = r * r;
let _overlap_end = r * r * r;
let z = "end";
}
}
fn complexer_example() {
fn gen_id(x: u32, y: u32) -> u32 {
let x = x & 0x0000_ffff;
let y = (y & 0xffff_0000) << 16;
x | y
}
fn process_data(data: DataPack) {
let _ = data;
}
let x = 8;
let y = 9;
if (x > 7 && y < 13) || (x + y) % 2 == 1 {
let a = 0xcafe;
let b = 0xffff00ff;
let e_id = gen_id(a, b);
println!("From the a `{}` to the b `{}`", a, b);
let pack = DataPack {
id: e_id,
name: "Player 1".to_string(),
some_data: vec![0x12, 0x34, 0x56, 0x78, 0x90],
};
process_data(pack);
} else {
let a = 0xcafe;
let b = 0xffff00ff;
let e_id = gen_id(a, b);
println!("The new ID is '{}'", e_id);
let pack = DataPack {
id: e_id,
name: "Player 1".to_string(),
some_data: vec![0x12, 0x34, 0x56, 0x78, 0x90],
};
process_data(pack);
}
}
fn main() {}