use pre-commit hooks for linting (#590)

use pre-commit hooks for linting. Currently this runs codespell for spelling errors, and cargo-fmt and cargo-clippy for coding style.
benfred · Jun 2, 2023 · 013a8a8 · 013a8a8
1 parent a838872
commit 013a8a8
Show file tree

Hide file tree

Showing 39 changed files with 2,869 additions and 1,410 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -13,8 +13,16 @@ env:
   CARGO_TERM_COLOR: always
 
 jobs:
+  lint:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - uses: actions/setup-python@v4
+      - uses: pre-commit/[email protected]
+
   build-linux-armv7:
     runs-on: [self-hosted, linux, arm]
+    needs: [lint]
     steps:
     - name: Setup python
       run: |
@@ -28,6 +36,7 @@ jobs:
 
   build:
     runs-on: ${{ matrix.os }}
+    needs: [lint]
     strategy:
       fail-fast: false
       matrix:
@@ -82,6 +91,7 @@ jobs:
 
   build-linux-cross:
     runs-on: ubuntu-latest
+    needs: [lint]
     strategy:
       fail-fast: false
       matrix:
@@ -109,6 +119,7 @@ jobs:
 
   build-freebsd:
     runs-on: macos-latest
+    needs: [lint]
     timeout-minutes: 30
     strategy:
       matrix:

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -0,0 +1,21 @@
+repos:
+      - repo: https://github.com/codespell-project/codespell
+        rev: v2.2.4
+        hooks:
+        - id: codespell
+          additional_dependencies: [tomli]
+          args: ["--toml", "pyproject.toml"]
+          exclude: (?x)^(ci/testdata.*|images.*)$
+      - repo: https://github.com/doublify/pre-commit-rust
+        rev: v1.0
+        hooks:
+        - id: fmt
+        - id: cargo-check
+      - repo: local
+        hooks:
+        - id: cargo-clippy
+          name: cargo clippy
+          entry: cargo clippy -- -D warnings
+          language: system
+          files: \.rs$
+          pass_filenames: false
diff --git a/build.rs b/build.rs
@@ -8,6 +8,6 @@ fn main() {
     match env::var("CARGO_CFG_TARGET_OS").unwrap().as_ref() {
         "windows" => println!("cargo:rustc-cfg=unwind"),
         "linux" => println!("cargo:rustc-cfg=unwind"),
-        _ => { }
+        _ => {}
     }
 }
diff --git a/ci/testdata/cython_test.pyx b/ci/testdata/cython_test.pyx
@@ -5,7 +5,7 @@ from cython cimport floating
 
 cpdef sqrt(floating value):
     # solve for the square root of value by finding the zeros of
-    #   'x * x - value = 0' using newtons meethod
+    #   'x * x - value = 0' using newtons method
     cdef double x = value / 2
     for _ in range(8):
         x -= (x * x - value) / (2 * x)

diff --git a/generate_bindings.py b/generate_bindings.py
@@ -153,6 +153,7 @@ def extract_bindings(cpython_path, version, configure=False):
         o.write("#![allow(clippy::default_trait_access)]\n")
         o.write("#![allow(clippy::cast_lossless)]\n")
         o.write("#![allow(clippy::trivially_copy_pass_by_ref)]\n\n")
+        o.write("#![allow(clippy::upper_case_acronyms)]\n\n")
         o.write(open(os.path.join(cpython_path, "bindgen_output.rs")).read())
 
 

diff --git a/pyproject.toml b/pyproject.toml
@@ -16,3 +16,7 @@ classifiers = [
 
 [tool.maturin]
 bindings = "bin"
+
+[tool.codespell]
+ignore-words-list = "crate"
+skip = "./.git,./.github,./target,./ci/testdata,./images/"
diff --git a/src/binary_parser.rs b/src/binary_parser.rs
@@ -1,10 +1,8 @@
-
 use std::collections::HashMap;
 use std::fs::File;
 use std::path::Path;
 
 use anyhow::Error;
-use goblin;
 use goblin::Object;
 use memmap::Mmap;
 
@@ -15,7 +13,7 @@ pub struct BinaryInfo {
     pub bss_size: u64,
     pub offset: u64,
     pub addr: u64,
-    pub size: u64
+    pub size: u64,
 }
 
 impl BinaryInfo {
@@ -42,12 +40,18 @@ pub fn parse_binary(filename: &Path, addr: u64, size: u64) -> Result<BinaryInfo,
             let mach = match mach {
                 goblin::mach::Mach::Binary(mach) => mach,
                 goblin::mach::Mach::Fat(fat) => {
-                    let arch = fat.iter_arches().find(|arch|
-                        match arch {
+                    let arch = fat
+                        .iter_arches()
+                        .find(|arch| match arch {
                             Ok(arch) => arch.is_64(),
-                            Err(_) => false
-                        }
-                    ).ok_or_else(|| format_err!("Failed to find 64 bit arch in FAT archive in {}", filename.display()))??;
+                            Err(_) => false,
+                        })
+                        .ok_or_else(|| {
+                            format_err!(
+                                "Failed to find 64 bit arch in FAT archive in {}",
+                                filename.display()
+                            )
+                        })??;
                     let bytes = &buffer[arch.offset as usize..][..arch.size as usize];
                     goblin::mach::MachO::parse(bytes, 0)?
                 }
@@ -69,31 +73,51 @@ pub fn parse_binary(filename: &Path, addr: u64, size: u64) -> Result<BinaryInfo,
                     let (name, value) = symbol?;
                     // almost every symbol we care about starts with an extra _, remove to normalize
                     // with the entries seen on linux/windows
-                    if name.starts_with('_') {
-                        symbols.insert(name[1..].to_string(), value.n_value + offset);
+                    if let Some(stripped_name) = name.strip_prefix('_') {
+                        symbols.insert(stripped_name.to_string(), value.n_value + offset);
                     }
-
                 }
             }
-            Ok(BinaryInfo{filename: filename.to_owned(), symbols, bss_addr, bss_size, offset, addr, size})
+            Ok(BinaryInfo {
+                filename: filename.to_owned(),
+                symbols,
+                bss_addr,
+                bss_size,
+                offset,
+                addr,
+                size,
+            })
         }
 
         Object::Elf(elf) => {
-            let bss_header = elf.section_headers
+            let bss_header = elf
+                .section_headers
                 .iter()
-                .find(|ref header| header.sh_type == goblin::elf::section_header::SHT_NOBITS)
-                .ok_or_else(|| format_err!("Failed to find BSS section header in {}", filename.display()))?;
-
-            let program_header = elf.program_headers
+                .find(|header| header.sh_type == goblin::elf::section_header::SHT_NOBITS)
+                .ok_or_else(|| {
+                    format_err!(
+                        "Failed to find BSS section header in {}",
+                        filename.display()
+                    )
+                })?;
+
+            let program_header = elf
+                .program_headers
                 .iter()
-                .find(|ref header|
-                    header.p_type == goblin::elf::program_header::PT_LOAD &&
-                    header.p_flags & goblin::elf::program_header::PF_X != 0)
-                .ok_or_else(|| format_err!("Failed to find executable PT_LOAD program header in {}", filename.display()))?;
+                .find(|header| {
+                    header.p_type == goblin::elf::program_header::PT_LOAD
+                        && header.p_flags & goblin::elf::program_header::PF_X != 0
+                })
+                .ok_or_else(|| {
+                    format_err!(
+                        "Failed to find executable PT_LOAD program header in {}",
+                        filename.display()
+                    )
+                })?;
 
             // p_vaddr may be larger than the map address in case when the header has an offset and
             // the map address is relatively small. In this case we can default to 0.
-            let offset = offset.checked_sub(program_header.p_vaddr).unwrap_or(0);
+            let offset = offset.saturating_sub(program_header.p_vaddr);
 
             for sym in elf.syms.iter() {
                 let name = elf.strtab[sym.st_name].to_string();
@@ -103,36 +127,49 @@ pub fn parse_binary(filename: &Path, addr: u64, size: u64) -> Result<BinaryInfo,
                 let name = elf.dynstrtab[dynsym.st_name].to_string();
                 symbols.insert(name, dynsym.st_value + offset);
             }
-            Ok(BinaryInfo{filename: filename.to_owned(),
-                          symbols,
-                          bss_addr: bss_header.sh_addr + offset,
-                          bss_size: bss_header.sh_size,
-                          offset,
-                          addr,
-                          size})
-        },
+            Ok(BinaryInfo {
+                filename: filename.to_owned(),
+                symbols,
+                bss_addr: bss_header.sh_addr + offset,
+                bss_size: bss_header.sh_size,
+                offset,
+                addr,
+                size,
+            })
+        }
         Object::PE(pe) => {
             for export in pe.exports {
                 if let Some(name) = export.name {
                     if let Some(export_offset) = export.offset {
-                        symbols.insert(name.to_string(), export_offset as u64 + offset as u64);
+                        symbols.insert(name.to_string(), export_offset as u64 + offset);
                     }
                 }
             }
 
             pe.sections
                 .iter()
-                .find(|ref section| section.name.starts_with(b".data"))
-                .ok_or_else(|| format_err!("Failed to find .data section in PE binary of {}", filename.display()))
+                .find(|section| section.name.starts_with(b".data"))
+                .ok_or_else(|| {
+                    format_err!(
+                        "Failed to find .data section in PE binary of {}",
+                        filename.display()
+                    )
+                })
                 .map(|data_section| {
                     let bss_addr = u64::from(data_section.virtual_address) + offset;
                     let bss_size = u64::from(data_section.virtual_size);
 
-                    BinaryInfo{filename: filename.to_owned(), symbols, bss_addr, bss_size, offset, addr, size}
+                    BinaryInfo {
+                        filename: filename.to_owned(),
+                        symbols,
+                        bss_addr,
+                        bss_size,
+                        offset,
+                        addr,
+                        size,
+                    }
                 })
-        },
-        _ => {
-            Err(format_err!("Unhandled binary type"))
         }
+        _ => Err(format_err!("Unhandled binary type")),
     }
 }