Closed Bug 1359350 Opened 2 years ago Closed 2 years ago

Add no-eval rule to eslint config

Categories

(Firefox Build System :: Lint and Formatting, enhancement)

3 Branch
enhancement
Not set

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: freddyb, Assigned: freddyb)

References

()

Details

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #1358050 +++

We discourage the use of eval.
string arguments to setInterval/setTimeout are disallowed with 1358050.

I suggest we also add no-eval
FWIW, I misremembered that no-implied-eval included eval. but it was only setTimeout, setInterval, so here's the second round. Patch incoming.
Assignee: nobody → fbraun
Status: NEW → ASSIGNED
Standard8 suggested I file bugs for the whitelisted failures and he will take some of them as mentored bugs.
Still looking into how to do that fast and automatically.
Comment on attachment 8865424 [details]
Bug 1359350: Add no-eval rule to eslint config (and whitelist failures in tests)

https://reviewboard.mozilla.org/r/137092/#review140152

Looks good. r=Standard8

For filing the bugs, I think it might be worth looking at grouping issues into various areas, e.g. just one bug for dom/indexedDB to cover fixing all the tests there as they have the same issue, etc.

::: .eslintrc.js:22
(Diff revision 2)
> -
>      // No (!foo in bar) or (!object instanceof Class)
>      "no-unsafe-negation": "error",
>      // No eval() and no strings in the first param of setTimeout or setInterval
>      "no-implied-eval": "error",
> +    "no-eval": "error",

Please can you also add this to tools/lint/eslint/eslint-plugin-mozilla/lib/configs/recommended.js so that we have it there for when we apply it globally (and also for our github projects that use the plugin).

Whilst you're there, could you also bump the version number in tools/lint/eslint/eslint-plugin-mozilla/package.json to 0.2.47 - I really ought to do another release for that soon (generally it doesn't need bumping on each update of the plugin files now, but I've not done a release for a while).
Comment on attachment 8865424 [details]
Bug 1359350: Add no-eval rule to eslint config (and whitelist failures in tests)

https://reviewboard.mozilla.org/r/137092/#review140154
Attachment #8865424 - Flags: review?(standard8) → review+
Thanks for the speedy review.
Waiting for try results before asking for checkin-needed: https://reviewboard.mozilla.org/r/137092/#comment181908
All green.
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/f71cd863be6e
Add no-eval rule to eslint config (and whitelist failures in tests) r=standard8
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/f71cd863be6e
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Product: Testing → Firefox Build System
Version: Version 3 → 3 Branch
You need to log in before you can comment on or make changes to this bug.