From cde0b164d28382da7de892172d9129f21f8bb179 Mon Sep 17 00:00:00 2001 From: est31 Date: Sat, 4 Mar 2023 04:24:09 +0100 Subject: [PATCH 1/6] tidy: allow direct format args capture in macro --- src/tools/tidy/src/style.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs index 9ecb30529cc92..8b10c635ef4f3 100644 --- a/src/tools/tidy/src/style.rs +++ b/src/tools/tidy/src/style.rs @@ -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); } @@ -351,7 +351,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') { @@ -441,7 +441,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 { From 7a686bf41dbaf065203336c1e99e0406de621587 Mon Sep 17 00:00:00 2001 From: est31 Date: Fri, 3 Mar 2023 08:04:53 +0100 Subject: [PATCH 2/6] tidy: enforce comment blocks to have even number of backticks Some comments may be formed like: // This function takes a tuple `(Vec, // Box<[u8]>)` and transforms it into `Vec`. where the "back-ticked" section wraps around. Therefore, we can't make a single-line based lint. We also cannot make the lint paragraph based, as it would otherwise complain about inline code blocks: /// ``` /// use super::Foo; /// /// fn main() { Foo::new(); } /// ``` For the future, one could introduce some checks to treat code blocks specially, but such code would make the check even more complicated. --- src/tools/tidy/src/style.rs | 56 +++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 9 deletions(-) diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs index 8b10c635ef4f3..af6231ed0df7a 100644 --- a/src/tools/tidy/src/style.rs +++ b/src/tools/tidy/src/style.rs @@ -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. @@ -415,15 +418,50 @@ 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::>() - .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::>() + .windows(4) + .any(|cs| matches!(cs, ['.', ' ', ' ', last] if last.is_alphabetic())) + { + err(DOUBLE_SPACE_AFTER_DOT) + } + + if line.contains("//") { + let (start_line, mut backtick_count) = comment_block.unwrap_or((i + 1, 0)); + let line_backticks = line.chars().filter(|ch| *ch == '`').count(); + let comment_text = line.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 { From 7f4cc178f07fca98aee1b58bb4f82b2f45f8afac Mon Sep 17 00:00:00 2001 From: est31 Date: Sat, 4 Mar 2023 04:25:36 +0100 Subject: [PATCH 3/6] Address the new odd backticks tidy lint in compiler/ --- compiler/rustc_builtin_macros/src/deriving/generic/mod.rs | 1 + .../rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs | 1 + compiler/rustc_error_codes/src/error_codes/E0368.md | 2 +- compiler/rustc_error_codes/src/error_codes/E0710.md | 4 ++-- .../error_reporting/nice_region_error/static_impl_trait.rs | 2 +- compiler/rustc_middle/src/mir/pretty.rs | 1 + compiler/rustc_mir_build/src/build/matches/mod.rs | 1 + .../src/traits/error_reporting/suggestions.rs | 2 +- compiler/rustc_type_ir/src/fold.rs | 2 +- 9 files changed, 10 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs index 1f819beeb5d7d..6b3053fdfac7e 100644 --- a/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs +++ b/compiler/rustc_builtin_macros/src/deriving/generic/mod.rs @@ -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<'_>, diff --git a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs index 93419d27a6236..978141917c6b0 100644 --- a/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs +++ b/compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs @@ -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>, diff --git a/compiler/rustc_error_codes/src/error_codes/E0368.md b/compiler/rustc_error_codes/src/error_codes/E0368.md index 7b9d933482131..b18e8758d712e 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0368.md +++ b/compiler/rustc_error_codes/src/error_codes/E0368.md @@ -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` } ``` diff --git a/compiler/rustc_error_codes/src/error_codes/E0710.md b/compiler/rustc_error_codes/src/error_codes/E0710.md index b7037ea611ba2..84d55d524267e 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0710.md +++ b/compiler/rustc_error_codes/src/error_codes/E0710.md @@ -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 } diff --git a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs index b06ff10d86eb0..22c1e3871175e 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/nice_region_error/static_impl_trait.rs @@ -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: diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index d8829e3e782c5..7e51953599d5a 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -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, "`")?, diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 9414d9bfa0863..3fb8a6db2d27a 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -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: // diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 9ab753c5a4826..c93c26cc04b9e 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -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 diff --git a/compiler/rustc_type_ir/src/fold.rs b/compiler/rustc_type_ir/src/fold.rs index ee4ef57c38f11..3a053d4c6a997 100644 --- a/compiler/rustc_type_ir/src/fold.rs +++ b/compiler/rustc_type_ir/src/fold.rs @@ -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) From 9475717ea302b43dcca2ccf49628525cfe82fbf7 Mon Sep 17 00:00:00 2001 From: est31 Date: Sat, 4 Mar 2023 06:20:03 +0100 Subject: [PATCH 4/6] Add a fixme and address a more non trivial case Co-authored-by: nils <48135649+Nilstrieb@users.noreply.github.com> --- compiler/rustc_hir_typeck/src/closure.rs | 2 +- compiler/rustc_trait_selection/src/solve/assembly.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/closure.rs b/compiler/rustc_hir_typeck/src/closure.rs index d84fabb783490..773ac0e40c571 100644 --- a/compiler/rustc_hir_typeck/src/closure.rs +++ b/compiler/rustc_hir_typeck/src/closure.rs @@ -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` /// diff --git a/compiler/rustc_trait_selection/src/solve/assembly.rs b/compiler/rustc_trait_selection/src/solve/assembly.rs index dec9f8016b0c1..626b44d08e7e6 100644 --- a/compiler/rustc_trait_selection/src/solve/assembly.rs +++ b/compiler/rustc_trait_selection/src/solve/assembly.rs @@ -247,7 +247,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>( &mut self, goal: Goal<'tcx, G>, From 3a20d52694c9a293d2fbf1aa7cc1c1643b3e30ec Mon Sep 17 00:00:00 2001 From: est31 Date: Sat, 4 Mar 2023 06:55:35 +0100 Subject: [PATCH 5/6] Extend the tidy lint to ftl files --- src/tools/tidy/src/style.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs index af6231ed0df7a..e3f98f42d7d44 100644 --- a/src/tools/tidy/src/style.rs +++ b/src/tools/tidy/src/style.rs @@ -430,7 +430,12 @@ pub fn check(path: &Path, bad: &mut bool) { err(DOUBLE_SPACE_AFTER_DOT) } - if line.contains("//") { + if filename.ends_with(".ftl") { + let line_backticks = line.chars().filter(|ch| *ch == '`').count(); + if line_backticks % 2 == 1 { + suppressible_tidy_err!(err, skip_odd_backticks, "odd number of backticks"); + } + } else if line.contains("//") { let (start_line, mut backtick_count) = comment_block.unwrap_or((i + 1, 0)); let line_backticks = line.chars().filter(|ch| *ch == '`').count(); let comment_text = line.split("//").nth(1).unwrap(); From b2aeb071370afeefceec4d21734e801837dd72e4 Mon Sep 17 00:00:00 2001 From: est31 Date: Sat, 4 Mar 2023 21:41:24 +0100 Subject: [PATCH 6/6] Use trimmed instead of line for performance --- src/tools/tidy/src/style.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs index e3f98f42d7d44..75a4586cb7f1c 100644 --- a/src/tools/tidy/src/style.rs +++ b/src/tools/tidy/src/style.rs @@ -431,14 +431,14 @@ pub fn check(path: &Path, bad: &mut bool) { } if filename.ends_with(".ftl") { - let line_backticks = line.chars().filter(|ch| *ch == '`').count(); + 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 line.contains("//") { + } else if trimmed.contains("//") { let (start_line, mut backtick_count) = comment_block.unwrap_or((i + 1, 0)); - let line_backticks = line.chars().filter(|ch| *ch == '`').count(); - let comment_text = line.split("//").nth(1).unwrap(); + 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();