clippy: Fix some warnings in components/script/timers.rs

[jahielkomu]

Fixed clippy warnings in components/script/timers.rs

• ./mach build -d does not report any errors
• ./mach test-tidy does not report any errors
• These changes are part of Fix as many clippy problems as possible #31500
• These changes do not require tests because it doesn't change functionality

[jahielkomu]

Hello @mrobinson, I hope you are well.
This is komuhangi, an outreachy applicant.
Please kindly review.

[mrobinson]

Please don't add dead code. I think you can replace all calls JSTimers::new() with JSTimers::default() everywhere and move the contents of new() into default().

[jahielkomu]

Thank you @mrobinson
Indeed the implementation of JsTimers::default behavior is unused.
However won't the change and replacement of new() with default() have effects on the use of JsTimers::new()

[mrobinson]

Yes, every usage of JSTimers::new() should be replaced with JSTimers::default().

[jahielkomu]

Thank you @mrobinson Indeed the implementation of JsTimers::default behavior is unused. However won't the change and replacement of new() with default() have effects on the use of JsTimers::new()

Apologies @mrobinson
I have reread your review and it does make sense.

[jahielkomu]

I have update the PR @mrobinson
Please kindly review.

[mrobinson]

This method should also be the implementation of the Default trait. Here's an example of this from the rust documentation: https://doc.rust-lang.org/std/default/trait.Default.html#how-can-i-implement-default

[mrobinson]

This method should also be the implementation of the Default trait. Here's an example of this from the rust documentation: https://doc.rust-lang.org/std/default/trait.Default.html#how-can-i-implement-default

[jahielkomu]

Hello @mrobinson.

I have update the PR, please review and revert.
I did not think that it was necessary to change the visibility of the default associated function to default it to the surrounding visibility.

[mrobinson]

You'll need to have the default() method inside a block like this:

impl Default for JsTimers {
 ...
}

but the rest of the methods will need to be inside a block that looks like this:

impl JsTimers {
 ...
}

[mrobinson]

I have update the PR, please review and revert. I did not think that it was necessary to change the visibility of the default associated function to default it to the surrounding visibility.

@jahielkomu I left a comment above. The build is failing because you have surrounded all JsTimer structure functions with the Default implementation block. The thing is that you need only put this kind of block around the functions necessary to implement the Default trait.

[jahielkomu]

I have update the PR, please review and revert. I did not think that it was necessary to change the visibility of the default associated function to default it to the surrounding visibility.

@jahielkomu I left a comment above. The build is failing because you have surrounded all JsTimer structure functions with the Default implementation block. The thing is that you need only put this kind of block around the functions necessary to implement the Default trait.

Yes @mrobinson
I realized I confused myself from the beginning.

I have updated the PR and have separated the Default implementation from the rest of the methods as in

impl Default for JsTimers {
...
}

then

impl JsTimers {
...
}

Instead of

impl JsTimers {
impl Default for JsTimers {
...
} 
...
}

Please review and revert.
Thank you.