ESLint Error Fixes

[Brittany-Reid]

ESLint provides us with error messages and fixes for code snippets, with a basic implemention in branch v2.0, however, there may be room to improve these. Possibilities include:

• Adding additional fixes for common errors.
• Looking for existing extensions to ESLint.
• Adding additional rules to ESLint that aren't part of the recommended list.

Todo:

• [ ] Run ESLint for all code snippets and find the most common errors
• [ ] Confirm error correctness and ESLint configuration
• [ ] See the impact of ESLint's existing fixes on error count

The error statistics are related to issue #48

[Brittany-Reid]

I need to investigate the eslint configuration, for the 2 million code snippets, the list of errors has some oddities.

We have errors such as alphabetize/_ from an eslint plugin, where the name is not very descriptive. I believe the config was originally only using default fixes but then I added some environment options, it's possible that's where this is coming from?

But we have odder cases: errors named rule-code [rule-code] and <rule>, what are these meant to even be?

We need to be able to explain the errors that appear in our data.

[Brittany-Reid]

I ran with the rule object empty and now only get parsing errors and the odd cases. Found this, seems it may be using our project config file that I use for the eslint VSCode plugin:

If so, these cases are still odd for our config. They should have names and belong to a project.

[Brittany-Reid]

It's not that, we're using the Linter object that shouldn't use our config.

Printed the code for one of the errors and received this:

/* eslint @fasttime/no-spaces-in-call-expression: "error" */\n\nfn ();

This inline rule says to error at this line. We can disable it with allowInlineConfig.

[Brittany-Reid]

Cool, it works. The info now only shows parser errors and rules from the defined ruleset.

[Brittany-Reid]

An analysis of errors can be found here: info.log

I printed 10 examples for each error, except in cases where there were less than 10 occurrences. The printed snippets are not a random sample, so show the first 10 occurances in the dataset (you will notice for common errors the package names they come from tend to be at the start of the alphabet). It may be a good idea to randomize on the next run.

Analysis

### Parsing error: Unexpected token 9/10 occurances were non-javascript code, once was a code snippet with results annotated on each line but not commented out. The nonJS code includes HTML, terminal commands and JSX (react). This error is a combination of all unexpected token 'x' errors, so I believe it would be a good idea to analyse the individual cases. ### Parsing error: 'import' and 'export' may appear only with 'sourceType: module' The REPL doesn't allow import and export statements, I have already implemented a rule fix for this that converts it to require. Export doesn't require a conversion as there's no reason to export from a REPL script, best to remove these lines or comment them out. ### no-multi-spaces A warning from standard, not categorized as a style change. I have commented this one out for now, I don't believe it tells us anything interesting about code quality. I'm happy to discuss this more if we end up using warning some way (for now, we just use errors). ### Parsing error: Unexpected character Again, this one is a merger of multiple errors for specific characters. The difference from token seems to be that this triggers on `#` or irregular characters. Most of the snippets are incorrect comment style using `#`, the others are non-code using weird characters like an example containing ASCII art. One is a Chinese string that I can't immediately see what caused the error (full-width comma maybe?). Should give this a look individually, maybe the `#` case could be converted to `//` if common? ### no-redeclare Warning for no redeclare of variables, however the code will still run. Common as many code snippets show multiple ways to import a project or do some task. ### no-labels Warning for best practice from standard. Many of the code snippets here don't actually use labels but have a JSON object as part of the snippet, for example, to show results. ``` //@13w/soap client.describe() // returns { MyService: { MyPort: { MyFunction: { input: { name: 'string' } } } } } ``` ### eqeqeq Warning for `==` instead of `===`, straightforward. Comes with a fix. ### no-new Warning for using new without assigning. Many code snippets do this to show how to create an object. Not too useful for code quality. ### Parsing error: Identifier has already been declared This one triggers for redeclaring constant variables. This is common in code snippets to show multiple ways to import packages. In this example, it's also showing multiple ways to do something: ``` const root = db.ref('/'); // Or const root = db.ref(''); ``` Within the REPL it may just be a good idea to convert all `const` to `var`, comment out the second case, or rename the variable to root2. ### node/no-path-concat, Warning for not using the path package to concatenate directory strings, to enforce cross platform compatibility. ### no-irregular-whitespace Warning for weird whitespace, doesn't have a fix built in. I ran a code snippet and it was fine, but some whitespace could cause issues. Possible to fix by removal if it would be useful. ### no-sequences Warning for sequences. Looks fine. One case was just a line of numbers seperated by commas. ### Parsing error: Invalid regular expression All weird uses of `/`, typically for a directory. At this point the occurances were so rare I decided to stop. I checked all the non-parsing error rules marked error however. For no dupe-keys, I believe the following should just give a warning: ``` //@alfalab/rollup-plugin-postcss postcss({ modules: true, // Or with custom options for `postcss-modules` modules: {} }) ``` Some others will still run just have unintended behaviour.