Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add clippy lint GitHub Actions CI #12

Merged
merged 3 commits into from
Sep 20, 2024
Merged

Add clippy lint GitHub Actions CI #12

merged 3 commits into from
Sep 20, 2024

Conversation

weiji14
Copy link
Member

@weiji14 weiji14 commented May 16, 2024

Run cargo clippy --all-targets --all-features on Continuous Integration on every Pull Request and push to main.

TODO:

Run `cargo clippy --all-targets --all-features` on Continuous Integration on every Pull Request and push to main. Adapted from https://doc.rust-lang.org/stable/clippy/continuous_integration/github_actions.html
@weiji14 weiji14 added this to the v0.1.0 milestone May 16, 2024
@weiji14 weiji14 self-assigned this May 16, 2024
Colorize the terminal output in the GitHub Actions CI logs.
@weiji14
Copy link
Member Author

weiji14 commented May 16, 2024

Copy of clippy lint errors from https://github.com/georust/geotiff/actions/runs/9116866203/job/25066420149?pr=12#step:3:31:

error: name `BYTE` contains a capitalized acronym
 --> src/lowlevel.rs:4:10
  |
4 | pub type BYTE      = u8;
  |          ^^^^ help: consider making the acronym lowercase, except the initial letter: `Byte`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
  = note: `-D clippy::upper-case-acronyms` implied by `-D warnings`
  = help: to override `-D warnings` add `#[allow(clippy::upper_case_acronyms)]`

error: name `SHORT` contains a capitalized acronym
 --> src/lowlevel.rs:5:10
  |
5 | pub type SHORT     = u16;
  |          ^^^^^ help: consider making the acronym lowercase, except the initial letter: `Short`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

error: name `LONG` contains a capitalized acronym
 --> src/lowlevel.rs:6:10
  |
6 | pub type LONG      = u32;
  |          ^^^^ help: consider making the acronym lowercase, except the initial letter: `Long`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

error: name `ASCII` contains a capitalized acronym
 --> src/lowlevel.rs:7:10
  |
7 | pub type ASCII     = String;
  |          ^^^^^ help: consider making the acronym lowercase, except the initial letter (notice the capitalization): `Ascii`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

error: name `RATIONAL` contains a capitalized acronym
 --> src/lowlevel.rs:8:10
  |
8 | pub type RATIONAL  = (u32, u32);
  |          ^^^^^^^^ help: consider making the acronym lowercase, except the initial letter: `Rational`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

error: name `SBYTE` contains a capitalized acronym
 --> src/lowlevel.rs:9:10
  |
9 | pub type SBYTE     = i8;
  |          ^^^^^ help: consider making the acronym lowercase, except the initial letter: `Sbyte`
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

error: name `SSHORT` contains a capitalized acronym
  --> src/lowlevel.rs:10:10
   |
10 | pub type SSHORT    = i16;
   |          ^^^^^^ help: consider making the acronym lowercase, except the initial letter: `Sshort`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

error: name `SLONG` contains a capitalized acronym
  --> src/lowlevel.rs:11:10
   |
11 | pub type SLONG     = i32;
   |          ^^^^^ help: consider making the acronym lowercase, except the initial letter: `Slong`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

error: name `SRATIONAL` contains a capitalized acronym
  --> src/lowlevel.rs:12:10
   |
12 | pub type SRATIONAL = (i32, i32);
   |          ^^^^^^^^^ help: consider making the acronym lowercase, except the initial letter: `Srational`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

error: name `FLOAT` contains a capitalized acronym
  --> src/lowlevel.rs:13:10
   |
13 | pub type FLOAT     = f32;
   |          ^^^^^ help: consider making the acronym lowercase, except the initial letter: `Float`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

error: name `DOUBLE` contains a capitalized acronym
  --> src/lowlevel.rs:14:10
   |
14 | pub type DOUBLE    = f64;
   |          ^^^^^^ help: consider making the acronym lowercase, except the initial letter: `Double`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

error: all variants have the same postfix: `Value`
  --> src/lowlevel.rs:65:1
   |
65 | / pub enum TagValue {
66 | |     ByteValue(BYTE),
67 | |     AsciiValue(ASCII),
68 | |     ShortValue(SHORT),
...  |
76 | |     DoubleValue(DOUBLE),
77 | | }
   | |_^
   |
   = help: remove the postfixes and use full paths to the variants instead of glob imports
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names
   = note: `-D clippy::enum-variant-names` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::enum_variant_names)]`

error: name `LZW` contains a capitalized acronym
  --> src/lowlevel.rs:93:5
   |
93 |     LZW      = 5,
   |     ^^^ help: consider making the acronym lowercase, except the initial letter (notice the capitalization): `Lzw`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

error: name `OJPEG` contains a capitalized acronym
  --> src/lowlevel.rs:94:5
   |
94 |     OJPEG    = 6,
   |     ^^^^^ help: consider making the acronym lowercase, except the initial letter: `Ojpeg`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

error: name `JPEG` contains a capitalized acronym
  --> src/lowlevel.rs:95:5
   |
95 |     JPEG     = 7,
   |     ^^^^ help: consider making the acronym lowercase, except the initial letter: `Jpeg`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

error: name `RGB` contains a capitalized acronym
   --> src/lowlevel.rs:124:5
    |
124 |     RGB,
    |     ^^^ help: consider making the acronym lowercase, except the initial letter: `Rgb`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

error: the borrowed expression implements the required traits
  --> src/reader.rs:25:37
   |
25 |         let mut reader = File::open(&filepath)?;
   |                                     ^^^^^^^^^ help: change this to: `filepath`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
   = note: `-D clippy::needless-borrows-for-generic-args` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::needless_borrows_for_generic_args)]`

error: useless use of `format!`
  --> src/reader.rs:45:54
   |
45 |             None => Err(Error::new(ErrorKind::Other, format!("Invalid byte order in header."))),
   |                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.to_string()`: `"Invalid byte order in header.".to_string()`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#useless_format
   = note: `-D clippy::useless-format` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::useless_format)]`

error: you don't need to add `&` to all patterns
   --> src/reader.rs:124:9
    |
124 | /         match tpe {
125 | |             &TagType::ByteTag => TagValue::ByteValue(vec[0]),
126 | |             &TagType::ASCIITag => TagValue::AsciiValue(String::from_utf8_lossy(&vec).to_string()),
127 | |             &TagType::ShortTag => TagValue::ShortValue(Endian::read_u16(&vec[..])),
...   |
138 | |             &TagType::UndefinedTag => TagValue::ByteValue(0),
139 | |         }
    | |_________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#match_ref_pats
    = note: `-D clippy::match-ref-pats` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::match_ref_pats)]`
help: instead of prefixing all patterns with `&`, you can dereference the expression
    |
124 ~         match *tpe {
125 ~             TagType::ByteTag => TagValue::ByteValue(vec[0]),
126 ~             TagType::ASCIITag => TagValue::AsciiValue(String::from_utf8_lossy(&vec).to_string()),
127 ~             TagType::ShortTag => TagValue::ShortValue(Endian::read_u16(&vec[..])),
128 ~             TagType::LongTag => TagValue::LongValue(Endian::read_u32(&vec[..])),
129 ~             TagType::RationalTag => TagValue::RationalValue((Endian::read_u32(&vec[..(len / 2)]),
130 |                                                               Endian::read_u32(&vec[(len / 2)..]))),
131 ~             TagType::SignedByteTag => TagValue::SignedByteValue(vec[0] as i8),
132 ~             TagType::SignedShortTag => TagValue::SignedShortValue(Endian::read_i16(&vec[..])),
133 ~             TagType::SignedLongTag => TagValue::SignedLongValue(Endian::read_i32(&vec[..])),
134 ~             TagType::SignedRationalTag => TagValue::SignedRationalValue((Endian::read_i32(&vec[..(len / 2)]),
135 |                                                                           Endian::read_i32(&vec[(len / 2)..]))),
136 ~             TagType::FloatTag => TagValue::FloatValue(Endian::read_f32(&vec[..])),
137 ~             TagType::DoubleTag => TagValue::DoubleValue(Endian::read_f64(&vec[..])),
138 ~             TagType::UndefinedTag => TagValue::ByteValue(0),
    |

error: casting integer literal to `usize` is unnecessary
   --> src/reader.rs:147:18
    |
147 |             0 => 0 as usize,
    |                  ^^^^^^^^^^ help: try: `0_usize`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
    = note: `-D clippy::unnecessary-cast` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::unnecessary_cast)]`

error: casting integer literal to `u16` is unnecessary
   --> src/reader.rs:259:18
    |
259 |             _ => 0 as u16,
    |                  ^^^^^^^^ help: try: `0_u16`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

error: casting integer literal to `u16` is unnecessary
   --> src/reader.rs:263:18
    |
263 |             _ => 0 as u16,
    |                  ^^^^^^^^ help: try: `0_u16`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

error: casting integer literal to `u16` is unnecessary
   --> src/reader.rs:267:18
    |
267 |             _ => 0 as u16,
    |                  ^^^^^^^^ help: try: `0_u16`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

error: casting integer literal to `u16` is unnecessary
   --> src/reader.rs:281:18
    |
281 |             _ => 0 as u16,
    |                  ^^^^^^^^ help: try: `0_u16`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
   --> src/reader.rs:285:13
    |
285 | /             match v {
286 | |                 TagValue::LongValue(v) => offsets.push(*v),
287 | |                 _ => (),
288 | |             };
    | |_____________^ help: try: `if let TagValue::LongValue(v) = v { offsets.push(*v) }`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_match
    = note: `-D clippy::single-match` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::single_match)]`

error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
   --> src/reader.rs:292:13
    |
292 | /             match v {
293 | |                 TagValue::LongValue(v) => byte_counts.push(*v),
294 | |                 _ => (),
295 | |             };
    | |_____________^ help: try: `if let TagValue::LongValue(v) = v { byte_counts.push(*v) }`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_match

error: casting to the same type is unnecessary (`usize` -> `usize`)
   --> src/reader.rs:314:30
    |
314 |                 if curr_y >= img[curr_x].len() as usize {
    |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `img[curr_x].len()`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast

error: could not compile `geotiff` (lib) due to 27 previous errors
warning: build failed, waiting for other jobs to finish...
error: could not compile `geotiff` (lib test) due to 27 previous errors
Error: Process completed with exit code 101.

Will fix these lint errors after a couple of other refactoring PRs.

@weiji14
Copy link
Member Author

weiji14 commented Sep 8, 2024

Most clippy lints resolved after big refactoring in #17. Debating on whether to enable other clippy lint groups listed at https://doc.rust-lang.org/stable/clippy/index.html 🤔

Edit: Will just go with the default lint groups for now and add more as needed.

@weiji14 weiji14 marked this pull request as ready for review September 8, 2024 22:48
@weiji14 weiji14 merged commit 6dcad4b into main Sep 20, 2024
2 checks passed
@weiji14 weiji14 deleted the gh_action/clippy branch September 20, 2024 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant