Skip to content

Introduce a cargo clippy run #2025

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 0 additions & 30 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,36 +10,6 @@ env:
CARGO_TERM_COLOR: always

jobs:
format:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Install formatting dependencies
run: |
sudo apt update
sudo apt install gettext yapf3

- name: Install nightly rustfmt
run: |
rustup default nightly
rustup component add rustfmt

- name: Check formatting
uses: dprint/check@v2.2

typos:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Check for typos
uses: crate-ci/typos@v1.23.2
with:
config: ./.github/typos.toml

cargo:
strategy:
matrix:
Expand Down
53 changes: 53 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
name: Lint

on:
pull_request:
push:
branches:
- main

env:
CARGO_TERM_COLOR: always

jobs:
format:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Install formatting dependencies
run: |
sudo apt update
sudo apt install gettext yapf3

- name: Install nightly rustfmt
run: |
rustup default nightly
rustup component add rustfmt

- name: Check formatting
uses: dprint/check@v2.2

typos:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Check for typos
uses: crate-ci/typos@v1.23.2
with:
config: ./.github/typos.toml

clippy:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Setup Rust cache
uses: ./.github/workflows/setup-rust-cache

- name: Clippy
run: cargo clippy
2 changes: 1 addition & 1 deletion mdbook-course/src/bin/course-schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ fn timediff(actual: u64, target: u64, slop: u64) -> String {
} else if actual + slop < target {
format!("{}: ({} short)", duration(actual), duration(target - actual),)
} else {
format!("{}", duration(actual))
duration(actual).to_string()
}
}

Expand Down
2 changes: 1 addition & 1 deletion mdbook-course/src/bin/mdbook-course.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ fn main() {
);
let matches = app.get_matches();

if let Some(_) = matches.subcommand_matches("supports") {
if matches.subcommand_matches("supports").is_some() {
// Support all renderers.
process::exit(0);
}
Expand Down
20 changes: 9 additions & 11 deletions mdbook-course/src/course.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ impl Courses {
fn course_mut(&mut self, name: impl AsRef<str>) -> &mut Course {
let name = name.as_ref();
if let Some(found_idx) =
self.courses.iter().position(|course| &course.name == name)
self.courses.iter().position(|course| course.name == name)
{
return &mut self.courses[found_idx];
}
Expand All @@ -177,9 +177,7 @@ impl Courses {
&self,
chapter: &Chapter,
) -> Option<(&Course, &Session, &Segment, &Slide)> {
let Some(ref source_path) = chapter.source_path else {
return None;
};
let source_path = chapter.source_path.as_ref()?;

for course in self {
for session in course {
Expand All @@ -193,7 +191,7 @@ impl Courses {
}
}

return None;
None
}
}

Expand All @@ -202,7 +200,7 @@ impl<'a> IntoIterator for &'a Courses {
type IntoIter = std::slice::Iter<'a, Course>;

fn into_iter(self) -> Self::IntoIter {
(&self.courses).into_iter()
self.courses.iter()
}
}

Expand All @@ -216,7 +214,7 @@ impl Course {
fn session_mut(&mut self, name: impl AsRef<str>) -> &mut Session {
let name = name.as_ref();
if let Some(found_idx) =
self.sessions.iter().position(|session| &session.name == name)
self.sessions.iter().position(|session| session.name == name)
{
return &mut self.sessions[found_idx];
}
Expand Down Expand Up @@ -275,7 +273,7 @@ impl<'a> IntoIterator for &'a Course {
type IntoIter = std::slice::Iter<'a, Session>;

fn into_iter(self) -> Self::IntoIter {
(&self.sessions).into_iter()
self.sessions.iter()
}
}

Expand Down Expand Up @@ -350,7 +348,7 @@ impl<'a> IntoIterator for &'a Session {
type IntoIter = std::slice::Iter<'a, Segment>;

fn into_iter(self) -> Self::IntoIter {
(&self.segments).into_iter()
self.segments.iter()
}
}

Expand Down Expand Up @@ -403,7 +401,7 @@ impl<'a> IntoIterator for &'a Segment {
type IntoIter = std::slice::Iter<'a, Slide>;

fn into_iter(self) -> Self::IntoIter {
(&self.slides).into_iter()
self.slides.iter()
}
}

Expand Down Expand Up @@ -451,7 +449,7 @@ impl Slide {
pub fn is_sub_chapter(&self, chapter: &Chapter) -> bool {
// The first `source_path` in the slide is the "parent" chapter, so anything
// else is a sub-chapter.
chapter.source_path.as_ref() != self.source_paths.get(0)
chapter.source_path.as_ref() != self.source_paths.first()
}

/// Return the total duration of this slide.
Expand Down
2 changes: 1 addition & 1 deletion mdbook-course/src/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ impl<const N: usize> Table<N> {
for cell in iter {
write!(f, " {} |", cell)?;
}
write!(f, "\n")
writeln!(f)
}
}

Expand Down
2 changes: 1 addition & 1 deletion mdbook-course/src/replacements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub fn replace(
}
["course", "outline", course_name] => {
let Some(course) = courses.find_course(course_name) else {
return format!("not found - {}", captures[0].to_string());
return format!("not found - {}", &captures[0]);
};
course.schedule()
}
Expand Down
2 changes: 1 addition & 1 deletion mdbook-exerciser/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub fn process(output_directory: &Path, input_contents: &str) -> anyhow::Result<
Event::Text(text) => {
info!("Text: {:?}", text);
if let Some(output_file) = &mut current_file {
output_file.write(text.as_bytes())?;
output_file.write_all(text.as_bytes())?;
}
}
Event::End(TagEnd::CodeBlock) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ async fn main() -> Result<(), tokio_websockets::Error> {
println!("From server: {}", text);
}
},
Some(Err(err)) => return Err(err.into()),
Some(Err(err)) => return Err(err),
None => return Ok(()),
}
}
Expand Down
4 changes: 1 addition & 3 deletions src/error-handling/exercise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,7 @@ enum ParserError {
fn parse(input: &str) -> Result<Expression, ParserError> {
let mut tokens = tokenize(input);

fn parse_expr<'a>(
tokens: &mut Tokenizer<'a>,
) -> Result<Expression, ParserError> {
fn parse_expr(tokens: &mut Tokenizer<'_>) -> Result<Expression, ParserError> {
let tok = tokens.next().ok_or(ParserError::UnexpectedEOF)??;
let expr = match tok {
Token::Number(num) => {
Expand Down
4 changes: 2 additions & 2 deletions src/iterators/exercise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ where
N: Copy + std::ops::Sub<Output = N>,
{
// ANCHOR_END: offset_differences
let a = (&values).into_iter();
let b = (&values).into_iter().cycle().skip(offset);
let a = values.iter();
let b = values.iter().cycle().skip(offset);
a.zip(b).map(|(a, b)| *b - *a).collect()
}

Expand Down
2 changes: 2 additions & 0 deletions src/tuples-and-arrays/exercise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// Iterators are covered later.
#[allow(clippy::needless_range_loop)]
// ANCHOR: solution
// ANCHOR: transpose
fn transpose(matrix: [[i32; 3]; 3]) -> [[i32; 3]; 3] {
Expand Down
2 changes: 2 additions & 0 deletions src/types-and-values/exercise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

// Omitting `return` is covered later.
#[allow(clippy::needless_return)]
// ANCHOR: solution
// ANCHOR: fib
fn fib(n: u32) -> u32 {
Expand Down
3 changes: 2 additions & 1 deletion src/unsafe-rust/exercise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#[allow(clippy::upper_case_acronyms)]
// ANCHOR: solution
// ANCHOR: ffi
mod ffi {
Expand Down Expand Up @@ -40,7 +41,7 @@ mod ffi {
}

// Layout according to the macOS man page for dir(5).
#[cfg(all(target_os = "macos"))]
#[cfg(target_os = "macos")]
#[repr(C)]
pub struct dirent {
pub d_fileno: u64,
Expand Down
9 changes: 3 additions & 6 deletions third_party/rust-on-exercism/health-statistics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,9 @@ impl User {
patient_name: &self.name,
visit_count: self.visit_count as u32,
height_change: measurements.height - self.height,
blood_pressure_change: match self.last_blood_pressure {
Some(lbp) => {
Some((bp.0 as i32 - lbp.0 as i32, bp.1 as i32 - lbp.1 as i32))
}
None => None,
},
Comment on lines -40 to -45
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably exclude third_party/ somehow..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, good idea! Let me figure out how to do that before merging!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the settings are done in the workspace Cargo.toml and every Cargo.toml has a workspace = true in the [lints] section as described in https://doc.rust-lang.org/cargo/reference/workspaces.html#the-lints-table we could tell cargo clippy what to do in the third_party Cargo.toml.

This would enable a central configuration for clippy and gives the option to override specific settings for individual components

blood_pressure_change: self
.last_blood_pressure
.map(|lbp| (bp.0 as i32 - lbp.0 as i32, bp.1 as i32 - lbp.1 as i32)),
};
self.height = measurements.height;
self.last_blood_pressure = Some(bp);
Expand Down