From 8ef504c01d09732bb450dcba33fa19400d0bab06 Mon Sep 17 00:00:00 2001 From: Skyler Grey Date: Mon, 18 Dec 2023 16:25:40 +0000 Subject: [PATCH] Make eslint in make check fail on eslint warnings As discussed [on Matrix](https://col.la/suppresseslintindevelopment) the set of things we want to pick up in CI and development are different. Specifically, there are some things that only get in the way during development but should not be allowed in merged code. Examples are 'debugger' statemens, dead code (e.g. resulting from 'if (false)') and unused variables. After #7822 there are no more eslint warnings, so we can use it as a separate state for these issues that should only block in CI. This commit make make check run eslint and fail if it receives any warnings, and changes those 3 errors so that they only emit warnings. It's expected that there are more annoying warnings which I've missed, if you have any please make a followup change! This commit explicitly does not deal with formatting-related issues (e.g. requiring single quotes for strings in eslint) as though they can be annoying there's no reason why they should change between development and master... another followup to improve linting experience could be to use an autoformatter so the computer fixes the formatting for you, however that's out-of-scope for this commit Change-Id: I036afac5ef5056a9cc2effc21e31165aa1436ad2 Signed-off-by: Skyler Grey --- Makefile.am | 10 +++++++++- browser/.eslintrc | 6 +++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Makefile.am b/Makefile.am index 7d48b91a029b5..ff42180ccea68 100644 --- a/Makefile.am +++ b/Makefile.am @@ -630,6 +630,14 @@ compile_commands: $(abs_srcdir)/compile_commands.json browser/node_modules: browser/package.json browser/archived-packages @cd browser && npm install +eslint: browser/node_modules + browser/node_modules/.bin/eslint browser/src browser/js browser/admin/src \ + --max-warnings 0 \ + --resolve-plugins-relative-to browser \ + --ignore-path browser/.eslintignore \ + --no-eslintrc \ + --config browser/.eslintrc + install-exec-hook: cd $(DESTDIR)$(bindir) && \ $(LN_S) coolconfig loolconfig && \ @@ -671,7 +679,7 @@ check-for-system-nss: fi \ fi -check: check-for-system-nss check-recursive +check: check-for-system-nss check-recursive eslint $(GEN_COVERAGE_COMMAND) coverage-report: diff --git a/browser/.eslintrc b/browser/.eslintrc index fcaf8bb0f14c1..f3e275b858027 100644 --- a/browser/.eslintrc +++ b/browser/.eslintrc @@ -20,7 +20,11 @@ "no-control-regex": 0, "no-useless-escape": 0, "semi": 2, - "no-redeclare": 0 + "no-redeclare": 0, + /// Rules that are set to warn will fail in CI but not when building for development: + "no-debugger": 1, + "no-unreachable": 1, + "no-unused-vars": 1, }, "globals": { "L": true,