Much stricter linting

[faustbrian]

Linting at the moment is fairly lax and even basics like consistent sorting of functions, variables and objects are missing. What I mean by lax is that for example a mix of null and undefined are used and there are no linting rules to enforce undefined, something like https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-null.md#why for example.

If you're open to a stricter set of linting rules I would create a list of rules I think make sense to start with and then PR them, including all of the initial fixes.

[acolytec3]

I like the thought here. We just integrated a strict-boolean-expressions rule in this pr and have yet to really take it too seriously. I like the idea here if you want to propose the list you think makes sense and it could be something we pick up once the breaking releases are out.

[faustbrian]

Will get a list together in the next days and post it here with reasoning for the biggest rules.

[acolytec3]

Cool. #1935 this was a list suggested by @ryanio that we had started to look at a while back

[faustbrian]

The first batch of additions I would suggest are what you could describe as cosmetic because they are primarily targeted at improving the visuals of the code by making it easier to scan imports, exports and all kinds of code that has values that can be sorted, like the keys of an object. The vast majority of those rules is auto fixable, meaning they are a quick improvement and make it easier to scan code because it will enforce consistent sorting.

module.exports = {
	extends: [
		'plugin:import/recommended',
		'plugin:import/typescript',
	],
	plugins: [
		'import',
		'simple-import-sort',
		'sort-destructure-keys',
		'sort-keys-fix',
		'unused-imports',
	],
	rules: {
		'import/default': 'error',
		'import/export': 'error',
		'import/exports-last': 'error',
		'import/extensions': 'off',
		'import/first': 'error',
		'import/group-exports': 'error',
		'import/namespace': 'error',
		'import/no-absolute-path': 'error',
		'import/no-anonymous-default-export': 'error',
		'import/no-cycle': 'error',
		'import/no-deprecated': 'error',
		'import/no-duplicates': 'error',
		'import/no-dynamic-require': 'error',
		'import/no-extraneous-dependencies': 'error',
		'import/no-mutable-exports': 'error',
		'import/no-namespace': 'error',
		'import/no-restricted-paths': 'error',
		'import/no-self-import': 'error',
		'import/no-unresolved': 'error',
		'import/no-unused-modules': 'error',
		'import/no-useless-path-segments': 'error',
		'import/no-webpack-loader-syntax': 'error',
		'import/order': 'error',
		'sort-destructure-keys/sort-destructure-keys': 'error',
		'sort-keys-fix/sort-keys-fix': [
			'error',
			'asc',
			{
				caseSensitive: true,
			},
		],
	},
};

[faustbrian]

First PR submitted #2086

[gabrocheleau]

Generally strongly in favour of this, with some reservations on a few points:

• As mentioned in one of my comments on chore: set up eslint-plugin-simple-import-sort and eslint-plugin-import #2086, I think we should make sure not to bloat up the linting config with unnecessary rules, so I feel like each rule should ideally be justified + result in an actual update.
• About the object key ordering, while I enjoy sorted keys in many contexts (and apply that rule in most other projects I've worked on), I think in this repo, we've been relying heavily on "semantic ordering", for example hardfork names ordered chronologically, function object args ordered semantically, etc. So I feel like alphabetic sorting would not make sense as a general rule.

[faustbrian]

Wrapping up some test refactor PRs for the trie package and then posting the next batch of rule suggestions.

[holgerd77]

Yeah, I guess going very much along with the overall sentiment respectively feedback from the team, so strongly in favor of additional/stricter linting, but also a) not too many rule updates at a time (in doubt rather 1 - eventually 2,3 - per PR) to not get lost on seeing the effects and b) yes - strongly going with @gabrocheleau - an - at least short - reasoning would be necessary for every linting rule - even if it's a bit of additional work - to just have the occasion to give every rule a short reflection and reduce the risk of side effects (I liked the semantic ordering argument from Gabriel a lot e.g.) by not being able to grasp the full picture.

[faustbrian]

Bit busy this week, will continue with this on the weekend or next week.

[faustbrian]

@holgerd77 I have some more rules lined up but will wait with it until the releases are out.