fullstacksjs / eslint-config

Fullstacks eslint config
MIT License
42 stars 3 forks source link

Add "padding-line-between-statements" rule #10

Closed erfanansari closed 1 year ago

erfanansari commented 1 year ago

I've used this rule with the following config for a while. It has cleaned the codebase and made navigating through statements much more convenient. Take a look at it please and see if it is suitable.

        "padding-line-between-statements": [
            "warn",
            {
                "blankLine": "always",
                "prev": "*",
                "next": [
                    "export",
                    "return",
                    "class",
                    "for",
                    "while",
                    "switch",
                    "do",
                    "directive",
                    "function",
                    "iife",
                    "block-like",
                    "multiline-const",
                    "multiline-let"
                ]
            }
        ],
ASafaeirad commented 1 year ago

Thank you for the contribution, I'll test that in our codebases and create a PR.

ASafaeirad commented 1 year ago

Well, some suggestions, first

// if statement after variable declaration.
const handledKeys = [
    'Backspace',
    'ArrowLeft',
    'ArrowRight',
];
if (handledKeys.includes(e.code)) e.preventDefault();
useEffect(() => {
  if (!isMounted.current) {
    isMounted.current = true;
    return;
  }

  return effect();
}, []);

I'll investigate more in my codebases. What's your take on this?

erfanansari commented 1 year ago

Thanks for your feedback! So for the first issue, it is possible to add "if" to the next property, in that case, it warns even if the if statement is only one line. For the Second issue, I think it is fine like this, you could remove "return" from the next property if you want to but I think it's legit to leave a blank line before the return statement.

ASafaeirad commented 1 year ago

It's a very good idea to have padding before return. I'll improve the rule to have both options.

ASafaeirad commented 1 year ago

This is what worked for me. There are some false positives, but I can't be stricter because the current rule's selectors are insufficient.

  "@typescript-eslint/padding-line-between-statements": [     
      "warn",
      {
        "blankLine": "always",
        "prev": "*",
        "next": [
          "class",
          "for",
          "while",
          "switch",
          "do",
          "directive",
          "function",
          "iife",
          "block-like"
        ]
      },
      {
        "blankLine": "always",
        "prev": ["block-like", "block"],
        "next": ["return"]
      },
      {
        "blankLine": "always",
        "prev": ["block-like"],
        "next": ["const", "let", "var"]
      },
      {
        "blankLine": "always",
        "prev": ["const", "let", "var"],
        "next": ["type", "interface"]
      },
      {
        "blankLine": "always",
        "prev": ["multiline-const", "multiline-let", "multiline-var"],
        "next": ["if"]
      }
ASafaeirad commented 1 year ago

Released: https://github.com/fullstacksjs/eslint-config/releases/tag/v8.9.0