Changes to prettier and eslint setup

[VitGottwald]

Setting up prettier and eslint to properly work side by side with various editors, cli, and with vscode extenstions can be tricky.

Guidelines

I followed these guidelines

0. use a fixed version of prettier
1. allow prettier to be used from various editors, cli and other environments like linters
2. format code on save in VSCode for all formats supported by prettier
3. ESLint should avoid doing formatting lints to avoid clashing with prettier
4. .editorconfig should be in sync with prettier config
5. use as few prettier configuration options a necessary
6. no lint warnings

Details

Use a fixed version of prettier

To avoid accidental updates and unexpected formatting changes in the future. Notice the --exact argument in installation instructions. That is why 0.

Allow prettier to be used from various environments

For this to reliably work a configuration file is needed. I used the .config.js extension that is being increasingly adopted by various JS/TS tooling (with eslint being a notable exception so far).

Format code on save in VSCode for all formats supported by prettier

Formatting is not an issue while authoring code and should not be linted for. Every time a change is made in editor, in order for it to get to disk, it has to be saved. At that moment formatting happens and the file is saved properly formatted. The vscode extension for prettier is used.

While the extension setup was created in the original branch it was done in a way that is officially dicouraged by the extension.

ESLint should avoid doing formatting lints

To achcive this prettier config for eslint is used in the eslintrc.yam, but no rules are enabled

.editorconfig should be in sync with prettier config

This resulted in a small change.

Use as few prettier configuration options a necessary

Many of the options in the original branch were prettier defaults so I removed them. But I also made these changes:

• trailing comma es5
• arrow parentheses always
• end of line `nl' (prettier default)

all of these are the default since prettier 2.0.0 and the links provide explanation why that is preferred.

No lint warnings

Allowing lint warnings makes developers live miserable. Those who strive to have a clean editor or have an OCD for underlined text suffer. While they meticulously fix all the complaints from linter. The next time they checkout or pull, their editor lights up again - because someone else did not care about a warning and CI passed.

The only reasonable solution for this problem that I am aware of is turn all warnings into errors and discuss, what is really needed.

Luckily for us, the linter is separate from compiler and even with failing lints, we can prototype and develop. But CI shall not pass.

CI

Checking code in CI for formatting errors is ensured by running prettier CLI on the whole repository. This is done concurrently to ESlint linting using the concurrently package.

[VitGottwald]

I should have said that this might be best reviewed commit-by-commit because one of the commits does reformatting of json and markdown files and that creates some noise in the overall PR diff.

[phaumer]

@VitGottwald thanks for this and sorry for the slow reply. I am switching back and forth between your and my branch and looked at all the individual commits. Still have a couple of questions:

• The format on save does not work for me in your branch. Even if I put the Prettier VS Code extension back. Not even cmd-option-f formats. I have to explicitly run npm run pretty to get indentation corrected. Am I missing something? In my branch using the linter I would get errors for formatting problems plus save would auto-correct it without any Prettier VS Code extension being installed. (Yes, my intention should have been to follow the advise of the Prettier VS Code extension Readme you have quoted to not use the extension as that was my goal: to not rely on developers installing another VS Code extension, but have the package.json force the required tools. But my branch with the eslint doing the formatting on save did exactly that though. So here I might misunderstand you arguments and we should continue discussing.)
• I would like to put MD back on the ignore list, because I do not understand why prettier formats in the way it does. For example, for a bullet point it adds four extra spaces, which I see no reason. As I write a lot of MD files I am actually using markdownlint (https://marketplace.visualstudio.com/items?itemName=DavidAnson.vscode-markdownlint) for MD files instead and the four spaces are a violation there.
• Also generated .js files I think we should avoid reformatting; not sure if that is the case in your config though.
• I agree with the warning vs error thing. My thinking was rather to use warnings in a transition period to distinguish items that must be fixed right away, from things that we gradually fix over time (where the OCS people, including myself can lead the charge). Zowe Explorer is full of JavaScript style code and we won't be able to eliminate all any's right away; or uppercase constant that seem to stem from some C++ background.

[VitGottwald]

Sorry if my comments were confusing @phaumer . To format on save in VSCode, the prettier extension has to be installed. I have never seen a problem with that and in my experience, that is the common setup that most people use.

[VitGottwald]

I understand that it is possible to utilise eslint to run prettier and fix the formatting. But I think it is not ideal for several reasons

• formatting errors show up in editor and disturb while typing code
• people that have the prettier extension installed now get two errors for any formatting issue (one from eslint and another one from the prettier extension)

These issues are valid for vscode and other editors as well.

[VitGottwald]

There is one more reason that I wanted to bring up separately. I personally do not have high trust in the eslint --fix option. I have seen it mess up things at times. After doing some learning and writing a few of my own eslint plugins, I think I understand why.

While ESlint was the first tool that clearly separated javascript parsing and allowed rules to be written against the program's AST rather than against its textual representation, it is not true for --fix. The fixer makes changes to the program's text and that can be tricky.

In order to reformat programs on save through eslint, the --fix option has to be turned on. And we get all fixers from all rules. We are now mixing two separate concerns:

• reformatting the code to match a desired style (prettier)
• fixing fixable lint errors (eslint rules)

The main idea of prettier is to reformat program text to match standards without changing the program itself. I other words, to reformat the text, but preserve the program's AST. This is very powerful.

If we turn on --fix on save, we are mixing prettier with eslint rules and we lose the AST invariant guarantee.

[VitGottwald]

I added out directory into .prettierignore so that we would not format the compiled .js files.

[VitGottwald]

Not sure why the formatting does not work on your end 🤔 I checked with @Alexandru-Dumitru and it worked for him. He is using windows I am on a mac.

[VitGottwald]

The four spaces in markdown were caused by the editorconfig setting. I changed it to 2 in bac8234 and I believe it now behaves as you expect.

[VitGottwald]

In regards to eslint rules, I think we should have a discussion what rules we want present. And when we decide to adopt a rule, make it an error and fix the code in the same pr.

I have some reservations about some of the currently selected rules, but I think that discussion can wait. I would like to have an agreement on the setup of the tooling first.

[VitGottwald]

As a general comment to improve code quality, I think we should utilise the typescript compiler first. For example we do not use strict type checking at all.

[VitGottwald]

Just created an issue for strict compiler checks in zowe explorer https://github.com/zowe/vscode-extension-for-zowe/issues/852 and additional compiler linting options https://github.com/zowe/vscode-extension-for-zowe/issues/853 .

[phaumer]

@VitGottwald my apologies again. As I am going on vacation next week I just had a lot of IBM work to finish. Here my replies

• I figured my problem out: I had some user settings that overwrote the workspace settings for formatting. I removed those and now Prettier works again. This kind of proof my point, though: asking people to install another extension is another point of failure, because a) people just don't do it (especially if we have people from the outside contributing code) and b) there are more points of failure such as with my user settings. We also had some bad experiences recently in our own IBM team with prettier as they changed their default multiple times such as adding comma to list automatically or adding parentheses to lambda causing every file that was saved to change completely without warning users. Anyway, I am definitely willing to compromise and let the team decide, but my vote would still let the linter do the formatting (and I like these errors while editing, because if people are not formatting they get a reminder) and get rid of the Prettier VS Code extension from my laptop. For the fix problem I would look at each rule individually to see what kind of changes we are talking about.

• I agree with switching on Typescript settings first. But we probably need a strategy for transitioning our code base as we probably have to do a long changes and fixes. I still like the warning idea and working towards close to zero warnings. VS Code let's you filter the scope of the warnings it shows you as well.

I am now merging your changes and we could continue the discussion on specific rules in the issue in Zowe Explorer perhaps (will be back on the 22nd).

[VitGottwald]

Hi @phaumer no worries, there is no rush. To address your concerns:

1. Literally everyone I have meet in the community had the prettier extension installed. It is the default setup with nearly 7 million downloads. If we want standard setup, it includes the vscode extension.
2. I believe the reason why you got formatting changes was because prettier was not installed with exact version and allowed for an update. I explained it and fixed that in my first commit. Also this behaviour would happen irrespectively of how you run it, whether through a vscode extension or through eslint.
3. One of the benefits of prettier is that we can focus on thinking about logic rather than formatting. Developers are free to type any way they want - even put it in a single line, then hit save and the formatting is correct. This is a new generation of formatters. Instead of telling you all the time that you are doing something wrong, it fixes it for you without bothering. And you know you can rely on it, it does not change the semantics (AST).
4. Enabling --fix means to me that I no longer can trust the tool. Prettier is heavily tested to preserve the AST. But eslint fixers are not. I personally do not want to mix these two together. I want prettier to do formatting for me and fix linting errors myself.