Add CI job for linting using cargo clippy

[echeran]

According to the landing page for this Github Action for Clippy, it will insert clippy warnings inline into your PR. In my test case, it appended a separate pseudo-job called "clippy" into the Github Actions UI page for the workflow run that also lists the warning.

Closes #98

[coveralls]

Pull Request Test Coverage Report for Build 3021ee7358159a3922b9b60b455243201b9ffbb8-PR-161

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 87.199%

---

Totals
Change from base Build a9740d75fad4b69e6fd5c546095d468eaafb69a5:	0.0%
Covered Lines:	688
Relevant Lines:	789

---

💛 - Coveralls

[sffc]

Clippy found some problems. The check should fail, but it's still green.

What are the HTML annotations? Is there a way to make those visible?

Clippy results: 0 ICE, 0 errors, 3 warnings, 0 notes, 0 help
##[error]Unable to create clippy annotations! Reason: HttpError: Resource not accessible by integration
##[warning]It seems that this Action is executed from the forked repository.
##[warning]GitHub Actions are not allowed to create Check annotations, when executed for a forked repos. See https://github.com/actions-rs/clippy-check/issues/2 for details.
Posting clippy checks here instead.
warning: you seem to be trying to use `&Box<T>`. Consider using just `&T`
   --> components/data-provider/src/lib.rs:107:20
    |
107 |         let boxed: &mut Box<dyn CloneableAny> = self.payload.to_mut();
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&mut dyn CloneableAny`
    |
    = note: `#[warn(clippy::borrowed_box)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#borrowed_box


warning: explicit lifetimes given in parameter types where they could be elided (or replaced with `'_` if needed by type declaration)
   --> components/data-provider/src/lib.rs:150:5
    |
150 |     pub fn with_borrowed_payload<'d, T: 'static + Clone + Debug>(self, t: &'d T) -> Response<'d> {
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: `#[warn(clippy::needless_lifetimes)]` on by default
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_lifetimes


warning: defining a method called `from_str` on this type; consider implementing the `std::str::FromStr` trait or choosing a less ambiguous name
  --> components/data-provider-json/src/lib.rs:45:5
   |
45 | /     pub fn from_str(data: &str) -> Result<Self, Error> {

[echeran]

I added new changes to address Shane's comment about making the job fail on clippy warnings too (not just on errors). The changes made this PR now make the CI tests fail as expected until Shane's fix in #162 is merged.

FYI: Sorry for force-pushing, I was trying to test changes to address Shane's comment on my personal fork and forgot that the branch is also connected to a PR. Afterwards, I still needed to add a workaround within the fix, and remembered to keep that a separate commit.

[echeran]

I'm not sure what you meant by HTML annotations -- are you referring to the links within each of the warning messages? I don't know how to make those links' text clickable.

The best I could do was to add a workaround in the latest commit so that you could have the warnings inserted into Github UI (via the Rust community-supported action) while still having the PR checks fail on warnings.

[sffc]

If you don't cache the cargo build, does that avoid the need for the cargo clean?

[echeran]

Good catch -- without the caching step (which is restoring the cache at the beginning), cargo clippy doesn't need the cargo clean step in order to still fail on warnings. I updated accordingly. This holds true, at least on CI, because each Github Actions job is run on a fresh VM image. (Hence, the need to start off each job with actions/checkout@v2 to clone the repo in the job's VM.)

[sffc]

Rather than making a workaround to a deficiency in actions-rs (the lack of ability to make the check fail on warnings), should we consider fixing the problem upstream? It would help simplify our code, and then the whole ecosystem can benefit. I filed an issue: actions-rs/clippy-check#82

[echeran]

Gah, after doing some poking around, I discovered that the desired fix was already made in v1.0.2, and I was just using v1 like in the example docs. I set the version to v1.0.7 to get the latest, FWIW. The change seems to work in my test. I'll update the new upstream issue accordingly.

[sffc]

Nice!

I think the order of operations should be:

1. Merge Fix cargo clippy warnings in data provider code #162, which needs a new rubber stamp
2. Merge master into this branch (or squash+rebase if you prefer)
3. If the new action is green, then merge!

[echeran]

Now that #162 is merged, the clippy linter action passes. I rebased the branch locally against the latest from master and then did a push-force, so need another stamp on the PR.

[sffc]

Approved!

Nit: For the future, IMHO, if you're going to change history (rebase), you might as well squash while you're at it, if the intermediate commits aren't meaningful.