Skip to content

Commit

Permalink
Auto merge of #10884 - Centri3:needless_raw_string_hashes, r=dswij
Browse files Browse the repository at this point in the history
New lint [`needless_raw_string_hashes`]

Emits a warning when there are an extraneous number of hashes(?) around a raw string literal, for example `r##"I'm a "raw string literal"!"##` or `cr#"crunb"#`

Closes #10882

I think this could also fit in `style` as well, rather than `complexity`.

changelog: Add [`needless_raw_string_hashes`] and [`needless_raw_string`] lints
  • Loading branch information
bors committed Jun 27, 2023
2 parents ecdea8c + 8cb6c86 commit c710b48
Show file tree
Hide file tree
Showing 33 changed files with 434 additions and 95 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5048,6 +5048,8 @@ Released 2018-09-13
[`needless_pass_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value
[`needless_question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_question_mark
[`needless_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
[`needless_raw_string_hashes`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_raw_string_hashes
[`needless_raw_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_raw_strings
[`needless_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
[`needless_splitn`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_splitn
[`needless_update`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_update
Expand Down Expand Up @@ -5412,4 +5414,5 @@ Released 2018-09-13
[`min-ident-chars-threshold`]: https://doc.rust-lang.org/clippy/lint_configuration.html#min-ident-chars-threshold
[`accept-comment-above-statement`]: https://doc.rust-lang.org/clippy/lint_configuration.html#accept-comment-above-statement
[`accept-comment-above-attributes`]: https://doc.rust-lang.org/clippy/lint_configuration.html#accept-comment-above-attributes
[`allow-one-hash-in-raw-strings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-one-hash-in-raw-strings
<!-- end autogenerated links to configuration documentation -->
10 changes: 10 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -717,3 +717,13 @@ Whether to accept a safety comment to be placed above the attributes for the `un
* [`undocumented_unsafe_blocks`](https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks)


## `allow-one-hash-in-raw-strings`
Whether to allow `r#""#` when `r""` can be used

**Default Value:** `false` (`bool`)

---
**Affected lints:**
* [`unnecessary_raw_string_hashes`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_raw_string_hashes)


2 changes: 2 additions & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,8 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::ranges::RANGE_MINUS_ONE_INFO,
crate::ranges::RANGE_PLUS_ONE_INFO,
crate::ranges::REVERSED_EMPTY_RANGES_INFO,
crate::raw_strings::NEEDLESS_RAW_STRINGS_INFO,
crate::raw_strings::NEEDLESS_RAW_STRING_HASHES_INFO,
crate::rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT_INFO,
crate::read_zero_byte_vec::READ_ZERO_BYTE_VEC_INFO,
crate::redundant_async_block::REDUNDANT_ASYNC_BLOCK_INFO,
Expand Down
7 changes: 7 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ mod pub_use;
mod question_mark;
mod question_mark_used;
mod ranges;
mod raw_strings;
mod rc_clone_in_vec_init;
mod read_zero_byte_vec;
mod redundant_async_block;
Expand Down Expand Up @@ -1061,6 +1062,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
def_id_to_usage: rustc_data_structures::fx::FxHashMap::default(),
})
});
let needless_raw_string_hashes_allow_one = conf.allow_one_hash_in_raw_strings;
store.register_early_pass(move || {
Box::new(raw_strings::RawStrings {
needless_raw_string_hashes_allow_one,
})
});
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
143 changes: 143 additions & 0 deletions clippy_lints/src/raw_strings.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
use std::{iter::once, ops::ControlFlow};

use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet};
use rustc_ast::{
ast::{Expr, ExprKind},
token::LitKind,
};
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_tool_lint, impl_lint_pass};

declare_clippy_lint! {
/// ### What it does
/// Checks for raw string literals where a string literal can be used instead.
///
/// ### Why is this bad?
/// It's just unnecessary, but there are many cases where using a raw string literal is more
/// idiomatic than a string literal, so it's opt-in.
///
/// ### Example
/// ```rust
/// let r = r"Hello, world!";
/// ```
/// Use instead:
/// ```rust
/// let r = "Hello, world!";
/// ```
#[clippy::version = "1.72.0"]
pub NEEDLESS_RAW_STRINGS,
restriction,
"suggests using a string literal when a raw string literal is unnecessary"
}
declare_clippy_lint! {
/// ### What it does
/// Checks for raw string literals with an unnecessary amount of hashes around them.
///
/// ### Why is this bad?
/// It's just unnecessary, and makes it look like there's more escaping needed than is actually
/// necessary.
///
/// ### Example
/// ```rust
/// let r = r###"Hello, "world"!"###;
/// ```
/// Use instead:
/// ```rust
/// let r = r#"Hello, "world"!"#;
/// ```
#[clippy::version = "1.72.0"]
pub NEEDLESS_RAW_STRING_HASHES,
style,
"suggests reducing the number of hashes around a raw string literal"
}
impl_lint_pass!(RawStrings => [NEEDLESS_RAW_STRINGS, NEEDLESS_RAW_STRING_HASHES]);

pub struct RawStrings {
pub needless_raw_string_hashes_allow_one: bool,
}

impl EarlyLintPass for RawStrings {
fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) {
if !in_external_macro(cx.sess(), expr.span)
&& let ExprKind::Lit(lit) = expr.kind
&& let LitKind::StrRaw(max) | LitKind::ByteStrRaw(max) | LitKind::CStrRaw(max) = lit.kind
{
let str = lit.symbol.as_str();
let prefix = match lit.kind {
LitKind::StrRaw(..) => "r",
LitKind::ByteStrRaw(..) => "br",
LitKind::CStrRaw(..) => "cr",
_ => unreachable!(),
};
if !snippet(cx, expr.span, prefix).trim().starts_with(prefix) {
return;
}

if !str.contains(['\\', '"']) {
span_lint_and_sugg(
cx,
NEEDLESS_RAW_STRINGS,
expr.span,
"unnecessary raw string literal",
"try",
format!("{}\"{}\"", prefix.replace('r', ""), lit.symbol),
Applicability::MachineApplicable,
);

return;
}

let req = {
let mut following_quote = false;
let mut req = 0;
// `once` so a raw string ending in hashes is still checked
let num = str.as_bytes().iter().chain(once(&0)).try_fold(0u8, |acc, &b| {
match b {
b'"' => (following_quote, req) = (true, 1),
// I'm a bit surprised the compiler didn't optimize this out, there's no
// branch but it still ends up doing an unnecessary comparison, it's:
// - cmp r9b,1h
// - sbb cl,-1h
// which will add 1 if it's true. With this change, it becomes:
// - add cl,r9b
// isn't that so much nicer?
b'#' => req += u8::from(following_quote),
_ => {
if following_quote {
following_quote = false;

if req == max {
return ControlFlow::Break(req);
}

return ControlFlow::Continue(acc.max(req));
}
},
}

ControlFlow::Continue(acc)
});

match num {
ControlFlow::Continue(num) | ControlFlow::Break(num) => num,
}
};

if req < max {
let hashes = "#".repeat(req as usize);

span_lint_and_sugg(
cx,
NEEDLESS_RAW_STRING_HASHES,
expr.span,
"unnecessary hashes around raw string literal",
"try",
format!(r#"{prefix}{hashes}"{}"{hashes}"#, lit.symbol),
Applicability::MachineApplicable,
);
}
}
}
}
6 changes: 5 additions & 1 deletion clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,10 +543,14 @@ define_Conf! {
///
/// Whether to accept a safety comment to be placed above the statement containing the `unsafe` block
(accept_comment_above_statement: bool = false),
/// Lint: UNDOCUMENTED_UNSAFE_BLOCKS.
/// Lint: UNDOCUMENTED_UNSAFE_BLOCKS.
///
/// Whether to accept a safety comment to be placed above the attributes for the `unsafe` block
(accept_comment_above_attributes: bool = false),
/// Lint: UNNECESSARY_RAW_STRING_HASHES.
///
/// Whether to allow `r#""#` when `r""` can be used
(allow_one_hash_in_raw_strings: bool = false),
}

/// Search for the configuration file.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ struct StandardFormulations<'a> {
impl AlmostStandardFormulation {
pub fn new() -> Self {
let standard_formulations = vec![StandardFormulations {
wrong_pattern: Regex::new(r"^(Check for|Detects? uses?)").unwrap(),
wrong_pattern: Regex::new("^(Check for|Detects? uses?)").unwrap(),
correction: "Checks for",
}];
Self { standard_formulations }
Expand Down
4 changes: 2 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::env;
use std::path::PathBuf;
use std::process::{self, Command};

const CARGO_CLIPPY_HELP: &str = r#"Checks a package to catch common mistakes and improve your Rust code.
const CARGO_CLIPPY_HELP: &str = "Checks a package to catch common mistakes and improve your Rust code.
Usage:
cargo clippy [options] [--] [<opts>...]
Expand All @@ -31,7 +31,7 @@ with:
You can use tool lints to allow or deny lints from your code, e.g.:
#[allow(clippy::needless_lifetimes)]
"#;
";

fn show_help() {
println!("{CARGO_CLIPPY_HELP}");
Expand Down
2 changes: 1 addition & 1 deletion tests/compile-test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ fn base_config(test_dir: &str) -> compiletest::Config {
config.program.args.push("-Dwarnings".into());

// Normalize away slashes in windows paths.
config.stderr_filter(r#"\\"#, "/");
config.stderr_filter(r"\\", "/");

//config.build_base = profile_path.join("test").join(test_dir);
config.program.program = profile_path.join(if cfg!(windows) {
Expand Down
30 changes: 15 additions & 15 deletions tests/lint_message_convention.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,16 @@ impl Message {
// also no punctuation (except for "?" ?) at the end of a line
static REGEX_SET: LazyLock<RegexSet> = LazyLock::new(|| {
RegexSet::new([
r"error: [A-Z]",
r"help: [A-Z]",
r"warning: [A-Z]",
r"note: [A-Z]",
r"try this: [A-Z]",
r"error: .*[.!]$",
r"help: .*[.!]$",
r"warning: .*[.!]$",
r"note: .*[.!]$",
r"try this: .*[.!]$",
"error: [A-Z]",
"help: [A-Z]",
"warning: [A-Z]",
"note: [A-Z]",
"try this: [A-Z]",
"error: .*[.!]$",
"help: .*[.!]$",
"warning: .*[.!]$",
"note: .*[.!]$",
"try this: .*[.!]$",
])
.unwrap()
});
Expand All @@ -39,11 +39,11 @@ impl Message {
static EXCEPTIONS_SET: LazyLock<RegexSet> = LazyLock::new(|| {
RegexSet::new([
r"\.\.\.$",
r".*C-like enum variant discriminant is not portable to 32-bit targets",
r".*Intel x86 assembly syntax used",
r".*AT&T x86 assembly syntax used",
r"note: Clippy version: .*",
r"the compiler unexpectedly panicked. this is a bug.",
".*C-like enum variant discriminant is not portable to 32-bit targets",
".*Intel x86 assembly syntax used",
".*AT&T x86 assembly syntax used",
"note: Clippy version: .*",
"the compiler unexpectedly panicked. this is a bug.",
])
.unwrap()
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//@compile-flags: --crate-name conf_disallowed_methods

#![allow(clippy::needless_raw_strings)]
#![warn(clippy::disallowed_methods)]
#![allow(clippy::useless_vec)]

Expand Down
Loading

0 comments on commit c710b48

Please sign in to comment.