Fix cargo clippy warnings in data provider code

[sffc]

Wait until #160 is ready before merging this one.

[coveralls]

Pull Request Test Coverage Report for Build 8a09ae6b8b41947ba0c6bc2c99b43b339f69c1a6-PR-162

• 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 87.199%

---

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

---

💛 - Coveralls

[zbraniecki]

In callers, instead of including FromStr, you can just do x.parse()

[sffc]

Thanks. The problem with .parse is that the compiler often doesn't have enough information to infer the type. I find FromStr clearer when the primary goal is constructing an object.

let a: A = "foo".parse();
let a = A::from_str("foo");

What level is your comment? If your comment is optional, I'd rather leave it as-is. If your comment is suggestion or required, I'll take your advice and change it.

[zbraniecki]

The problem with .parse is that the compiler often doesn't have enough information to infer the type. I find FromStr clearer when the primary goal is constructing an object.

In my experience that's actually a good thing. It basically means that if the context is strong enough (for example, you're passing the foo.parse() to a function that accepts the target), then you can skip the annotation.
Otherwise, you need to annotate either via Target::from_str or via let x: Target = foo.parse(). Both are totally ok.

In result, I'm quite flexibly using both in my experience but found myself using parse more often :)

What level is your comment? If your comment is optional, I'd rather leave it as-is. If your comment is suggestion or required, I'll take your advice and change it.

Suggestion, optional. Feel free to use from_str. It's correct, understood by users and there's no problem with it.
My guess is that as we polish the APIs more, we'll end up using parse more, but not exclusively.

[echeran]

I updated #161 according to your comments to make the clippy linting strict as it should be (fail on warnings). When I run this PR with the new strict check command -- cargo clean && cargo clippy --all-targets --all-features -- -D warnings -- I get a couple of new warnings (errors), but they're minor:

error: Constants have by default a `'static` lifetime
  --> components/data-provider-json/tests/test_no_std.rs:11:14
   |
11 | const DATA: &'static str = r#"{
   |             -^^^^^^^---- help: consider removing `'static`: `&str`
   |
   = note: `-D clippy::redundant-static-lifetimes` implied by `-D warnings`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes

error: unneeded `return` statement
  --> components/data-provider-json/tests/test_no_std.rs:26:5
   |
26 | /     return provider
27 | |         .load(&datap::Request {
28 | |             langid: "en-US".parse().unwrap(),
29 | |             category: datap::Category::Decimal,
...  |
32 | |         })
33 | |         .unwrap();
   | |__________________^
   |
   = note: `-D clippy::needless-return` implied by `-D warnings`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
help: remove `return`
   |
26 |     provider
27 |         .load(&datap::Request {
28 |             langid: "en-US".parse().unwrap(),
29 |             category: datap::Category::Decimal,
30 |             key: datap::decimal::Key::SymbolsV1.into(),
31 |             payload: None,
 ...

error: this import is redundant
 --> components/data-provider-json/tests/test_file_io.rs:1:1
  |
1 | use icu_data_provider_json;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it entirely
  |
  = note: `-D clippy::single-component-path-imports` implied by `-D warnings`
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports

error: aborting due to 2 previous errors

error: could not compile `icu-data-provider-json`.

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: aborting due to previous error

error: could not compile `icu-data-provider-json`.

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed

[sffc]

I fixed the additional warnings uncovered by cargo clippy --all-targets --all-features

[echeran]

Don't forget the -- -D warnings part of the command. And the cargo clean && part ensures that the error code is non-zero on warnings (matters more for CI than for local testing). With cargo clean && cargo clippy --all-targets --all-features -- -D warnings, which is currently what #161 is set up with, I still get one last little thing:

error: this import is redundant
 --> components/data-provider-json/tests/test_file_io.rs:1:1
  |
1 | use icu_data_provider_json;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove it entirely
  |
  = note: `-D clippy::single-component-path-imports` implied by `-D warnings`
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports

error: aborting due to previous error

error: could not compile `icu-data-provider-json`.

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed

[echeran]

One last little clippy warning to clear. I put the details in this comment

[sffc]

Odd, because I don't get that warning on my computer.

omnicu$ cargo clean && cargo clippy --all-targets --all-features -- -D warnings
   Compiling autocfg v1.0.0
   Compiling proc-macro2 v1.0.18
...
    Finished dev [unoptimized + debuginfo] target(s) in 29.54s
omnicu$ cargo clippy --all-targets --all-features -- -D warnings
    Finished dev [unoptimized + debuginfo] target(s) in 0.05s
omnicu$ cargo --version
cargo 1.42.0 (86334295e 2020-01-31)
omnicu$ cargo clippy --version
clippy 0.0.212 (4ee1206 2020-02-01)
omnicu$

[sffc]

OK, I ran rustup update and now I'm able to reproduce the warning. Pushed a fix.

[echeran]

LGTM.

FYI, this PR here needs to be merged first before #161 can be merged.

[sffc]

@zbraniecki @nciric I need at least one approval from a code owner