From 9f5bfe24eb254fbe3f745062a4c9ce1c76ff800f Mon Sep 17 00:00:00 2001 From: GnomedDev Date: Sat, 21 Sep 2024 12:46:08 +0100 Subject: [PATCH 01/87] Implement lint for regex::Regex compilation inside a loop --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/regex.rs | 62 +++++++++++++++++++++++++++++- tests/ui/regex.rs | 28 +++++++++++++- tests/ui/regex.stderr | 52 ++++++++++++++++++++++++- 5 files changed, 141 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e5d1688e4a..c385405e08a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5873,6 +5873,7 @@ Released 2018-09-13 [`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref [`ref_option_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_option_ref [`ref_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_patterns +[`regex_creation_in_loops`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_creation_in_loops [`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro [`renamed_function_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#renamed_function_params [`repeat_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_once diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 16c64830e70..25a224a47ff 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -637,6 +637,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::ref_patterns::REF_PATTERNS_INFO, crate::reference::DEREF_ADDROF_INFO, crate::regex::INVALID_REGEX_INFO, + crate::regex::REGEX_CREATION_IN_LOOPS_INFO, crate::regex::TRIVIAL_REGEX_INFO, crate::repeat_vec_with_capacity::REPEAT_VEC_WITH_CAPACITY_INFO, crate::reserve_after_initialization::RESERVE_AFTER_INITIALIZATION_INFO, diff --git a/clippy_lints/src/regex.rs b/clippy_lints/src/regex.rs index f6ef02b7c23..1671737db01 100644 --- a/clippy_lints/src/regex.rs +++ b/clippy_lints/src/regex.rs @@ -55,6 +55,44 @@ declare_clippy_lint! { "trivial regular expressions" } +declare_clippy_lint! { + /// ### What it does + /// + /// Checks for [regex](https://crates.io/crates/regex) compilation inside a loop with a literal. + /// + /// ### Why is this bad? + /// + /// Compiling a regex is a much more expensive operation than using one, and a compiled regex can be used multiple times. + /// This is documented as an antipattern [on the regex documentation](https://docs.rs/regex/latest/regex/#avoid-re-compiling-regexes-especially-in-a-loop) + /// + /// ### Example + /// ```no_run + /// # let haystacks = [""]; + /// # const MY_REGEX: &str = "a.b"; + /// for haystack in haystacks { + /// let regex = regex::Regex::new(MY_REGEX).unwrap(); + /// if regex.is_match(haystack) { + /// // Perform operation + /// } + /// } + /// ``` + /// can be replaced with + /// ```no_run + /// # let haystacks = [""]; + /// # const MY_REGEX: &str = "a.b"; + /// let regex = regex::Regex::new(MY_REGEX).unwrap(); + /// for haystack in haystacks { + /// if regex.is_match(haystack) { + /// // Perform operation + /// } + /// } + /// ``` + #[clippy::version = "1.83.0"] + pub REGEX_CREATION_IN_LOOPS, + perf, + "regular expression compilation performed in a loop" +} + #[derive(Copy, Clone)] enum RegexKind { Unicode, @@ -66,9 +104,10 @@ enum RegexKind { #[derive(Default)] pub struct Regex { definitions: DefIdMap, + loop_stack: Vec, } -impl_lint_pass!(Regex => [INVALID_REGEX, TRIVIAL_REGEX]); +impl_lint_pass!(Regex => [INVALID_REGEX, TRIVIAL_REGEX, REGEX_CREATION_IN_LOOPS]); impl<'tcx> LateLintPass<'tcx> for Regex { fn check_crate(&mut self, cx: &LateContext<'tcx>) { @@ -96,12 +135,33 @@ impl<'tcx> LateLintPass<'tcx> for Regex { && let Some(def_id) = path_def_id(cx, fun) && let Some(regex_kind) = self.definitions.get(&def_id) { + if let Some(&loop_span) = self.loop_stack.last() + && (matches!(arg.kind, ExprKind::Lit(_)) || const_str(cx, arg).is_some()) + { + span_lint_and_help( + cx, + REGEX_CREATION_IN_LOOPS, + fun.span, + "compiling a regex in a loop", + Some(loop_span), + "move the regex construction outside this loop", + ); + } + match regex_kind { RegexKind::Unicode => check_regex(cx, arg, true), RegexKind::UnicodeSet => check_set(cx, arg, true), RegexKind::Bytes => check_regex(cx, arg, false), RegexKind::BytesSet => check_set(cx, arg, false), } + } else if let ExprKind::Loop(_, _, _, span) = expr.kind { + self.loop_stack.push(span); + } + } + + fn check_expr_post(&mut self, _: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + if matches!(expr.kind, ExprKind::Loop(..)) { + self.loop_stack.pop(); } } } diff --git a/tests/ui/regex.rs b/tests/ui/regex.rs index 4fb6c08bb44..3352239892c 100644 --- a/tests/ui/regex.rs +++ b/tests/ui/regex.rs @@ -5,7 +5,7 @@ clippy::needless_borrow, clippy::needless_borrows_for_generic_args )] -#![warn(clippy::invalid_regex, clippy::trivial_regex)] +#![warn(clippy::invalid_regex, clippy::trivial_regex, clippy::regex_creation_in_loops)] extern crate regex; @@ -118,7 +118,33 @@ fn trivial_regex() { let _ = BRegex::new(r"\b{start}word\b{end}"); } +fn regex_creation_in_loops() { + loop { + let regex = Regex::new("a.b"); + //~^ ERROR: compiling a regex in a loop + let regex = BRegex::new("a.b"); + //~^ ERROR: compiling a regex in a loop + #[allow(clippy::regex_creation_in_loops)] + let allowed_regex = Regex::new("a.b"); + + if true { + let regex = Regex::new("a.b"); + //~^ ERROR: compiling a regex in a loop + } + + for _ in 0..10 { + let nested_regex = Regex::new("a.b"); + //~^ ERROR: compiling a regex in a loop + } + } + + for i in 0..10 { + let dependant_regex = Regex::new(&format!("{i}")); + } +} + fn main() { syntax_error(); trivial_regex(); + regex_creation_in_loops(); } diff --git a/tests/ui/regex.stderr b/tests/ui/regex.stderr index e936208d8d7..38e14b16316 100644 --- a/tests/ui/regex.stderr +++ b/tests/ui/regex.stderr @@ -195,5 +195,55 @@ LL | let binary_trivial_empty = BRegex::new("^$"); | = help: consider using `str::is_empty` -error: aborting due to 24 previous errors +error: compiling a regex in a loop + --> tests/ui/regex.rs:123:21 + | +LL | let regex = Regex::new("a.b"); + | ^^^^^^^^^^ + | +help: move the regex construction outside this loop + --> tests/ui/regex.rs:122:5 + | +LL | loop { + | ^^^^ + = note: `-D clippy::regex-creation-in-loops` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::regex_creation_in_loops)]` + +error: compiling a regex in a loop + --> tests/ui/regex.rs:125:21 + | +LL | let regex = BRegex::new("a.b"); + | ^^^^^^^^^^^ + | +help: move the regex construction outside this loop + --> tests/ui/regex.rs:122:5 + | +LL | loop { + | ^^^^ + +error: compiling a regex in a loop + --> tests/ui/regex.rs:131:25 + | +LL | let regex = Regex::new("a.b"); + | ^^^^^^^^^^ + | +help: move the regex construction outside this loop + --> tests/ui/regex.rs:122:5 + | +LL | loop { + | ^^^^ + +error: compiling a regex in a loop + --> tests/ui/regex.rs:136:32 + | +LL | let nested_regex = Regex::new("a.b"); + | ^^^^^^^^^^ + | +help: move the regex construction outside this loop + --> tests/ui/regex.rs:135:9 + | +LL | for _ in 0..10 { + | ^^^^^^^^^^^^^^ + +error: aborting due to 28 previous errors From b522e7a944341d463bf9091bd72faffcc807d875 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 12 Aug 2024 20:39:25 +0200 Subject: [PATCH 02/87] Generate lint list in HTML directly instead of JS --- .github/deploy.sh | 1 - .gitignore | 1 + Cargo.toml | 2 + clippy_dev/src/serve.rs | 2 +- rinja.toml | 8 ++ tests/compile-test.rs | 33 +++++- .../{index.html => index_template.html} | 103 ++++++++--------- util/gh-pages/script.js | 108 ++++++++++-------- 8 files changed, 152 insertions(+), 106 deletions(-) create mode 100644 rinja.toml rename util/gh-pages/{index.html => index_template.html} (79%) diff --git a/.github/deploy.sh b/.github/deploy.sh index 5b4b4be4e36..6cebbb7801b 100644 --- a/.github/deploy.sh +++ b/.github/deploy.sh @@ -9,7 +9,6 @@ echo "Making the docs for master" mkdir out/master/ cp util/gh-pages/index.html out/master cp util/gh-pages/script.js out/master -cp util/gh-pages/lints.json out/master cp util/gh-pages/style.css out/master if [[ -n $TAG_NAME ]]; then diff --git a/.gitignore b/.gitignore index 181b71a658b..a7c25b29021 100644 --- a/.gitignore +++ b/.gitignore @@ -34,6 +34,7 @@ out # gh pages docs util/gh-pages/lints.json +util/gh-pages/index.html # rustfmt backups *.rs.bk diff --git a/Cargo.toml b/Cargo.toml index cf810798d8c..c7383520741 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,8 @@ toml = "0.7.3" walkdir = "2.3" filetime = "0.2.9" itertools = "0.12" +pulldown-cmark = "0.11" +rinja = { version = "0.3", default-features = false, features = ["config"] } # UI test dependencies clippy_utils = { path = "clippy_utils" } diff --git a/clippy_dev/src/serve.rs b/clippy_dev/src/serve.rs index cc14cd8dae6..0216d884e2d 100644 --- a/clippy_dev/src/serve.rs +++ b/clippy_dev/src/serve.rs @@ -19,7 +19,7 @@ pub fn run(port: u16, lint: Option) -> ! { }); loop { - if mtime("util/gh-pages/lints.json") < mtime("clippy_lints/src") { + if mtime("util/gh-pages/index.html") < mtime("clippy_lints/src") { Command::new(env::var("CARGO").unwrap_or("cargo".into())) .arg("collect-metadata") .spawn() diff --git a/rinja.toml b/rinja.toml new file mode 100644 index 00000000000..5fa682788bd --- /dev/null +++ b/rinja.toml @@ -0,0 +1,8 @@ +[general] +dirs = ["util/gh-pages/"] +default_syntax = "mixed" + +[[syntax]] +name = "mixed" +expr_start = "{(" +expr_end = ")}" diff --git a/tests/compile-test.rs b/tests/compile-test.rs index af2aa519257..0bd6ac67770 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -8,6 +8,8 @@ use clippy_config::ClippyConfiguration; use clippy_lints::LintInfo; use clippy_lints::declared_lints::LINTS; use clippy_lints::deprecated_lints::{DEPRECATED, DEPRECATED_VERSION, RENAMED}; +use pulldown_cmark::{Options, Parser, html}; +use rinja::{Template, filters::Safe}; use serde::{Deserialize, Serialize}; use test_utils::IS_RUSTC_TEST_SUITE; use ui_test::custom_flags::Flag; @@ -385,6 +387,22 @@ fn ui_cargo_toml_metadata() { } } +#[derive(Template)] +#[template(path = "index_template.html")] +struct Renderer<'a> { + lints: &'a Vec, +} + +impl<'a> Renderer<'a> { + fn markdown(&self, input: &str) -> Safe { + let parser = Parser::new_ext(input, Options::all()); + let mut html_output = String::new(); + html::push_html(&mut html_output, parser); + // Oh deer, what a hack :O + Safe(html_output.replace(", @@ -559,4 +576,14 @@ impl LintMetadata { applicability: Applicability::Unspecified, } } + + fn applicability_str(&self) -> &str { + match self.applicability { + Applicability::MachineApplicable => "MachineApplicable", + Applicability::HasPlaceholders => "HasPlaceholders", + Applicability::MaybeIncorrect => "MaybeIncorrect", + Applicability::Unspecified => "Unspecified", + _ => panic!("needs to update this code"), + } + } } diff --git a/util/gh-pages/index.html b/util/gh-pages/index_template.html similarity index 79% rename from util/gh-pages/index.html rename to util/gh-pages/index_template.html index f3d7e504fdf..ee134eaaa98 100644 --- a/util/gh-pages/index.html +++ b/util/gh-pages/index_template.html @@ -55,16 +55,8 @@ Otherwise, have a great day =^.^= -
- - - - -
+
+
@@ -188,9 +180,7 @@ Otherwise, have a great day =^.^=
- +
- -
-
-

-
- {{lint.id}} - - - 📋 - -
+ {% for lint in lints %} +
+
+

+
+ {(lint.id)} + + + 📋 + +
-
- {{lint.group}} +
+ {(lint.group)} - {{lint.level}} + {(lint.level)} - − - + -
-

-
+ − + + +

+ + -
-
-
- -
- Applicability: - {{lint.applicability}} - (?) -
- -
- {{lint.group == "deprecated" ? "Deprecated" : "Added"}} in: - {{lint.version}} -
- -
- Related Issues -
- -
- View Source +
+
{(markdown(lint.docs))}
+
+ {# Applicability #} +
+ Applicability: + {( lint.applicability_str() )} + (?) +
+ +
+ {% if lint.group == "deprecated" %}Deprecated{% else %} Added{% endif %} in: + {(lint.version)} +
+ +
+ Related Issues +
+ +
+ View Source +
-
- + + {% endfor %}
diff --git a/util/gh-pages/script.js b/util/gh-pages/script.js index 1a5330bc0e5..8942628d5da 100644 --- a/util/gh-pages/script.js +++ b/util/gh-pages/script.js @@ -1,21 +1,4 @@ (function () { - const md = window.markdownit({ - html: true, - linkify: true, - typographer: true, - highlight: function (str, lang) { - if (lang && hljs.getLanguage(lang)) { - try { - return '
' +
-                        hljs.highlight(str, { language: lang, ignoreIllegals: true }).value +
-                        '
'; - } catch (__) {} - } - - return '
' + md.utils.escapeHtml(str) + '
'; - } - }); - function scrollToLint(lintId) { const target = document.getElementById(lintId); if (!target) { @@ -41,15 +24,6 @@ } angular.module("clippy", []) - .filter('markdown', function ($sce) { - return function (text) { - return $sce.trustAsHtml( - md.render(text || '') - // Oh deer, what a hack :O - .replace(' { + if (searchState.timeout !== null) { + clearTimeout(searchState.timeout); + searchState.timeout = null + } + }, + resetInputTimeout: () => { + searchState.clearInputTimeout(); + setTimeout(searchState.filterLints, 50); + }, + filterLints: () => { + let searchStr = searchState.value.trim().toLowerCase(); + if (searchStr.startsWith("clippy::")) { + searchStr = searchStr.slice(8); + } + const terms = searchStr.split(" "); + + onEachLazy(document.querySelectorAll("article"), lint => { + // Search by id + if (lint.id.indexOf(searchStr.replaceAll("-", "_")) !== -1) { + el.style.display = ""; + return; + } + // Search the description + // The use of `for`-loops instead of `foreach` enables us to return early + const docsLowerCase = lint.docs.toLowerCase(); + for (index = 0; index < terms.length; index++) { + // This is more likely and will therefore be checked first + if (docsLowerCase.indexOf(terms[index]) !== -1) { + continue; + } + + if (lint.id.indexOf(terms[index]) !== -1) { + continue; + } + + return false; + } + }); + }, +}; + +function handleInputChanged(event) { + if (event.target !== document.activeElement) { + return; + } + searchState.resetInputTimeout(); +} + function storeValue(settingName, value) { try { localStorage.setItem(`clippy-lint-list-${settingName}`, value); @@ -627,3 +643,5 @@ if (prefersDark.matches && !theme) { } let disableShortcuts = loadValue('disable-shortcuts') === "true"; document.getElementById("disable-shortcuts").checked = disableShortcuts; + +hljs.highlightAll(); From 9661ba07408c901d9521e27ee8453f769ab12744 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 13 Aug 2024 20:47:42 +0200 Subject: [PATCH 03/87] Update `cargo dev serve` command to look over the correct files --- clippy_dev/src/serve.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/clippy_dev/src/serve.rs b/clippy_dev/src/serve.rs index 0216d884e2d..d367fefec61 100644 --- a/clippy_dev/src/serve.rs +++ b/clippy_dev/src/serve.rs @@ -19,7 +19,9 @@ pub fn run(port: u16, lint: Option) -> ! { }); loop { - if mtime("util/gh-pages/index.html") < mtime("clippy_lints/src") { + let index_time = mtime("util/gh-pages/index.html"); + + if index_time < mtime("clippy_lints/src") || index_time < mtime("util/gh-pages/index_template.html") { Command::new(env::var("CARGO").unwrap_or("cargo".into())) .arg("collect-metadata") .spawn() From 0055cebaa3c0ab3cb0921efa20b18b0f291ae3ea Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 13 Aug 2024 20:47:54 +0200 Subject: [PATCH 04/87] Replace search with vanilla JS --- tests/compile-test.rs | 2 +- util/gh-pages/index_template.html | 29 +++++++------ util/gh-pages/script.js | 68 ++++++++++++------------------- 3 files changed, 43 insertions(+), 56 deletions(-) diff --git a/tests/compile-test.rs b/tests/compile-test.rs index 0bd6ac67770..00627dc0bb1 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -10,7 +10,7 @@ use clippy_lints::declared_lints::LINTS; use clippy_lints::deprecated_lints::{DEPRECATED, DEPRECATED_VERSION, RENAMED}; use pulldown_cmark::{Options, Parser, html}; use rinja::{Template, filters::Safe}; -use serde::{Deserialize, Serialize}; +use serde::Deserialize; use test_utils::IS_RUSTC_TEST_SUITE; use ui_test::custom_flags::Flag; use ui_test::custom_flags::rustfix::RustfixMode; diff --git a/util/gh-pages/index_template.html b/util/gh-pages/index_template.html index ee134eaaa98..5baa03da2ea 100644 --- a/util/gh-pages/index_template.html +++ b/util/gh-pages/index_template.html @@ -180,9 +180,9 @@ Otherwise, have a great day =^.^=
- + - @@ -199,22 +199,22 @@ Otherwise, have a great day =^.^=
{% for lint in lints %} -
+

{(lint.id)} - - + 📋
- {(lint.group)} + {(lint.group)} - {(lint.level)} + {(lint.level)} − @@ -223,7 +223,7 @@ Otherwise, have a great day =^.^=

-
+
{(markdown(lint.docs))}
{# Applicability #} @@ -232,18 +232,21 @@ Otherwise, have a great day =^.^= {( lint.applicability_str() )} (?)
- + {# Clippy version #}
{% if lint.group == "deprecated" %}Deprecated{% else %} Added{% endif %} in: {(lint.version)}
- + {# Open related issues #}
Related Issues
- -
- View Source + + {# Jump to source #} + {% if let Some(id_location) = lint.id_location %} +
+ View Source + {% endif %}
diff --git a/util/gh-pages/script.js b/util/gh-pages/script.js index 8942628d5da..eb563a0f242 100644 --- a/util/gh-pages/script.js +++ b/util/gh-pages/script.js @@ -361,42 +361,6 @@ return $scope.groups[lint.group]; }; - $scope.bySearch = function (lint, index, array) { - let searchStr = $scope.search; - // It can be `null` I haven't missed this value - if (searchStr == null) { - return true; - } - searchStr = searchStr.toLowerCase(); - if (searchStr.startsWith("clippy::")) { - searchStr = searchStr.slice(8); - } - - // Search by id - if (lint.id.indexOf(searchStr.replaceAll("-", "_")) !== -1) { - return true; - } - - // Search the description - // The use of `for`-loops instead of `foreach` enables us to return early - const terms = searchStr.split(" "); - const docsLowerCase = lint.docs.toLowerCase(); - for (index = 0; index < terms.length; index++) { - // This is more likely and will therefore be checked first - if (docsLowerCase.indexOf(terms[index]) !== -1) { - continue; - } - - if (lint.id.indexOf(terms[index]) !== -1) { - continue; - } - - return false; - } - - return true; - } - $scope.byApplicabilities = function (lint) { return $scope.applicabilities[lint.applicability]; }; @@ -472,6 +436,11 @@ function getQueryVariable(variable) { window.searchState = { timeout: null, inputElem: document.getElementById("search-input"), + lastSearch: '', + clearInput: () => { + searchState.inputElem.value = ""; + searchState.filterLints(); + }, clearInputTimeout: () => { if (searchState.timeout !== null) { clearTimeout(searchState.timeout); @@ -483,32 +452,38 @@ window.searchState = { setTimeout(searchState.filterLints, 50); }, filterLints: () => { - let searchStr = searchState.value.trim().toLowerCase(); + searchState.clearInputTimeout(); + + let searchStr = searchState.inputElem.value.trim().toLowerCase(); if (searchStr.startsWith("clippy::")) { searchStr = searchStr.slice(8); } + if (searchState.lastSearch === searchStr) { + return; + } + searchState.lastSearch = searchStr; const terms = searchStr.split(" "); onEachLazy(document.querySelectorAll("article"), lint => { // Search by id if (lint.id.indexOf(searchStr.replaceAll("-", "_")) !== -1) { - el.style.display = ""; + lint.style.display = ""; return; } // Search the description // The use of `for`-loops instead of `foreach` enables us to return early - const docsLowerCase = lint.docs.toLowerCase(); + const docsLowerCase = lint.textContent.toLowerCase(); for (index = 0; index < terms.length; index++) { // This is more likely and will therefore be checked first if (docsLowerCase.indexOf(terms[index]) !== -1) { - continue; + return; } if (lint.id.indexOf(terms[index]) !== -1) { - continue; + return; } - return false; + lint.style.display = "none"; } }); }, @@ -631,7 +606,16 @@ function generateSettings() { ); } +function generateSearch() { + searchState.inputElem.addEventListener("change", handleInputChanged); + searchState.inputElem.addEventListener("input", handleInputChanged); + searchState.inputElem.addEventListener("keydown", handleInputChanged); + searchState.inputElem.addEventListener("keyup", handleInputChanged); + searchState.inputElem.addEventListener("paste", handleInputChanged); +} + generateSettings(); +generateSearch(); // loading the theme after the initial load const prefersDark = window.matchMedia("(prefers-color-scheme: dark)"); From 574e3dd9226774d5a11cec900ec5322fbf7d6f28 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 13 Aug 2024 22:34:14 +0200 Subject: [PATCH 05/87] Finish porting lint functionalities to vanilla JS --- util/gh-pages/index_template.html | 19 ++- util/gh-pages/script.js | 230 +++++++++++++++++------------- util/gh-pages/style.css | 4 + 3 files changed, 140 insertions(+), 113 deletions(-) diff --git a/util/gh-pages/index_template.html b/util/gh-pages/index_template.html index 5baa03da2ea..6571283cbf4 100644 --- a/util/gh-pages/index_template.html +++ b/util/gh-pages/index_template.html @@ -199,14 +199,13 @@ Otherwise, have a great day =^.^=
{% for lint in lints %} -
-
+
{# Open related issues #}
- Related Issues + Related Issues
{# Jump to source #} {% if let Some(id_location) = lint.id_location %}
- View Source + View Source {% endif %}
@@ -315,7 +313,6 @@ Otherwise, have a great day =^.^= - diff --git a/util/gh-pages/script.js b/util/gh-pages/script.js index eb563a0f242..902827b97e4 100644 --- a/util/gh-pages/script.js +++ b/util/gh-pages/script.js @@ -1,19 +1,4 @@ (function () { - function scrollToLint(lintId) { - const target = document.getElementById(lintId); - if (!target) { - return; - } - target.scrollIntoView(); - } - - function scrollToLintByURL($scope, $location) { - const removeListener = $scope.$on('ngRepeatFinished', function (ngRepeatFinishedEvent) { - scrollToLint($location.path().substring(1)); - removeListener(); - }); - } - function selectGroup($scope, selectedGroup) { const groups = $scope.groups; for (const group in groups) { @@ -365,39 +350,12 @@ return $scope.applicabilities[lint.applicability]; }; - // Show details for one lint - $scope.openLint = function (lint) { - $scope.open[lint.id] = true; - $location.path(lint.id); - }; - $scope.toggleExpansion = function(lints, isExpanded) { lints.forEach(lint => { $scope.open[lint.id] = isExpanded; }); } - $scope.copyToClipboard = function (lint) { - const clipboard = document.getElementById("clipboard-" + lint.id); - if (clipboard) { - let resetClipboardTimeout = null; - const resetClipboardIcon = clipboard.innerHTML; - - function resetClipboard() { - resetClipboardTimeout = null; - clipboard.innerHTML = resetClipboardIcon; - } - - navigator.clipboard.writeText("clippy::" + lint.id); - - clipboard.innerHTML = "✓"; - if (resetClipboardTimeout !== null) { - clearTimeout(resetClipboardTimeout); - } - resetClipboardTimeout = setTimeout(resetClipboard, 1000); - } - } - // Get data $scope.open = {}; $scope.loading = true; @@ -413,8 +371,6 @@ selectGroup($scope, selectedGroup.toLowerCase()); } - scrollToLintByURL($scope, $location); - setTimeout(function () { const el = document.getElementById('filter-input'); if (el) { el.focus() } @@ -433,6 +389,65 @@ function getQueryVariable(variable) { } } +function storeValue(settingName, value) { + try { + localStorage.setItem(`clippy-lint-list-${settingName}`, value); + } catch (e) { } +} + +function loadValue(settingName) { + return localStorage.getItem(`clippy-lint-list-${settingName}`); +} + +function setTheme(theme, store) { + let enableHighlight = false; + let enableNight = false; + let enableAyu = false; + + switch(theme) { + case "ayu": + enableAyu = true; + break; + case "coal": + case "navy": + enableNight = true; + break; + case "rust": + enableHighlight = true; + break; + default: + enableHighlight = true; + theme = "light"; + break; + } + + document.getElementsByTagName("body")[0].className = theme; + + document.getElementById("githubLightHighlight").disabled = enableNight || !enableHighlight; + document.getElementById("githubDarkHighlight").disabled = !enableNight && !enableAyu; + + document.getElementById("styleHighlight").disabled = !enableHighlight; + document.getElementById("styleNight").disabled = !enableNight; + document.getElementById("styleAyu").disabled = !enableAyu; + + if (store) { + storeValue("theme", theme); + } else { + document.getElementById(`theme-choice`).value = theme; + } +} + +// loading the theme after the initial load +const prefersDark = window.matchMedia("(prefers-color-scheme: dark)"); +const theme = loadValue('theme'); +if (prefersDark.matches && !theme) { + setTheme("coal", false); +} else { + setTheme(theme, false); +} +let disableShortcuts = loadValue('disable-shortcuts') === "true"; +document.getElementById("disable-shortcuts").checked = disableShortcuts; + window.searchState = { timeout: null, inputElem: document.getElementById("search-input"), @@ -486,6 +501,11 @@ window.searchState = { lint.style.display = "none"; } }); + if (searchStr.length > 0) { + window.location.hash = `/${searchStr}`; + } else { + window.location.hash = ''; + } }, }; @@ -496,54 +516,6 @@ function handleInputChanged(event) { searchState.resetInputTimeout(); } -function storeValue(settingName, value) { - try { - localStorage.setItem(`clippy-lint-list-${settingName}`, value); - } catch (e) { } -} - -function loadValue(settingName) { - return localStorage.getItem(`clippy-lint-list-${settingName}`); -} - -function setTheme(theme, store) { - let enableHighlight = false; - let enableNight = false; - let enableAyu = false; - - switch(theme) { - case "ayu": - enableAyu = true; - break; - case "coal": - case "navy": - enableNight = true; - break; - case "rust": - enableHighlight = true; - break; - default: - enableHighlight = true; - theme = "light"; - break; - } - - document.getElementsByTagName("body")[0].className = theme; - - document.getElementById("githubLightHighlight").disabled = enableNight || !enableHighlight; - document.getElementById("githubDarkHighlight").disabled = !enableNight && !enableAyu; - - document.getElementById("styleHighlight").disabled = !enableHighlight; - document.getElementById("styleNight").disabled = !enableNight; - document.getElementById("styleAyu").disabled = !enableAyu; - - if (store) { - storeValue("theme", theme); - } else { - document.getElementById(`theme-choice`).value = theme; - } -} - function handleShortcut(ev) { if (ev.ctrlKey || ev.altKey || ev.metaKey || disableShortcuts) { return; @@ -584,6 +556,52 @@ function onEachLazy(lazyArray, func) { } } +function expandLintId(lintId) { + searchState.inputElem.value = lintId; + searchState.filterLints(); + + // Expand the lint. + const lintElem = document.getElementById(lintId); + const isCollapsed = lintElem.classList.remove("collapsed"); + lintElem.querySelector(".label-doc-folding").innerText = "-"; +} + +// Show details for one lint +function openLint(event) { + event.preventDefault(); + event.stopPropagation(); + expandLintId(event.target.getAttribute("href").slice(1)); +} + +function expandLint(lintId) { + const lintElem = document.getElementById(lintId); + const isCollapsed = lintElem.classList.toggle("collapsed"); + lintElem.querySelector(".label-doc-folding").innerText = isCollapsed ? "+" : "-"; +} + +function copyToClipboard(event) { + event.preventDefault(); + event.stopPropagation(); + + const clipboard = event.target; + + let resetClipboardTimeout = null; + const resetClipboardIcon = clipboard.innerHTML; + + function resetClipboard() { + resetClipboardTimeout = null; + clipboard.innerHTML = resetClipboardIcon; + } + + navigator.clipboard.writeText("clippy::" + clipboard.parentElement.id.slice(5)); + + clipboard.innerHTML = "✓"; + if (resetClipboardTimeout !== null) { + clearTimeout(resetClipboardTimeout); + } + resetClipboardTimeout = setTimeout(resetClipboard, 1000); +} + function handleBlur(event) { const parent = document.getElementById("settings-dropdown"); if (!parent.contains(document.activeElement) && @@ -617,15 +635,23 @@ function generateSearch() { generateSettings(); generateSearch(); -// loading the theme after the initial load -const prefersDark = window.matchMedia("(prefers-color-scheme: dark)"); -const theme = loadValue('theme'); -if (prefersDark.matches && !theme) { - setTheme("coal", false); -} else { - setTheme(theme, false); +function scrollToLint(lintId) { + const target = document.getElementById(lintId); + if (!target) { + return; + } + target.scrollIntoView(); + expandLintId(lintId); } -let disableShortcuts = loadValue('disable-shortcuts') === "true"; -document.getElementById("disable-shortcuts").checked = disableShortcuts; -hljs.highlightAll(); +// If the page we arrive on has link to a given lint, we scroll to it. +function scrollToLintByURL() { + const lintId = window.location.hash.substring(2); + if (lintId.length > 0) { + scrollToLint(lintId); + } +} + +scrollToLintByURL(); + +onEachLazy(document.querySelectorAll("pre > code.language-rust"), el => hljs.highlightElement(el)); diff --git a/util/gh-pages/style.css b/util/gh-pages/style.css index a9485d51104..9af566b5aa9 100644 --- a/util/gh-pages/style.css +++ b/util/gh-pages/style.css @@ -396,3 +396,7 @@ body { background: var(--bg); color: var(--fg); } + +article.collapsed .lint-docs { + display: none; +} \ No newline at end of file From f2193c680cc5e2d9cf711af0a9bb92b15887c892 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 13 Aug 2024 23:43:35 +0200 Subject: [PATCH 06/87] Completely remove angular and generate parts of settings in JS --- util/gh-pages/index_template.html | 80 ++--- util/gh-pages/script.js | 469 +++++------------------------- 2 files changed, 102 insertions(+), 447 deletions(-) diff --git a/util/gh-pages/index_template.html b/util/gh-pages/index_template.html index 6571283cbf4..5a7af7d562e 100644 --- a/util/gh-pages/index_template.html +++ b/util/gh-pages/index_template.html @@ -25,7 +25,7 @@ Otherwise, have a great day =^.^= - +
@@ -59,121 +59,90 @@ Otherwise, have a great day =^.^=
-
+
-
    +
-
+
-
    +
-
+
-
+
-
    +
@@ -189,10 +158,10 @@ Otherwise, have a great day =^.^=
- -
@@ -315,7 +284,6 @@ Otherwise, have a great day =^.^= - diff --git a/util/gh-pages/script.js b/util/gh-pages/script.js index 902827b97e4..6f506956e7c 100644 --- a/util/gh-pages/script.js +++ b/util/gh-pages/script.js @@ -1,394 +1,3 @@ -(function () { - function selectGroup($scope, selectedGroup) { - const groups = $scope.groups; - for (const group in groups) { - if (groups.hasOwnProperty(group)) { - groups[group] = group === selectedGroup; - } - } - } - - angular.module("clippy", []) - .directive('filterDropdown', function ($document) { - return { - restrict: 'A', - link: function ($scope, $element, $attr) { - $element.bind('click', function (event) { - if (event.target.closest('button')) { - $element.toggleClass('open'); - } else { - $element.addClass('open'); - } - $element.addClass('open-recent'); - }); - - $document.bind('click', function () { - if (!$element.hasClass('open-recent')) { - $element.removeClass('open'); - } - $element.removeClass('open-recent'); - }) - } - } - }) - .directive('onFinishRender', function ($timeout) { - return { - restrict: 'A', - link: function (scope, element, attr) { - if (scope.$last === true) { - $timeout(function () { - scope.$emit(attr.onFinishRender); - }); - } - } - }; - }) - .controller("lintList", function ($scope, $http, $location, $timeout) { - // Level filter - const LEVEL_FILTERS_DEFAULT = {allow: true, warn: true, deny: true, none: true}; - $scope.levels = { ...LEVEL_FILTERS_DEFAULT }; - $scope.byLevels = function (lint) { - return $scope.levels[lint.level]; - }; - - const GROUPS_FILTER_DEFAULT = { - cargo: true, - complexity: true, - correctness: true, - nursery: true, - pedantic: true, - perf: true, - restriction: true, - style: true, - suspicious: true, - deprecated: false, - } - - $scope.groups = { - ...GROUPS_FILTER_DEFAULT - }; - - $scope.versionFilters = { - "≥": {enabled: false, minorVersion: null }, - "≤": {enabled: false, minorVersion: null }, - "=": {enabled: false, minorVersion: null }, - }; - - // Map the versionFilters to the query parameters in a way that is easier to work with in a URL - const versionFilterKeyMap = { - "≥": "gte", - "≤": "lte", - "=": "eq" - }; - const reverseVersionFilterKeyMap = Object.fromEntries( - Object.entries(versionFilterKeyMap).map(([key, value]) => [value, key]) - ); - - const APPLICABILITIES_FILTER_DEFAULT = { - MachineApplicable: true, - MaybeIncorrect: true, - HasPlaceholders: true, - Unspecified: true, - }; - - $scope.applicabilities = { - ...APPLICABILITIES_FILTER_DEFAULT - } - - // loadFromURLParameters retrieves filter settings from the URL parameters and assigns them - // to corresponding $scope variables. - function loadFromURLParameters() { - // Extract parameters from URL - const urlParameters = $location.search(); - - // Define a helper function that assigns URL parameters to a provided scope variable - const handleParameter = (parameter, scopeVariable, defaultValues) => { - if (urlParameters[parameter]) { - const items = urlParameters[parameter].split(','); - for (const key in scopeVariable) { - if (scopeVariable.hasOwnProperty(key)) { - scopeVariable[key] = items.includes(key); - } - } - } else if (defaultValues) { - for (const key in defaultValues) { - if (scopeVariable.hasOwnProperty(key)) { - scopeVariable[key] = defaultValues[key]; - } - } - } - }; - - handleParameter('levels', $scope.levels, LEVEL_FILTERS_DEFAULT); - handleParameter('groups', $scope.groups, GROUPS_FILTER_DEFAULT); - handleParameter('applicabilities', $scope.applicabilities, APPLICABILITIES_FILTER_DEFAULT); - - // Handle 'versions' parameter separately because it needs additional processing - if (urlParameters.versions) { - const versionFilters = urlParameters.versions.split(','); - for (const versionFilter of versionFilters) { - const [key, minorVersion] = versionFilter.split(':'); - const parsedMinorVersion = parseInt(minorVersion); - - // Map the key from the URL parameter to its original form - const originalKey = reverseVersionFilterKeyMap[key]; - - if (originalKey in $scope.versionFilters && !isNaN(parsedMinorVersion)) { - $scope.versionFilters[originalKey].enabled = true; - $scope.versionFilters[originalKey].minorVersion = parsedMinorVersion; - } - } - } - - // Load the search parameter from the URL path - const searchParameter = $location.path().substring(1); // Remove the leading slash - if (searchParameter) { - $scope.search = searchParameter; - $scope.open[searchParameter] = true; - scrollToLintByURL($scope, $location); - } - } - - // updateURLParameter updates the URL parameter with the given key to the given value - function updateURLParameter(filterObj, urlKey, defaultValue = {}, processFilter = filter => filter) { - const parameter = Object.keys(filterObj) - .filter(filter => filterObj[filter]) - .sort() - .map(processFilter) - .filter(Boolean) // Filters out any falsy values, including null - .join(','); - - const defaultParameter = Object.keys(defaultValue) - .filter(filter => defaultValue[filter]) - .sort() - .map(processFilter) - .filter(Boolean) // Filters out any falsy values, including null - .join(','); - - // if we ended up back at the defaults, just remove it from the URL - if (parameter === defaultParameter) { - $location.search(urlKey, null); - } else { - $location.search(urlKey, parameter || null); - } - } - - // updateVersionURLParameter updates the version URL parameter with the given version filters - function updateVersionURLParameter(versionFilters) { - updateURLParameter( - versionFilters, - 'versions', {}, - versionFilter => versionFilters[versionFilter].enabled && versionFilters[versionFilter].minorVersion != null - ? `${versionFilterKeyMap[versionFilter]}:${versionFilters[versionFilter].minorVersion}` - : null - ); - } - - // updateAllURLParameters updates all the URL parameters with the current filter settings - function updateAllURLParameters() { - updateURLParameter($scope.levels, 'levels', LEVEL_FILTERS_DEFAULT); - updateURLParameter($scope.groups, 'groups', GROUPS_FILTER_DEFAULT); - updateVersionURLParameter($scope.versionFilters); - updateURLParameter($scope.applicabilities, 'applicabilities', APPLICABILITIES_FILTER_DEFAULT); - } - - // Add $watches to automatically update URL parameters when the data changes - $scope.$watch('levels', function (newVal, oldVal) { - if (newVal !== oldVal) { - updateURLParameter(newVal, 'levels', LEVEL_FILTERS_DEFAULT); - } - }, true); - - $scope.$watch('groups', function (newVal, oldVal) { - if (newVal !== oldVal) { - updateURLParameter(newVal, 'groups', GROUPS_FILTER_DEFAULT); - } - }, true); - - $scope.$watch('versionFilters', function (newVal, oldVal) { - if (newVal !== oldVal) { - updateVersionURLParameter(newVal); - } - }, true); - - $scope.$watch('applicabilities', function (newVal, oldVal) { - if (newVal !== oldVal) { - updateURLParameter(newVal, 'applicabilities', APPLICABILITIES_FILTER_DEFAULT) - } - }, true); - - // Watch for changes in the URL path and update the search and lint display - $scope.$watch(function () { return $location.path(); }, function (newPath) { - const searchParameter = newPath.substring(1); - if ($scope.search !== searchParameter) { - $scope.search = searchParameter; - $scope.open[searchParameter] = true; - scrollToLintByURL($scope, $location); - } - }); - - let debounceTimeout; - $scope.$watch('search', function (newVal, oldVal) { - if (newVal !== oldVal) { - if (debounceTimeout) { - $timeout.cancel(debounceTimeout); - } - - debounceTimeout = $timeout(function () { - $location.path(newVal); - }, 1000); - } - }); - - $scope.$watch(function () { return $location.search(); }, function (newParameters) { - loadFromURLParameters(); - }, true); - - $scope.updatePath = function () { - if (debounceTimeout) { - $timeout.cancel(debounceTimeout); - } - - $location.path($scope.search); - } - - $scope.toggleLevels = function (value) { - const levels = $scope.levels; - for (const key in levels) { - if (levels.hasOwnProperty(key)) { - levels[key] = value; - } - } - }; - - $scope.toggleGroups = function (value) { - const groups = $scope.groups; - for (const key in groups) { - if (groups.hasOwnProperty(key)) { - groups[key] = value; - } - } - }; - - $scope.toggleApplicabilities = function (value) { - const applicabilities = $scope.applicabilities; - for (const key in applicabilities) { - if (applicabilities.hasOwnProperty(key)) { - applicabilities[key] = value; - } - } - } - - $scope.resetGroupsToDefault = function () { - $scope.groups = { - ...GROUPS_FILTER_DEFAULT - }; - }; - - $scope.selectedValuesCount = function (obj) { - return Object.values(obj).filter(x => x).length; - } - - $scope.clearVersionFilters = function () { - for (const filter in $scope.versionFilters) { - $scope.versionFilters[filter] = { enabled: false, minorVersion: null }; - } - } - - $scope.versionFilterCount = function(obj) { - return Object.values(obj).filter(x => x.enabled).length; - } - - $scope.updateVersionFilters = function() { - for (const filter in $scope.versionFilters) { - const minorVersion = $scope.versionFilters[filter].minorVersion; - - // 1.29.0 and greater - if (minorVersion && minorVersion > 28) { - $scope.versionFilters[filter].enabled = true; - continue; - } - - $scope.versionFilters[filter].enabled = false; - } - } - - $scope.byVersion = function(lint) { - const filters = $scope.versionFilters; - for (const filter in filters) { - if (filters[filter].enabled) { - const minorVersion = filters[filter].minorVersion; - - // Strip the "pre " prefix for pre 1.29.0 lints - const lintVersion = lint.version.startsWith("pre ") ? lint.version.substring(4, lint.version.length) : lint.version; - const lintMinorVersion = lintVersion.substring(2, 4); - - switch (filter) { - // "=" gets the highest priority, since all filters are inclusive - case "=": - return (lintMinorVersion == minorVersion); - case "≥": - if (lintMinorVersion < minorVersion) { return false; } - break; - case "≤": - if (lintMinorVersion > minorVersion) { return false; } - break; - default: - return true - } - } - } - - return true; - } - - $scope.byGroups = function (lint) { - return $scope.groups[lint.group]; - }; - - $scope.byApplicabilities = function (lint) { - return $scope.applicabilities[lint.applicability]; - }; - - $scope.toggleExpansion = function(lints, isExpanded) { - lints.forEach(lint => { - $scope.open[lint.id] = isExpanded; - }); - } - - // Get data - $scope.open = {}; - $scope.loading = true; - - // This will be used to jump into the source code of the version that this documentation is for. - $scope.docVersion = window.location.pathname.split('/')[2] || "master"; - - // Set up the filters from the URL parameters before we start loading the data - loadFromURLParameters(); - - const selectedGroup = getQueryVariable("sel"); - if (selectedGroup) { - selectGroup($scope, selectedGroup.toLowerCase()); - } - - setTimeout(function () { - const el = document.getElementById('filter-input'); - if (el) { el.focus() } - }, 0); - }); -})(); - -function getQueryVariable(variable) { - const query = window.location.search.substring(1); - const vars = query.split('&'); - for (const entry of vars) { - const pair = entry.split('='); - if (decodeURIComponent(pair[0]) == variable) { - return decodeURIComponent(pair[1]); - } - } -} - function storeValue(settingName, value) { try { localStorage.setItem(`clippy-lint-list-${settingName}`, value); @@ -611,6 +220,34 @@ function handleBlur(event) { } } +function toggleExpansion(expand) { + onEachLazy( + document.querySelectorAll("article"), + expand ? el => el.classList.remove("collapsed") : el => el.classList.add("collapsed"), + ); +} + +function generateListOfOptions(list, elementId) { + let html = ''; + let nbEnabled = 0; + for (const [key, value] of Object.entries(list)) { + const attr = value ? " checked" : ""; + html += `\ +
  • \ + \ +
    • `; + if (value) { + nbEnabled += 1; + } + } + + const elem = document.getElementById(elementId); + elem.previousElementSibling.querySelector(".badge").innerText = `${nbEnabled}`; + elem.innerHTML += html; +} + function generateSettings() { const settings = document.getElementById("settings-dropdown"); const settingsButton = settings.querySelector(".settings-icon") @@ -622,6 +259,56 @@ function generateSettings() { settingsMenu.querySelectorAll("input"), el => el.onblur = handleBlur, ); + + const LEVEL_FILTERS_DEFAULT = {allow: true, warn: true, deny: true, none: true}; + generateListOfOptions(LEVEL_FILTERS_DEFAULT, "lint-levels"); + + // Generate lint groups. + const GROUPS_FILTER_DEFAULT = { + cargo: true, + complexity: true, + correctness: true, + deprecated: false, + nursery: true, + pedantic: true, + perf: true, + restriction: true, + style: true, + suspicious: true, + }; + generateListOfOptions(GROUPS_FILTER_DEFAULT, "lint-groups"); + + const APPLICABILITIES_FILTER_DEFAULT = { + Unspecified: true, + Unresolved: true, + MachineApplicable: true, + MaybeIncorrect: true, + HasPlaceholders: true + }; + generateListOfOptions(APPLICABILITIES_FILTER_DEFAULT, "lint-applicabilities"); + + const VERSIONS_FILTERS = { + "≥": {enabled: false, minorVersion: null }, + "≤": {enabled: false, minorVersion: null }, + "=": {enabled: false, minorVersion: null }, + }; + + let html = ''; + for (const kind of ["≥", "≤", "="]) { + html += `\ +
  • \ + \ + 1. \ + \ + .0\ +
    • `; + } + document.getElementById("version-filter-selector").innerHTML += html; } function generateSearch() { From a235cbd8ac347f323084c99d198f96d27269ec85 Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Mon, 23 Sep 2024 11:58:28 -0700 Subject: [PATCH 07/87] `module_name_repetitions`: don't warn if the item is in a private module. Fixes . There is still a warning (as there should be) if the item is reexported by name, but not by glob; that would require further work to examine the names in the glob, and I haven't looked into that. Credit to @Centri3 for suggesting approximately this simple fix in . However, per later comment , I am not making it configuration-dependent, but *always* checking public items in public modules only. --- clippy_lints/src/item_name_repetitions.rs | 33 +++++++++++++++---- clippy_lints/src/literal_representation.rs | 1 - clippy_lints/src/macro_use.rs | 1 - clippy_lints/src/vec.rs | 1 - .../allowed_prefixes/item_name_repetitions.rs | 2 +- .../item_name_repetitions.rs | 2 +- tests/ui/module_name_repetitions.rs | 18 +++++++++- tests/ui/module_name_repetitions.stderr | 8 ++++- 8 files changed, 53 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/item_name_repetitions.rs b/clippy_lints/src/item_name_repetitions.rs index 66a8a3167a4..511317c8b43 100644 --- a/clippy_lints/src/item_name_repetitions.rs +++ b/clippy_lints/src/item_name_repetitions.rs @@ -50,11 +50,28 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Detects type names that are prefixed or suffixed by the - /// containing module's name. + /// Detects public item names that are prefixed or suffixed by the + /// containing public module's name. /// /// ### Why is this bad? - /// It requires the user to type the module name twice. + /// It requires the user to type the module name twice in each usage, + /// especially if they choose to import the module rather than its contents. + /// + /// Lack of such repetition is also the style used in the Rust standard library; + /// e.g. `io::Error` and `fmt::Error` rather than `io::IoError` and `fmt::FmtError`; + /// and `array::from_ref` rather than `array::array_from_ref`. + /// + /// ### Known issues + /// Glob re-exports are ignored; e.g. this will not warn even though it should: + /// + /// ```no_run + /// pub mod foo { + /// mod iteration { + /// pub struct FooIter {} + /// } + /// pub use iteration::*; // creates the path `foo::FooIter` + /// } + /// ``` /// /// ### Example /// ```no_run @@ -389,12 +406,12 @@ impl LateLintPass<'_> for ItemNameRepetitions { let item_name = item.ident.name.as_str(); let item_camel = to_camel_case(item_name); if !item.span.from_expansion() && is_present_in_source(cx, item.span) { - if let [.., (mod_name, mod_camel, owner_id)] = &*self.modules { + if let [.., (mod_name, mod_camel, mod_owner_id)] = &*self.modules { // constants don't have surrounding modules if !mod_camel.is_empty() { if mod_name == &item.ident.name && let ItemKind::Mod(..) = item.kind - && (!self.allow_private_module_inception || cx.tcx.visibility(owner_id.def_id).is_public()) + && (!self.allow_private_module_inception || cx.tcx.visibility(mod_owner_id.def_id).is_public()) { span_lint( cx, @@ -403,9 +420,13 @@ impl LateLintPass<'_> for ItemNameRepetitions { "module has the same name as its containing module", ); } + // The `module_name_repetitions` lint should only trigger if the item has the module in its // name. Having the same name is accepted. - if cx.tcx.visibility(item.owner_id).is_public() && item_camel.len() > mod_camel.len() { + if cx.tcx.visibility(item.owner_id).is_public() + && cx.tcx.visibility(mod_owner_id.def_id).is_public() + && item_camel.len() > mod_camel.len() + { let matching = count_match_start(mod_camel, &item_camel); let rmatching = count_match_end(mod_camel, &item_camel); let nchars = mod_camel.chars().count(); diff --git a/clippy_lints/src/literal_representation.rs b/clippy_lints/src/literal_representation.rs index 81f2a03fb55..e2dcb20f906 100644 --- a/clippy_lints/src/literal_representation.rs +++ b/clippy_lints/src/literal_representation.rs @@ -412,7 +412,6 @@ impl LiteralDigitGrouping { } } -#[expect(clippy::module_name_repetitions)] pub struct DecimalLiteralRepresentation { threshold: u64, } diff --git a/clippy_lints/src/macro_use.rs b/clippy_lints/src/macro_use.rs index bd6b3f1a47b..d5d6226f0aa 100644 --- a/clippy_lints/src/macro_use.rs +++ b/clippy_lints/src/macro_use.rs @@ -43,7 +43,6 @@ impl MacroRefData { } #[derive(Default)] -#[expect(clippy::module_name_repetitions)] pub struct MacroUseImports { /// the actual import path used and the span of the attribute above it. The value is /// the location, where the lint should be emitted. diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs index ce4f41e854d..0fdef51d6fc 100644 --- a/clippy_lints/src/vec.rs +++ b/clippy_lints/src/vec.rs @@ -17,7 +17,6 @@ use rustc_middle::ty::layout::LayoutOf; use rustc_session::impl_lint_pass; use rustc_span::{DesugaringKind, Span, sym}; -#[expect(clippy::module_name_repetitions)] pub struct UselessVec { too_large_for_stack: u64, msrv: Msrv, diff --git a/tests/ui-toml/item_name_repetitions/allowed_prefixes/item_name_repetitions.rs b/tests/ui-toml/item_name_repetitions/allowed_prefixes/item_name_repetitions.rs index 4142ced5f6b..2ae673a6def 100644 --- a/tests/ui-toml/item_name_repetitions/allowed_prefixes/item_name_repetitions.rs +++ b/tests/ui-toml/item_name_repetitions/allowed_prefixes/item_name_repetitions.rs @@ -1,7 +1,7 @@ #![warn(clippy::module_name_repetitions)] #![allow(dead_code)] -mod foo { +pub mod foo { // #12544 - shouldn't warn if item name consists only of an allowed prefix and a module name. // In this test, allowed prefixes are configured to be ["bar"]. diff --git a/tests/ui-toml/item_name_repetitions/allowed_prefixes_extend/item_name_repetitions.rs b/tests/ui-toml/item_name_repetitions/allowed_prefixes_extend/item_name_repetitions.rs index b132305d01c..dbd61992c0d 100644 --- a/tests/ui-toml/item_name_repetitions/allowed_prefixes_extend/item_name_repetitions.rs +++ b/tests/ui-toml/item_name_repetitions/allowed_prefixes_extend/item_name_repetitions.rs @@ -1,7 +1,7 @@ #![warn(clippy::module_name_repetitions)] #![allow(dead_code)] -mod foo { +pub mod foo { // #12544 - shouldn't warn if item name consists only of an allowed prefix and a module name. // In this test, allowed prefixes are configured to be all of the default prefixes and ["bar"]. diff --git a/tests/ui/module_name_repetitions.rs b/tests/ui/module_name_repetitions.rs index b75ef87ab36..71d8ac7a1f0 100644 --- a/tests/ui/module_name_repetitions.rs +++ b/tests/ui/module_name_repetitions.rs @@ -3,7 +3,7 @@ #![warn(clippy::module_name_repetitions)] #![allow(dead_code)] -mod foo { +pub mod foo { pub fn foo() {} pub fn foo_bar() {} //~^ ERROR: item name starts with its containing module's name @@ -20,6 +20,22 @@ mod foo { // Should not warn pub struct Foobar; + // #8524 - shouldn't warn when item is declared in a private module... + mod error { + pub struct Error; + pub struct FooError; + } + pub use error::Error; + // ... but should still warn when the item is reexported to create a *public* path with repetition. + pub use error::FooError; + //~^ ERROR: item name starts with its containing module's name + + // FIXME: This should also warn because it creates the public path `foo::FooIter`. + mod iter { + pub struct FooIter; + } + pub use iter::*; + // #12544 - shouldn't warn if item name consists only of an allowed prefix and a module name. pub fn to_foo() {} pub fn into_foo() {} diff --git a/tests/ui/module_name_repetitions.stderr b/tests/ui/module_name_repetitions.stderr index bffb08f6f87..8fd8b394875 100644 --- a/tests/ui/module_name_repetitions.stderr +++ b/tests/ui/module_name_repetitions.stderr @@ -31,5 +31,11 @@ error: item name starts with its containing module's name LL | pub struct Foo7Bar; | ^^^^^^^ -error: aborting due to 5 previous errors +error: item name starts with its containing module's name + --> tests/ui/module_name_repetitions.rs:30:20 + | +LL | pub use error::FooError; + | ^^^^^^^^ + +error: aborting due to 6 previous errors From 4997ee7afcda865406352f755e9e3b44958560bd Mon Sep 17 00:00:00 2001 From: blyxyas Date: Fri, 6 Sep 2024 00:26:02 +0200 Subject: [PATCH 08/87] Turn declare_clippy_lint into a declarative macro --- clippy_lints/Cargo.toml | 1 - clippy_lints/src/declare_clippy_lint.rs | 162 +++++++++++++++++++++ clippy_lints/src/lib.rs | 6 +- clippy_utils/src/lib.rs | 2 +- declare_clippy_lint/Cargo.toml | 13 -- declare_clippy_lint/src/lib.rs | 182 ------------------------ tests/versioncheck.rs | 1 - 7 files changed, 167 insertions(+), 200 deletions(-) create mode 100644 clippy_lints/src/declare_clippy_lint.rs delete mode 100644 declare_clippy_lint/Cargo.toml delete mode 100644 declare_clippy_lint/src/lib.rs diff --git a/clippy_lints/Cargo.toml b/clippy_lints/Cargo.toml index d1188940b46..55f1d31b4ad 100644 --- a/clippy_lints/Cargo.toml +++ b/clippy_lints/Cargo.toml @@ -13,7 +13,6 @@ arrayvec = { version = "0.7", default-features = false } cargo_metadata = "0.18" clippy_config = { path = "../clippy_config" } clippy_utils = { path = "../clippy_utils" } -declare_clippy_lint = { path = "../declare_clippy_lint" } itertools = "0.12" quine-mc_cluskey = "0.2" regex-syntax = "0.8" diff --git a/clippy_lints/src/declare_clippy_lint.rs b/clippy_lints/src/declare_clippy_lint.rs new file mode 100644 index 00000000000..b1e39c70baa --- /dev/null +++ b/clippy_lints/src/declare_clippy_lint.rs @@ -0,0 +1,162 @@ +#[macro_export] +#[allow(clippy::crate_in_macro_def)] +macro_rules! declare_clippy_lint { + (@ + $(#[doc = $lit:literal])* + pub $lint_name:ident, + $category:ident, + $lintcategory:expr, + $desc:literal, + $version_expr:expr, + $version_lit:literal + ) => { + rustc_session::declare_tool_lint! { + $(#[doc = $lit])* + #[clippy::version = $version_lit] + pub clippy::$lint_name, + $category, + $desc, + report_in_external_macro:true + } + + pub(crate) static ${concat($lint_name, _INFO)}: &'static crate::LintInfo = &crate::LintInfo { + lint: &$lint_name, + category: $lintcategory, + explanation: concat!($($lit,"\n",)*), + location: concat!(file!(), "#L", line!()), + version: $version_expr + }; + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + restriction, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Allow, crate::LintCategory::Restriction, $desc, + Some($version), $version + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + style, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Warn, crate::LintCategory::Style, $desc, + Some($version), $version + + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + correctness, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Deny, crate::LintCategory::Correctness, $desc, + Some($version), $version + + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + perf, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Warn, crate::LintCategory::Perf, $desc, + Some($version), $version + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + complexity, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Warn, crate::LintCategory::Complexity, $desc, + Some($version), $version + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + suspicious, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Warn, crate::LintCategory::Suspicious, $desc, + Some($version), $version + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + nursery, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Allow, crate::LintCategory::Nursery, $desc, + Some($version), $version + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + pedantic, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Allow, crate::LintCategory::Pedantic, $desc, + Some($version), $version + } + }; + ( + $(#[doc = $lit:literal])* + #[clippy::version = $version:literal] + pub $lint_name:ident, + cargo, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Allow, crate::LintCategory::Cargo, $desc, + Some($version), $version + } + }; + + ( + $(#[doc = $lit:literal])* + pub $lint_name:ident, + internal, + $desc:literal + ) => { + declare_clippy_lint! {@ + $(#[doc = $lit])* + pub $lint_name, Allow, crate::LintCategory::Internal, $desc, + None, "0.0.0" + } + }; +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1d41f568f37..efc9fe6f315 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1,6 +1,7 @@ #![feature(array_windows)] #![feature(binary_heap_into_iter_sorted)] #![feature(box_patterns)] +#![feature(macro_metavar_expr_concat)] #![feature(control_flow_enum)] #![feature(f128)] #![feature(f16)] @@ -59,9 +60,10 @@ extern crate rustc_trait_selection; extern crate thin_vec; #[macro_use] -extern crate clippy_utils; +mod declare_clippy_lint; + #[macro_use] -extern crate declare_clippy_lint; +extern crate clippy_utils; #[cfg_attr(feature = "internal", allow(clippy::missing_clippy_version_attribute))] mod utils; diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index a925549b0bf..bf7484f7902 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -4,6 +4,7 @@ #![feature(f128)] #![feature(f16)] #![feature(if_let_guard)] +#![feature(macro_metavar_expr_concat)] #![feature(let_chains)] #![feature(never_type)] #![feature(rustc_private)] @@ -129,7 +130,6 @@ use crate::consts::{ConstEvalCtxt, Constant, mir_to_const}; use crate::higher::Range; use crate::ty::{adt_and_variant_of_res, can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type}; use crate::visitors::for_each_expr_without_closures; - use rustc_middle::hir::nested_filter; #[macro_export] diff --git a/declare_clippy_lint/Cargo.toml b/declare_clippy_lint/Cargo.toml deleted file mode 100644 index 67a1f7cc72c..00000000000 --- a/declare_clippy_lint/Cargo.toml +++ /dev/null @@ -1,13 +0,0 @@ -[package] -name = "declare_clippy_lint" -version = "0.1.83" -edition = "2021" -publish = false - -[lib] -proc-macro = true - -[dependencies] -itertools = "0.12" -quote = "1.0.21" -syn = "2.0" diff --git a/declare_clippy_lint/src/lib.rs b/declare_clippy_lint/src/lib.rs deleted file mode 100644 index fefc1a0a6c4..00000000000 --- a/declare_clippy_lint/src/lib.rs +++ /dev/null @@ -1,182 +0,0 @@ -#![feature(let_chains, proc_macro_span)] -// warn on lints, that are included in `rust-lang/rust`s bootstrap -#![warn(rust_2018_idioms, unused_lifetimes)] - -use proc_macro::TokenStream; -use quote::{format_ident, quote}; -use syn::parse::{Parse, ParseStream}; -use syn::{Attribute, Error, Expr, ExprLit, Ident, Lit, LitStr, Meta, Result, Token, parse_macro_input}; - -fn parse_attr(path: [&'static str; LEN], attr: &Attribute) -> Option { - if let Meta::NameValue(name_value) = &attr.meta { - let path_idents = name_value.path.segments.iter().map(|segment| &segment.ident); - - if itertools::equal(path_idents, path) - && let Expr::Lit(ExprLit { lit: Lit::Str(s), .. }) = &name_value.value - { - return Some(s.clone()); - } - } - - None -} - -struct ClippyLint { - attrs: Vec, - version: Option, - explanation: String, - name: Ident, - category: Ident, - description: LitStr, -} - -impl Parse for ClippyLint { - fn parse(input: ParseStream<'_>) -> Result { - let attrs = input.call(Attribute::parse_outer)?; - - let mut in_code = false; - let mut explanation = String::new(); - let mut version = None; - for attr in &attrs { - if let Some(lit) = parse_attr(["doc"], attr) { - let value = lit.value(); - let line = value.strip_prefix(' ').unwrap_or(&value); - - if let Some(lang) = line.strip_prefix("```") { - let tag = lang.split_once(',').map_or(lang, |(left, _)| left); - if !in_code && matches!(tag, "" | "rust" | "ignore" | "should_panic" | "no_run" | "compile_fail") { - explanation += "```rust\n"; - } else { - explanation += line; - explanation.push('\n'); - } - in_code = !in_code; - } else if !(in_code && line.starts_with("# ")) { - explanation += line; - explanation.push('\n'); - } - } else if let Some(lit) = parse_attr(["clippy", "version"], attr) { - if let Some(duplicate) = version.replace(lit) { - return Err(Error::new_spanned(duplicate, "duplicate clippy::version")); - } - } else { - return Err(Error::new_spanned(attr, "unexpected attribute")); - } - } - - input.parse::()?; - let name = input.parse()?; - input.parse::()?; - - let category = input.parse()?; - input.parse::()?; - - let description = input.parse()?; - - Ok(Self { - attrs, - version, - explanation, - name, - category, - description, - }) - } -} - -/// Macro used to declare a Clippy lint. -/// -/// Every lint declaration consists of 4 parts: -/// -/// 1. The documentation, which is used for the website and `cargo clippy --explain` -/// 2. The `LINT_NAME`. See [lint naming][lint_naming] on lint naming conventions. -/// 3. The `lint_level`, which is a mapping from *one* of our lint groups to `Allow`, `Warn` or -/// `Deny`. The lint level here has nothing to do with what lint groups the lint is a part of. -/// 4. The `description` that contains a short explanation on what's wrong with code where the lint -/// is triggered. -/// -/// Currently the categories `style`, `correctness`, `suspicious`, `complexity` and `perf` are -/// enabled by default. As said in the README.md of this repository, if the lint level mapping -/// changes, please update README.md. -/// -/// # Example -/// -/// ```ignore -/// use rustc_session::declare_tool_lint; -/// -/// declare_clippy_lint! { -/// /// ### What it does -/// /// Checks for ... (describe what the lint matches). -/// /// -/// /// ### Why is this bad? -/// /// Supply the reason for linting the code. -/// /// -/// /// ### Example -/// /// ```rust -/// /// Insert a short example of code that triggers the lint -/// /// ``` -/// /// -/// /// Use instead: -/// /// ```rust -/// /// Insert a short example of improved code that doesn't trigger the lint -/// /// ``` -/// #[clippy::version = "1.65.0"] -/// pub LINT_NAME, -/// pedantic, -/// "description" -/// } -/// ``` -/// [lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints -#[proc_macro] -pub fn declare_clippy_lint(input: TokenStream) -> TokenStream { - let ClippyLint { - attrs, - version, - explanation, - name, - category, - description, - } = parse_macro_input!(input as ClippyLint); - - let mut category = category.to_string(); - - let level = format_ident!("{}", match category.as_str() { - "correctness" => "Deny", - "style" | "suspicious" | "complexity" | "perf" => "Warn", - "pedantic" | "restriction" | "cargo" | "nursery" | "internal" => "Allow", - _ => panic!("unknown category {category}"), - },); - - let info_name = format_ident!("{name}_INFO"); - - (&mut category[0..1]).make_ascii_uppercase(); - let category_variant = format_ident!("{category}"); - - let name_span = name.span().unwrap(); - let location = format!("{}#L{}", name_span.source_file().path().display(), name_span.line()); - - let version = match version { - Some(version) => quote!(Some(#version)), - None => quote!(None), - }; - - let output = quote! { - rustc_session::declare_tool_lint! { - #(#attrs)* - pub clippy::#name, - #level, - #description, - report_in_external_macro: true - } - - pub(crate) static #info_name: &'static crate::LintInfo = &crate::LintInfo { - lint: &#name, - category: crate::LintCategory::#category_variant, - explanation: #explanation, - location: #location, - version: #version, - }; - }; - - TokenStream::from(output) -} diff --git a/tests/versioncheck.rs b/tests/versioncheck.rs index 68328333937..e29898f068d 100644 --- a/tests/versioncheck.rs +++ b/tests/versioncheck.rs @@ -24,7 +24,6 @@ fn consistent_clippy_crate_versions() { let clippy_version = read_version("Cargo.toml"); let paths = [ - "declare_clippy_lint/Cargo.toml", "clippy_config/Cargo.toml", "clippy_lints/Cargo.toml", "clippy_utils/Cargo.toml", From 0735031be94cd4f6a102e83e72630fae04a18090 Mon Sep 17 00:00:00 2001 From: blyxyas Date: Wed, 25 Sep 2024 13:42:21 +0200 Subject: [PATCH 09/87] Move tag removing to --explain --- clippy_lints/src/lib.rs | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index efc9fe6f315..e927d9a2d5b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -524,8 +524,28 @@ impl LintInfo { pub fn explain(name: &str) -> i32 { let target = format!("clippy::{}", name.to_ascii_uppercase()); + if let Some(info) = declared_lints::LINTS.iter().find(|info| info.lint.name == target) { - println!("{}", info.explanation); + // Remove tags and hidden code: + let mut explanation = String::with_capacity(128); + let mut in_code = false; + for line in info.explanation.lines().map(|line| line.trim()) { + if let Some(lang) = line.strip_prefix("```") { + let tag = lang.split_once(',').map_or(lang, |(left, _)| left); + if !in_code && matches!(tag, "" | "rust" | "ignore" | "should_panic" | "no_run" | "compile_fail") { + explanation += "```rust\n"; + } else { + explanation += line; + explanation.push('\n'); + } + in_code = !in_code; + } else if !(in_code && line.starts_with("# ")) { + explanation += line; + explanation.push('\n'); + } + } + + println!("{}", explanation); // Check if the lint has configuration let mut mdconf = get_configuration_metadata(); let name = name.to_ascii_lowercase(); From daf730caedb638696ee704e22942617cf5848e5f Mon Sep 17 00:00:00 2001 From: blyxyas Date: Thu, 26 Sep 2024 00:23:38 +0200 Subject: [PATCH 10/87] Add `sanitize_explanation` --- clippy_lints/src/lib.rs | 45 +++++++++++++++++++++++------------------ tests/compile-test.rs | 7 ++++++- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index e927d9a2d5b..0bfbb92dfda 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -522,30 +522,35 @@ impl LintInfo { } } +// Remove code tags and code behind '# 's, as they are not needed for the lint docs and --explain +pub fn sanitize_explanation(raw_docs: &str) -> String { + // Remove tags and hidden code: + let mut explanation = String::with_capacity(128); + let mut in_code = false; + for line in raw_docs.lines().map(|line| line.trim()) { + if let Some(lang) = line.strip_prefix("```") { + let tag = lang.split_once(',').map_or(lang, |(left, _)| left); + if !in_code && matches!(tag, "" | "rust" | "ignore" | "should_panic" | "no_run" | "compile_fail") { + explanation += "```rust\n"; + } else { + explanation += line; + explanation.push('\n'); + } + in_code = !in_code; + } else if !(in_code && line.starts_with("# ")) { + explanation += line; + explanation.push('\n'); + } + } + + explanation +} + pub fn explain(name: &str) -> i32 { let target = format!("clippy::{}", name.to_ascii_uppercase()); if let Some(info) = declared_lints::LINTS.iter().find(|info| info.lint.name == target) { - // Remove tags and hidden code: - let mut explanation = String::with_capacity(128); - let mut in_code = false; - for line in info.explanation.lines().map(|line| line.trim()) { - if let Some(lang) = line.strip_prefix("```") { - let tag = lang.split_once(',').map_or(lang, |(left, _)| left); - if !in_code && matches!(tag, "" | "rust" | "ignore" | "should_panic" | "no_run" | "compile_fail") { - explanation += "```rust\n"; - } else { - explanation += line; - explanation.push('\n'); - } - in_code = !in_code; - } else if !(in_code && line.starts_with("# ")) { - explanation += line; - explanation.push('\n'); - } - } - - println!("{}", explanation); + println!("{}", sanitize_explanation(info.explanation)); // Check if the lint has configuration let mut mdconf = get_configuration_metadata(); let name = name.to_ascii_lowercase(); diff --git a/tests/compile-test.rs b/tests/compile-test.rs index af2aa519257..23dd41235bd 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -5,9 +5,9 @@ use cargo_metadata::Message; use cargo_metadata::diagnostic::{Applicability, Diagnostic}; use clippy_config::ClippyConfiguration; -use clippy_lints::LintInfo; use clippy_lints::declared_lints::LINTS; use clippy_lints::deprecated_lints::{DEPRECATED, DEPRECATED_VERSION, RENAMED}; +use clippy_lints::{LintInfo, sanitize_explanation}; use serde::{Deserialize, Serialize}; use test_utils::IS_RUSTC_TEST_SUITE; use ui_test::custom_flags::Flag; @@ -444,7 +444,12 @@ impl DiagnosticCollector { iter::zip(DEPRECATED, DEPRECATED_VERSION) .map(|((lint, reason), version)| LintMetadata::new_deprecated(lint, reason, version)), ) + .map(|mut metadata| { + metadata.docs = sanitize_explanation(&metadata.docs); + metadata + }) .collect(); + metadata.sort_unstable_by(|a, b| a.id.cmp(&b.id)); let json = serde_json::to_string_pretty(&metadata).unwrap(); From d232d094b8e2776a6a1e88f478442fd98e2e70b4 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Wed, 18 Sep 2024 10:34:27 -0700 Subject: [PATCH 11/87] Remove the `control_flow_enum` feature from clippy --- clippy_lints/src/lib.rs | 1 - clippy_utils/src/lib.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1d41f568f37..54c3758459c 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1,7 +1,6 @@ #![feature(array_windows)] #![feature(binary_heap_into_iter_sorted)] #![feature(box_patterns)] -#![feature(control_flow_enum)] #![feature(f128)] #![feature(f16)] #![feature(if_let_guard)] diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 6dbc3334157..6c0413c1808 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1,6 +1,5 @@ #![feature(array_chunks)] #![feature(box_patterns)] -#![feature(control_flow_enum)] #![feature(f128)] #![feature(f16)] #![feature(if_let_guard)] From c8725f70e77a08974a1d51bd360345a2ab7fabc6 Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Thu, 26 Sep 2024 17:44:59 +0200 Subject: [PATCH 12/87] add proc macro check to needless_return --- clippy_lints/src/returns.rs | 2 +- tests/ui/needless_return.fixed | 8 ++ tests/ui/needless_return.rs | 8 ++ tests/ui/needless_return.stderr | 128 ++++++++++++++++---------------- 4 files changed, 81 insertions(+), 65 deletions(-) diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index 3754fdddedf..6754912f765 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -407,7 +407,7 @@ fn check_final_expr<'tcx>( } } - if ret_span.from_expansion() { + if ret_span.from_expansion() || is_from_proc_macro(cx, expr) { return; } diff --git a/tests/ui/needless_return.fixed b/tests/ui/needless_return.fixed index c5c570690b4..1507bc50a75 100644 --- a/tests/ui/needless_return.fixed +++ b/tests/ui/needless_return.fixed @@ -1,3 +1,4 @@ +//@aux-build:proc_macros.rs #![feature(yeet_expr)] #![allow(unused)] #![allow( @@ -9,6 +10,9 @@ )] #![warn(clippy::needless_return)] +extern crate proc_macros; +use proc_macros::with_span; + use std::cell::RefCell; macro_rules! the_answer { @@ -359,4 +363,8 @@ fn issue12907() -> String { "".split("").next().unwrap().to_string() } +fn issue13458() { + with_span!(span return); +} + fn main() {} diff --git a/tests/ui/needless_return.rs b/tests/ui/needless_return.rs index 738611391df..c59bfced6d6 100644 --- a/tests/ui/needless_return.rs +++ b/tests/ui/needless_return.rs @@ -1,3 +1,4 @@ +//@aux-build:proc_macros.rs #![feature(yeet_expr)] #![allow(unused)] #![allow( @@ -9,6 +10,9 @@ )] #![warn(clippy::needless_return)] +extern crate proc_macros; +use proc_macros::with_span; + use std::cell::RefCell; macro_rules! the_answer { @@ -369,4 +373,8 @@ fn issue12907() -> String { return "".split("").next().unwrap().to_string(); } +fn issue13458() { + with_span!(span return); +} + fn main() {} diff --git a/tests/ui/needless_return.stderr b/tests/ui/needless_return.stderr index da0fa220d8c..d3c2a6badc0 100644 --- a/tests/ui/needless_return.stderr +++ b/tests/ui/needless_return.stderr @@ -1,5 +1,5 @@ error: unneeded `return` statement - --> tests/ui/needless_return.rs:25:5 + --> tests/ui/needless_return.rs:29:5 | LL | return true; | ^^^^^^^^^^^ @@ -13,7 +13,7 @@ LL + true | error: unneeded `return` statement - --> tests/ui/needless_return.rs:29:5 + --> tests/ui/needless_return.rs:33:5 | LL | return true; | ^^^^^^^^^^^ @@ -25,7 +25,7 @@ LL + true | error: unneeded `return` statement - --> tests/ui/needless_return.rs:34:5 + --> tests/ui/needless_return.rs:38:5 | LL | return true;;; | ^^^^^^^^^^^ @@ -37,7 +37,7 @@ LL + true | error: unneeded `return` statement - --> tests/ui/needless_return.rs:39:5 + --> tests/ui/needless_return.rs:43:5 | LL | return true;; ; ; | ^^^^^^^^^^^ @@ -49,7 +49,7 @@ LL + true | error: unneeded `return` statement - --> tests/ui/needless_return.rs:44:9 + --> tests/ui/needless_return.rs:48:9 | LL | return true; | ^^^^^^^^^^^ @@ -61,7 +61,7 @@ LL + true | error: unneeded `return` statement - --> tests/ui/needless_return.rs:46:9 + --> tests/ui/needless_return.rs:50:9 | LL | return false; | ^^^^^^^^^^^^ @@ -73,7 +73,7 @@ LL + false | error: unneeded `return` statement - --> tests/ui/needless_return.rs:52:17 + --> tests/ui/needless_return.rs:56:17 | LL | true => return false, | ^^^^^^^^^^^^ @@ -84,7 +84,7 @@ LL | true => false, | ~~~~~ error: unneeded `return` statement - --> tests/ui/needless_return.rs:54:13 + --> tests/ui/needless_return.rs:58:13 | LL | return true; | ^^^^^^^^^^^ @@ -96,7 +96,7 @@ LL + true | error: unneeded `return` statement - --> tests/ui/needless_return.rs:61:9 + --> tests/ui/needless_return.rs:65:9 | LL | return true; | ^^^^^^^^^^^ @@ -108,7 +108,7 @@ LL + true | error: unneeded `return` statement - --> tests/ui/needless_return.rs:63:16 + --> tests/ui/needless_return.rs:67:16 | LL | let _ = || return true; | ^^^^^^^^^^^ @@ -119,7 +119,7 @@ LL | let _ = || true; | ~~~~ error: unneeded `return` statement - --> tests/ui/needless_return.rs:67:5 + --> tests/ui/needless_return.rs:71:5 | LL | return the_answer!(); | ^^^^^^^^^^^^^^^^^^^^ @@ -131,7 +131,7 @@ LL + the_answer!() | error: unneeded `return` statement - --> tests/ui/needless_return.rs:70:21 + --> tests/ui/needless_return.rs:74:21 | LL | fn test_void_fun() { | _____________________^ @@ -146,7 +146,7 @@ LL + fn test_void_fun() { | error: unneeded `return` statement - --> tests/ui/needless_return.rs:75:11 + --> tests/ui/needless_return.rs:79:11 | LL | if b { | ___________^ @@ -161,7 +161,7 @@ LL + if b { | error: unneeded `return` statement - --> tests/ui/needless_return.rs:77:13 + --> tests/ui/needless_return.rs:81:13 | LL | } else { | _____________^ @@ -176,7 +176,7 @@ LL + } else { | error: unneeded `return` statement - --> tests/ui/needless_return.rs:85:14 + --> tests/ui/needless_return.rs:89:14 | LL | _ => return, | ^^^^^^ @@ -187,7 +187,7 @@ LL | _ => (), | ~~ error: unneeded `return` statement - --> tests/ui/needless_return.rs:93:24 + --> tests/ui/needless_return.rs:97:24 | LL | let _ = 42; | ________________________^ @@ -202,7 +202,7 @@ LL + let _ = 42; | error: unneeded `return` statement - --> tests/ui/needless_return.rs:96:14 + --> tests/ui/needless_return.rs:100:14 | LL | _ => return, | ^^^^^^ @@ -213,7 +213,7 @@ LL | _ => (), | ~~ error: unneeded `return` statement - --> tests/ui/needless_return.rs:109:9 + --> tests/ui/needless_return.rs:113:9 | LL | return String::from("test"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -225,7 +225,7 @@ LL + String::from("test") | error: unneeded `return` statement - --> tests/ui/needless_return.rs:111:9 + --> tests/ui/needless_return.rs:115:9 | LL | return String::new(); | ^^^^^^^^^^^^^^^^^^^^ @@ -237,7 +237,7 @@ LL + String::new() | error: unneeded `return` statement - --> tests/ui/needless_return.rs:133:32 + --> tests/ui/needless_return.rs:137:32 | LL | bar.unwrap_or_else(|_| return) | ^^^^^^ @@ -248,7 +248,7 @@ LL | bar.unwrap_or_else(|_| {}) | ~~ error: unneeded `return` statement - --> tests/ui/needless_return.rs:137:21 + --> tests/ui/needless_return.rs:141:21 | LL | let _ = || { | _____________________^ @@ -263,7 +263,7 @@ LL + let _ = || { | error: unneeded `return` statement - --> tests/ui/needless_return.rs:140:20 + --> tests/ui/needless_return.rs:144:20 | LL | let _ = || return; | ^^^^^^ @@ -274,7 +274,7 @@ LL | let _ = || {}; | ~~ error: unneeded `return` statement - --> tests/ui/needless_return.rs:146:32 + --> tests/ui/needless_return.rs:150:32 | LL | res.unwrap_or_else(|_| return Foo) | ^^^^^^^^^^ @@ -284,18 +284,6 @@ help: remove `return` LL | res.unwrap_or_else(|_| Foo) | ~~~ -error: unneeded `return` statement - --> tests/ui/needless_return.rs:155:5 - | -LL | return true; - | ^^^^^^^^^^^ - | -help: remove `return` - | -LL - return true; -LL + true - | - error: unneeded `return` statement --> tests/ui/needless_return.rs:159:5 | @@ -309,7 +297,19 @@ LL + true | error: unneeded `return` statement - --> tests/ui/needless_return.rs:164:9 + --> tests/ui/needless_return.rs:163:5 + | +LL | return true; + | ^^^^^^^^^^^ + | +help: remove `return` + | +LL - return true; +LL + true + | + +error: unneeded `return` statement + --> tests/ui/needless_return.rs:168:9 | LL | return true; | ^^^^^^^^^^^ @@ -321,7 +321,7 @@ LL + true | error: unneeded `return` statement - --> tests/ui/needless_return.rs:166:9 + --> tests/ui/needless_return.rs:170:9 | LL | return false; | ^^^^^^^^^^^^ @@ -333,7 +333,7 @@ LL + false | error: unneeded `return` statement - --> tests/ui/needless_return.rs:172:17 + --> tests/ui/needless_return.rs:176:17 | LL | true => return false, | ^^^^^^^^^^^^ @@ -344,7 +344,7 @@ LL | true => false, | ~~~~~ error: unneeded `return` statement - --> tests/ui/needless_return.rs:174:13 + --> tests/ui/needless_return.rs:178:13 | LL | return true; | ^^^^^^^^^^^ @@ -356,7 +356,7 @@ LL + true | error: unneeded `return` statement - --> tests/ui/needless_return.rs:181:9 + --> tests/ui/needless_return.rs:185:9 | LL | return true; | ^^^^^^^^^^^ @@ -368,7 +368,7 @@ LL + true | error: unneeded `return` statement - --> tests/ui/needless_return.rs:183:16 + --> tests/ui/needless_return.rs:187:16 | LL | let _ = || return true; | ^^^^^^^^^^^ @@ -379,7 +379,7 @@ LL | let _ = || true; | ~~~~ error: unneeded `return` statement - --> tests/ui/needless_return.rs:187:5 + --> tests/ui/needless_return.rs:191:5 | LL | return the_answer!(); | ^^^^^^^^^^^^^^^^^^^^ @@ -391,7 +391,7 @@ LL + the_answer!() | error: unneeded `return` statement - --> tests/ui/needless_return.rs:190:33 + --> tests/ui/needless_return.rs:194:33 | LL | async fn async_test_void_fun() { | _________________________________^ @@ -406,7 +406,7 @@ LL + async fn async_test_void_fun() { | error: unneeded `return` statement - --> tests/ui/needless_return.rs:195:11 + --> tests/ui/needless_return.rs:199:11 | LL | if b { | ___________^ @@ -421,7 +421,7 @@ LL + if b { | error: unneeded `return` statement - --> tests/ui/needless_return.rs:197:13 + --> tests/ui/needless_return.rs:201:13 | LL | } else { | _____________^ @@ -436,7 +436,7 @@ LL + } else { | error: unneeded `return` statement - --> tests/ui/needless_return.rs:205:14 + --> tests/ui/needless_return.rs:209:14 | LL | _ => return, | ^^^^^^ @@ -447,7 +447,7 @@ LL | _ => (), | ~~ error: unneeded `return` statement - --> tests/ui/needless_return.rs:218:9 + --> tests/ui/needless_return.rs:222:9 | LL | return String::from("test"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -459,7 +459,7 @@ LL + String::from("test") | error: unneeded `return` statement - --> tests/ui/needless_return.rs:220:9 + --> tests/ui/needless_return.rs:224:9 | LL | return String::new(); | ^^^^^^^^^^^^^^^^^^^^ @@ -471,7 +471,7 @@ LL + String::new() | error: unneeded `return` statement - --> tests/ui/needless_return.rs:236:5 + --> tests/ui/needless_return.rs:240:5 | LL | return format!("Hello {}", "world!"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -483,7 +483,7 @@ LL + format!("Hello {}", "world!") | error: unneeded `return` statement - --> tests/ui/needless_return.rs:277:9 + --> tests/ui/needless_return.rs:281:9 | LL | return true; | ^^^^^^^^^^^ @@ -497,7 +497,7 @@ LL ~ } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:279:9 + --> tests/ui/needless_return.rs:283:9 | LL | return false; | ^^^^^^^^^^^^ @@ -509,7 +509,7 @@ LL ~ } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:286:13 + --> tests/ui/needless_return.rs:290:13 | LL | return 10; | ^^^^^^^^^ @@ -524,7 +524,7 @@ LL ~ } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:289:13 + --> tests/ui/needless_return.rs:293:13 | LL | return 100; | ^^^^^^^^^^ @@ -537,7 +537,7 @@ LL ~ } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:297:9 + --> tests/ui/needless_return.rs:301:9 | LL | return 0; | ^^^^^^^^ @@ -549,7 +549,7 @@ LL ~ } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:304:13 + --> tests/ui/needless_return.rs:308:13 | LL | return *(x as *const isize); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -564,7 +564,7 @@ LL ~ } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:306:13 + --> tests/ui/needless_return.rs:310:13 | LL | return !*(x as *const isize); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -577,7 +577,7 @@ LL ~ } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:313:20 + --> tests/ui/needless_return.rs:317:20 | LL | let _ = 42; | ____________________^ @@ -594,7 +594,7 @@ LL + let _ = 42; | error: unneeded `return` statement - --> tests/ui/needless_return.rs:320:20 + --> tests/ui/needless_return.rs:324:20 | LL | let _ = 42; return; | ^^^^^^^ @@ -606,7 +606,7 @@ LL + let _ = 42; | error: unneeded `return` statement - --> tests/ui/needless_return.rs:332:9 + --> tests/ui/needless_return.rs:336:9 | LL | return Ok(format!("ok!")); | ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -618,7 +618,7 @@ LL + Ok(format!("ok!")) | error: unneeded `return` statement - --> tests/ui/needless_return.rs:334:9 + --> tests/ui/needless_return.rs:338:9 | LL | return Err(format!("err!")); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -630,7 +630,7 @@ LL + Err(format!("err!")) | error: unneeded `return` statement - --> tests/ui/needless_return.rs:340:9 + --> tests/ui/needless_return.rs:344:9 | LL | return if true { 1 } else { 2 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -642,7 +642,7 @@ LL + if true { 1 } else { 2 } | error: unneeded `return` statement - --> tests/ui/needless_return.rs:344:9 + --> tests/ui/needless_return.rs:348:9 | LL | return if b1 { 0 } else { 1 } | if b2 { 2 } else { 3 } | if b3 { 4 } else { 5 }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -654,7 +654,7 @@ LL + (if b1 { 0 } else { 1 } | if b2 { 2 } else { 3 } | if b3 { 4 } else | error: unneeded `return` statement - --> tests/ui/needless_return.rs:365:5 + --> tests/ui/needless_return.rs:369:5 | LL | return { "a".to_string() } + "b" + { "c" }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -666,7 +666,7 @@ LL + ({ "a".to_string() } + "b" + { "c" }) | error: unneeded `return` statement - --> tests/ui/needless_return.rs:369:5 + --> tests/ui/needless_return.rs:373:5 | LL | return "".split("").next().unwrap().to_string(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 55834a362c1e849bbab9cb0adc641a22be598d7e Mon Sep 17 00:00:00 2001 From: y21 <30553356+y21@users.noreply.github.com> Date: Thu, 26 Sep 2024 17:49:14 +0200 Subject: [PATCH 13/87] deal with differing syntax contexts for subexpressions in check_proc_macro --- clippy_utils/src/check_proc_macro.rs | 135 ++++++++++++++++----------- tests/ui/dbg_macro/dbg_macro.fixed | 2 +- tests/ui/dbg_macro/dbg_macro.rs | 2 +- 3 files changed, 83 insertions(+), 56 deletions(-) diff --git a/clippy_utils/src/check_proc_macro.rs b/clippy_utils/src/check_proc_macro.rs index 9143d292f67..9a18bff3c7a 100644 --- a/clippy_utils/src/check_proc_macro.rs +++ b/clippy_utils/src/check_proc_macro.rs @@ -141,62 +141,89 @@ fn path_search_pat(path: &Path<'_>) -> (Pat, Pat) { /// Get the search patterns to use for the given expression fn expr_search_pat(tcx: TyCtxt<'_>, e: &Expr<'_>) -> (Pat, Pat) { - match e.kind { - ExprKind::ConstBlock(_) => (Pat::Str("const"), Pat::Str("}")), - // Parenthesis are trimmed from the text before the search patterns are matched. - // See: `span_matches_pat` - ExprKind::Tup([]) => (Pat::Str(")"), Pat::Str("(")), - ExprKind::Unary(UnOp::Deref, e) => (Pat::Str("*"), expr_search_pat(tcx, e).1), - ExprKind::Unary(UnOp::Not, e) => (Pat::Str("!"), expr_search_pat(tcx, e).1), - ExprKind::Unary(UnOp::Neg, e) => (Pat::Str("-"), expr_search_pat(tcx, e).1), - ExprKind::Lit(lit) => lit_search_pat(&lit.node), - ExprKind::Array(_) | ExprKind::Repeat(..) => (Pat::Str("["), Pat::Str("]")), - ExprKind::Call(e, []) | ExprKind::MethodCall(_, e, [], _) => (expr_search_pat(tcx, e).0, Pat::Str("(")), - ExprKind::Call(first, [.., last]) - | ExprKind::MethodCall(_, first, [.., last], _) - | ExprKind::Binary(_, first, last) - | ExprKind::Tup([first, .., last]) - | ExprKind::Assign(first, last, _) - | ExprKind::AssignOp(_, first, last) => (expr_search_pat(tcx, first).0, expr_search_pat(tcx, last).1), - ExprKind::Tup([e]) | ExprKind::DropTemps(e) => expr_search_pat(tcx, e), - ExprKind::Cast(e, _) | ExprKind::Type(e, _) => (expr_search_pat(tcx, e).0, Pat::Str("")), - ExprKind::Let(let_expr) => (Pat::Str("let"), expr_search_pat(tcx, let_expr.init).1), - ExprKind::If(..) => (Pat::Str("if"), Pat::Str("}")), - ExprKind::Loop(_, Some(_), _, _) | ExprKind::Block(_, Some(_)) => (Pat::Str("'"), Pat::Str("}")), - ExprKind::Loop(_, None, LoopSource::Loop, _) => (Pat::Str("loop"), Pat::Str("}")), - ExprKind::Loop(_, None, LoopSource::While, _) => (Pat::Str("while"), Pat::Str("}")), - ExprKind::Loop(_, None, LoopSource::ForLoop, _) | ExprKind::Match(_, _, MatchSource::ForLoopDesugar) => { - (Pat::Str("for"), Pat::Str("}")) - }, - ExprKind::Match(_, _, MatchSource::Normal) => (Pat::Str("match"), Pat::Str("}")), - ExprKind::Match(e, _, MatchSource::TryDesugar(_)) => (expr_search_pat(tcx, e).0, Pat::Str("?")), - ExprKind::Match(e, _, MatchSource::AwaitDesugar) | ExprKind::Yield(e, YieldSource::Await { .. }) => { - (expr_search_pat(tcx, e).0, Pat::Str("await")) - }, - ExprKind::Closure(&Closure { body, .. }) => (Pat::Str(""), expr_search_pat(tcx, tcx.hir().body(body).value).1), - ExprKind::Block( - Block { - rules: BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided), - .. + fn expr_search_pat_inner(tcx: TyCtxt<'_>, e: &Expr<'_>, outer_span: Span) -> (Pat, Pat) { + // The expression can have subexpressions in different contexts, in which case + // building up a search pattern from the macro expansion would lead to false positives; + // e.g. `return format!(..)` would be considered to be from a proc macro + // if we build up a pattern for the macro expansion and compare it to the invocation `format!()`. + // So instead we return an empty pattern such that `span_matches_pat` always returns true. + if !e.span.eq_ctxt(outer_span) { + return (Pat::Str(""), Pat::Str("")); + } + + match e.kind { + ExprKind::ConstBlock(_) => (Pat::Str("const"), Pat::Str("}")), + // Parenthesis are trimmed from the text before the search patterns are matched. + // See: `span_matches_pat` + ExprKind::Tup([]) => (Pat::Str(")"), Pat::Str("(")), + ExprKind::Unary(UnOp::Deref, e) => (Pat::Str("*"), expr_search_pat_inner(tcx, e, outer_span).1), + ExprKind::Unary(UnOp::Not, e) => (Pat::Str("!"), expr_search_pat_inner(tcx, e, outer_span).1), + ExprKind::Unary(UnOp::Neg, e) => (Pat::Str("-"), expr_search_pat_inner(tcx, e, outer_span).1), + ExprKind::Lit(lit) => lit_search_pat(&lit.node), + ExprKind::Array(_) | ExprKind::Repeat(..) => (Pat::Str("["), Pat::Str("]")), + ExprKind::Call(e, []) | ExprKind::MethodCall(_, e, [], _) => { + (expr_search_pat_inner(tcx, e, outer_span).0, Pat::Str("(")) }, - None, - ) => (Pat::Str("unsafe"), Pat::Str("}")), - ExprKind::Block(_, None) => (Pat::Str("{"), Pat::Str("}")), - ExprKind::Field(e, name) => (expr_search_pat(tcx, e).0, Pat::Sym(name.name)), - ExprKind::Index(e, _, _) => (expr_search_pat(tcx, e).0, Pat::Str("]")), - ExprKind::Path(ref path) => qpath_search_pat(path), - ExprKind::AddrOf(_, _, e) => (Pat::Str("&"), expr_search_pat(tcx, e).1), - ExprKind::Break(Destination { label: None, .. }, None) => (Pat::Str("break"), Pat::Str("break")), - ExprKind::Break(Destination { label: Some(name), .. }, None) => (Pat::Str("break"), Pat::Sym(name.ident.name)), - ExprKind::Break(_, Some(e)) => (Pat::Str("break"), expr_search_pat(tcx, e).1), - ExprKind::Continue(Destination { label: None, .. }) => (Pat::Str("continue"), Pat::Str("continue")), - ExprKind::Continue(Destination { label: Some(name), .. }) => (Pat::Str("continue"), Pat::Sym(name.ident.name)), - ExprKind::Ret(None) => (Pat::Str("return"), Pat::Str("return")), - ExprKind::Ret(Some(e)) => (Pat::Str("return"), expr_search_pat(tcx, e).1), - ExprKind::Struct(path, _, _) => (qpath_search_pat(path).0, Pat::Str("}")), - ExprKind::Yield(e, YieldSource::Yield) => (Pat::Str("yield"), expr_search_pat(tcx, e).1), - _ => (Pat::Str(""), Pat::Str("")), + ExprKind::Call(first, [.., last]) + | ExprKind::MethodCall(_, first, [.., last], _) + | ExprKind::Binary(_, first, last) + | ExprKind::Tup([first, .., last]) + | ExprKind::Assign(first, last, _) + | ExprKind::AssignOp(_, first, last) => ( + expr_search_pat_inner(tcx, first, outer_span).0, + expr_search_pat_inner(tcx, last, outer_span).1, + ), + ExprKind::Tup([e]) | ExprKind::DropTemps(e) => expr_search_pat_inner(tcx, e, outer_span), + ExprKind::Cast(e, _) | ExprKind::Type(e, _) => (expr_search_pat_inner(tcx, e, outer_span).0, Pat::Str("")), + ExprKind::Let(let_expr) => (Pat::Str("let"), expr_search_pat_inner(tcx, let_expr.init, outer_span).1), + ExprKind::If(..) => (Pat::Str("if"), Pat::Str("}")), + ExprKind::Loop(_, Some(_), _, _) | ExprKind::Block(_, Some(_)) => (Pat::Str("'"), Pat::Str("}")), + ExprKind::Loop(_, None, LoopSource::Loop, _) => (Pat::Str("loop"), Pat::Str("}")), + ExprKind::Loop(_, None, LoopSource::While, _) => (Pat::Str("while"), Pat::Str("}")), + ExprKind::Loop(_, None, LoopSource::ForLoop, _) | ExprKind::Match(_, _, MatchSource::ForLoopDesugar) => { + (Pat::Str("for"), Pat::Str("}")) + }, + ExprKind::Match(_, _, MatchSource::Normal) => (Pat::Str("match"), Pat::Str("}")), + ExprKind::Match(e, _, MatchSource::TryDesugar(_)) => { + (expr_search_pat_inner(tcx, e, outer_span).0, Pat::Str("?")) + }, + ExprKind::Match(e, _, MatchSource::AwaitDesugar) | ExprKind::Yield(e, YieldSource::Await { .. }) => { + (expr_search_pat_inner(tcx, e, outer_span).0, Pat::Str("await")) + }, + ExprKind::Closure(&Closure { body, .. }) => ( + Pat::Str(""), + expr_search_pat_inner(tcx, tcx.hir().body(body).value, outer_span).1, + ), + ExprKind::Block( + Block { + rules: BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided), + .. + }, + None, + ) => (Pat::Str("unsafe"), Pat::Str("}")), + ExprKind::Block(_, None) => (Pat::Str("{"), Pat::Str("}")), + ExprKind::Field(e, name) => (expr_search_pat_inner(tcx, e, outer_span).0, Pat::Sym(name.name)), + ExprKind::Index(e, _, _) => (expr_search_pat_inner(tcx, e, outer_span).0, Pat::Str("]")), + ExprKind::Path(ref path) => qpath_search_pat(path), + ExprKind::AddrOf(_, _, e) => (Pat::Str("&"), expr_search_pat_inner(tcx, e, outer_span).1), + ExprKind::Break(Destination { label: None, .. }, None) => (Pat::Str("break"), Pat::Str("break")), + ExprKind::Break(Destination { label: Some(name), .. }, None) => { + (Pat::Str("break"), Pat::Sym(name.ident.name)) + }, + ExprKind::Break(_, Some(e)) => (Pat::Str("break"), expr_search_pat_inner(tcx, e, outer_span).1), + ExprKind::Continue(Destination { label: None, .. }) => (Pat::Str("continue"), Pat::Str("continue")), + ExprKind::Continue(Destination { label: Some(name), .. }) => { + (Pat::Str("continue"), Pat::Sym(name.ident.name)) + }, + ExprKind::Ret(None) => (Pat::Str("return"), Pat::Str("return")), + ExprKind::Ret(Some(e)) => (Pat::Str("return"), expr_search_pat_inner(tcx, e, outer_span).1), + ExprKind::Struct(path, _, _) => (qpath_search_pat(path).0, Pat::Str("}")), + ExprKind::Yield(e, YieldSource::Yield) => (Pat::Str("yield"), expr_search_pat_inner(tcx, e, outer_span).1), + _ => (Pat::Str(""), Pat::Str("")), + } } + + expr_search_pat_inner(tcx, e, e.span) } fn fn_header_search_pat(header: FnHeader) -> Pat { diff --git a/tests/ui/dbg_macro/dbg_macro.fixed b/tests/ui/dbg_macro/dbg_macro.fixed index e3525191423..bda9221a5e1 100644 --- a/tests/ui/dbg_macro/dbg_macro.fixed +++ b/tests/ui/dbg_macro/dbg_macro.fixed @@ -1,5 +1,5 @@ #![warn(clippy::dbg_macro)] -#![allow(clippy::unnecessary_operation, clippy::no_effect)] +#![allow(clippy::unnecessary_operation, clippy::no_effect, clippy::unit_arg)] fn foo(n: u32) -> u32 { if let Some(n) = n.checked_sub(4) { n } else { n } diff --git a/tests/ui/dbg_macro/dbg_macro.rs b/tests/ui/dbg_macro/dbg_macro.rs index 80606c2db05..8244254026b 100644 --- a/tests/ui/dbg_macro/dbg_macro.rs +++ b/tests/ui/dbg_macro/dbg_macro.rs @@ -1,5 +1,5 @@ #![warn(clippy::dbg_macro)] -#![allow(clippy::unnecessary_operation, clippy::no_effect)] +#![allow(clippy::unnecessary_operation, clippy::no_effect, clippy::unit_arg)] fn foo(n: u32) -> u32 { if let Some(n) = dbg!(n.checked_sub(4)) { n } else { n } From d1dec199eea9678b0b3d728100ff356b0cd6f216 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Mon, 2 Sep 2024 02:20:35 -0400 Subject: [PATCH 14/87] Add manual_ignore_cast_cmp lint --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/manual_ignore_case_cmp.rs | 127 +++++ tests/ui/manual_ignore_case_cmp.fixed | 107 ++++ tests/ui/manual_ignore_case_cmp.rs | 107 ++++ tests/ui/manual_ignore_case_cmp.stderr | 546 +++++++++++++++++++++ 7 files changed, 891 insertions(+) create mode 100644 clippy_lints/src/manual_ignore_case_cmp.rs create mode 100644 tests/ui/manual_ignore_case_cmp.fixed create mode 100644 tests/ui/manual_ignore_case_cmp.rs create mode 100644 tests/ui/manual_ignore_case_cmp.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d253d52531..98be47ec5a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5621,6 +5621,7 @@ Released 2018-09-13 [`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map [`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten [`manual_hash_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one +[`manual_ignore_case_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ignore_case_cmp [`manual_inspect`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_inspect [`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed [`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 9cec672beb0..0b37dffd732 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -306,6 +306,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::manual_float_methods::MANUAL_IS_FINITE_INFO, crate::manual_float_methods::MANUAL_IS_INFINITE_INFO, crate::manual_hash_one::MANUAL_HASH_ONE_INFO, + crate::manual_ignore_case_cmp::MANUAL_IGNORE_CASE_CMP_INFO, crate::manual_is_ascii_check::MANUAL_IS_ASCII_CHECK_INFO, crate::manual_is_power_of_two::MANUAL_IS_POWER_OF_TWO_INFO, crate::manual_let_else::MANUAL_LET_ELSE_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1d41f568f37..56722e7f537 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -206,6 +206,7 @@ mod manual_clamp; mod manual_div_ceil; mod manual_float_methods; mod manual_hash_one; +mod manual_ignore_case_cmp; mod manual_is_ascii_check; mod manual_is_power_of_two; mod manual_let_else; @@ -944,5 +945,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo)); store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions)); store.register_late_pass(move |_| Box::new(unused_trait_names::UnusedTraitNames::new(conf))); + store.register_late_pass(|_| Box::new(manual_ignore_case_cmp::ManualIgnoreCaseCmp)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/manual_ignore_case_cmp.rs b/clippy_lints/src/manual_ignore_case_cmp.rs new file mode 100644 index 00000000000..dabfac3f613 --- /dev/null +++ b/clippy_lints/src/manual_ignore_case_cmp.rs @@ -0,0 +1,127 @@ +use crate::manual_ignore_case_cmp::MatchType::{Literal, ToAscii}; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::snippet_with_applicability; +use clippy_utils::ty::{get_type_diagnostic_name, is_type_diagnostic_item, is_type_lang_item}; +use rustc_ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir::ExprKind::{Binary, Lit, MethodCall}; +use rustc_hir::{BinOpKind, Expr, LangItem}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty; +use rustc_middle::ty::{Ty, UintTy}; +use rustc_session::declare_lint_pass; +use rustc_span::{Span, sym}; + +declare_clippy_lint! { + /// ### What it does + /// Checks for manual case-insensitive ASCII comparison. + /// + /// ### Why is this bad? + /// The `eq_ignore_ascii_case` method is faster because it does not allocate + /// memory for the new strings, and it is more readable. + /// + /// ### Example + /// ```no_run + /// fn compare(a: &str, b: &str) -> bool { + /// a.to_ascii_lowercase() == b.to_ascii_lowercase() || a.to_ascii_lowercase() == "abc" + /// } + /// ``` + /// Use instead: + /// ```no_run + /// fn compare(a: &str, b: &str) -> bool { + /// a.eq_ignore_ascii_case(b) || a.eq_ignore_ascii_case("abc") + /// } + /// ``` + #[clippy::version = "1.82.0"] + pub MANUAL_IGNORE_CASE_CMP, + perf, + "manual case-insensitive ASCII comparison" +} + +declare_lint_pass!(ManualIgnoreCaseCmp => [MANUAL_IGNORE_CASE_CMP]); + +enum MatchType<'a, 'b> { + ToAscii(bool, Ty<'a>), + Literal(&'b LitKind), +} + +fn get_ascii_type<'a, 'b>(cx: &LateContext<'a>, kind: rustc_hir::ExprKind<'b>) -> Option<(Span, MatchType<'a, 'b>)> { + if let MethodCall(path, expr, _, _) = kind { + let is_lower = match path.ident.name.as_str() { + "to_ascii_lowercase" => true, + "to_ascii_uppercase" => false, + _ => return None, + }; + let ty_raw = cx.typeck_results().expr_ty(expr); + let ty = ty_raw.peel_refs(); + if needs_ref_to_cmp(cx, ty) + || ty.is_str() + || ty.is_slice() + || matches!(get_type_diagnostic_name(cx, ty), Some(sym::OsStr | sym::OsString)) + { + return Some((expr.span, ToAscii(is_lower, ty_raw))); + } + } else if let Lit(expr) = kind { + return Some((expr.span, Literal(&expr.node))); + } + None +} + +/// Returns true if the type needs to be dereferenced to be compared +fn needs_ref_to_cmp(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + ty.is_char() + || *ty.kind() == ty::Uint(UintTy::U8) + || is_type_diagnostic_item(cx, ty, sym::Vec) + || is_type_lang_item(cx, ty, LangItem::String) +} + +impl LateLintPass<'_> for ManualIgnoreCaseCmp { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) { + // check if expression represents a comparison of two strings + // using .to_ascii_lowercase() or .to_ascii_uppercase() methods, + // or one of the sides is a literal + // Offer to replace it with .eq_ignore_ascii_case() method + if let Binary(op, left, right) = &expr.kind + && (op.node == BinOpKind::Eq || op.node == BinOpKind::Ne) + && let Some((left_span, left_val)) = get_ascii_type(cx, left.kind) + && let Some((right_span, right_val)) = get_ascii_type(cx, right.kind) + && match (&left_val, &right_val) { + (ToAscii(l_lower, ..), ToAscii(r_lower, ..)) if l_lower == r_lower => true, + (ToAscii(..), Literal(..)) | (Literal(..), ToAscii(..)) => true, + _ => false, + } + { + let deref = match right_val { + ToAscii(_, ty) if needs_ref_to_cmp(cx, ty) => "&", + ToAscii(..) => "", + Literal(ty) => { + if let LitKind::Char(_) | LitKind::Byte(_) = ty { + "&" + } else { + "" + } + }, + }; + let neg = if op.node == BinOpKind::Ne { "!" } else { "" }; + span_lint_and_then( + cx, + MANUAL_IGNORE_CASE_CMP, + expr.span, + "manual case-insensitive ASCII comparison", + |diag| { + let mut app = Applicability::MachineApplicable; + diag.span_suggestion_verbose( + expr.span, + "consider using `.eq_ignore_ascii_case()` instead", + format!( + "{neg}{}.eq_ignore_ascii_case({deref}{})", + snippet_with_applicability(cx, left_span, "_", &mut app), + snippet_with_applicability(cx, right_span, "_", &mut app) + ), + app, + ); + }, + ); + } + } +} diff --git a/tests/ui/manual_ignore_case_cmp.fixed b/tests/ui/manual_ignore_case_cmp.fixed new file mode 100644 index 00000000000..53a124f59c8 --- /dev/null +++ b/tests/ui/manual_ignore_case_cmp.fixed @@ -0,0 +1,107 @@ +#![allow(clippy::all)] +#![deny(clippy::manual_ignore_case_cmp)] + +use std::ffi::{OsStr, OsString}; + +fn main() {} + +fn variants(a: &str, b: &str) { + if a.eq_ignore_ascii_case(b) { + return; + } + if a.eq_ignore_ascii_case(b) { + return; + } + let r = a.eq_ignore_ascii_case(b); + let r = r || a.eq_ignore_ascii_case(b); + r && a.eq_ignore_ascii_case(&b.to_uppercase()); + // != + if !a.eq_ignore_ascii_case(b) { + return; + } + if !a.eq_ignore_ascii_case(b) { + return; + } + let r = !a.eq_ignore_ascii_case(b); + let r = r || !a.eq_ignore_ascii_case(b); + r && !a.eq_ignore_ascii_case(&b.to_uppercase()); +} + +fn unsupported(a: char, b: char) { + // TODO:: these are rare, and might not be worth supporting + a.to_ascii_lowercase() == char::to_ascii_lowercase(&b); + char::to_ascii_lowercase(&a) == b.to_ascii_lowercase(); + char::to_ascii_lowercase(&a) == char::to_ascii_lowercase(&b); +} + +fn char(a: char, b: char) { + a.eq_ignore_ascii_case(&b); + a.to_ascii_lowercase() == *&b.to_ascii_lowercase(); + *&a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.eq_ignore_ascii_case(&'a'); + 'a'.eq_ignore_ascii_case(&b); +} +fn u8(a: u8, b: u8) { + a.eq_ignore_ascii_case(&b); + a.eq_ignore_ascii_case(&b'a'); + b'a'.eq_ignore_ascii_case(&b); +} +fn ref_str(a: &str, b: &str) { + a.eq_ignore_ascii_case(b); + a.to_uppercase().eq_ignore_ascii_case(b); + a.eq_ignore_ascii_case("a"); + "a".eq_ignore_ascii_case(b); +} +fn ref_ref_str(a: &&str, b: &&str) { + a.eq_ignore_ascii_case(b); + a.to_uppercase().eq_ignore_ascii_case(b); + a.eq_ignore_ascii_case("a"); + "a".eq_ignore_ascii_case(b); +} +fn string(a: String, b: String) { + a.eq_ignore_ascii_case(&b); + a.eq_ignore_ascii_case("a"); + "a".eq_ignore_ascii_case(&b); + &a.to_ascii_lowercase() == &b.to_ascii_lowercase(); + &&a.to_ascii_lowercase() == &&b.to_ascii_lowercase(); + a.eq_ignore_ascii_case("a"); + "a".eq_ignore_ascii_case(&b); +} +fn ref_string(a: String, b: &String) { + a.eq_ignore_ascii_case(b); + a.eq_ignore_ascii_case("a"); + "a".eq_ignore_ascii_case(b); + + b.eq_ignore_ascii_case(&a); + b.eq_ignore_ascii_case("a"); + "a".eq_ignore_ascii_case(&a); +} +fn string_ref_str(a: String, b: &str) { + a.eq_ignore_ascii_case(b); + a.eq_ignore_ascii_case("a"); + "a".eq_ignore_ascii_case(b); + + b.eq_ignore_ascii_case(&a); + b.eq_ignore_ascii_case("a"); + "a".eq_ignore_ascii_case(&a); +} +fn ref_u8slice(a: &[u8], b: &[u8]) { + a.eq_ignore_ascii_case(b); +} +fn u8vec(a: Vec, b: Vec) { + a.eq_ignore_ascii_case(&b); +} +fn ref_u8vec(a: Vec, b: &Vec) { + a.eq_ignore_ascii_case(b); + b.eq_ignore_ascii_case(&a); +} +fn ref_osstr(a: &OsStr, b: &OsStr) { + a.eq_ignore_ascii_case(b); +} +fn osstring(a: OsString, b: OsString) { + a.eq_ignore_ascii_case(b); +} +fn ref_osstring(a: OsString, b: &OsString) { + a.eq_ignore_ascii_case(b); + b.eq_ignore_ascii_case(a); +} diff --git a/tests/ui/manual_ignore_case_cmp.rs b/tests/ui/manual_ignore_case_cmp.rs new file mode 100644 index 00000000000..2a4d84b30ac --- /dev/null +++ b/tests/ui/manual_ignore_case_cmp.rs @@ -0,0 +1,107 @@ +#![allow(clippy::all)] +#![deny(clippy::manual_ignore_case_cmp)] + +use std::ffi::{OsStr, OsString}; + +fn main() {} + +fn variants(a: &str, b: &str) { + if a.to_ascii_lowercase() == b.to_ascii_lowercase() { + return; + } + if a.to_ascii_uppercase() == b.to_ascii_uppercase() { + return; + } + let r = a.to_ascii_lowercase() == b.to_ascii_lowercase(); + let r = r || a.to_ascii_uppercase() == b.to_ascii_uppercase(); + r && a.to_ascii_lowercase() == b.to_uppercase().to_ascii_lowercase(); + // != + if a.to_ascii_lowercase() != b.to_ascii_lowercase() { + return; + } + if a.to_ascii_uppercase() != b.to_ascii_uppercase() { + return; + } + let r = a.to_ascii_lowercase() != b.to_ascii_lowercase(); + let r = r || a.to_ascii_uppercase() != b.to_ascii_uppercase(); + r && a.to_ascii_lowercase() != b.to_uppercase().to_ascii_lowercase(); +} + +fn unsupported(a: char, b: char) { + // TODO:: these are rare, and might not be worth supporting + a.to_ascii_lowercase() == char::to_ascii_lowercase(&b); + char::to_ascii_lowercase(&a) == b.to_ascii_lowercase(); + char::to_ascii_lowercase(&a) == char::to_ascii_lowercase(&b); +} + +fn char(a: char, b: char) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == *&b.to_ascii_lowercase(); + *&a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == 'a'; + 'a' == b.to_ascii_lowercase(); +} +fn u8(a: u8, b: u8) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == b'a'; + b'a' == b.to_ascii_lowercase(); +} +fn ref_str(a: &str, b: &str) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_uppercase().to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == "a"; + "a" == b.to_ascii_lowercase(); +} +fn ref_ref_str(a: &&str, b: &&str) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_uppercase().to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == "a"; + "a" == b.to_ascii_lowercase(); +} +fn string(a: String, b: String) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == "a"; + "a" == b.to_ascii_lowercase(); + &a.to_ascii_lowercase() == &b.to_ascii_lowercase(); + &&a.to_ascii_lowercase() == &&b.to_ascii_lowercase(); + a.to_ascii_lowercase() == "a"; + "a" == b.to_ascii_lowercase(); +} +fn ref_string(a: String, b: &String) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == "a"; + "a" == b.to_ascii_lowercase(); + + b.to_ascii_lowercase() == a.to_ascii_lowercase(); + b.to_ascii_lowercase() == "a"; + "a" == a.to_ascii_lowercase(); +} +fn string_ref_str(a: String, b: &str) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); + a.to_ascii_lowercase() == "a"; + "a" == b.to_ascii_lowercase(); + + b.to_ascii_lowercase() == a.to_ascii_lowercase(); + b.to_ascii_lowercase() == "a"; + "a" == a.to_ascii_lowercase(); +} +fn ref_u8slice(a: &[u8], b: &[u8]) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); +} +fn u8vec(a: Vec, b: Vec) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); +} +fn ref_u8vec(a: Vec, b: &Vec) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); + b.to_ascii_lowercase() == a.to_ascii_lowercase(); +} +fn ref_osstr(a: &OsStr, b: &OsStr) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); +} +fn osstring(a: OsString, b: OsString) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); +} +fn ref_osstring(a: OsString, b: &OsString) { + a.to_ascii_lowercase() == b.to_ascii_lowercase(); + b.to_ascii_lowercase() == a.to_ascii_lowercase(); +} diff --git a/tests/ui/manual_ignore_case_cmp.stderr b/tests/ui/manual_ignore_case_cmp.stderr new file mode 100644 index 00000000000..11e8b8aebb5 --- /dev/null +++ b/tests/ui/manual_ignore_case_cmp.stderr @@ -0,0 +1,546 @@ +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:9:8 + | +LL | if a.to_ascii_lowercase() == b.to_ascii_lowercase() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> tests/ui/manual_ignore_case_cmp.rs:2:9 + | +LL | #![deny(clippy::manual_ignore_case_cmp)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | if a.eq_ignore_ascii_case(b) { + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:12:8 + | +LL | if a.to_ascii_uppercase() == b.to_ascii_uppercase() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | if a.eq_ignore_ascii_case(b) { + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:15:13 + | +LL | let r = a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | let r = a.eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:16:18 + | +LL | let r = r || a.to_ascii_uppercase() == b.to_ascii_uppercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | let r = r || a.eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:17:10 + | +LL | r && a.to_ascii_lowercase() == b.to_uppercase().to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | r && a.eq_ignore_ascii_case(&b.to_uppercase()); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:19:8 + | +LL | if a.to_ascii_lowercase() != b.to_ascii_lowercase() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | if !a.eq_ignore_ascii_case(b) { + | ~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:22:8 + | +LL | if a.to_ascii_uppercase() != b.to_ascii_uppercase() { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | if !a.eq_ignore_ascii_case(b) { + | ~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:25:13 + | +LL | let r = a.to_ascii_lowercase() != b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | let r = !a.eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:26:18 + | +LL | let r = r || a.to_ascii_uppercase() != b.to_ascii_uppercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | let r = r || !a.eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:27:10 + | +LL | r && a.to_ascii_lowercase() != b.to_uppercase().to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | r && !a.eq_ignore_ascii_case(&b.to_uppercase()); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:38:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case(&b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:41:5 + | +LL | a.to_ascii_lowercase() == 'a'; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case(&'a'); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:42:5 + | +LL | 'a' == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | 'a'.eq_ignore_ascii_case(&b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:45:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case(&b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:46:5 + | +LL | a.to_ascii_lowercase() == b'a'; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case(&b'a'); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:47:5 + | +LL | b'a' == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | b'a'.eq_ignore_ascii_case(&b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:50:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:51:5 + | +LL | a.to_uppercase().to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.to_uppercase().eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:52:5 + | +LL | a.to_ascii_lowercase() == "a"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case("a"); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:53:5 + | +LL | "a" == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | "a".eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:56:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:57:5 + | +LL | a.to_uppercase().to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.to_uppercase().eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:58:5 + | +LL | a.to_ascii_lowercase() == "a"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case("a"); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:59:5 + | +LL | "a" == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | "a".eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:62:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case(&b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:63:5 + | +LL | a.to_ascii_lowercase() == "a"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case("a"); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:64:5 + | +LL | "a" == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | "a".eq_ignore_ascii_case(&b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:67:5 + | +LL | a.to_ascii_lowercase() == "a"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case("a"); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:68:5 + | +LL | "a" == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | "a".eq_ignore_ascii_case(&b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:71:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:72:5 + | +LL | a.to_ascii_lowercase() == "a"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case("a"); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:73:5 + | +LL | "a" == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | "a".eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:75:5 + | +LL | b.to_ascii_lowercase() == a.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | b.eq_ignore_ascii_case(&a); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:76:5 + | +LL | b.to_ascii_lowercase() == "a"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | b.eq_ignore_ascii_case("a"); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:77:5 + | +LL | "a" == a.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | "a".eq_ignore_ascii_case(&a); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:80:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:81:5 + | +LL | a.to_ascii_lowercase() == "a"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case("a"); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:82:5 + | +LL | "a" == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | "a".eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:84:5 + | +LL | b.to_ascii_lowercase() == a.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | b.eq_ignore_ascii_case(&a); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:85:5 + | +LL | b.to_ascii_lowercase() == "a"; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | b.eq_ignore_ascii_case("a"); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:86:5 + | +LL | "a" == a.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | "a".eq_ignore_ascii_case(&a); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:89:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:92:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case(&b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:95:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:96:5 + | +LL | b.to_ascii_lowercase() == a.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | b.eq_ignore_ascii_case(&a); + | ~~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:99:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:102:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:105:5 + | +LL | a.to_ascii_lowercase() == b.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | a.eq_ignore_ascii_case(b); + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: manual case-insensitive ASCII comparison + --> tests/ui/manual_ignore_case_cmp.rs:106:5 + | +LL | b.to_ascii_lowercase() == a.to_ascii_lowercase(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +help: consider using `.eq_ignore_ascii_case()` instead + | +LL | b.eq_ignore_ascii_case(a); + | ~~~~~~~~~~~~~~~~~~~~~~~~~ + +error: aborting due to 49 previous errors + From 13e2633f193838eadcddd5bf5842b9ce0a1f17dd Mon Sep 17 00:00:00 2001 From: blyxyas Date: Tue, 1 Oct 2024 19:39:18 +0200 Subject: [PATCH 15/87] Also sanitize configuration --- clippy_config/src/conf.rs | 24 ++++++++++++++++++++++++ clippy_config/src/lib.rs | 2 +- clippy_lints/src/lib.rs | 26 +------------------------- tests/compile-test.rs | 6 +----- 4 files changed, 27 insertions(+), 31 deletions(-) diff --git a/clippy_config/src/conf.rs b/clippy_config/src/conf.rs index 757620341cc..df13be80098 100644 --- a/clippy_config/src/conf.rs +++ b/clippy_config/src/conf.rs @@ -97,6 +97,30 @@ impl ConfError { } } +// Remove code tags and code behind '# 's, as they are not needed for the lint docs and --explain +pub fn sanitize_explanation(raw_docs: &str) -> String { + // Remove tags and hidden code: + let mut explanation = String::with_capacity(128); + let mut in_code = false; + for line in raw_docs.lines().map(str::trim) { + if let Some(lang) = line.strip_prefix("```") { + let tag = lang.split_once(',').map_or(lang, |(left, _)| left); + if !in_code && matches!(tag, "" | "rust" | "ignore" | "should_panic" | "no_run" | "compile_fail") { + explanation += "```rust\n"; + } else { + explanation += line; + explanation.push('\n'); + } + in_code = !in_code; + } else if !(in_code && line.starts_with("# ")) { + explanation += line; + explanation.push('\n'); + } + } + + explanation +} + macro_rules! wrap_option { () => { None diff --git a/clippy_config/src/lib.rs b/clippy_config/src/lib.rs index c63d98a0a13..42651521f8d 100644 --- a/clippy_config/src/lib.rs +++ b/clippy_config/src/lib.rs @@ -26,5 +26,5 @@ mod metadata; pub mod msrvs; pub mod types; -pub use conf::{Conf, get_configuration_metadata, lookup_conf_file}; +pub use conf::{Conf, get_configuration_metadata, lookup_conf_file, sanitize_explanation}; pub use metadata::ClippyConfiguration; diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 0bfbb92dfda..71e7b56408e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -396,7 +396,7 @@ mod zero_sized_map_values; mod zombie_processes; // end lints modules, do not remove this comment, it’s used in `update_lints` -use clippy_config::{Conf, get_configuration_metadata}; +use clippy_config::{Conf, get_configuration_metadata, sanitize_explanation}; use clippy_utils::macros::FormatArgsStorage; use rustc_data_structures::fx::FxHashSet; use rustc_lint::{Lint, LintId}; @@ -522,30 +522,6 @@ impl LintInfo { } } -// Remove code tags and code behind '# 's, as they are not needed for the lint docs and --explain -pub fn sanitize_explanation(raw_docs: &str) -> String { - // Remove tags and hidden code: - let mut explanation = String::with_capacity(128); - let mut in_code = false; - for line in raw_docs.lines().map(|line| line.trim()) { - if let Some(lang) = line.strip_prefix("```") { - let tag = lang.split_once(',').map_or(lang, |(left, _)| left); - if !in_code && matches!(tag, "" | "rust" | "ignore" | "should_panic" | "no_run" | "compile_fail") { - explanation += "```rust\n"; - } else { - explanation += line; - explanation.push('\n'); - } - in_code = !in_code; - } else if !(in_code && line.starts_with("# ")) { - explanation += line; - explanation.push('\n'); - } - } - - explanation -} - pub fn explain(name: &str) -> i32 { let target = format!("clippy::{}", name.to_ascii_uppercase()); diff --git a/tests/compile-test.rs b/tests/compile-test.rs index 23dd41235bd..8734bd41364 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -5,9 +5,9 @@ use cargo_metadata::Message; use cargo_metadata::diagnostic::{Applicability, Diagnostic}; use clippy_config::ClippyConfiguration; +use clippy_lints::LintInfo; use clippy_lints::declared_lints::LINTS; use clippy_lints::deprecated_lints::{DEPRECATED, DEPRECATED_VERSION, RENAMED}; -use clippy_lints::{LintInfo, sanitize_explanation}; use serde::{Deserialize, Serialize}; use test_utils::IS_RUSTC_TEST_SUITE; use ui_test::custom_flags::Flag; @@ -444,10 +444,6 @@ impl DiagnosticCollector { iter::zip(DEPRECATED, DEPRECATED_VERSION) .map(|((lint, reason), version)| LintMetadata::new_deprecated(lint, reason, version)), ) - .map(|mut metadata| { - metadata.docs = sanitize_explanation(&metadata.docs); - metadata - }) .collect(); metadata.sort_unstable_by(|a, b| a.id.cmp(&b.id)); From 3ac3c3495173d2b93c585e0df37667f7633b9ab5 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 29 Sep 2024 22:23:44 -0400 Subject: [PATCH 16/87] Remove crashes and fix tests --- tests/ui/crashes/ice-12284.rs | 10 ---------- 1 file changed, 10 deletions(-) delete mode 100644 tests/ui/crashes/ice-12284.rs diff --git a/tests/ui/crashes/ice-12284.rs b/tests/ui/crashes/ice-12284.rs deleted file mode 100644 index 8d1dbface8e..00000000000 --- a/tests/ui/crashes/ice-12284.rs +++ /dev/null @@ -1,10 +0,0 @@ -#![allow(incomplete_features)] -#![feature(unnamed_fields)] - -#[repr(C)] -struct Foo { - _: struct { - }, -} - -fn main() {} From ce22fd34d9c35ad08da74d82bea4227f8dba77f7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 2 Oct 2024 21:52:44 -0400 Subject: [PATCH 17/87] Remove redundant in_trait from hir::TyKind::OpaqueDef --- clippy_lints/src/extra_unused_type_parameters.rs | 2 +- clippy_lints/src/lifetimes.rs | 2 +- clippy_lints/src/manual_async_fn.rs | 2 +- clippy_utils/src/hir_utils.rs | 3 +-- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/extra_unused_type_parameters.rs b/clippy_lints/src/extra_unused_type_parameters.rs index bf9388b4a70..a7fb9535e79 100644 --- a/clippy_lints/src/extra_unused_type_parameters.rs +++ b/clippy_lints/src/extra_unused_type_parameters.rs @@ -199,7 +199,7 @@ impl<'cx, 'tcx> Visitor<'tcx> for TypeWalker<'cx, 'tcx> { fn visit_ty(&mut self, t: &'tcx Ty<'tcx>) { if let Some((def_id, _)) = t.peel_refs().as_generic_param() { self.ty_params.remove(&def_id); - } else if let TyKind::OpaqueDef(id, _, _) = t.kind { + } else if let TyKind::OpaqueDef(id, _) = t.kind { // Explicitly walk OpaqueDef. Normally `walk_ty` would do the job, but it calls // `visit_nested_item`, which checks that `Self::NestedFilter::INTER` is set. We're // using `OnlyBodies`, so the check ends up failing and the type isn't fully walked. diff --git a/clippy_lints/src/lifetimes.rs b/clippy_lints/src/lifetimes.rs index 755afe959b3..d3cc5ea628c 100644 --- a/clippy_lints/src/lifetimes.rs +++ b/clippy_lints/src/lifetimes.rs @@ -523,7 +523,7 @@ impl<'a, 'tcx> Visitor<'tcx> for RefVisitor<'a, 'tcx> { fn visit_ty(&mut self, ty: &'tcx Ty<'_>) { match ty.kind { - TyKind::OpaqueDef(item, bounds, _) => { + TyKind::OpaqueDef(item, bounds) => { let map = self.cx.tcx.hir(); let item = map.item(item); let len = self.lts.len(); diff --git a/clippy_lints/src/manual_async_fn.rs b/clippy_lints/src/manual_async_fn.rs index fc3bba9e512..7097c85156c 100644 --- a/clippy_lints/src/manual_async_fn.rs +++ b/clippy_lints/src/manual_async_fn.rs @@ -105,7 +105,7 @@ fn future_trait_ref<'tcx>( cx: &LateContext<'tcx>, ty: &'tcx Ty<'tcx>, ) -> Option<(&'tcx TraitRef<'tcx>, Vec)> { - if let TyKind::OpaqueDef(item_id, bounds, false) = ty.kind + if let TyKind::OpaqueDef(item_id, bounds) = ty.kind && let item = cx.tcx.hir().item(item_id) && let ItemKind::OpaqueTy(opaque) = &item.kind && let Some(trait_ref) = opaque.bounds.iter().find_map(|bound| { diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 76900379ac7..b2d75125ccd 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -1115,9 +1115,8 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } }, TyKind::Path(ref qpath) => self.hash_qpath(qpath), - TyKind::OpaqueDef(_, arg_list, in_trait) => { + TyKind::OpaqueDef(_, arg_list) => { self.hash_generic_args(arg_list); - in_trait.hash(&mut self.s); }, TyKind::TraitObject(_, lifetime, _) => { self.hash_lifetime(lifetime); From cbd64a4868a84e980101be7779b7b17156dff383 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 2 Oct 2024 22:04:18 -0400 Subject: [PATCH 18/87] Use named fields for OpaqueTyOrigin --- clippy_lints/src/len_zero.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index 8bc2a56af99..0cbcdbeb90a 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -313,7 +313,7 @@ fn extract_future_output<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<& kind: ItemKind::OpaqueTy(opaque), .. } = item - && let OpaqueTyOrigin::AsyncFn(_) = opaque.origin + && let OpaqueTyOrigin::AsyncFn { .. } = opaque.origin && let [GenericBound::Trait(trait_ref, _)] = &opaque.bounds && let Some(segment) = trait_ref.trait_ref.path.segments.last() && let Some(generic_args) = segment.args From 2080634d336ac73a00aa256993ee035da7226aca Mon Sep 17 00:00:00 2001 From: coekjan Date: Wed, 2 Oct 2024 23:51:52 +0800 Subject: [PATCH 19/87] Fix lint `manual_slice_size_calculation` when the slice is ref more than once --- .../src/manual_slice_size_calculation.rs | 14 +++++----- tests/ui/manual_slice_size_calculation.fixed | 4 +++ tests/ui/manual_slice_size_calculation.rs | 4 +++ tests/ui/manual_slice_size_calculation.stderr | 26 ++++++++++++++----- 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/clippy_lints/src/manual_slice_size_calculation.rs b/clippy_lints/src/manual_slice_size_calculation.rs index b24a0f4695a..a984a7f9520 100644 --- a/clippy_lints/src/manual_slice_size_calculation.rs +++ b/clippy_lints/src/manual_slice_size_calculation.rs @@ -45,10 +45,11 @@ impl<'tcx> LateLintPass<'tcx> for ManualSliceSizeCalculation { && !expr.span.from_expansion() // Does not apply inside const because size_of_val is not cost in stable. && !is_in_const_context(cx) - && let Some(receiver) = simplify(cx, left, right) + && let Some((receiver, refs_count)) = simplify(cx, left, right) { let ctxt = expr.span.ctxt(); let mut app = Applicability::MachineApplicable; + let deref = "*".repeat(refs_count - 1); let val_name = snippet_with_context(cx, receiver.span, ctxt, "slice", &mut app).0; let Some(sugg) = std_or_core(cx) else { return }; @@ -58,7 +59,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualSliceSizeCalculation { expr.span, "manual slice size calculation", "try", - format!("{sugg}::mem::size_of_val({val_name})"), + format!("{sugg}::mem::size_of_val({deref}{val_name})"), app, ); } @@ -69,7 +70,7 @@ fn simplify<'tcx>( cx: &LateContext<'tcx>, expr1: &'tcx Expr<'tcx>, expr2: &'tcx Expr<'tcx>, -) -> Option<&'tcx Expr<'tcx>> { +) -> Option<(&'tcx Expr<'tcx>, usize)> { let expr1 = expr_or_init(cx, expr1); let expr2 = expr_or_init(cx, expr2); @@ -80,13 +81,14 @@ fn simplify_half<'tcx>( cx: &LateContext<'tcx>, expr1: &'tcx Expr<'tcx>, expr2: &'tcx Expr<'tcx>, -) -> Option<&'tcx Expr<'tcx>> { +) -> Option<(&'tcx Expr<'tcx>, usize)> { if !expr1.span.from_expansion() // expr1 is `[T1].len()`? && let ExprKind::MethodCall(method_path, receiver, _, _) = expr1.kind && method_path.ident.name == sym::len && let receiver_ty = cx.typeck_results().expr_ty(receiver) - && let ty::Slice(ty1) = receiver_ty.peel_refs().kind() + && let (receiver_ty, refs_count) = clippy_utils::ty::walk_ptrs_ty_depth(receiver_ty) + && let ty::Slice(ty1) = receiver_ty.kind() // expr2 is `size_of::()`? && let ExprKind::Call(func, _) = expr2.kind && let ExprKind::Path(ref func_qpath) = func.kind @@ -96,7 +98,7 @@ fn simplify_half<'tcx>( // T1 == T2? && *ty1 == ty2 { - Some(receiver) + Some((receiver, refs_count)) } else { None } diff --git a/tests/ui/manual_slice_size_calculation.fixed b/tests/ui/manual_slice_size_calculation.fixed index 62b372f4b8d..0603b30e346 100644 --- a/tests/ui/manual_slice_size_calculation.fixed +++ b/tests/ui/manual_slice_size_calculation.fixed @@ -10,11 +10,15 @@ use proc_macros::external; fn main() { let v_i32 = Vec::::new(); let s_i32 = v_i32.as_slice(); + let s_i32_ref = &s_i32; + let s_i32_ref_ref = &s_i32_ref; // True positives: let _ = std::mem::size_of_val(s_i32); // WARNING let _ = std::mem::size_of_val(s_i32); // WARNING let _ = std::mem::size_of_val(s_i32) * 5; // WARNING + let _ = std::mem::size_of_val(*s_i32_ref); // WARNING + let _ = std::mem::size_of_val(**s_i32_ref_ref); // WARNING let len = s_i32.len(); let size = size_of::(); diff --git a/tests/ui/manual_slice_size_calculation.rs b/tests/ui/manual_slice_size_calculation.rs index d59f5fd8b94..14093e653c0 100644 --- a/tests/ui/manual_slice_size_calculation.rs +++ b/tests/ui/manual_slice_size_calculation.rs @@ -10,11 +10,15 @@ use proc_macros::external; fn main() { let v_i32 = Vec::::new(); let s_i32 = v_i32.as_slice(); + let s_i32_ref = &s_i32; + let s_i32_ref_ref = &s_i32_ref; // True positives: let _ = s_i32.len() * size_of::(); // WARNING let _ = size_of::() * s_i32.len(); // WARNING let _ = size_of::() * s_i32.len() * 5; // WARNING + let _ = size_of::() * s_i32_ref.len(); // WARNING + let _ = size_of::() * s_i32_ref_ref.len(); // WARNING let len = s_i32.len(); let size = size_of::(); diff --git a/tests/ui/manual_slice_size_calculation.stderr b/tests/ui/manual_slice_size_calculation.stderr index 4bd8a4fdf17..0397f3a4969 100644 --- a/tests/ui/manual_slice_size_calculation.stderr +++ b/tests/ui/manual_slice_size_calculation.stderr @@ -1,5 +1,5 @@ error: manual slice size calculation - --> tests/ui/manual_slice_size_calculation.rs:15:13 + --> tests/ui/manual_slice_size_calculation.rs:17:13 | LL | let _ = s_i32.len() * size_of::(); // WARNING | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(s_i32)` @@ -8,40 +8,52 @@ LL | let _ = s_i32.len() * size_of::(); // WARNING = help: to override `-D warnings` add `#[allow(clippy::manual_slice_size_calculation)]` error: manual slice size calculation - --> tests/ui/manual_slice_size_calculation.rs:16:13 + --> tests/ui/manual_slice_size_calculation.rs:18:13 | LL | let _ = size_of::() * s_i32.len(); // WARNING | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(s_i32)` error: manual slice size calculation - --> tests/ui/manual_slice_size_calculation.rs:17:13 + --> tests/ui/manual_slice_size_calculation.rs:19:13 | LL | let _ = size_of::() * s_i32.len() * 5; // WARNING | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(s_i32)` +error: manual slice size calculation + --> tests/ui/manual_slice_size_calculation.rs:20:13 + | +LL | let _ = size_of::() * s_i32_ref.len(); // WARNING + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(*s_i32_ref)` + error: manual slice size calculation --> tests/ui/manual_slice_size_calculation.rs:21:13 | +LL | let _ = size_of::() * s_i32_ref_ref.len(); // WARNING + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(**s_i32_ref_ref)` + +error: manual slice size calculation + --> tests/ui/manual_slice_size_calculation.rs:25:13 + | LL | let _ = len * size_of::(); // WARNING | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(s_i32)` error: manual slice size calculation - --> tests/ui/manual_slice_size_calculation.rs:22:13 + --> tests/ui/manual_slice_size_calculation.rs:26:13 | LL | let _ = s_i32.len() * size; // WARNING | ^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(s_i32)` error: manual slice size calculation - --> tests/ui/manual_slice_size_calculation.rs:23:13 + --> tests/ui/manual_slice_size_calculation.rs:27:13 | LL | let _ = len * size; // WARNING | ^^^^^^^^^^ help: try: `std::mem::size_of_val(s_i32)` error: manual slice size calculation - --> tests/ui/manual_slice_size_calculation.rs:25:13 + --> tests/ui/manual_slice_size_calculation.rs:29:13 | LL | let _ = external!(&[1u64][..]).len() * size_of::(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::mem::size_of_val(external!(&[1u64][..]))` -error: aborting due to 7 previous errors +error: aborting due to 9 previous errors From aeb548a4f76c742608c2b7154461814440048506 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 14 Aug 2024 16:40:00 +0200 Subject: [PATCH 20/87] Put back interactions with settings menus --- util/gh-pages/index_template.html | 78 ++++++++++------------------ util/gh-pages/script.js | 85 +++++++++++++++++++------------ util/gh-pages/style.css | 14 ++++- 3 files changed, 91 insertions(+), 86 deletions(-) diff --git a/util/gh-pages/index_template.html b/util/gh-pages/index_template.html index 5a7af7d562e..a18948a2060 100644 --- a/util/gh-pages/index_template.html +++ b/util/gh-pages/index_template.html @@ -27,7 +27,7 @@ Otherwise, have a great day =^.^=
    -
    +
    Theme
    - All - +
  • - +
    • -
    +
    -
    -
    -
    - - -
    +
    + +
    -
    +
    -