Fix most clippy lints #5400

kadiwa4 · 2022-06-22T13:49:08Z

I didn't fix the clippy::too_many_arguments and clippy::uninlined_format_args warnings.

xxchan

I've browsed changes. Mostly LGTM, but there are some non-trivial changes.

So

It might be better to fix in multiple smaller PRs to make review easier.
It might be better to add a clippy check in CI to avoid future lint issues. BTW, I think #[expect(lint)] Tracking issue for RFC 2383, "Lint Reasons RFC" rust#54503 is useful to progressively fix lints.

@ytmimi @calebcartwright I suggest we either close (maybe split in smaller steps like above) or accept this PR, otherwise it will get stale often.

src/cargo-fmt/test/mod.rs

xxchan · 2023-07-29T11:06:49Z

src/chains.rs

@@ -202,7 +202,7 @@ impl ChainItemKind {
    fn is_tup_field_access(expr: &ast::Expr) -> bool {
        match expr.kind {
            ast::ExprKind::Field(_, ref field) => {
-                field.name.to_string().chars().all(|c| c.is_digit(10))
+                field.name.to_string().bytes().all(|c| c.is_ascii_digit())


Why chars to bytes? Seems not equivalent 👀

It's true that it is unrelated. But if all the chars are ASCII digits, then none of them are multi-byte, so it is equivalent

xxchan · 2023-07-29T11:14:40Z

src/imports.rs

+impl PartialEq for UseSegment {
+    fn eq(&self, other: &Self) -> bool {
+        self.kind == other.kind
+    }
+}
+
 impl Hash for UseSegment {
    fn hash<H: Hasher>(&self, state: &mut H) {
        self.kind.hash(state);


This is inconsistent eq and hash? May need to check the correctness.

xxchan · 2023-07-29T11:18:22Z

src/lib.rs

-            Box::new(emitter::FilesWithBackupEmitter::default())
+            <Box<emitter::FilesWithBackupEmitter>>::default()
        }
        EmitMode::Files => Box::new(emitter::FilesEmitter::new(
            config.print_misformatted_file_names(),
        )),
        EmitMode::Stdout | EmitMode::Coverage => {
            Box::new(emitter::StdoutEmitter::new(config.verbose()))
        }
-        EmitMode::Json => Box::new(emitter::JsonEmitter::default()),
-        EmitMode::ModifiedLines => Box::new(emitter::ModifiedLinesEmitter::default()),
-        EmitMode::Checkstyle => Box::new(emitter::CheckstyleEmitter::default()),
+        EmitMode::Json => <Box<emitter::JsonEmitter>>::default(),
+        EmitMode::ModifiedLines => <Box<emitter::ModifiedLinesEmitter>>::default(),
+        EmitMode::Checkstyle => <Box<emitter::CheckstyleEmitter>>::default(),


I feel it's not better than before. 🤔

Certainly not easier to understand, but maybe a bit more efficient in a future version of Rust. But you're right, it doesn't seem to be worth it.

xxchan · 2023-07-29T11:24:01Z

src/matches.rs

+            let semicolon =
+                if context.config.version() != Version::One && semicolon_for_expr(context, body) {
+                    ";"
                } else {
                    ""
                };
-                let semicolon = if context.config.version() == Version::One {
-                    ""
-                } else {
-                    if semicolon_for_expr(context, body) {
-                        ";"
-                    } else {
-                        ""
-                    }
-                };
-                ("{", format!("{}{}}}{}", semicolon, indent_str, comma))


Maybe the original version's logic is more clear.

xxchan · 2023-07-29T11:26:32Z

src/overflow.rs

-            // splitn(2, *).next().unwrap() is always safe.
-            let rewrite_first_line = Some(rewrite.splitn(2, '\n').next().unwrap().to_owned());
+            // split(*).next().unwrap() never panics.
+            let rewrite_first_line = Some(rewrite.split('\n').next().unwrap().to_owned());


Is it split_once instead of split?

I can also use split_once but that's longer

let rewrite_first_line = Some( rewrite .split_once('\n') .map_or(&*rewrite, |(f, _)| f) .to_owned(), );

Maybe str::lines is the best option here?

xxchan · 2023-07-29T11:29:28Z

src/utils.rs

-    unicode_str_width(s.splitn(2, '\n').next().unwrap_or(""))
+    unicode_str_width(s.split('\n').next().unwrap_or(""))
 }

 /// The width of the last line in s.
 #[inline]
 pub(crate) fn last_line_width(s: &str) -> usize {
-    unicode_str_width(s.rsplitn(2, '\n').next().unwrap_or(""))
+    unicode_str_width(s.rsplit('\n').next().unwrap_or(""))


xxchan · 2023-07-29T11:29:38Z

src/visitor.rs

-            let first_lines = comment_snippet.splitn(2, '/').next().unwrap_or("");
+            let first_lines = comment_snippet.split('/').next().unwrap_or("");


ytmimi added the pr-not-reviewed label Jun 22, 2022

kadiwa4 force-pushed the clippy_lints branch from 97abf6b to df64073 Compare June 23, 2022 19:11

kadiwa4 force-pushed the clippy_lints branch 2 times, most recently from e522bf4 to 3a804cd Compare July 1, 2022 14:25

ytmimi added p-low pr-merge-conflict This PR has merge conflicts that must be resolved labels Jul 28, 2022

kadiwa4 force-pushed the clippy_lints branch from 3a804cd to 60d3b4e Compare July 28, 2022 20:40

ytmimi removed the pr-merge-conflict This PR has merge conflicts that must be resolved label Aug 8, 2022

ytmimi mentioned this pull request Aug 8, 2022

Fix portion clippy issues #5491

Closed

kadiwa4 force-pushed the clippy_lints branch from 60d3b4e to fd1d7e0 Compare January 25, 2023 18:06

kadiwa4 force-pushed the clippy_lints branch from fd1d7e0 to 0d53aea Compare July 1, 2023 12:16

kadiwa4 force-pushed the clippy_lints branch 2 times, most recently from 78175a8 to dfdb05a Compare July 20, 2023 06:55

xxchan reviewed Jul 29, 2023

View reviewed changes

kadiwa4 force-pushed the clippy_lints branch from dfdb05a to e732afb Compare September 24, 2023 09:49

kadiwa4 mentioned this pull request Sep 24, 2023

Rename IntoOverflowableItem to satisfy clippy #5924

Open

ytmimi mentioned this pull request Jan 5, 2024

fix a couply of clippy findings #6007

Merged

kadiwa4 added 7 commits January 9, 2024 12:17

fix clippy lints regarding lazy/eager evaluation

a93ab9b

fix clippy lints about PartialEq/Eq implementation

904de72

fix clippy::format_{push_string,collect}

68bd868

fix clippy::collapsible_(else_)if

e9933eb

rename IntoOverflowableItem to satisfy clippy

7b6d83f

fix clippy lints about borrowing

220bb45

fix other clippy lints

24cc6da

kadiwa4 force-pushed the clippy_lints branch from e732afb to 24cc6da Compare January 9, 2024 12:41

Rawk mentioned this pull request Sep 1, 2024

Fix many lint warnings #6304

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix most clippy lints #5400

Fix most clippy lints #5400

kadiwa4 commented Jun 22, 2022 •

edited

Loading

xxchan left a comment

xxchan Jul 29, 2023

kadiwa4 Jul 29, 2023

xxchan Jul 29, 2023

xxchan Jul 29, 2023

kadiwa4 Jul 29, 2023

xxchan Jul 29, 2023

xxchan Jul 29, 2023

kadiwa4 Jul 29, 2023 •

edited

Loading

xxchan Jul 29, 2023

xxchan Jul 29, 2023

		let first_lines = comment_snippet.splitn(2, '/').next().unwrap_or("");
		let first_lines = comment_snippet.split('/').next().unwrap_or("");

Fix most clippy lints #5400

Are you sure you want to change the base?

Fix most clippy lints #5400

Conversation

kadiwa4 commented Jun 22, 2022 • edited Loading

xxchan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kadiwa4 Jul 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kadiwa4 commented Jun 22, 2022 •

edited

Loading

kadiwa4 Jul 29, 2023 •

edited

Loading