Skip to content

Commit

Permalink
Introduce a cargo clippy run
Browse files Browse the repository at this point in the history
This will help catch stylistic problems in our Rust code.
  • Loading branch information
mgeisler committed Apr 24, 2024
1 parent 8bd2dd0 commit 3917067
Show file tree
Hide file tree
Showing 15 changed files with 80 additions and 59 deletions.
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.20.9
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.20.9
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 @@ -101,9 +101,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,
},
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

0 comments on commit 3917067

Please sign in to comment.