Latex-workshop: 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

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

Breaking rules compared to tslint.

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

Non default eslint rules we want to enforce

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

Extra changes

• [x] 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.

[tamuratak]

I prefer type to interface. To my understanding, the difference is little. However, the declaration merging of interface might become a trouble-maker. See link.

[tamuratak]

We can use prefer-arrow-callback as an alternative to only-arrow-functions.

I prefer

prefer-arrow-callback: [ "error", { "allowUnboundThis": false } ]

This setting

prohibits the use of function expressions as callbacks or function arguments entirely, without exception.

[tamuratak]

I want to add@typescript-eslint/ban-ts-ignore. See link.

[tamuratak]

We had better exclude src/lib/**/*.ts using .eslintignore.

I have posted separated comments to enable them to be hidden if resolved.

[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