Merge pull request #153 from tgross35/clippy-ci

Run clippy checks in CI
rust-lang · Dec 29, 2024 · 7a17f11 · 7a17f11
2 parents d6cac97 + 56619ab
commit 7a17f11
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 40 deletions.
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -1,5 +1,10 @@
 name: CI
 
+env:
+  CARGO_TERM_VERBOSE: true
+  RUSTDOCFLAGS: -Dwarnings
+  RUSTFLAGS: -Dwarnings
+
 on:
   pull_request:
   push:
@@ -36,6 +41,23 @@ jobs:
 
     - run: cargo test --all
 
+  clippy:
+    name: Clippy
+    runs-on: ubuntu-24.04
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v4
+
+    - name: Update rust
+      run: |
+        # use beta since it gives us near-latest fixes but isn't as volatile as nightly
+        rustup default beta
+        rustup component add clippy
+        rustup update --no-self-update
+
+    # FIXME(msrv): suggestions do not work in 1.23, nor dows `#![allow(clippy::...)]`
+    - run: cargo clippy --all -- -Aclippy::while_let_loop
+
   msrv:
     name: Check building with the MSRV
     runs-on: ubuntu-24.04
@@ -65,6 +87,7 @@ jobs:
   success:
     needs:
       - test
+      - clippy
       - msrv
       - rustfmt
     runs-on: ubuntu-latest

diff --git a/src/lib.rs b/src/lib.rs
@@ -70,6 +70,7 @@ extern crate doc_comment;
 doctest!("../README.md");
 
 use std::cmp;
+use std::cmp::Ordering;
 use std::error::Error;
 use std::fmt;
 use std::fs;
@@ -620,52 +621,56 @@ impl Pattern {
 
                     let count = i - old;
 
-                    if count > 2 {
-                        return Err(PatternError {
-                            pos: old + 2,
-                            msg: ERROR_WILDCARDS,
-                        });
-                    } else if count == 2 {
-                        // ** can only be an entire path component
-                        // i.e. a/**/b is valid, but a**/b or a/**b is not
-                        // invalid matches are treated literally
-                        let is_valid = if i == 2 || path::is_separator(chars[i - count - 1]) {
-                            // it ends in a '/'
-                            if i < chars.len() && path::is_separator(chars[i]) {
-                                i += 1;
-                                true
-                            // or the pattern ends here
-                            // this enables the existing globbing mechanism
-                            } else if i == chars.len() {
-                                true
-                            // `**` ends in non-separator
+                    match count.cmp(&2) {
+                        Ordering::Greater => {
+                            return Err(PatternError {
+                                pos: old + 2,
+                                msg: ERROR_WILDCARDS,
+                            })
+                        }
+                        Ordering::Equal => {
+                            // ** can only be an entire path component
+                            // i.e. a/**/b is valid, but a**/b or a/**b is not
+                            // invalid matches are treated literally
+                            let is_valid = if i == 2 || path::is_separator(chars[i - count - 1]) {
+                                // it ends in a '/'
+                                if i < chars.len() && path::is_separator(chars[i]) {
+                                    i += 1;
+                                    true
+                                // or the pattern ends here
+                                // this enables the existing globbing mechanism
+                                } else if i == chars.len() {
+                                    true
+                                // `**` ends in non-separator
+                                } else {
+                                    return Err(PatternError {
+                                        pos: i,
+                                        msg: ERROR_RECURSIVE_WILDCARDS,
+                                    });
+                                }
+                            // `**` begins with non-separator
                             } else {
                                 return Err(PatternError {
-                                    pos: i,
+                                    pos: old - 1,
                                     msg: ERROR_RECURSIVE_WILDCARDS,
                                 });
-                            }
-                        // `**` begins with non-separator
-                        } else {
-                            return Err(PatternError {
-                                pos: old - 1,
-                                msg: ERROR_RECURSIVE_WILDCARDS,
-                            });
-                        };
+                            };
 
-                        if is_valid {
-                            // collapse consecutive AnyRecursiveSequence to a
-                            // single one
+                            if is_valid {
+                                // collapse consecutive AnyRecursiveSequence to a
+                                // single one
 
-                            let tokens_len = tokens.len();
+                                let tokens_len = tokens.len();
 
-                            if !(tokens_len > 1 && tokens[tokens_len - 1] == AnyRecursiveSequence) {
-                                is_recursive = true;
-                                tokens.push(AnyRecursiveSequence);
+                                if !(tokens_len > 1
+                                    && tokens[tokens_len - 1] == AnyRecursiveSequence)
+                                {
+                                    is_recursive = true;
+                                    tokens.push(AnyRecursiveSequence);
+                                }
                             }
                         }
-                    } else {
-                        tokens.push(AnySequence);
+                        Ordering::Less => tokens.push(AnySequence),
                     }
                 }
                 '[' => {
@@ -1029,7 +1034,7 @@ fn chars_eq(a: char, b: char, case_sensitive: bool) -> bool {
         true
     } else if !case_sensitive && a.is_ascii() && b.is_ascii() {
         // FIXME: work with non-ascii chars properly (issue #9084)
-        a.to_ascii_lowercase() == b.to_ascii_lowercase()
+        a.eq_ignore_ascii_case(&b)
     } else {
         a == b
     }
@@ -1166,8 +1171,7 @@ mod test {
                 .ok()
                 .and_then(|p| match p.components().next().unwrap() {
                     Component::Prefix(prefix_component) => {
-                        let path = Path::new(prefix_component.as_os_str());
-                        path.join("*");
+                        let path = Path::new(prefix_component.as_os_str()).join("*");
                         Some(path.to_path_buf())
                     }
                     _ => panic!("no prefix in this path"),