Skip to content

Commit

Permalink
Rollup merge of #108726 - est31:backticks_matchmaking_tidy, r=Nilstrieb
Browse files Browse the repository at this point in the history
tidy: enforce comment blocks to have an even number of backticks

After PR #108694, most unmatched backticks in `compiler/` comments have been eliminated. This PR adds a tidy lint to ensure no new unmatched backticks are added, and either addresses the lint in the remaining instances it found, or allows it.

Very often, backtick containing sections wrap around lines, for example:

```Rust
// This function takes a tuple `(Vec<String>,
// Box<[u8]>)` and transforms it into `Vec<u8>`.
```

The lint is implemented to work on top of blocks, counting each line with a `//` into a block, and counting if there are an odd or even number of backticks in the entire block, instead of looking at just a single line.
  • Loading branch information
matthiaskrgr committed Mar 12, 2023
2 parents 501ad02 + b2aeb07 commit 9668ae5
Show file tree
Hide file tree
Showing 12 changed files with 69 additions and 21 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_builtin_macros/src/deriving/generic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1052,6 +1052,7 @@ impl<'a> MethodDef<'a> {
/// ::core::hash::Hash::hash(&{ self.y }, state)
/// }
/// }
/// ```
fn expand_struct_method_body<'b>(
&self,
cx: &mut ExtCtxt<'_>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,7 @@ fn build_enum_variant_member_di_node<'ll, 'tcx>(
/// DW_TAG_structure_type (type of variant 1)
/// DW_TAG_structure_type (type of variant 2)
/// DW_TAG_structure_type (type of variant 3)
/// ```
struct VariantMemberInfo<'a, 'll> {
variant_index: VariantIdx,
variant_name: Cow<'a, str>,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_error_codes/src/error_codes/E0368.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl Add for Foo {
fn main() {
let mut x: Foo = Foo(5);
x += Foo(7); // error, `+= cannot be applied to the type `Foo`
x += Foo(7); // error, `+=` cannot be applied to the type `Foo`
}
```

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_error_codes/src/error_codes/E0710.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ An unknown tool name was found in a scoped lint.
Erroneous code examples:

```compile_fail,E0710
#[allow(clipp::filter_map)] // error!`
#[allow(clipp::filter_map)] // error!
fn main() {
// business logic
}
```

```compile_fail,E0710
#[warn(clipp::filter_map)] // error!`
#[warn(clipp::filter_map)] // error!
fn main() {
// business logic
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
///
/// Here:
/// - E would be `fn(&u32) -> &u32`.
/// - S would be `fn(&u32) ->
/// - S would be `fn(&u32) -> ?T`
/// - E' is `&'!0 u32 -> &'!0 u32`
/// - S' is `&'?0 u32 -> ?T`
///
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
let (mention_influencer, influencer_point) =
if sup_origin.span().overlaps(param.param_ty_span) {
// Account for `async fn` like in `async-await/issues/issue-62097.rs`.
// The desugaring of `async `fn`s causes `sup_origin` and `param` to point at the same
// The desugaring of `async fn`s causes `sup_origin` and `param` to point at the same
// place (but with different `ctxt`, hence `overlaps` instead of `==` above).
//
// This avoids the following:
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ fn dump_matched_mir_node<'tcx, F>(
// see notes on #41697 above
let def_path =
ty::print::with_forced_impl_filename_line!(tcx.def_path_str(body.source.def_id()));
// ignore-tidy-odd-backticks the literal below is fine
write!(file, "// MIR for `{}", def_path)?;
match body.source.promoted {
None => write!(file, "`")?,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1886,6 +1886,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// let place = Foo::new();
// match place { Foo { .. } if { let tmp1 = &place; inspect(*tmp1) }
// => { let tmp2 = place; feed(tmp2) }, ... }
// ```
//
// And an input like:
//
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_trait_selection/src/solve/assembly.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,8 @@ impl<'tcx> EvalCtxt<'_, 'tcx> {
///
/// To deal with this, we first try to normalize the self type and add the candidates for the normalized
/// self type to the list of candidates in case that succeeds. Note that we can't just eagerly return in
/// this case as projections as self types add `
/// this case as projections as self types add
// FIXME complete the unfinished sentence above
fn assemble_candidates_after_normalizing_self_ty<G: GoalKind<'tcx>>(
&mut self,
goal: Goal<'tcx, G>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2220,7 +2220,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
// - `BuiltinDerivedObligation` with a generator witness (A)
// - `BuiltinDerivedObligation` with a generator (A)
// - `BuiltinDerivedObligation` with `impl std::future::Future` (A)
// - `BindingObligation` with `impl_send (Send requirement)
// - `BindingObligation` with `impl_send` (Send requirement)
//
// The first obligation in the chain is the most useful and has the generator that captured
// the type. The last generator (`outer_generator` below) has information about where the
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_type_ir/src/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
//! It defines a "skeleton" of how they should be folded.
//! - `TypeSuperFoldable`. This is implemented only for each type of interest,
//! and defines the folding "skeleton" for these types.
//! - `TypeFolder`/`FallibleTypeFolder. One of these is implemented for each
//! - `TypeFolder`/`FallibleTypeFolder`. One of these is implemented for each
//! folder. This defines how types of interest are folded.
//!
//! This means each fold is a mixture of (a) generic folding operations, and (b)
Expand Down
69 changes: 56 additions & 13 deletions src/tools/tidy/src/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,9 @@ fn contains_ignore_directive(can_contain: bool, contents: &str, check: &str) ->
}

macro_rules! suppressible_tidy_err {
($err:ident, $skip:ident, $msg:expr) => {
($err:ident, $skip:ident, $msg:literal) => {
if let Directive::Deny = $skip {
$err($msg);
$err(&format!($msg));
} else {
$skip = Directive::Ignore(true);
}
Expand Down Expand Up @@ -300,10 +300,13 @@ pub fn check(path: &Path, bad: &mut bool) {
contains_ignore_directive(can_contain, &contents, "leading-newlines");
let mut skip_copyright = contains_ignore_directive(can_contain, &contents, "copyright");
let mut skip_dbg = contains_ignore_directive(can_contain, &contents, "dbg");
let mut skip_odd_backticks =
contains_ignore_directive(can_contain, &contents, "odd-backticks");
let mut leading_new_lines = false;
let mut trailing_new_lines = 0;
let mut lines = 0;
let mut last_safety_comment = false;
let mut comment_block: Option<(usize, usize)> = None;
let is_test = file.components().any(|c| c.as_os_str() == "tests");
// scanning the whole file for multiple needles at once is more efficient than
// executing lines times needles separate searches.
Expand Down Expand Up @@ -351,7 +354,7 @@ pub fn check(path: &Path, bad: &mut bool) {
suppressible_tidy_err!(
err,
skip_line_length,
&format!("line longer than {max_columns} chars")
"line longer than {max_columns} chars"
);
}
if !is_style_file && line.contains('\t') {
Expand Down Expand Up @@ -415,15 +418,55 @@ pub fn check(path: &Path, bad: &mut bool) {

// For now only enforce in compiler
let is_compiler = || file.components().any(|c| c.as_os_str() == "compiler");
if is_compiler()
&& line.contains("//")
&& line
.chars()
.collect::<Vec<_>>()
.windows(4)
.any(|cs| matches!(cs, ['.', ' ', ' ', last] if last.is_alphabetic()))
{
err(DOUBLE_SPACE_AFTER_DOT)

if is_compiler() {
if line.contains("//")
&& line
.chars()
.collect::<Vec<_>>()
.windows(4)
.any(|cs| matches!(cs, ['.', ' ', ' ', last] if last.is_alphabetic()))
{
err(DOUBLE_SPACE_AFTER_DOT)
}

if filename.ends_with(".ftl") {
let line_backticks = trimmed.chars().filter(|ch| *ch == '`').count();
if line_backticks % 2 == 1 {
suppressible_tidy_err!(err, skip_odd_backticks, "odd number of backticks");
}
} else if trimmed.contains("//") {
let (start_line, mut backtick_count) = comment_block.unwrap_or((i + 1, 0));
let line_backticks = trimmed.chars().filter(|ch| *ch == '`').count();
let comment_text = trimmed.split("//").nth(1).unwrap();
// This check ensures that we don't lint for code that has `//` in a string literal
if line_backticks % 2 == 1 {
backtick_count += comment_text.chars().filter(|ch| *ch == '`').count();
}
comment_block = Some((start_line, backtick_count));
} else {
if let Some((start_line, backtick_count)) = comment_block.take() {
if backtick_count % 2 == 1 {
let mut err = |msg: &str| {
tidy_error!(bad, "{}:{start_line}: {msg}", file.display());
};
let block_len = (i + 1) - start_line;
if block_len == 1 {
suppressible_tidy_err!(
err,
skip_odd_backticks,
"comment with odd number of backticks"
);
} else {
suppressible_tidy_err!(
err,
skip_odd_backticks,
"{block_len}-line comment block with odd number of backticks"
);
}
}
}
}
}
}
if leading_new_lines {
Expand All @@ -441,7 +484,7 @@ pub fn check(path: &Path, bad: &mut bool) {
n => suppressible_tidy_err!(
err,
skip_trailing_newlines,
&format!("too many trailing newlines ({n})")
"too many trailing newlines ({n})"
),
};
if lines > LINES {
Expand Down

0 comments on commit 9668ae5

Please sign in to comment.