From 02e15da51b8dabfbf06287efcfc3b4d279fce132 Mon Sep 17 00:00:00 2001
From: Ralph Sennhauser <ralph.sennhauser@gmail.com>
Date: Wed, 4 Sep 2024 17:34:54 +0200
Subject: [PATCH] Add eslint pre-commit hook

This replaces the old arclint eslint setup 1:1 rule wise, only porting
the configuration to a format recent eslint can read.

Further remove the arclint setup as it is no longer of use.

Thanks to Stan for reviewing all needed fixes to Javascript code to
allow for adding this without compromises.

Fixes: #7812
Signed-off-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
---
 .gitignore                                  |   4 +
 .pre-commit-config.yaml                     |  10 ++
 build/arclint/README.md                     |  12 --
 build/arclint/configs/eslintrc.json         |  97 --------------
 build/arclint/dummies/eslint.bat            |   2 -
 build/arclint/dummies/eslint.php            |  45 -------
 build/arclint/pyrolint/src/ESLintLinter.php | 122 -----------------
 eslint.config.mjs                           | 139 ++++++++++++++++++++
 package.json                                |  11 ++
 source/tools/lint/README.md                 |  14 ++
 10 files changed, 178 insertions(+), 278 deletions(-)
 delete mode 100644 build/arclint/configs/eslintrc.json
 delete mode 100644 build/arclint/dummies/eslint.bat
 delete mode 100755 build/arclint/dummies/eslint.php
 delete mode 100644 build/arclint/pyrolint/src/ESLintLinter.php
 create mode 100644 eslint.config.mjs
 create mode 100644 package.json

diff --git a/.gitignore b/.gitignore
index 57c5f806f4..033a65b67e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -87,6 +87,10 @@ binaries/data/mods/**/*.jpg
 # Vulkan SPIR-V shaders
 binaries/data/mods/*/shaders/spirv
 
+# eslint
+node_modules
+package-lock.json
+
 # Windows specific data
 desktop.ini
 Thumbs.db
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index d920583fa1..08d6b58f57 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -72,3 +72,13 @@ repos:
       - id: yamllint
         args:
           - --strict
+  - repo: https://github.com/pre-commit/mirrors-eslint
+    rev: v9.27.0
+    hooks:
+      - id: eslint
+        additional_dependencies:
+          - eslint@9.27.0
+          - eslint-plugin-brace-rules@0.1.6
+        args:
+          - --max-warnings=0
+          - --no-warn-ignored
diff --git a/build/arclint/README.md b/build/arclint/README.md
index 6db47cea95..f0e58a499c 100644
--- a/build/arclint/README.md
+++ b/build/arclint/README.md
@@ -8,7 +8,6 @@ https://secure.phabricator.com/book/phabricator/article/arcanist_lint/
 
 - `text` is configured to detect whitespace issues.
 - `json` detects JSON syntax errors.
-- `eslint`, if installed, will run on javascript files.
 
 ## Installation
 
@@ -21,14 +20,3 @@ Configuration is at the root of the project, under `.arclint`.
 ### Installing linters
 
 We provide dummy replacement for external linters, so that they are not required.
-
-#### eslint
-
-Installation via npm is recommended. The linter assumes a global installation
-of both eslint and the "brace-rules" plugin.
-
-```
-npm install -g eslint@latest eslint-plugin-brace-rules`
-```
-
-See also https://eslint.org/docs/user-guide/getting-started
diff --git a/build/arclint/configs/eslintrc.json b/build/arclint/configs/eslintrc.json
deleted file mode 100644
index 2db5faa384..0000000000
--- a/build/arclint/configs/eslintrc.json
+++ /dev/null
@@ -1,97 +0,0 @@
-{
-    "parserOptions": {
-        "ecmaVersion": 2022
-    },
-    "plugins": [
-        "brace-rules"
-    ],
-    "rules": {
-        "no-caller": 1,
-        "no-cond-assign": 1,
-        "no-constant-condition": ["error", { "checkLoops": false }],
-        "no-dupe-args": 1,
-        "no-dupe-keys": 1,
-        "no-duplicate-case": 1,
-        "no-empty": 1,
-        "no-extra-boolean-cast": 0,
-        "no-extra-parens": 0,
-        "no-extra-semi": 1,
-        "no-floating-decimal": 1,
-        "no-func-assign": 1,
-        "no-negated-in-lhs": 1,
-        "no-obj-calls": 1,
-        "no-unreachable": 1,
-        "no-use-before-define": ["error", "nofunc"],
-        "use-isnan": 1,
-        "valid-jsdoc": 0,
-        "valid-typeof": 1,
-
-        "block-scoped-var": 0,
-        "consistent-return": 1,
-        "default-case": 1,
-        "dot-notation": 1,
-        "no-else-return": 1,
-        "no-invalid-this": 1,
-        "no-loop-func": 0,
-        "no-multi-spaces": ["warn", { "ignoreEOLComments": true }],
-        "no-new": 1,
-        "no-redeclare": 0,
-        "no-return-assign": 1,
-        "no-self-assign": 1,
-        "no-self-compare": 1,
-        "no-unmodified-loop-condition": 1,
-        "no-unused-expressions": 1,
-        "no-unused-labels": 1,
-        "no-useless-concat": 0,
-        "yoda": 1,
-
-        "no-delete-var": 1,
-        "no-label-var": 1,
-        "no-shadow-restricted-names": 1,
-        "no-shadow": 1,
-        "no-undef": 0,
-        "no-undef-init": 1,
-        "no-unused-vars": 0,
-
-        "comma-spacing": 1,
-        "indent": ["warn", "tab", { "outerIIFEBody": 0 }],
-        "key-spacing": 1,
-        "new-cap": 0,
-        "new-parens": 1,
-        "no-mixed-spaces-and-tabs": ["warn", "smart-tabs"],
-        "no-multi-assign": 1,
-        "no-trailing-spaces": 1,
-        "no-unneeded-ternary": 1,
-        "no-irregular-whitespace": 1,
-        "object-curly-spacing": ["warn", "always"],
-        "operator-assignment": 1,
-        "operator-linebreak": ["warn", "after"],
-        "quote-props": 1,
-        "semi": 1,
-        "semi-spacing": 1,
-        "space-before-function-paren": ["warn", "never"],
-        "space-in-parens": 1,
-        "space-unary-ops": 1,
-        "spaced-comment": ["warn", "always"],
-
-        "no-class-assign": 1,
-        "no-const-assign": 1,
-        "no-dupe-class-members" : 1,
-        "prefer-const": 1,
-
-        "brace-rules/brace-on-same-line": ["warn", {
-            "FunctionDeclaration": "never",
-            "FunctionExpression": "ignore",
-            "ArrowFunctionExpression": "always",
-            "IfStatement": "never",
-            "TryStatement": "ignore",
-            "CatchClause": "ignore",
-            "DoWhileStatement": "never",
-            "WhileStatement": "never",
-            "ForStatement": "never",
-            "ForInStatement": "never",
-            "ForOfStatement": "never",
-            "SwitchStatement": "never"
-        }, { "allowSingleLine": true }]
-    }
-}
diff --git a/build/arclint/dummies/eslint.bat b/build/arclint/dummies/eslint.bat
deleted file mode 100644
index 8505eb7433..0000000000
--- a/build/arclint/dummies/eslint.bat
+++ /dev/null
@@ -1,2 +0,0 @@
-@echo off
-php build\arclint\dummies\eslint.php
diff --git a/build/arclint/dummies/eslint.php b/build/arclint/dummies/eslint.php
deleted file mode 100755
index 84a7e68e12..0000000000
--- a/build/arclint/dummies/eslint.php
+++ /dev/null
@@ -1,45 +0,0 @@
-#!/usr/bin/env php
-<?php
-/**
- * Copyright (C) 2021 Wildfire Games.
- * This file is part of 0 A.D.
- *
- * 0 A.D. is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 2 of the License, or
- * (at your option) any later version.
- *
- * 0 A.D. is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with 0 A.D.  If not, see <http://www.gnu.org/licenses/>.
- */
-
-
-/**
- * This file replaces eslint if the former is not found, to avoid failure in 'arc lint'.
- * It is written in PHP as we can assume php is installed if arcanist is to work at all.
- * It mimics `eslint --format json`.
- * Set the VERBOSE env variable to generate an 'advice' level lint message.
- */
-
-$verbose = getenv("VERBOSE") ? getenv("VERBOSE") : false;
-
-$advice = <<<EOD
-{
-	"filePath": "build/arclint/dummies/eslint.php",
-	"messages": [{
-		"ruleId": "skipped",
-		"severity": 0,
-		"message": "ESLint not found - skipped",
-		"line": 23,
-		"column": 0
-	}]
-}
-EOD;
-
-fwrite(STDOUT, $verbose ? "[$advice]" : "[]");
-?>
diff --git a/build/arclint/pyrolint/src/ESLintLinter.php b/build/arclint/pyrolint/src/ESLintLinter.php
deleted file mode 100644
index e54cccad3e..0000000000
--- a/build/arclint/pyrolint/src/ESLintLinter.php
+++ /dev/null
@@ -1,122 +0,0 @@
-<?php
-/**
- * Copyright 2016 Pinterest, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-/**
- * Copyright 2021 Wildfire Games.
- * Lints JavaScript via ESlint.
- * Heavily modified from pinterest/arcanist-linters. Original licence above.
- */
-final class ESLintLinter extends ArcanistExternalLinter {
-
-  private $config;
-
-  public function getLinterName() {
-    return 'ESLINT';
-  }
-
-  public function getLinterConfigurationName() {
-    return 'eslint';
-  }
-
-  public function getDefaultBinary() {
-    return 'eslint';
-  }
-
-  public function getInstallInstructions() {
-    return pht(
-      'Install eslint using npm install -g eslint.',
-      'Install the brace-rule plugin using',
-      'npm install -g eslint-plugin-brace-rules');
-  }
-
-  protected function getMandatoryFlags() {
-    list($err, $stdout, $stderr) = exec_manual('npm root -g');
-    return array(
-      '--format=json',
-      '--no-color',
-      '--config',
-      $this->config,
-      // This allows globally installing plugins even with eslint 6+
-      '--resolve-plugins-relative-to',
-      strtok($stdout, "\n")
-    );
-  }
-
-  public function getLinterConfigurationOptions() {
-    $options = array(
-      'config' => array(
-        'type' => 'optional string',
-        'help' => pht('Link to the config file.'),
-      ),
-    );
-    return $options + parent::getLinterConfigurationOptions();
-  }
-
-  public function setLinterConfigurationValue($key, $value) {
-    switch ($key) {
-      case 'config':
-        $this->config = $value;
-        return;
-    }
-    return parent::setLinterConfigurationValue($key, $value);
-  }
-
-  protected function parseLinterOutput($path, $err, $stdout, $stderr) {
-    // Gate on $stderr b/c $err (exit code) is expected.
-    if ($stderr) {
-      return false;
-    }
-
-    $json = json_decode($stdout, true);
-    $messages = array();
-
-    foreach ($json as $file) {
-      foreach ($file['messages'] as $offense) {
-        // Skip file ignored warning: if a file is ignored by .eslintingore
-        // but linted explicitly (by arcanist), a warning will be reported,
-        // containing only: `{fatal:false,severity:1,message:...}`.
-        if (strpos($offense['message'], "File ignored ") === 0) {
-          continue;
-        }
-
-        $message = new ArcanistLintMessage();
-        $message->setPath($file['filePath']);
-        $message->setSeverity($this->mapSeverity(idx($offense, 'severity', '0')));
-        $message->setName(nonempty(idx($offense, 'ruleId'), 'unknown'));
-        $message->setDescription(idx($offense, 'message'));
-        $message->setLine(idx($offense, 'line'));
-        $message->setChar(idx($offense, 'column'));
-        $message->setCode($this->getLinterName());
-        $messages[] = $message;
-      }
-    }
-
-    return $messages;
-  }
-
-  private function mapSeverity($eslintSeverity) {
-    switch($eslintSeverity) {
-      case '0':
-        return ArcanistLintSeverity::SEVERITY_ADVICE;
-      case '1':
-        return ArcanistLintSeverity::SEVERITY_WARNING;
-      case '2':
-      default:
-        return ArcanistLintSeverity::SEVERITY_ERROR;
-    }
-  }
-}
diff --git a/eslint.config.mjs b/eslint.config.mjs
new file mode 100644
index 0000000000..63e9cf5565
--- /dev/null
+++ b/eslint.config.mjs
@@ -0,0 +1,139 @@
+// Hack to get eslint run via pre-commit to find the braces plugin in the pre-commit cache,
+// should be 'import braceRules from "eslint-plugin-brace-rules";'
+import { createRequire } from 'node:module';
+const require = createRequire(import.meta.url);
+const braceRules = require("eslint-plugin-brace-rules");
+
+
+const configIgnores = {
+	"ignores": [
+		"binaries/data/mods/mod/globalscripts/sprintf.js",
+		"libraries/",
+		"source/tools/profiler2/jquery*",
+		"source/tools/replayprofile/jquery*",
+		"source/tools/templatesanalyzer/tablefilter/",
+	],
+};
+
+
+const configEslintBase = {
+	"rules": {
+		"no-caller": 1,
+		"no-cond-assign": 1,
+
+		"no-constant-condition": ["error", {
+			"checkLoops": false,
+		}],
+
+		"no-dupe-args": 1,
+		"no-dupe-keys": 1,
+		"no-duplicate-case": 1,
+		"no-empty": 1,
+		"no-extra-boolean-cast": 0,
+		"no-extra-parens": 0,
+		"no-extra-semi": 1,
+		"no-floating-decimal": 1,
+		"no-func-assign": 1,
+		"no-negated-in-lhs": 1,
+		"no-obj-calls": 1,
+		"no-unreachable": 1,
+		"no-use-before-define": ["error", "nofunc"],
+		"use-isnan": 1,
+		"valid-jsdoc": 0,
+		"valid-typeof": 1,
+		"block-scoped-var": 0,
+		"consistent-return": 1,
+		"default-case": 1,
+		"dot-notation": 1,
+		"no-else-return": 1,
+		"no-invalid-this": 1,
+		"no-loop-func": 0,
+
+		"no-multi-spaces": ["warn", {
+			"ignoreEOLComments": true,
+		}],
+
+		"no-new": 1,
+		"no-redeclare": 0,
+		"no-return-assign": 1,
+		"no-self-assign": 1,
+		"no-self-compare": 1,
+		"no-unmodified-loop-condition": 1,
+		"no-unused-expressions": 1,
+		"no-unused-labels": 1,
+		"no-useless-concat": 0,
+		"yoda": 1,
+		"no-delete-var": 1,
+		"no-label-var": 1,
+		"no-shadow-restricted-names": 1,
+		"no-shadow": 1,
+		"no-undef": 0,
+		"no-undef-init": 1,
+		"no-unused-vars": 0,
+		"comma-spacing": 1,
+
+		"indent": ["warn", "tab", {
+			"outerIIFEBody": 0,
+		}],
+
+		"key-spacing": 1,
+		"new-cap": 0,
+		"new-parens": 1,
+		"no-mixed-spaces-and-tabs": ["warn", "smart-tabs"],
+		"no-multi-assign": 1,
+		"no-trailing-spaces": 1,
+		"no-unneeded-ternary": 1,
+		"no-irregular-whitespace": 1,
+		"object-curly-spacing": ["warn", "always"],
+		"operator-assignment": 1,
+		"operator-linebreak": ["warn", "after"],
+		"quote-props": 1,
+		"semi": 1,
+		"semi-spacing": 1,
+		"space-before-function-paren": ["warn", "never"],
+		"space-in-parens": 1,
+		"space-unary-ops": 1,
+		"spaced-comment": ["warn", "always"],
+		"no-class-assign": 1,
+		"no-const-assign": 1,
+		"no-dupe-class-members": 1,
+		"prefer-const": 1,
+	}
+};
+
+
+const configBracesRules = {
+	"plugins": {
+		"brace-rules": braceRules
+	},
+
+	"rules": {
+		"brace-rules/brace-on-same-line": [
+			"warn",
+			{
+				"FunctionDeclaration": "never",
+				"FunctionExpression": "ignore",
+				"ArrowFunctionExpression": "always",
+				"IfStatement": "never",
+				"TryStatement": "ignore",
+				"CatchClause": "ignore",
+				"DoWhileStatement": "never",
+				"WhileStatement": "never",
+				"ForStatement": "never",
+				"ForInStatement": "never",
+				"ForOfStatement": "never",
+				"SwitchStatement": "never",
+			},
+			{
+				"allowSingleLine": true,
+			}
+		],
+	}
+};
+
+
+const configs = [configIgnores, configEslintBase];
+configs[1].plugins = { ...configBracesRules.plugins };
+Object.assign(configs[1].rules, configBracesRules.rules);
+
+export default configs;
diff --git a/package.json b/package.json
new file mode 100644
index 0000000000..18f087c8d1
--- /dev/null
+++ b/package.json
@@ -0,0 +1,11 @@
+{
+	"type": "module",
+	"devDependencies": {
+		"eslint": "^9.27.0",
+		"eslint-plugin-brace-rules": "^0.1.6"
+	},
+	"scripts": {
+		"lint": "eslint",
+		"lint:fix": "eslint --fix"
+	}
+}
diff --git a/source/tools/lint/README.md b/source/tools/lint/README.md
index 0b47d7f327..424f26b863 100644
--- a/source/tools/lint/README.md
+++ b/source/tools/lint/README.md
@@ -20,3 +20,17 @@ Adding library cfg's for other deps could improve cppchecks ability to find issu
 ## copyright
 
 A linter for checking copyright dates in file headers are up to date.
+
+## eslint
+
+For eslint run 'pre-commt run eslint -a'
+
+### Installation and IDE integration
+
+Install Node.js and then run 'npm install' in the repo root.
+
+Now you can run eslint as 'npm run-script lint' or if you want eslint to try
+fix the issues 'npm run-script lint:fix'.
+
+After having installed eslint you might want to add an eslint extension to your
+editor to get inline warnings and to allow for auto-formatting.