Skip to content

New lint [needless_raw_string_hashes] #10884

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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