Fix no-unused-vars issues

[ghost]

This PR seeks to resolve all no-unused-vars ESLint issues in the codebase. Although useful on its own, it is focused on helping to land #6945

Each commit message references the commit where the changes should have been done, as a way to help others to review.

cc @nb

Test live: https://calypso.live/?branch=fix/no-unused-vars

[ghost]

This is ready to be reviewed @nb

Now that all no-unused-vars issues in the codebase are fixed, I would propose set this as an error, to help landing #6945

[aduth]

Because we extend eslint-config-wpcalypso we can simply remove the line here because it only existed to downgrade from the default error to warning.

[nb]

It looks like unused arguments are rarely legitimate, @nssw, @aduth, what do you think about relaxing the rule?

[nb]

@nssw this needs a rebase – a lot of files were moved around (sorry, you weren’t lucky :) ).

[ghost]

Rebase done. I had already rebased yesterday but I guess this was bound to happen with such a cross-project PR :)

It looks like unused arguments are rarely legitimate

Thanks for the comment @nb ! I was hoping to receive something like this. I haven't left a comment for reviewers about that to avoid anchoring the topic.

So: I am not sure about this. On the one hand, I like how leaving not used arguments/variables/etc in some places (signatures for functions that process events, errs/success callbacks where you do not use the arguments, positional variables, etc) documents the code and the expectations, in a manner. On the other hand, I am not sure whether having the exception for those cases could be a source of subtle bugs yet to comprehend or whether it is a bad practice for some other reason.

I have chosen the first route, just because, well, I needed to choose one. But I would be equally happy to choose the other if more eyes see this as a code smell somehow.

[nb]

I have chosen the first route, just because, well, I needed to choose one. But I would be equally happy to choose the other if more eyes see this as a code smell somehow.

I think it’s okay for now. After looking through your changes, the cases when we need to add line-level exception are few enough.

I read through the changes and I noticed two more things:

• In few occasions it was hard for me to know which is the unused variable we added an exception for – maybe we should add some extra clarification for each exception saying what’s the variable and why are we adding the exception?
• The whitespace/indentation changes were often distracting for me, do we need them in this PR?

[nb]

Squashing and merging, thanks for working and following up on that @nssw!

[ghost]

Thanks to you @nb for you patience, the good suggestions and the time you have taken for reviewing this!

[gziolo]

Nice one! We need more tasks like that 👍