Opened 5 months ago

Last modified 3 months ago

#5524 assigned enhancement

Have all JS source code pass ESLint

Reported by: Krinkle Owned by: Krinkle
Priority: Nice to Have Milestone: Backlog
Component: Build & Packages Keywords:
Cc: Patch: phab:D2103, phab:D2070

Description

ESLint helps in three main areas:

  • valid syntax (because syntax errors might not be found easily otherwise, given code is loaded on-demand only),
  • consistent style for the code (style rules),
  • detect and prevent common bugs by disallowing fragile or broken code (quality rules).

By default it only does syntax validation. But r20364 introduced a few style and quality rules as well.

Right now, there's a lot of violations in the source code, causing noise during code review.

It seems there may be a few rules we no longer agree with, in which case we should disable or change them to match our desired outcome.

For other cases, we should just fix them wholesale once and have a clean build going forward.

The style rules can be easily and safely auto-fixed by eslint --fix, which performs non-semantic changes only. The quality violations are best addressed in smaller changes focussing on an individual area only with understanding of local context.

Change History (10)

comment:1 Changed 5 months ago by Krinkle

Prior work:

Recent work:

  • r22419: Fix eslint semicolon-related warnings in all components,
  • r22472: Migrate no-floating-decimal rule from jshint to eslint.

comment:2 Changed 5 months ago by Krinkle

Owner: set to Krinkle
Status: assignednew

comment:3 Changed 5 months ago by Krinkle

Patch: phab:D2103, phab:D2070
Status: newassigned

comment:4 Changed 5 months ago by elexis

In 22529:

Disable eslint curly rule, because none of its configurable enforcement reflects the curly brace convention of this repository, refs #5524.

Differential Revision: https://code.wildfiregames.com/D2070
Patch By: Krinkle
Reviewed By: Imarok

comment:5 Changed 4 months ago by elexis

In 22651:

Hide ESLint no-unmodified-loop-condition hint for a while (const) in gamesetup.js, refs #5524.

Differential Revision: https://code.wildfiregames.com/D2147
Patch By: Krinkle

comment:6 Changed 4 months ago by elexis

In 22654:

Duplicate clearChatMessages since that ends up being cleaner
than having a stray function in gui/common/ that refers to page specific names and logic with a try-catch block to shield against reference errors, refs #3396, rP17465, rP17467.

Incidentally fixes ESLint no-empty hint triggered for empty catch blocks, refs #5524, D2146.

comment:7 Changed 4 months ago by elexis

In 22655:

Fix ESLint no-multi-assign, default-case, no-empty, no-shadow warnings, refs #5524.

Differential Revision: https://code.wildfiregames.com/D2146
Based on patch By: Krinkle

comment:8 Changed 4 months ago by elexis

In 22803:

Remove PETRA Module and Augmentation pattern from rP14441, refs #2322, and in Petra since rP14865.

The objective of rP14441 to have only one global for multiple AIs is still achieved by not changing the resulting object structure.
The advantage of not using the pattern is to have less parental scopes, i.e. more localization and separation of concerns.
This resembles the successful simulation script component system.
The stated benefits of the Augmentation pattern were never used, because there are (luckily) no such private variables or globals passed as arguments.

Fixes an ESLint warning about using PETRA before it is defined, refs #5524, D1993.
Fixes the missing level of indentation if indentation should represent scoping proportionally.

Remove superfluous, thus misleading name PetraBot in PETRA.PetraBot assignment.

Differential Revision: https://code.wildfiregames.com/D2103
Patch By: Krinkle
Related comments by: Yves, wraitii, Philip, leper on #0ad-dev on December 2013, see #2322
Source was: http://www.adequatelygood.com/JavaScript-Module-Pattern-In-Depth.html

comment:9 Changed 3 months ago by elexis

In 22835:

Fix some Petra whitespace; with rP22803/D2103 fixes all ESLint issues in petra/, refs #5524.

Differential Revision: https://code.wildfiregames.com/D1993
Patch By: Krinkle

comment:10 Changed 3 months ago by elexis

In 22893:

Enable no-caller and no-irregular-whitespace in ESLint, refs #5524.

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

Differential Revision: https://code.wildfiregames.com/D2237
Patch By: Krinkle

Note: See TracTickets for help on using tickets.