[#174416949] Replace tslint with eslint #752

BurnedMarshal · 2021-01-08T17:16:35Z

List of Changes

Replace tslint and italia-tslint-rules with @pagopa/eslint-config
Add eslint configuration from @pagopa/eslint-config/strong
Autofix all autofixable rules
Replace tslint:disable with eslint-disable
Change lint script inside package.json

Motivation and Context

TSLint has been deprecated in favour of ESLint. This PR replace TSLint with ESLint and italia-tslint-rules with the new package @pagopa/eslint-config. Some rules from @pagopa/eslint-config/strong has been disabled to minimize code refactoring.

How Has This Been Tested?

Local yarn lint and yarn build execution
Local yarn test execution

Types of changes

Chore
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

My change requires a change to the documentation.
I have updated the documentation accordingly.

Question:

Can we enable eslint even on .test.ts files? So we could have the same code style restrictions on tests.

pagopa-github-bot · 2021-01-08T17:43:40Z

Affected stories

⚙️ #174416949: Sostituire TSLint con ESLint nei nostri progetti

New dependencies added: @pagopa/eslint-config and eslint-plugin-prettier.

@pagopa/eslint-config

Author: Unknown

Description: This package provide the following ESLint custom rules for Typescript projects.

Homepage: https://github.com/pagopa/eslint-rules#readme


Created	14 days ago
Last Updated	about 1 hour ago
License	MIT
Maintainers	5
Releases	3
Direct Dependencies	`@typescript-eslint/eslint-plugin`, `@typescript-eslint/eslint-plugin-tslint`, `@typescript-eslint/parser`, `eslint`, `eslint-config-prettier`, `eslint-plugin-extra-rules`, `eslint-plugin-fp`, `eslint-plugin-functional`, `eslint-plugin-import`, `eslint-plugin-jsdoc`, `eslint-plugin-no-credentials`, `eslint-plugin-prefer-arrow`, `eslint-plugin-prettier`, `eslint-plugin-react`, `eslint-plugin-sonarjs` and `prettier`

README

PagoPA ESLint config

This package provide the following ESLint custom rules for Typescript projects.

recommendend
react
strong

This repository replace italia-tslint-rules after TSLint deprecation.

The following TSLint rules (included inside italia-tslint-rules) are not supported for eslint at the moment and are missing in this package:

bool-param-default
max-union-size
no-accessor-field-mismatch
no-array-delete // Mitigated by no-delete
no-case-with-or
no-dead-store
no-duplicate-in-composite
no-empty-array
no-extra-semicolon // Mitigated by prettier
no-empty-destructuring // Mititgated by no-empty-pattern
no-gratuitous-expressions
no-hardcoded-credentials
no-ignored-initial-value // Mitigated by no-param-reassign, no-let
no-in-misuse
no-inferred-empty-object-type
no-invalid-await // Mititgated by await-thenable
no-invariant-return
no-misleading-array-reverse // Mitigated by immutable-data
no-misspelled-operator // Mitigated by prettier
no-nested-switch
no-nested-template-literals
no-statements-same-line // Mitigated by prettier
no-try-promise
no-tslint-disable-all
no-unconditional-jump
no-undefined-argument
no-unenclosed-multiline-block // Mitigated by prettier
no-unthrown-error
no-unused-array // Mitigated by no-unused-vars
no-useless-increment
no-useless-intersection
prefer-promise-shorthand
promise-must-complete
use-primitive-type

This list has been produced following these steps:

Follow TSLint to ESLint migration guide on the project io-backend
Running npx tslint-to-eslint-config that produces a list of TSLint rules not available for ESLint (77 rules at the moment)
Manually check each one for an alternative ESLint rules/plugins
Verify each alternative ESLint rule/plugin on a testing file

Usage

Installation and Configuration

To use this package install as devDependecy inside any typescript project with

yarn install -D @pagopa/eslint-config

Create on the project an .eslintrc.js file with the following content

module.exports = {
  "extends": [
    "@pagopa/eslint-config/strong",
  ],
  "rules": {
    // Any project level custom rule
  }
}

Add inside the package.json file a lint and optionally a lint-autofix script as:

"scripts": {
  "lint": "eslint . -c .eslintrc.js --ext .ts,.tsx",
  "lint-autofix": "eslint . -c .eslintrc.js --ext .ts,.tsx --fix",
  ...
}

Migration from TSLint

Remove from the package.json every tslint reference:

tslint
italia-tslint-rules

Replace all the // tslint:disable-next-line with the proper // eslint-disable-next-line comment. If are present some // tslint:disable replace it with /* eslint-disable */ at the top of the file.

If you need to disable ESLint for some files create .eslintignore file with the list of folders or files that must be excluded from lint process. Copy the exclusion from tslint.json linterOptions.exclude

Delete all tslint related files (es. tslint.json).

Run yarn lint-autofix to refactorize the code automatically with all the auto-fixable ESLint rules.

eslint-plugin-prettier

Author: Teddy Katz

Description: Runs prettier as an eslint rule

Homepage: https://github.com/prettier/eslint-plugin-prettier#readme


Created	almost 4 years ago
Last Updated	18 days ago
License	MIT
Maintainers	3
Releases	25
Direct Dependencies	`prettier-linter-helpers`
Keywords	eslint, eslintplugin, eslint-plugin and prettier

README

eslint-plugin-prettier

Runs Prettier as an ESLint rule and reports differences as individual ESLint issues.

If your desired formatting does not match Prettier’s output, you should use a different tool such as prettier-eslint instead.

Sample

error: Insert `,` (prettier/prettier) at pkg/commons-atom/ActiveEditorRegistry.js:22:25:
  20 | import {
  21 |   observeActiveEditorsDebounced,
> 22 |   editorChangesDebounced
     |                         ^
  23 | } from './debounced';;
  24 |
  25 | import {observableFromSubscribeFunction} from '../commons-node/event';


error: Delete `;` (prettier/prettier) at pkg/commons-atom/ActiveEditorRegistry.js:23:21:
  21 |   observeActiveEditorsDebounced,
  22 |   editorChangesDebounced
> 23 | } from './debounced';;
     |                     ^
  24 |
  25 | import {observableFromSubscribeFunction} from '../commons-node/event';
  26 | import {cacheWhileSubscribed} from '../commons-node/observable';


2 errors found.

./node_modules/.bin/eslint --format codeframe pkg/commons-atom/ActiveEditorRegistry.js (code from nuclide).

Installation

npm install --save-dev eslint-plugin-prettier
npm install --save-dev --save-exact prettier

eslint-plugin-prettier does not install Prettier or ESLint for you. You must install these yourself.

Then, in your .eslintrc.json:

{
  "plugins": ["prettier"],
  "rules": {
    "prettier/prettier": "error"
  }
}

Recommended Configuration

This plugin works best if you disable all other ESLint rules relating to code formatting, and only enable rules that detect potential bugs. (If another active ESLint rule disagrees with prettier about how code should be formatted, it will be impossible to avoid lint errors.) You can use eslint-config-prettier to disable all formatting-related ESLint rules.

This plugin ships with a plugin:prettier/recommended config that sets up both the plugin and eslint-config-prettier in one go.

In addition to the above installation instructions, install eslint-config-prettier:
```
npm install --save-dev eslint-config-prettier
```
Then you need to add plugin:prettier/recommended as the last extension in your .eslintrc.json:
```
{
  "extends": ["plugin:prettier/recommended"]
}
```
You can then set Prettier's own options inside a .prettierrc file.
Some ESLint plugins (such as eslint-plugin-react) also contain rules that conflict with Prettier. Add extra exclusions for the plugins you use like so:
```
{
  "extends": [
    "plugin:prettier/recommended",
    "prettier/flowtype",
    "prettier/react"
  ]
}
```
For the list of every available exclusion rule set, please see the readme of eslint-config-prettier.

Exactly what does plugin:prettier/recommended do? Well, this is what it expands to:

{
  "extends": ["prettier"],
  "plugins": ["prettier"],
  "rules": {
    "prettier/prettier": "error",
    "arrow-body-style": "off",
    "prefer-arrow-callback": "off"
  }
}

"extends": ["prettier"] enables the main config from eslint-config-prettier, which turns off some ESLint core rules that conflict with Prettier.
"plugins": ["prettier"] registers this plugin.
"prettier/prettier": "error" turns on the rule provided by this plugin, which runs Prettier from within ESLint.
"arrow-body-style": "off" and "prefer-arrow-callback": "off" turns off two ESLint core rules that unfortunately are problematic with this plugin – see the next section.

`arrow-body-style` and `prefer-arrow-callback` issue

If you use arrow-body-style or prefer-arrow-callback together with the prettier/prettier rule from this plugin, you can in some cases end up with invalid code due to a bug in ESLint’s autofix – see issue #65.

For this reason, it’s recommended to turn off these rules. The plugin:prettier/recommended config does that for you.

You can still use these rules together with this plugin if you want, because the bug does not occur all the time. But if you do, you need to keep in mind that you might end up with invalid code, where you manually have to insert a missing closing parenthesis to get going again.

If you’re fixing large of amounts of previously unformatted code, consider temporarily disabling the prettier/prettier rule and running eslint --fix and prettier --write separately.

Options

Note: While it is possible to pass options to Prettier via your ESLint configuration file, it is not recommended because editor extensions such as prettier-atom and prettier-vscode will read .prettierrc, but won't read settings from ESLint, which can lead to an inconsistent experience.

The first option:
- An object representing options that will be passed into prettier. Example:
```
"prettier/prettier": ["error", {"singleQuote": true, "parser": "flow"}]
```
  NB: This option will merge and override any config set with .prettierrc files
The second option:
- An object with the following options
  - usePrettierrc: Enables loading of the Prettier configuration file, (default: true). May be useful if you are using multiple tools that conflict with each other, or do not wish to mix your ESLint settings with your Prettier configuration.
```
"prettier/prettier": ["error", {}, {
  "usePrettierrc": false
}]
```
  - fileInfoOptions: Options that are passed to prettier.getFileInfo to decide whether a file needs to be formatted. Can be used for example to opt-out from ignoring files located in node_modules directories.
```
"prettier/prettier": ["error", {}, {
  "fileInfoOptions": {
    "withNodeModules": true
  }
}]
```
The rule is autofixable -- if you run eslint with the --fix flag, your code will be formatted according to prettier style.

Contributing

See CONTRIBUTING.md

Generated by 🚫 dangerJS

codecov · 2021-01-08T17:45:08Z

Codecov Report

Merging #752 (f8e5a19) into master (f523227) will decrease coverage by 0.30%.
The diff coverage is 79.48%.

@@            Coverage Diff             @@
##           master     #752      +/-   ##
==========================================
- Coverage   80.16%   79.85%   -0.31%     
==========================================
  Files          70       71       +1     
  Lines        2319     2328       +9     
  Branches      369      372       +3     
==========================================
  Hits         1859     1859              
- Misses        447      456       +9     
  Partials       13       13

Impacted Files	Coverage Δ
src/clients/api.ts	`71.42% <0.00%> (ø)`
src/controllers/bonusController.ts	`100.00% <ø> (ø)`
src/controllers/messagesController.ts	`100.00% <ø> (ø)`
src/controllers/profileController.ts	`100.00% <ø> (ø)`
src/controllers/servicesController.ts	`83.33% <ø> (ø)`
src/controllers/userDataProcessingController.ts	`83.33% <ø> (ø)`
src/server.ts	`0.00% <0.00%> (ø)`
src/services/apiClientFactory.ts	`71.42% <ø> (ø)`
src/services/redisUserMetadataStorage.ts	`85.41% <ø> (ø)`
src/services/userDataProcessingService.ts	`67.39% <ø> (ø)`
... and 49 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6367908...f8e5a19. Read the comment docs.

gunzip · 2021-01-11T12:26:33Z

lgtm two things:

can you write upgrading notes in the readme here: https://github.com/pagopa/eslint-rules/ ?
if there are "scriptable" things (ie. renaming comments to disable tslint -> eslint), can you add the script into eslint-rules repo?

BurnedMarshal · 2021-01-11T15:52:33Z

lgtm two things:

can you write upgrading notes in the readme here: https://github.com/pagopa/eslint-rules/ ?

if there are "scriptable" things (ie. renaming comments to disable tslint -> eslint), can you add the script into eslint-rules repo?

@gunzip About 1 a more detailed REAME was provided pagopa/eslint-rules/pull/3

BurnedMarshal · 2021-01-11T15:59:17Z

lgtm two things:

can you write upgrading notes in the readme here: https://github.com/pagopa/eslint-rules/ ?

if there are "scriptable" things (ie. renaming comments to disable tslint -> eslint), can you add the script into eslint-rules repo?

@gunzip About 2 maybe could be used this package (link) for comment conversion. If you like it we could add reference to this package inside the README of @pagopa/eslint-rules

gunzip · 2021-01-11T17:19:40Z

@gunzip About 2 maybe could be used this package (link) for comment conversion. If you like it we could add reference to this package inside the README of @pagopa/eslint-rules

yup

balanza · 2021-01-11T11:24:11Z

.eslintignore

+**/__tests__/*
+**/__mocks__/*


Shouldn't use eslint on those files as well? I mean: I see the point of not having such files to block a build, but also I'd like to enforce code conventions on test code (and, to use prettier)

I like the idea. But enable the linter on these files generate 330 problems because tslint was previously disabled on mocks and tests.

balanza · 2021-01-11T11:27:15Z

src/app.ts

+  readonly allowNotifyIPSourceRange: ReadonlyArray<CIDR>;
+  readonly allowPagoPAIPSourceRange: ReadonlyArray<CIDR>;
+  readonly allowMyPortalIPSourceRange: ReadonlyArray<CIDR>;
+  readonly allowBPDIPSourceRange: ReadonlyArray<CIDR>;
+  readonly allowSessionHandleIPSourceRange: ReadonlyArray<CIDR>;


isn't that redundant?

No, the readonly keyword makes the prop like a const so it can't be reassigned (es. myVar.allowNotifyIPSourceRange = myValue is forbidden). ReadonlyArray instead make the array immutable. They work on different layers.

gunzip · 2021-01-21T13:32:56Z

why draft? can this be merged?

BurnedMarshal · 2021-01-21T13:37:39Z

why draft? can this be merged?

I remove the draft. Maybe a rebase is needed before we can merge.

…-config [#174416949] Restore app.ts layout

AleDore

LGTM

BurnedMarshal force-pushed the 174416949-migrate-to-eslint branch from 06f4176 to 644b605 Compare January 8, 2021 17:42

BurnedMarshal requested review from balanza, gunzip and cloudify January 11, 2021 11:09

gunzip approved these changes Jan 11, 2021

View reviewed changes

balanza reviewed Jan 11, 2021

View reviewed changes

gunzip approved these changes Jan 12, 2021

View reviewed changes

BurnedMarshal marked this pull request as ready for review January 21, 2021 13:38

BurnedMarshal requested review from AleDore and francescopersico as code owners January 21, 2021 13:38

BurnedMarshal force-pushed the 174416949-migrate-to-eslint branch from 594b4f9 to 45d5f9a Compare January 21, 2021 13:47

BurnedMarshal added 10 commits January 22, 2021 15:17

[#174416949] Replace italia-tslint-rules (tslint) with @pagopa/eslint…

7af0a8a

…-config [#174416949] Restore app.ts layout

[#174416949] Apply eslint to app.ts after rebase

0ba73ce

[#174416949] Refactor casting with unknown instead any

373e3fd

[#174416949] Fix MessageSubject import path

61d7455

[#174416949] Revert .test.ts changes

bb5b86f

[#174416949] Add NL EOF

bd65e6e

[#174416949] Remove useless tslint:disable comment

c27ac07

[#174416949] Remove useless eslint-disable-next-line

e495b69

[#174416949] Add eslint cache into lint script

9262585

[#174416949] Fix after rebase

b6d96e2

BurnedMarshal force-pushed the 174416949-migrate-to-eslint branch from 45d5f9a to b6d96e2 Compare January 22, 2021 14:18

BurnedMarshal added 2 commits January 22, 2021 15:24

[#174416949] Fix lint error (bug)

228fd69

[#174416949] Fix false positive no-shadow on enum

d25fea5

AleDore approved these changes Jan 22, 2021

View reviewed changes

[#174416949] Upgrade @pagopa/eslint-config

f8e5a19

BurnedMarshal merged commit 925e59f into master Jan 22, 2021

BurnedMarshal deleted the 174416949-migrate-to-eslint branch January 22, 2021 16:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#174416949] Replace tslint with eslint #752

[#174416949] Replace tslint with eslint #752

BurnedMarshal commented Jan 8, 2021 •

edited

pagopa-github-bot commented Jan 8, 2021 •

edited

PagoPA ESLint config

Usage

Installation and Configuration

Migration from TSLint

eslint-plugin-prettier

Sample

Installation

Recommended Configuration

`arrow-body-style` and `prefer-arrow-callback` issue

Options

Contributing

codecov bot commented Jan 8, 2021 •

edited

gunzip commented Jan 11, 2021

BurnedMarshal commented Jan 11, 2021

BurnedMarshal commented Jan 11, 2021

gunzip commented Jan 11, 2021

balanza Jan 11, 2021

BurnedMarshal Jan 12, 2021 •

edited

balanza Jan 11, 2021

BurnedMarshal Jan 12, 2021

gunzip commented Jan 21, 2021

BurnedMarshal commented Jan 21, 2021 •

edited

AleDore left a comment

		*/__tests__/
		*/__mocks__/

[#174416949] Replace tslint with eslint #752

[#174416949] Replace tslint with eslint #752

Conversation

BurnedMarshal commented Jan 8, 2021 • edited

List of Changes

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Question:

pagopa-github-bot commented Jan 8, 2021 • edited

Affected stories

@pagopa/eslint-config

PagoPA ESLint config

Usage

Installation and Configuration

Migration from TSLint

eslint-plugin-prettier

eslint-plugin-prettier

Sample

Installation

Recommended Configuration

arrow-body-style and prefer-arrow-callback issue

Options

Contributing

codecov bot commented Jan 8, 2021 • edited

Codecov Report

gunzip commented Jan 11, 2021

BurnedMarshal commented Jan 11, 2021

BurnedMarshal commented Jan 11, 2021

gunzip commented Jan 11, 2021

balanza Jan 11, 2021

Choose a reason for hiding this comment

BurnedMarshal Jan 12, 2021 • edited

Choose a reason for hiding this comment

balanza Jan 11, 2021

Choose a reason for hiding this comment

BurnedMarshal Jan 12, 2021

Choose a reason for hiding this comment

gunzip commented Jan 21, 2021

BurnedMarshal commented Jan 21, 2021 • edited

AleDore left a comment

Choose a reason for hiding this comment

BurnedMarshal commented Jan 8, 2021 •

edited

pagopa-github-bot commented Jan 8, 2021 •

edited

`arrow-body-style` and `prefer-arrow-callback` issue

codecov bot commented Jan 8, 2021 •

edited

BurnedMarshal Jan 12, 2021 •

edited

BurnedMarshal commented Jan 21, 2021 •

edited