Page MenuHomeWildfire Games

Linting: Enable no-caller and no-irregular-whitespace in ESLint
ClosedPublic

Authored by Krinkle on Sat, Aug 31, 5:25 PM.

Details

Summary

These are analogous to the JSHint options "noarg" and "nobsp", which are already enabled in build/jenkins/lint-config/jshint.json.

From https://jshint.com/docs/options/:

  • nonbsp: This option warns about "non-breaking whitespace" characters in source code.
  • noarg: This option prohibits the use of arguments.caller and arguments.callee.

From https://eslint.org/docs/rules/no-caller:

This rule is aimed at discouraging the use of deprecated and sub-optimal
code by disallowing use of arguments.caller and arguments.callee.

From https://eslint.org/docs/rules/no-irregular-whitespace:

Invalid or irregular whitespace makes code harder to debug in a similar nature to mixed tabs and spaces.
This rule disallows the following characters: \u00A0 <NBSP>, …

Test Plan

When running ESLint over the whole of data/binaries/ with eslint.json reduced to just these new keys, it finds zero existing violations of this rule.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Krinkle created this revision.Sat, Aug 31, 5:25 PM
Owners added a subscriber: Restricted Owners Package.Sat, Aug 31, 5:25 PM
Krinkle edited the summary of this revision. (Show Details)Sat, Aug 31, 5:26 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/523/display/redirect

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/14/display/redirect

elexis accepted this revision.Thu, Sep 12, 11:34 PM
elexis added a subscriber: elexis.

When running ESLint over the whole of data/binaries/ with eslint.json reduced to just these new keys, it finds zero existing violations of this rule.

I did not verify this, but from my experience you are accurate about these things.

Usually I would say its best to minimize bureucracy, and go through all options for once and for all so that we don't have a neverending list of small steps.
At least if the scope is managable.
I remember going through that list of options once, when the first version of the file was committed: rP20364 / D213.
Then again I don't mind putting an end to this patch now by committing it.

Yes I fully agree with banning whitespace that isnt space, tab or newline.

Yes I see that arguments.callee is already illegal in ES5 strict mode:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/arguments/callee

https://eslint.org/docs/rules/no-caller

This rule was introduced in ESLint 0.0.6.

https://eslint.org/docs/rules/no-irregular-whitespace

This rule was introduced in ESLint 0.9.0.

It looks like these options aren't new either:
https://github.com/eslint/eslint/releases?after=v5.12.1

So it seems like in the original review I only checked for correctness, but not compelteness.
So basically if we wanted to be complete, one can look at all the other options, meh.

Thanks for the patch Krinkle!

This revision is now accepted and ready to land.Thu, Sep 12, 11:34 PM