From 1adece89d69b9f877fed7021f398d5d2210cf01d 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 7d48b91a0..ff42180cc 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 fcaf8bb0f..f3e275b85 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,