Consistency/linting roadmap

[jlelong]

Following the long discussion in #1475, I think it would be nice to write a little roadmap to improve code consistency.

Before moving to eslint

• Remove unnecessary escape in regex: {, } need no escape and neither do [, ] when used inside character class. Rule no-useless-escape.
• Strings should single quotes. Backticks should be reserved for strings using substitution. Rule: quotes.
• Statements should not end with a ;.
• Remove all // @ts-ignore and fix the errors. These are trickier so I will wait until we merge Formatting towards eslint #1543. See PR Ban ts ignore #1545.

Breaking rules compared to tslint.

• Remove extra space before : when giving explicit type. Rule @typescript-eslint/type-annotation-spacing.

Non default eslint rules we want to enforce

• Prefer type over interface. See here.
• Ignore everything in src/lib.
• Properties can be declared in the constructor. set "@typescript-eslint/no-parameter-properties": "off".

Extra changes

• Convert .plist syntax files to .tmLanguage.json. I find .json files much easier to edit and maintain and they allow for more elaborate constructions not available in the .plist (say .xml) format. The vscode-tmlanguage extension can be used as a conversion tool.

Such changes may induce a lot of modifications and tend to blur file history. So, I want to make sure there is no objection before making any change. I will try to update this post with any further discussion.

The Done ticks refer to the developments in branch format.

[jlelong]

Some remaining errors, eslint is complaining about.

• Unexpected any. Specify a different type. @typescript-eslint/no-explicit-any. Fixing this often requires some extra code and is sometimes virtually impossible as in Promise<any> or { [string]: any}.
• Property cannot be declared in the constructor. @typescript-eslint/no-parameter-properties.

[jlelong]

What about stylistic rules?

• max-len
• newline-per-chained-call
• object-property-newline

[tamuratak]

I cannot find a strong reason to enforce @typescript-eslint/no-parameter-properties.

I think enforcing @typescript-eslint/no-explicit-any would reduce a number of bugs. But I am not sure it is feasible.

We can refer to the migration process by TypeScript team here. In it, both the options are turned off at present.

[jlelong]

I agree with you on enforcing @typescript-eslint/no-explicit-any but I am struggling a bit in particular when extending classes from the vscode API. I do not think we can change the signature...

[tamuratak]

To eliminate the warning, warning File ignored because of a matching ignore pattern, we have to call eslint with ., not with a glob pattern, src/**/*.ts. See eslint/eslint/issues/5623.

By adding the following entries to .eslintignore,

diff --git a/.eslintignore b/.eslintignore
index 5e8f5ff..e959428 100644
--- a/.eslintignore
+++ b/.eslintignore
@@ -1 +1,6 @@
 src/lib/**/*.ts
+dev/**/*.js
+resources/**/*.js
+out
+node_modules
+viewer

we have to call eslint with the --ext option,

./node_modules/.bin/eslint --ext .ts .

The extension will be able to be specified in a config file in the future release. See https://github.com/eslint/rfcs/tree/master/designs/2019-additional-lint-targets