Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing spaces around name interface and type #1606

Closed
valentineus opened this issue Feb 14, 2020 · 11 comments · Fixed by #4184
Closed

Missing spaces around name interface and type #1606

valentineus opened this issue Feb 14, 2020 · 11 comments · Fixed by #4184
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: new base rule extension New base rule extension required to handle a TS specific case formatting Related to whitespace/bracket formatting. We strongly recommend you use a formatter instead. package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@valentineus
Copy link

Repository with example.

Instruction

  1. Clone a repository:
git clone https://gitlab.com/valentineus/typescript-eslint-example.git
  1. Install dependencies:
npm install
  1. Run a test:
npm run lint

Code to check:

interface ITesstInterfase{
    key: string;
}

type TestType=() => void;

const variable1: TestType = () => {

    console.log("Hello, World!");

};

variable1();

Enabled all rules eslint and @typescript-eslint/eslint-plugin.

On line 1, missing spaces around interface ITesstInterfase.
On line 5, missing spaces around sign =.

Are there any rules governing such cases?

package version
@typescript-eslint/eslint-plugin 2.19.2
@typescript-eslint/parser 2.19.2
eslint 6.8.0
@valentineus valentineus added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Feb 14, 2020
@bradzacher
Copy link
Member

There is not rules which cover either. Our general guidance is to not rely upon a linter for formatting, and instead you should use a tool made for formatting, like prettier.

That being said:

  • type TestType=() => void; could be handled by an extended version of space-infix-ops.

  • interface ITestInterface{ could be handled by an extended version of space-before-blocks

Happy to accept a PR.

@bradzacher bradzacher added enhancement: new base rule extension New base rule extension required to handle a TS specific case and removed triage Waiting for maintainers to take a look labels Feb 14, 2020
@zanona
Copy link

zanona commented Jul 15, 2020

Our general guidance is to not rely upon a linter for formatting, and instead you should use a tool made for formatting, like prettier.

Hey, @bradzacher, sorry about the off-topic here but referring to your comment, would you recommend anything besides prettier? Or better, does anything besides prettier exists to format code nowadays? The problem is that being 'opinionated' like prettier, doesn't allow for customisation at all. So there must be another solution. :)

@richardsimko
Copy link

Our general guidance is to not rely upon a linter for formatting, and instead you should use a tool made for formatting, like prettier.

Another question on this topic, there are plenty of rules regarding formatting, what's the reasoning behind including them in that case? (comma-spacing, indend, keword-spacing just to name a few)

@bradzacher
Copy link
Member

To start with - a personal anecdote for you.

At the start of my career, I was all for tweaking and adjusting code style to match the team's style. I spent many hours configuring eslint (and other tools, like resharper for C#) to match the exact code style of our codebase. I did this for years across 5 or 6 codebases and 2 companies. But it was never quite right, there were edge cases that rules didn't catch, or cases that rules wouldn't enforce. It resulted in writing documents about style standards to help code reviewers catch incorrectly styled code. It was a large burden for everyone.

And then I started at facebook, where they enforce that all JS code across their ginormous monorepo is formatted using prettier, and have done so for many years. Initially I hated it - I am the engineer and I want to format my code in a certain way.

It only took me a week of using prettier to fall in love. No longer did I have to think about formatting. No longer did I have to think about indentation or brace placement. I just wrote code, hit save, and got a consistent code style.

When reviewing code I no longer had to look at the formatting or anything. I didn't have to keep an eye out for disabled formatting rules and question why. I just reviewed the code and thought about the logic.

It was a revelation - I went to all of my open source projects and I installed prettier and formatted the codebases, and I've never looked back.

Now-a-days I actually find I dislike working on codebases that don't use prettier now because those codebases are often inconsistently formatted. It's unpredictable and makes the code harder to read.

The moral of this story is to just try it - let go of the desire to control the styling and you'll find how much better your engineering life will be.


would you recommend anything besides prettier

🤷‍♂️ I've never used or wanted to use anything else. Prettier lists the prettierx project as a related "less opinionated" version of prettier. Never looked into it or used it however, so IDK if it's going to do what you want.

The problem is that being 'opinionated' like prettier, doesn't allow for customisation at all.

The reason that it doesn't allow for customisation is that customisation makes the codebase a maintenance nightmare. Case in point - the indent rule (#1824).

When you start adding options, people want more options. More options means more branches. More branches means exponentially more tests. It is a nightmare.

We see this problem within this project as well - some lint rules have ridiculous combinations of options and attempting to cover every use case leads to tests that are unapproachable and unmaintainable. For example this 3300 line monster test suite for the overly configurable member-delimited-style stylistic rule. The rule itself is only 239 lines, but it needs that much testing to cover every branch and case. It's straight up not worth it - nobody is ever going to be able to work on that rule ever again because the tests are so complicated.

Another question on this topic, there are plenty of rules regarding formatting, what's the reasoning behind including them in that case?

One reason really: people want them - engineers are opinionated AF and want to do things their way 😄

Earlier in my OSS "career", I was all for satisfying everyone - I personally implemented and enhanced extension rules for these stylistic rules, and I worked on new rules to help style TS code.

But since discovering prettier, I've gone in the other direction. I try to do as close to zero work on stylistic rules as I can now - I don't have enough time as it is to work on this project, so I don't want to (as I see it) waste my time on stylistic rules when I can instead work on higher value tasks like improving the core infra.

If contributors want to work on and implement or fix stylistic extension rules themselves, I'm not going to say no - I'll happily review the PRs and merge them. Extension rules I see as a bugfix instead - adding TS compatibility to the base rules, so blocking people working on them will ultimately negatively impact the ecosystem and detract from this project's value.

@zanona
Copy link

zanona commented Jul 16, 2020

@bradzacher, thank you so much for taking the time to tell us your experience and how that influenced your way of thinking, making you a better professional. It's really great reading about this, as sometimes, we become too obsessed with a single way of solving problems and forget about other possibilities. Thanks again.

@richardsimko
Copy link

@bradzacher Thanks for the long and detailed reply! I'll make sure to give prettier a try ;)

@FDIM
Copy link
Contributor

FDIM commented Mar 28, 2021

The issue related to space around type declaration is now fixed in the linked PR. Second issue with interface I'll address in a separate PR - it's a bit more work as space-before-blocks needs to be extended, yet to figure out how

@josephzidell
Copy link
Contributor

josephzidell commented Sep 9, 2021

@FDIM Have you had time to look at spacing around interface blocks?

@morganbarrett
Copy link

space-before-blocks should handle types before the open parenthesis when set to "never"
e.g function x(): number { vs function x(): number{
this could be an extra option for the extended version

@sbitenc
Copy link

sbitenc commented Sep 22, 2021

When is this coming out? I need this :)

@bradzacher
Copy link
Member

With any issue in opened in this project - it either has a visible progress in the form of an attached PR, or it has no progress.

We are a community run project. The volunteer maintainers spend most of their time triaging issues and reviewing PRs. This means that most issues will not progress unless a member of the community steps up and champions it.

If this issue is important to you - consider being that champion.

If not - please just subscribe to the issue and wait patiently.
Commenting asking for status updates does not bump issue priority in any way and just serves to spam everyone subscribed to the issue.

@JoshuaKGoldberg JoshuaKGoldberg added the accepting prs Go ahead, send a pull request that resolves this issue label Oct 25, 2021
FDIM added a commit to FDIM/typescript-eslint that referenced this issue Nov 18, 2021
FDIM added a commit to FDIM/typescript-eslint that referenced this issue Nov 18, 2021
FDIM added a commit to FDIM/typescript-eslint that referenced this issue Nov 18, 2021
FDIM added a commit to FDIM/typescript-eslint that referenced this issue Nov 20, 2021
FDIM added a commit to FDIM/typescript-eslint that referenced this issue Nov 20, 2021
FDIM added a commit to FDIM/typescript-eslint that referenced this issue Nov 20, 2021
FDIM added a commit to FDIM/typescript-eslint that referenced this issue Nov 20, 2021
FDIM added a commit to FDIM/typescript-eslint that referenced this issue Nov 20, 2021
FDIM added a commit to FDIM/typescript-eslint that referenced this issue Jan 1, 2022
FDIM added a commit to FDIM/typescript-eslint that referenced this issue Jan 10, 2022
FDIM added a commit to FDIM/typescript-eslint that referenced this issue Feb 13, 2022
FDIM added a commit to FDIM/typescript-eslint that referenced this issue Feb 13, 2022
FDIM added a commit to FDIM/typescript-eslint that referenced this issue Feb 13, 2022
@JoshuaKGoldberg JoshuaKGoldberg added the formatting Related to whitespace/bracket formatting. We strongly recommend you use a formatter instead. label Feb 24, 2022
JoshuaKGoldberg added a commit that referenced this issue Feb 24, 2022
…#4184)

Co-authored-by: Josh Goldberg <me@joshuakgoldberg.com>
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this issue Mar 8, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.12.1` -> `5.13.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/5.12.1/5.13.0) |
| [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.12.1` -> `5.13.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/5.12.1/5.13.0) |

---

### Release Notes

<details>
<summary>typescript-eslint/typescript-eslint (@&#8203;typescript-eslint/eslint-plugin)</summary>

### [`v5.13.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#&#8203;5130-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5121v5130-2022-02-28)

[Compare Source](typescript-eslint/typescript-eslint@v5.12.1...v5.13.0)

##### Bug Fixes

-   **eslint-plugin:** \[sort-type-union-intersection-members] Wrap the constructorType in parentheses ([#&#8203;4590](typescript-eslint/typescript-eslint#4590)) ([851bb90](typescript-eslint/typescript-eslint@851bb90))

##### Features

-   **eslint-plugin:** \[no-misused-promises] check more places for checksVoidReturn ([#&#8203;4541](typescript-eslint/typescript-eslint#4541)) ([052cf51](typescript-eslint/typescript-eslint@052cf51))
-   **eslint-plugin:** add `no-redundant-type-constituents` rule ([#&#8203;4378](typescript-eslint/typescript-eslint#4378)) ([63d051e](typescript-eslint/typescript-eslint@63d051e))
-   **eslint-plugin:** add `no-useless-empty-export` rule ([#&#8203;4380](typescript-eslint/typescript-eslint#4380)) ([823b945](typescript-eslint/typescript-eslint@823b945))
-   **eslint-plugin:** add extension rule `space-before-blocks` ([#&#8203;1606](typescript-eslint/typescript-eslint#1606)) ([#&#8203;4184](typescript-eslint/typescript-eslint#4184)) ([208b6d0](typescript-eslint/typescript-eslint@208b6d0))
-   **eslint-plugin:** added member group support to member-ordering rule ([#&#8203;4538](typescript-eslint/typescript-eslint#4538)) ([6afcaea](typescript-eslint/typescript-eslint@6afcaea))

#### [5.12.1](typescript-eslint/typescript-eslint@v5.12.0...v5.12.1) (2022-02-21)

##### Bug Fixes

-   **eslint-plugin:** \[no-unnecessary-type-arguments] fix comparison of types ([#&#8203;4555](typescript-eslint/typescript-eslint#4555)) ([fc3936e](typescript-eslint/typescript-eslint@fc3936e))

</details>

<details>
<summary>typescript-eslint/typescript-eslint (@&#8203;typescript-eslint/parser)</summary>

### [`v5.13.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#&#8203;5130-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5121v5130-2022-02-28)

[Compare Source](typescript-eslint/typescript-eslint@v5.12.1...v5.13.0)

**Note:** Version bump only for package [@&#8203;typescript-eslint/parser](https://github.com/typescript-eslint/parser)

#### [5.12.1](typescript-eslint/typescript-eslint@v5.12.0...v5.12.1) (2022-02-21)

**Note:** Version bump only for package [@&#8203;typescript-eslint/parser](https://github.com/typescript-eslint/parser)

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found.

🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Co-authored-by: 6543 <6543@noreply.codeberg.org>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1192
Reviewed-by: 6543 <6543@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue enhancement: new base rule extension New base rule extension required to handle a TS specific case formatting Related to whitespace/bracket formatting. We strongly recommend you use a formatter instead. package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants