1488006 - mozprofile: Fix clippy lint warnings and errors

Status: RESOLVED FIXED

(Testing :: Mozbase Rust, enhancement)

Description

[Andreas Tolfsen ❲:ato❳]

Per title.

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: mozprofile: format code

No functional changes, apart from running all the code through
rustfmt prior to fixing clippy lint warnings.

Comment 2

[Andreas Tolfsen ❲:ato❳]

Attachment: mozprofile: avoid redundant field names in struct init

Comment 3

[Andreas Tolfsen ❲:ato❳]

Attachment: mozprofile: drop needless lifetimes

Drops all the unnecessary lifetime definitions in mozprofile.

Comment 4

[Andreas Tolfsen ❲:ato❳]

Attachment: mozprofile: prefer byte string literals to as_bytes() where possible

Where string literals contain only ASCII characters, it is considered
better style to define byte strings using b"foo" rather than calling
"foo".as_bytes().

Comment 5

[Andreas Tolfsen ❲:ato❳]

Attachment: mozprofile: replace call to io::Write::write() with ::write_all()

io::Write::write() is not guaranteed to process the entire buffer.
It returns how many bytes were processed, which may be smaller than the
given buffer's length.  mozprofile does not need to deal with partial
writes, so all the calls to ::write() may be replaced with ::write_all().

Comment 6

[Andreas Tolfsen ❲:ato❳]

Attachment: mozprofile: loop over references to container

It is considered more idiomatic Rust to loop over references to
containers rather than calling the iteration protocol's x.iter()
or x.into_iter() explicitly.

Comment 7

[Andreas Tolfsen ❲:ato❳]

Attachment: mozprofile: avoid identical conversions

PrefReaderError::new already expects a &str.

Comment 8

[Andreas Tolfsen ❲:ato❳]

Attachment: mozprofile: turn single-character strings into chars

Where we have single-character strings we can convert them to chars
for efficiency.

Comment 9

[Andreas Tolfsen ❲:ato❳]

Attachment: mozprofile: deref token to avoid & prefix

It is considered more idiomatic to dereference the match expression
than to peek at each variant through a reference.

This silences a clippy lint warning.

Comment 10

[Andreas Tolfsen ❲:ato❳]

Attachment: mozprofile: avoid manual implementation of assign operator

Comment 11

[Andreas Tolfsen ❲:ato❳]

Attachment: mozprofile: avoid incidental functional call

There is a functional call after ok_or() which is called and
evaluated before escape_char is converted due to RHS evaluation.
We can avoid this by employing a closure with ok_or_else().

Comment 12

[Andreas Tolfsen ❲:ato❳]

Attachment: mozprofile: rename Position::new() to ::start()

Users might expect that Position can be used with the Default trait
if we define a ::new() function on it.  It might be clearer to rename
this function ::start() so that it is explicit what the position is.

This silences the following clippy warning:
https://rust-lang-nursery.github.io/rust-clippy/v0.0.212/index.html#new_without_default_derive

Comment 13

[James Graham [:jgraham]]

Comment on attachment 9005840 [details] [diff] [review]
mozprofile: avoid manual implementation of assign operator

Review of attachment 9005840 [details] [diff] [review]:
-----------------------------------------------------------------

<<= seems way more obscure to me; I'd prefer to ignore that lint.

Comment 14

[James Graham [:jgraham]]

Comment on attachment 9005842 [details] [diff] [review]
mozprofile: rename Position::new() to ::start()

Review of attachment 9005842 [details] [diff] [review]:
-----------------------------------------------------------------

This seems like a needless change to the public API for a very hypothetical problem.

Comment 15

[Andreas Tolfsen ❲:ato❳]

(In reply to James Graham [:jgraham] from comment #14)
> Comment on attachment 9005842 [details] [diff] [review]
> mozprofile: rename Position::new() to ::start()
> 
> Review of attachment 9005842 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This seems like a needless change to the public API for a very hypothetical
> problem.

Then we need to implement the Default trait instead to make clippy
happy.  I thought it was semantically better to rename it, but I
can’t submit a patch for other way.

Comment 16

[Andreas Tolfsen ❲:ato❳]

Attachment: webdriver: derive Default for prefreader::Position.

The user might expect to be able to use Default as the type can be
constructed without arguments.

Comment 17

[James Graham [:jgraham]]

Comment on attachment 9006244 [details] [diff] [review]
webdriver: derive Default for prefreader::Position.

Review of attachment 9006244 [details] [diff] [review]:
-----------------------------------------------------------------

I would also have been very happy to just change the configuration to silence the warning in this case.

Comment 18

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 9005840 [details] [diff] [review]
mozprofile: avoid manual implementation of assign operator

> Then we need to implement the Default trait instead to make clippy
> happy.  I thought it was semantically better to rename it, but I
> can’t submit a patch for other way.

I actually can’t figure out how to do this, so I’m going to drop
that patch.

Comment 19

[Andreas Tolfsen ❲:ato❳]

(In reply to James Graham [:jgraham] from comment #17)
> Comment on attachment 9006244 [details] [diff] [review]
> webdriver: derive Default for prefreader::Position.
> 
> Review of attachment 9006244 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I would also have been very happy to just change the configuration to
> silence the warning in this case.

Yeah, but I couldn’t figure out how to (-:

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/03fde1788da9
mozprofile: format code; r=jgraham
https://hg.mozilla.org/integration/mozilla-inbound/rev/a680beaf94c4
mozprofile: avoid redundant field names in struct init; r=jgraham
https://hg.mozilla.org/integration/mozilla-inbound/rev/86af1d4f044e
mozprofile: drop needless lifetimes; r=jgraham
https://hg.mozilla.org/integration/mozilla-inbound/rev/a03963a35bcd
mozprofile: prefer byte string literals to as_bytes() where possible; r=jgraham
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b9df7da7d39
mozprofile: replace call to io::Write::write() with ::write_all(); r=jgraham
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f018c99822e
mozprofile: loop over references to container; r=jgraham
https://hg.mozilla.org/integration/mozilla-inbound/rev/c8373045be47
mozprofile: avoid identical conversions; r=jgraham
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b38507b76cb
mozprofile: turn single-character strings into chars; r=jgraham
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e63eff2c381
mozprofile: deref token to avoid & prefix; r=jgraham
https://hg.mozilla.org/integration/mozilla-inbound/rev/4136b6f97a9a
mozprofile: avoid incidental functional call; r=jgraham
https://hg.mozilla.org/integration/mozilla-inbound/rev/41b45139750f
mozprofile: derive Default for prefreader::Position. r=jgraham

Comment 21

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/03fde1788da9
https://hg.mozilla.org/mozilla-central/rev/a680beaf94c4
https://hg.mozilla.org/mozilla-central/rev/86af1d4f044e
https://hg.mozilla.org/mozilla-central/rev/a03963a35bcd
https://hg.mozilla.org/mozilla-central/rev/7b9df7da7d39
https://hg.mozilla.org/mozilla-central/rev/7f018c99822e
https://hg.mozilla.org/mozilla-central/rev/c8373045be47
https://hg.mozilla.org/mozilla-central/rev/5b38507b76cb
https://hg.mozilla.org/mozilla-central/rev/1e63eff2c381
https://hg.mozilla.org/mozilla-central/rev/4136b6f97a9a
https://hg.mozilla.org/mozilla-central/rev/41b45139750f

Comment 22

[David Heiberg [:dheiberg]]

Attachment: Bug 1488006 - Use assignment operator instead of manual implementation r=jgraham

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69f4d4c7a175
Use assignment operator instead of manual implementation r=jgraham

Comment 24

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/69f4d4c7a175