Skip to content

Commit

Permalink
[WIP] Implement captured vars in format!()
Browse files Browse the repository at this point in the history
  • Loading branch information
nyurik committed Jul 24, 2022
1 parent 818a91e commit ce060e9
Show file tree
Hide file tree
Showing 11 changed files with 756 additions and 34 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3570,8 +3570,8 @@ Released 2018-09-13
[`fallible_impl_from`]: https://rust-lang.github.io/rust-clippy/master/index.html#fallible_impl_from
[`field_reassign_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#field_reassign_with_default
[`filetype_is_file`]: https://rust-lang.github.io/rust-clippy/master/index.html#filetype_is_file
[`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map
[`filter_map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_identity
[`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map
[`filter_map_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next
[`filter_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_next
[`find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#find_map
Expand Down Expand Up @@ -3631,6 +3631,7 @@ Released 2018-09-13
[`inline_asm_x86_att_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_att_syntax
[`inline_asm_x86_intel_syntax`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_asm_x86_intel_syntax
[`inline_fn_without_body`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_fn_without_body
[`inline_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#inline_format_args
[`inspect_for_each`]: https://rust-lang.github.io/rust-clippy/master/index.html#inspect_for_each
[`int_plus_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#int_plus_one
[`integer_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#integer_arithmetic
Expand Down
148 changes: 148 additions & 0 deletions clippy_lints/src/inline_format_args.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
// #![allow(unused_imports)]
// #![allow(unused_variables)]

// use std::fs::{create_dir_all, OpenOptions};
// use std::io::Write;

use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::macros::{FormatArgsArg, FormatArgsExpn, is_format_macro, root_macro_call_first_node};
use clippy_utils::source::{expand_past_previous_comma, snippet};
use if_chain::if_chain;

use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, Path, QPath};
use rustc_hir::def::Res;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::ExpnData;

declare_clippy_lint! {
/// ### What it does
///
/// ### Why is this bad?
///
/// ### Example
/// ```rust
/// // example code where clippy issues a warning
/// ```
/// Use instead:
/// ```rust
/// // example code which does not raise clippy warning
/// ```
#[clippy::version = "1.64.0"]
pub INLINE_FORMAT_ARGS,
nursery,
"default lint description"
}
declare_lint_pass!(InlineFormatArgs => [INLINE_FORMAT_ARGS]);

impl<'tcx> LateLintPass<'tcx> for InlineFormatArgs {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if_chain! {
// TODO: are all these needed? They wre mostly copy/pasted from other places.
if let Some(outer_macro) = root_macro_call_first_node(cx, expr);
if let Some(format_args) = FormatArgsExpn::find_nested(cx, expr, outer_macro.expn);
if !format_args.format_string_span.from_expansion();
let expr_expn_data = expr.span.ctxt().outer_expn_data();
let outermost_expn_data = outermost_expn_data(expr_expn_data);
if let Some(macro_def_id) = outermost_expn_data.macro_def_id;
if is_format_macro(cx, macro_def_id);
// TODO: do we need this?
// if let rustc_span::ExpnKind::Macro(_, name) = outermost_expn_data.kind;
if let Some(args) = format_args.args(cx);
if !args.is_empty();
then {
let mut changes = None;
for (i, arg) in args.iter().enumerate() {
if_chain! {
// If this condition is expensive, may want to move it to the end of this if chain?
if arg.argument_span.is_empty() || snippet(cx, arg.argument_span, "").trim_end().is_empty();
if let ExprKind::Path(QPath::Resolved(None, Path { span, res, segments })) = arg.value.kind;
if segments.len() == 1;
// TODO: do we need this?
if let Res::Local(_local_id) = res;
if !is_aliased(&args, i);
then {
let c = changes.get_or_insert_with(|| vec![]);
// TODO: is it ok to assume segments.len() == 1?
// if not, could use this instead:
// let var_name = snippet(cx, *span, "").trim_end();
// if var_name.is_empty() { continue; }
let var_name = segments[0].ident.name;
c.push((arg.argument_span, var_name.to_string()));
let arg_span = expand_past_previous_comma(cx, *span);
c.push((arg_span, "".to_string()));
}
}
}

if let Some(changes) = changes {
span_lint_and_then(
cx,
INLINE_FORMAT_ARGS,
outer_macro.span,
"REPLACE ME",
|diag| {
diag.multipart_suggestion(
&format!("some interesting message"),
changes,
Applicability::MachineApplicable,
);
},
);
}

// let dumps_dir = "expected";
// create_dir_all(dumps_dir).unwrap();
// let full_str = snippet(cx, outer_macro.span, "panic").to_string()
// .replace("/","--")
// .replace("\t","[TAB]")
// .replace("\n","[NL]")
// .replace("\\","--");
// let filename = format!("{dumps_dir}/{full_str}.txt");
// let mut file = OpenOptions::new()
// .write(true)
// .create(true)
// .truncate(true)
// .open(&filename)
// .unwrap();
// writeln!(file, "NAME {name:?}").unwrap();
// writeln!(file, "FMT {macro_def_id:?}").unwrap();
// for (i, part) in format_args.format_string_parts.iter().enumerate() {
// writeln!(file, "FMT_PART {i} = {:?}", part).unwrap();
// }
// for (i, part) in format_args.formatters.iter().enumerate() {
// writeln!(file, "FORMATTERS {i} = {:?}", part).unwrap();
// }
// for (i, part) in format_args.specs.iter().enumerate() {
// writeln!(file, "SPECS {i} = {:#?}", part).unwrap();
// }
// for (i, arg) in args.iter().enumerate() {
// writeln!(file, "#{i} = TRAIT={:?}, HAS_STRFMT={:?}, ALIAS={:?}, HAS_PRIM_FMT={:?}, SPAN={:?}, ARG_SPAN={:?}, SPEC={:#?}\nVALUE={:#?}",
// arg.format_trait, arg.has_string_formatting(), is_aliased(&args, i), arg.has_primitive_formatting(),
// snippet(cx, arg.span, "<<<..>>>"),
// snippet(cx, arg.argument_span, "<<<..>>>"),
// arg.spec, arg.value).unwrap();
// }
}
}
}
}

// TODO: if this is a common pattern, should this be in utils?
fn outermost_expn_data(expn_data: ExpnData) -> ExpnData {
if expn_data.call_site.from_expansion() {
outermost_expn_data(expn_data.call_site.ctxt().outer_expn_data())
} else {
expn_data
}
}

// Returns true if `args[i]` "refers to" or "is referred to by" another argument.
// TODO: this is not catching cases when the value is (also) used as precision or width
fn is_aliased(args: &[FormatArgsArg<'_>], i: usize) -> bool {
let value = args[i].value;
args.iter()
.enumerate()
.any(|(j, arg)| i != j && std::ptr::eq(value, arg.value))
}
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ store.register_lints(&[
inherent_to_string::INHERENT_TO_STRING_SHADOW_DISPLAY,
init_numbered_fields::INIT_NUMBERED_FIELDS,
inline_fn_without_body::INLINE_FN_WITHOUT_BODY,
inline_format_args::INLINE_FORMAT_ARGS,
int_plus_one::INT_PLUS_ONE,
invalid_upcast_comparisons::INVALID_UPCAST_COMPARISONS,
invalid_utf8_in_unchecked::INVALID_UTF8_IN_UNCHECKED,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_nursery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
LintId::of(floating_point_arithmetic::SUBOPTIMAL_FLOPS),
LintId::of(future_not_send::FUTURE_NOT_SEND),
LintId::of(index_refutable_slice::INDEX_REFUTABLE_SLICE),
LintId::of(inline_format_args::INLINE_FORMAT_ARGS),
LintId::of(let_if_seq::USELESS_LET_IF_SEQ),
LintId::of(methods::ITER_WITH_DRAIN),
LintId::of(missing_const_for_fn::MISSING_CONST_FOR_FN),
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ mod inherent_impl;
mod inherent_to_string;
mod init_numbered_fields;
mod inline_fn_without_body;
mod inline_format_args;
mod int_plus_one;
mod invalid_upcast_comparisons;
mod invalid_utf8_in_unchecked;
Expand Down Expand Up @@ -916,6 +917,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(move || Box::new(operators::Operators::new(verbose_bit_mask_threshold)));
store.register_late_pass(|| Box::new(invalid_utf8_in_unchecked::InvalidUtf8InUnchecked));
store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports));
store.register_late_pass(|| Box::new(inline_format_args::InlineFormatArgs));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
13 changes: 3 additions & 10 deletions clippy_lints/src/write.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
use clippy_utils::macros::{root_macro_call_first_node, FormatArgsArg, FormatArgsExpn, MacroCall};
use clippy_utils::source::snippet_opt;
use clippy_utils::source::{snippet_opt, expand_past_previous_comma};
use rustc_ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, HirIdMap, Impl, Item, ItemKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{sym, BytePos, Span};
use rustc_span::{sym, BytePos};

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -363,8 +363,8 @@ fn check_newline(cx: &LateContext<'_>, format_args: &FormatArgsExpn<'_>, macro_c
&format!("using `{name}!()` with a format string that ends in a single newline"),
|diag| {
let name_span = cx.sess().source_map().span_until_char(macro_call.span, '!');
let Some(format_snippet) = snippet_opt(cx, format_string_span) else { return };

let Some(format_snippet) = snippet_opt(cx, format_string_span) else { return };
if format_string_parts.len() == 1 && last.as_str() == "\n" {
// print!("\n"), write!(f, "\n")

Expand Down Expand Up @@ -546,10 +546,3 @@ fn conservative_unescape(literal: &str) -> Result<String, UnescapeErr> {

if err { Err(UnescapeErr::Lint) } else { Ok(unescaped) }
}

// Expand from `writeln!(o, "")` to `writeln!(o, "")`
// ^^ ^^^^
fn expand_past_previous_comma(cx: &LateContext<'_>, span: Span) -> Span {
let extended = cx.sess().source_map().span_extend_to_prev_char(span, ',', true);
extended.with_lo(extended.lo() - BytePos(1))
}
Loading

0 comments on commit ce060e9

Please sign in to comment.