Use more idiomatic rust, comment for lint logic

This commit is contained in:
Preston From 2022-07-28 00:10:19 -06:00
parent 1a08b17044
commit 1b2e05e212

View File

@ -18,7 +18,6 @@ use rustc_lint_defs::builtin::NAMED_ARGUMENTS_USED_POSITIONALLY;
use rustc_lint_defs::{BufferedEarlyLint, BuiltinLintDiagnostics, LintId}; use rustc_lint_defs::{BufferedEarlyLint, BuiltinLintDiagnostics, LintId};
use std::borrow::Cow; use std::borrow::Cow;
use std::collections::hash_map::Entry; use std::collections::hash_map::Entry;
use std::ops::Deref;
#[derive(PartialEq)] #[derive(PartialEq)]
enum ArgumentType { enum ArgumentType {
@ -71,14 +70,10 @@ impl PositionalNamedArg {
// In the case of a named argument whose position is implicit, there will not be a span // In the case of a named argument whose position is implicit, there will not be a span
// to replace. Instead, we insert the name after the `{`, which is the first character // to replace. Instead, we insert the name after the `{`, which is the first character
// of arg_span. // of arg_span.
if let Some(arg_span) = cx.arg_spans.get(self.cur_piece).copied() { return cx
return Some(Span::new( .arg_spans
arg_span.lo() + BytePos(1), .get(self.cur_piece)
arg_span.lo() + BytePos(1), .map(|arg_span| arg_span.with_lo(arg_span.lo() + BytePos(1)).shrink_to_lo());
self.positional_named_arg_span.ctxt(),
self.positional_named_arg_span.parent(),
));
}
} }
None None
@ -92,6 +87,43 @@ struct PositionalNamedArgsLint {
} }
impl PositionalNamedArgsLint { impl PositionalNamedArgsLint {
/// For a given positional argument, check if the index is for a named argument.
///
/// Since positional arguments are required to come before named arguments, if the positional
/// index is greater than or equal to the start of named arguments, we know it's a named
/// argument used positionally.
///
/// Example:
/// println!("{} {} {2}", 0, a=1, b=2);
///
/// In this case, the first piece (`{}`) would be ArgumentImplicitlyIs with an index of 0. The
/// total number of arguments is 3 and the number of named arguments is 2, so the start of named
/// arguments is index 1. Therefore, the index of 0 is okay.
///
/// The second piece (`{}`) would be ArgumentImplicitlyIs with an index of 1, which is the start
/// of named arguments, and so we should add a lint to use the named argument `a`.
///
/// The third piece (`{2}`) would be ArgumentIs with an index of 2, which is greater than the
/// start of named arguments, and so we should add a lint to use the named argument `b`.
///
/// This same check also works for width and precision formatting when either or both are
/// CountIsParam, which contains an index into the arguments.
fn maybe_add_positional_named_arg(
&mut self,
current_positional_arg: usize,
total_args_length: usize,
format_argument_index: usize,
ty: PositionalNamedArgType,
cur_piece: usize,
inner_span_to_replace: Option<rustc_parse_format::InnerSpan>,
names: &FxHashMap<Symbol, (usize, Span)>,
) {
let start_of_named_args = total_args_length - names.len();
if current_positional_arg >= start_of_named_args {
self.maybe_push(format_argument_index, ty, cur_piece, inner_span_to_replace, names)
}
}
/// Try constructing a PositionalNamedArg struct and pushing it into the vec of positional /// Try constructing a PositionalNamedArg struct and pushing it into the vec of positional
/// named arguments. If a named arg associated with `format_argument_index` cannot be found, /// named arguments. If a named arg associated with `format_argument_index` cannot be found,
/// a new item will not be added as the lint cannot be emitted in this case. /// a new item will not be added as the lint cannot be emitted in this case.
@ -100,30 +132,30 @@ impl PositionalNamedArgsLint {
format_argument_index: usize, format_argument_index: usize,
ty: PositionalNamedArgType, ty: PositionalNamedArgType,
cur_piece: usize, cur_piece: usize,
mut inner_span_to_replace: Option<rustc_parse_format::InnerSpan>, inner_span_to_replace: Option<rustc_parse_format::InnerSpan>,
names: &FxHashMap<Symbol, (usize, Span)>, names: &FxHashMap<Symbol, (usize, Span)>,
) { ) {
let named_arg = names let named_arg = names
.iter() .iter()
.find(|name| name.deref().1.0 == format_argument_index) .find(|&(_, &(index, _))| index == format_argument_index)
.map(|found| found.clone()); .map(|found| found.clone());
if let Some(named_arg) = named_arg { if let Some((&replacement, &(_, positional_named_arg_span))) = named_arg {
// In FormatSpec, `precision_span` starts at the leading `.`, which we want to keep in // In FormatSpec, `precision_span` starts at the leading `.`, which we want to keep in
// the lint suggestion, so increment `start` by 1 when `PositionalArgumentType` is // the lint suggestion, so increment `start` by 1 when `PositionalArgumentType` is
// `Precision`. // `Precision`.
if ty == PositionalNamedArgType::Precision { let inner_span_to_replace = if ty == PositionalNamedArgType::Precision {
inner_span_to_replace = inner_span_to_replace.map(|mut is| { inner_span_to_replace
is.start += 1; .map(|is| rustc_parse_format::InnerSpan { start: is.start + 1, end: is.end })
is } else {
}); inner_span_to_replace
} };
self.positional_named_args.push(PositionalNamedArg { self.positional_named_args.push(PositionalNamedArg {
ty, ty,
cur_piece, cur_piece,
inner_span_to_replace, inner_span_to_replace,
replacement: named_arg.0.clone(), replacement,
positional_named_arg_span: named_arg.1.1.clone(), positional_named_arg_span,
}); });
} }
} }
@ -386,30 +418,28 @@ impl<'a, 'b> Context<'a, 'b> {
// it's written second, so it should come after width/precision. // it's written second, so it should come after width/precision.
let pos = match arg.position { let pos = match arg.position {
parse::ArgumentIs(i, arg_end) => { parse::ArgumentIs(i, arg_end) => {
let start_of_named_args = self.args.len() - self.names.len(); self.unused_names_lint.maybe_add_positional_named_arg(
if self.curpiece >= start_of_named_args { i,
self.unused_names_lint.maybe_push( self.args.len(),
i, i,
PositionalNamedArgType::Arg, PositionalNamedArgType::Arg,
self.curpiece, self.curpiece,
arg_end, arg_end,
&self.names, &self.names,
); );
}
Exact(i) Exact(i)
} }
parse::ArgumentImplicitlyIs(i) => { parse::ArgumentImplicitlyIs(i) => {
let start_of_named_args = self.args.len() - self.names.len(); self.unused_names_lint.maybe_add_positional_named_arg(
if self.curpiece >= start_of_named_args { i,
self.unused_names_lint.maybe_push( self.args.len(),
i, i,
PositionalNamedArgType::Arg, PositionalNamedArgType::Arg,
self.curpiece, self.curpiece,
None, None,
&self.names, &self.names,
); );
}
Exact(i) Exact(i)
} }
parse::ArgumentNamed(s, span) => { parse::ArgumentNamed(s, span) => {
@ -491,16 +521,15 @@ impl<'a, 'b> Context<'a, 'b> {
match c { match c {
parse::CountImplied | parse::CountIs(..) => {} parse::CountImplied | parse::CountIs(..) => {}
parse::CountIsParam(i) => { parse::CountIsParam(i) => {
let start_of_named_args = self.args.len() - self.names.len(); self.unused_names_lint.maybe_add_positional_named_arg(
if i >= start_of_named_args { i,
self.unused_names_lint.maybe_push( self.args.len(),
i, i,
named_arg_type, named_arg_type,
self.curpiece, self.curpiece,
inner_span.clone(), *inner_span,
&self.names, &self.names,
); );
}
self.verify_arg_type(Exact(i), Count); self.verify_arg_type(Exact(i), Count);
} }
parse::CountIsName(s, span) => { parse::CountIsName(s, span) => {