Skip to content

DO NOT MERGE: Deny clippy warnings in example solutions#1012

Closed
petertseng wants to merge 3 commits intoexercism:masterfrom
petertseng:clippy-ex
Closed

DO NOT MERGE: Deny clippy warnings in example solutions#1012
petertseng wants to merge 3 commits intoexercism:masterfrom
petertseng:clippy-ex

Conversation

@petertseng
Copy link
Copy Markdown
Member

@petertseng petertseng commented Nov 18, 2020

The author isn't really interested in prioritising these but wants to
at least take a look at how the situation is.

@petertseng petertseng changed the title DO NOT MERGE: DO NOT MERGE: Deny clippy warnings in example solutions Nov 18, 2020
@petertseng
Copy link
Copy Markdown
Member Author

grep -B1 src/lib.rs gha-clippy-log.txt | sed 's/^20..-..-..T..:..:..........Z //' | grep 'warning\|error' | sort | uniq -c | sort -nr
     14 error: constants have by default a `'static` lifetime
     10 error: statics have by default a `'static` lifetime
      8 error: `if` chain can be rewritten with `match`
      6 error: Use sort_unstable instead of sort
      6 error: redundant clone
      6 error: casting a character literal to `u8` truncates
      6 error: called `.nth(0)` on a `std::iter::Iterator`, when `.next()` is equivalent
      4 error: writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used with non-Vec-based slices.
      4 error: unneeded unit return type
      2 error: you should consider adding a `Default` implementation for `SimpleLinkedList<T>`
      2 error: you should consider adding a `Default` implementation for `School`
      2 error: you should consider adding a `Default` implementation for `LinkedList<T>`
      2 error: you should consider adding a `Default` implementation for `graph::Graph`
      2 error: you should consider adding a `Default` implementation for `Forth`
      2 error: you should consider adding a `Default` implementation for `BowlingGame`
      2 error: using `Result.and_then(|x| Ok(y))`, which is more succinctly expressed as `map(|x| y)`
      2 error: Using `.iter().next()` on an array
      2 error: this `.into_iter()` call is equivalent to `.iter()` and will not move the `slice`
      2 error: this `else { if .. }` block can be collapsed
      2 error: redundant pattern matching, consider using `is_some()`
      2 error: methods called `into_*` usually take self by value; consider choosing a less ambiguous name
      2 error: it looks like the same item is being pushed into this Vec
      2 error: item `SimpleLinkedList<T>` has a public `len` method but no corresponding `is_empty` method
      2 error: item `LinkedList<T>` has a public `len` method but no corresponding `is_empty` method
      2 error: defining a method called `next` on this type; consider implementing the `std::iter::Iterator` trait or choosing a less ambiguous name
      2 error: avoid using `collect()` when not needed
      1 error: this `.into_iter()` call is equivalent to `.iter()` and will not move the `array`

Will consider fixing some of these at some point, but don't really find it urgent.

@coriolinus
Copy link
Copy Markdown
Member

At this point all lints with more than 2 errors reported should be fixed.

@petertseng petertseng reopened this Nov 18, 2020
The author isn't really interested in prioritising these but wants to
at least take a look at how the situation is.
@petertseng petertseng closed this Nov 18, 2020
@petertseng
Copy link
Copy Markdown
Member Author

petertseng commented Nov 18, 2020

Yes, that is true! Excellent.

      2 error: you should consider adding a `Default` implementation for `SimpleLinkedList<T>`
      2 error: you should consider adding a `Default` implementation for `School`
      2 error: you should consider adding a `Default` implementation for `Robot`
      2 error: you should consider adding a `Default` implementation for `Reactor<'a, T>`
      2 error: you should consider adding a `Default` implementation for `LinkedList<T>`
      2 error: you should consider adding a `Default` implementation for `graph::Graph`
      2 error: you should consider adding a `Default` implementation for `Forth`
      2 error: you should consider adding a `Default` implementation for `BowlingGame`
      2 error: methods called `into_*` usually take self by value; consider choosing a less ambiguous name
      2 error: item `SimpleLinkedList<T>` has a public `len` method but no corresponding `is_empty` method
      2 error: item `LinkedList<T>` has a public `len` method but no corresponding `is_empty` method
      2 error: defining a method called `next` on this type; consider implementing the `std::iter::Iterator` trait or choosing a less ambiguous name

(updated after #1025)

petertseng added a commit that referenced this pull request Nov 18, 2020
clippy::needless_collect

Instead, check whether element exists in collection during iteration.

Helps address #1012
petertseng added a commit that referenced this pull request Nov 20, 2020
It's understandable that clippy says the `next` here conflicts with
Iterator's `next`, but I really think that's the best name for it unless
we go with something like `forward` and `back`.

There is one other example of Cursors on the internet:
https://contain-rs.github.io/linked-list/linked_list/struct.Cursor.html
This one also uses `next`, so it seems they did not find a better
solution to this than we could.

Helps address #1011
Helps address #1012
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.

2 participants