style: add additional style rules & apply fixes #75

devonzara · 2023-04-02T01:10:20Z

Closes N/A

Description

Add additional style rules, bump @typescript-eslint-plugin and @typescript-eslint-parser versions, & apply style fixes.

Add object-shorthand [Changes] (Docs)
Ensures ES6 object literal shorthand is used.

Examples

Valid syntax:

const obj = {
  foo: 'bar',
  baz,
  qux() {
    //
  },
};

Invalid syntax:

const obj = {
  foo: 'bar',
  baz: baz,
  qux: function() {
    //
  },
};

Add @typescript-eslint/key-spacing [Change 1, Change 2] (Docs)
Ensures a space follows the colon in object literals.

Required updating typescript-eslint to 5.50.0 or higher (Release notes), updated to latest:
```
  "@typescript-eslint/eslint-plugin": "^5.57.0",
  "@typescript-eslint/parser": "^5.57.0",
```
Examples

Valid syntax:
```
const foo = { bar: 'baz' };
```
Invalid syntax:
```
const foo = { bar:'baz' };
```

Add @typescript-eslint/space-before-blocks [Changes] (Docs)
Ensures a space exists before the start of a block that is not on a new line.

Examples

Valid syntax:

if (foo) {
  //
}

function bar() {
  //
}

function bar() {}

class Baz() {
  //
}

class Baz()
{
  //
}

Invalid syntax:

if (foo){
  //
}

function bar(){
  //
}

function bar(){}

class Baz(){
  //
}

Add arrow-spacing [Changes] (Docs)
Ensures a space exists before and after the => in an arrow function.
Examples

Valid syntax:
```
() => {
  //
};
```
Invalid syntax:
```
()=>{
  //
};
```
Add arrow-parens [Changes] (Docs)
Ensures parenthesis around arrow function parameters are only used when required.
Examples

Valid syntax:
```
foo => {
  //
};

(foo, bar) => {
  //
};
```
Invalid syntax:
```
(foo) => {
  //
};

(foo, bar) => {
  //
};
```
Add @typescript-eslint/brace-style [Changes] (Docs)
Ensures 1tbs (one true brace style), where braces must be on the same line as the keywords they're associated with.
Examples

Valid syntax:
```
if (foo) {
  //
} else {
  //
}

try {
  //
} catch(error) {
  //
}
```
Invalid syntax:
```
if (foo)
{
  //
}
else {
  //
}

try
{
  //
}
catch(error)
{
  //
}
```

Add @typescript-eslint/member-delimiter-style [Changes] (Docs)
Ensures semicolon delimiters between members of interface and type literals.

Examples

Valid syntax:

interface Foo {
    name: string;
    greet(): string;
}

type Bar = { name: string }
type FooBar = { name: string; greet(): string }

Invalid syntax:

interface Foo {
    name: string
    greet(): string
}

interface Foo {
    name: string,
    greet(): string,
}

interface Foo {
    name: string;
    greet(): string
}

type FooBar = { name: string, greet(): string }

Add array-bracket-newline [Changes] (Docs)
Ensures array brackets are either both on new lines or are both on the same line.

Examples

Valid syntax:

const foo = [];

const foo = [ 1, 2, 3 ];

const foo = [
  1,
  2,
  3,
];

const foo = [{
  bar: baz
}];

const foo = [
  { bar: baz }
];

Invalid syntax:

const foo = [
];

const foo = [
  1, 2, 3 ];

const foo = [
  1,
  2,
  3];

const foo = [{
    bar: baz
  }
];

const foo = [
  { bar: baz }];

Add object-curly-newline [Changes] (Docs)
Ensures object braces are either both on new lines or are both on the same line.

Examples

Valid syntax:

const foo = {};

const foo = {
};

const foo = { bar: baz };

const foo = {
  bar: baz,
};

const { bar } = obj;

const {
  bar
} = obj;

Invalid syntax:

const foo = { bar: baz
};

const foo = {
  bar: baz};

const { bar
} = obj;

Add padded-blocks [Changes] (Docs)
Ensures there is not a new line before or after the body of a block.
Examples

Valid syntax:
```
if (foo) {
  //
}
```
Invalid syntax:
```
if (foo) {

  //

}

if (foo) {

  //
}

if (foo) {
  //

}
```

Add eslint-plugin-import/exports-last [Changes] (Docs)
Ensures all exports are declared at the bottom of the file.

Examples

Valid syntax:

const str = 'foo';
const bool = true;

export {
  str,
  bool,
};

const str = 'foo';

export const bool = true;

Invalid syntax:

export const bool = true;

const str = 'foo';

Switch from ESLint's indent to @typescript-eslint/indent [Changes] (Docs)
Ensures consistent indentation.

This has known issues, revert to ESLint's version if it becomes problematic.

Examples

Valid syntax:

function foo() {
  if (bar) {
    console.log('baz');
  } else {
    console.log('qux');
  }
}

switch(a){
  case "a":
    break;
  case "b":
    break;
}

Invalid syntax:

function foo() {
 if (bar) {
console.log('baz');
} else {
     console.log('qux');
 }
}

switch(a){
case "a":
    break;
case "b":
    break;
}

Add 'no-unused-vars': 'off' as per ts-eslint docs [Changes]

We're already using @typescript-eslint/no-unused-vars, the docs state to explicitly disable no-unused-vars to avoid conflicts.

Switch from ESLint's semi to @typescript-eslint/semi [Changes] (Docs)
Ensures semicolons are added after each statement, including TypeScript statements.

Examples

Valid syntax:

type ClientUser = Pick<User, 'id' | 'discord_user_id'>;

const foo = 'bar';

object.foo = function() {
    //
};

class Foo {
    bar = 1;
}

Invalid syntax:

type ClientUser = Pick<User, 'id' | 'discord_user_id'>

const foo = 'bar'

object.foo = function() {
    //
}

class Foo {
    bar = 1
}

Add padding-line-between-statements rule for directives [Changes] (Docs)
Ensures there is an empty line after directives.
Examples

Valid syntax:
```
'use strict';

const foo = 'bar';
```
Invalid syntax:
```
'use strict';
const foo = 'bar';
```
Removed server/migrations from ignorePatterns and overrides, and applied style fixes. [Changes]
Switch from ESLint's object-curly-spacing to @typescript-eslint/object-curly-spacing & apply fixes [Changes] (Docs)
Switch from ESLint's comma-dangle to @typescript-eslint/comma-dangle [Changes] (Docs)
Switch from ESLint's padding-line-between-statements to @typescript-eslint/padding-line-between-statements [Changes] (Docs)

Testing

Run npm run lint && npm run test:server to ensure nothing is broken.

Optional: Copy incorrect code examples from the description and check that ESLint catches them.

Type of change

Other (chores/linting rules)

Screenshots

N/A

Checklist:

Changes have new/updated automated tests, if applicable
Changes have new/updated docs, if applicable
I have performed a self-review of my own code
I have added comments on any new, hard-to-understand code
Changes have been manually tested
Changes generate no new errors or warnings

This rule ensures ```js const obj = { foo: 'bar', baz, } ``` Instead of allowing ```js const obj = { foo: 'bar', baz: baz, } ```

This rule ensures: ```js const foo = { bar: 'baz' }; ``` Instead of allowing: ```js const foo = { bar:'baz' }; ```

Ensures a space exists before the start of a block that is not on a new line. [Documentation](https://typescript-eslint.io/rules/space-before-blocks/) This rule ensures: ```js if (foo) { // } ``` Instead of allowing: ```js if (foo){ // } ```

Ensures a space exists before and after the `=>` in an arrow function. [Documentation](https://eslint.org/docs/latest/rules/arrow-spacing) This rule ensures: ```js () => { // }; ``` Instead of allowing: ```js ()=>{ // }; ```

Ensures parenthesis around arrow function parameters are only used when required. [Documentation](https://eslint.org/docs/latest/rules/arrow-parens) This rule ensures: ```js foo => { // }; (foo, bar) => { // }; ``` Instead of allowing: ```js (foo) => { // }; (foo, bar) => { // }; ```

Ensures 1tbs (one true brace style), where braces must be on the same line as the keywords they're associated with. [Documentation](https://typescript-eslint.io/rules/brace-style/) This rule ensures: ```js if (foo) { // } else { // } try { // } catch(error) { // } ``` Instead of allowing: ```js if (foo) { // } else { // } try { // } catch(error) { // } ```

Ensures semicolon delimiters between members of interface and type literals. [Documentation](https://typescript-eslint.io/rules/member-delimiter-style) This rule ensures: ```js interface Foo { name: string; greet(): string; } type Bar = { name: string } type FooBar = { name: string; greet(): string } ``` Instead of allowing: ```js interface Foo { name: string greet(): string } interface Foo { name: string, greet(): string, } interface Foo { name: string; greet(): string } type FooBar = { name: string, greet(): string } ```

Ensures array brackets are either both on new lines or are both on the same line. [Documentation](https://eslint.org/docs/latest/rules/array-bracket-newline) This rule ensures: ```js const foo = []; const foo = [ 1, 2, 3 ]; const foo = [ 1, 2, 3, ]; const foo = [{ bar: baz }]; const foo = [ { bar: baz } ]; ``` Instead of allowing: ```js const foo = [ ]; const foo = [ 1, 2, 3 ]; const foo = [ 1, 2, 3]; const foo = [{ bar: baz } ]; const foo = [ { bar: baz }]; ```

Ensures object braces are either both on new lines or are both on the same line. [Documentation](https://eslint.org/docs/latest/rules/object-curly-newline) This rule ensures: ```js const foo = {}; const foo = { bar: baz }; const foo = { bar: baz, }; const { bar } = obj; const { bar } = obj; ``` Instead of allowing: ```js const foo = { }; const foo = { bar: baz }; const foo = { bar: baz}; const { bar } = obj; ```

Ensures there is not a new line before or after the body of a block. [Documentation](https://eslint.org/docs/latest/rules/padded-blocks) This rule ensures: ```js if (foo) { // } ``` Instead of allowing: ```js if (foo) { // } if (foo) { // } if (foo) { // } ```

Ensures all exports are declared at the bottom of the file. [Documentation](https://github.com/import-js/eslint-plugin-import/blob/5680a1f8d41cd19f9c60d999a6fadf10994a0a64/docs/rules/exports-last.md) This rule ensures: ```js const str = 'foo'; const bool = true; export { str, bool, }; ``` ```js const str = 'foo'; export const bool = true; ``` Instead of allowing: ```js export const bool = true; const str = 'foo'; ```

Ensures consistent indentation. [Documentation](https://typescript-eslint.io/rules/indent) !! This has [known issues](typescript-eslint/typescript-eslint#1824), revert to ESLint's version if necessary. This rule ensures: ```js function foo() { if (bar) { console.log('baz'); } else { console.log('qux'); } } switch(a){ case "a": break; case "b": break; } ``` Instead of allowing: ```js function foo() { if (bar) { console.log('baz'); } else { console.log('qux'); } } switch(a){ case "a": break; case "b": break; } ```

We're already using `@typescript-eslint/no-unused-vars`, the [docs](https://typescript-eslint.io/rules/no-unused-vars) state to explicitly disable `no-unused-vars` to avoid conflicts.

Ensures semicolons are added after each statement, including TypeScript statements. [Documentation](https://typescript-eslint.io/rules/semi) This rule ensures: ```js type ClientUser = Pick<User, 'id' | 'discord_user_id'>; const foo = 'bar'; object.foo = function() { // }; class Foo { bar = 1; } ``` Instead of allowing: ```js type ClientUser = Pick<User, 'id' | 'discord_user_id'> const foo = 'bar' object.foo = function() { // } class Foo { bar = 1 } ```

Ensures there is an empty line after directives. [Documentation](https://eslint.org/docs/latest/rules/padding-line-between-statements) This rule ensures: ```js 'use strict'; const foo = 'bar'; ``` Instead of allowing: ```js 'use strict'; const foo = 'bar'; ```

devonzara · 2023-04-02T06:57:40Z

Phew! That was a lot... hopefully these changes will catch most of the nits and allow us to focus more on implementations. 👍🏼

There was one rule that eluded me... no-redundant-variables which is a feature in JetBrains IDEs, but I wasn't able to find an existing ESLint rule for checking this.

Example

const user = await User.create(userObject); // Local variable `user` is redundant.

return user;

Which implies that it can be simplified to:

return await User.create(userObject);

MainlyColors

LGTM
I like all the rules but have 2 comments

Is this a normal warning? I deleted the node_modules folder and did a clean install but the warning is still there.
Definition for rule '@typescript-eslint/key-spacing' was not found.eslint(@typescript-eslint/key-spacing)

And the rule still works, pasted in the example

2nd comment

This warning about using Eslint as a formatter is on the typescript-eslint docs for a lot of the rules, is there a reason we want to use ESlint for this instead of a formatter? I have no strong opinion either way, just for my own knowledge wanted to know the why

devonzara · 2023-04-03T17:33:29Z

@MainlyColors, Prettier is currently the most popular formatter. Unfortunately, it's very opinionated and not very customizable. Until it becomes more customizable, another formatter arises, or we decide to adopt their opinionated coding style; ESLint is still the best option, despite their counter recommendations.

Caleb-Cohen · 2023-04-03T17:51:25Z

in object-curly-newline why is

const foo = {
};

Valid, but the one below invalid?

const foo = { bar: baz
};

Caleb-Cohen

Left a question, otherwise LGTM.

devonzara · 2023-04-03T18:01:25Z

@Caleb-Cohen Ironically, if you look at the message on the commit, I had that case listed as invalid because I also misinterpreted it initially.

Reasoning: It's checking if the braces are consistent. In the first example, both braces are in fact on their own line. In the second example, the closing brace is sharing the line with its contents.

It would need a different rule if we wanted to force empty objects to be on a single line, though I don't know of one that exists.

timmyichen

Tested, LGTM!

Ensures spacing around keywords such as `if`, `else`, `function`, `try`, `catch`, etc. [Documentation](https://typescript-eslint.io/rules/keyword-spacing) This rule ensures: ```js if (foo) { //... } else if (bar) { //... } else { //... } ``` Instead of allowing: ```js if (foo) { //... }else if (bar) { //... }else { //... } ```

Ensures spacing around operators such as `+`, `-`, `?`, `:`, `=`, etc. [Documentation](https://typescript-eslint.io/rules/space-infix-ops) This rule ensures: ```js a + b a + b a ? b : c // `object-curly-spacing` would ensure spacing inside `{}` var {a = 0} = bar; // `key-spacing` would ensure a space after `:` const a = {b:1}; function foo(a = 0) {} ``` Instead of allowing: ```js a+b a+ b a +b a?b:c var {a=0}=bar; const a={b:1}; function foo(a=0) { } ```

devonzara · 2023-04-04T08:13:52Z

Added two more rules, which should be the last of them. Unless there are any objections, I'll merge in the evening. 👍🏼

devonzara added the developer experience label Apr 2, 2023

devonzara self-assigned this Apr 2, 2023

devonzara added 2 commits April 1, 2023 21:33

style: add object-shorthand rule & apply fixes

1d098f9

This rule ensures ```js const obj = { foo: 'bar', baz, } ``` Instead of allowing ```js const obj = { foo: 'bar', baz: baz, } ```

chore: update ts-eslint plugin & parser

69201da

devonzara force-pushed the dz-add-style-rules branch from 974be28 to bb23fcc Compare April 2, 2023 01:34

style: add ts-eslint key-spacing rule & apply fixes

9547350

This rule ensures: ```js const foo = { bar: 'baz' }; ``` Instead of allowing: ```js const foo = { bar:'baz' }; ```

devonzara force-pushed the dz-add-style-rules branch from bb23fcc to bdb34ec Compare April 2, 2023 02:35

devonzara added 4 commits April 1, 2023 23:45

style: add arrow-spacing rule

510969d

Ensures a space exists before and after the `=>` in an arrow function. [Documentation](https://eslint.org/docs/latest/rules/arrow-spacing) This rule ensures: ```js () => { // }; ``` Instead of allowing: ```js ()=>{ // }; ```

devonzara force-pushed the dz-add-style-rules branch from bdb34ec to 4b009df Compare April 2, 2023 04:11

devonzara added 11 commits April 2, 2023 00:26

style: add 'no-unused-vars': 'off' as per ts-eslint docs

3154ea0

We're already using `@typescript-eslint/no-unused-vars`, the [docs](https://typescript-eslint.io/rules/no-unused-vars) state to explicitly disable `no-unused-vars` to avoid conflicts.

style: organize eslintrc

6139fa9

style: remove ignores/overrides and apply style fixes

3c2f693

devonzara marked this pull request as ready for review April 2, 2023 06:46

devonzara requested review from timmyichen, Caleb-Cohen, code2244, dev-skworks, DevinCLane and intelagense as code owners April 2, 2023 06:46

devonzara requested review from jamiecha88, jmoldyvan, MainlyColors, MatthewBozin, wo1vin, sstrunks52005 and windyScripts as code owners April 2, 2023 06:46

MainlyColors approved these changes Apr 3, 2023

View reviewed changes

Caleb-Cohen approved these changes Apr 3, 2023

View reviewed changes

timmyichen approved these changes Apr 4, 2023

View reviewed changes

devonzara added 2 commits April 4, 2023 03:54

devonzara added 5 commits April 4, 2023 14:19

style: switch to ts-eslint object-curly-spacing & apply fixes

5f7f553

style: switch to ts-eslint comma-dangle

41949fc

style: switch to ts-eslint padding-line-between-statements

0231dcd

Merge branch 'main'

7639c06

style: apply style fixes

c886081

devonzara merged commit e4a5f28 into main Apr 4, 2023
2 checks passed

devonzara deleted the dz-add-style-rules branch April 4, 2023 19:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

style: add additional style rules & apply fixes #75

style: add additional style rules & apply fixes #75

devonzara commented Apr 2, 2023 •

edited

devonzara commented Apr 2, 2023 •

edited

MainlyColors left a comment •

edited

devonzara commented Apr 3, 2023 •

edited

Caleb-Cohen commented Apr 3, 2023

Caleb-Cohen left a comment

devonzara commented Apr 3, 2023 •

edited

timmyichen left a comment

devonzara commented Apr 4, 2023

style: add additional style rules & apply fixes #75

style: add additional style rules & apply fixes #75

Conversation

devonzara commented Apr 2, 2023 • edited

Description

Testing

Type of change

Screenshots

Checklist:

devonzara commented Apr 2, 2023 • edited

MainlyColors left a comment • edited

Choose a reason for hiding this comment

devonzara commented Apr 3, 2023 • edited

Caleb-Cohen commented Apr 3, 2023

Caleb-Cohen left a comment

Choose a reason for hiding this comment

devonzara commented Apr 3, 2023 • edited

timmyichen left a comment

Choose a reason for hiding this comment

devonzara commented Apr 4, 2023

devonzara commented Apr 2, 2023 •

edited

devonzara commented Apr 2, 2023 •

edited

MainlyColors left a comment •

edited

devonzara commented Apr 3, 2023 •

edited

devonzara commented Apr 3, 2023 •

edited