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

feat(eslint-plugin): add extension rule space-before-blocks (#1606) #4184

Merged
merged 2 commits into from Feb 24, 2022

Conversation

FDIM
Copy link
Contributor

@FDIM FDIM commented Nov 18, 2021

PR Checklist

Overview

I've added an extension rule for space-before-blocks that ensures that space before curly brace is enforced for enums and interfaces.

I'll add rule documentation file a bit later and in the meantime, feedback on PR is welcome as I'm not too familiar with this code base. Most specifically if there is any better way of handling schema without copying & extending. (PR is ready)

Impressive local setup, even debugging via vscode just worked!

EDIT: and even more impressive setup in CI, unit test for missing doc file was a nice surprise.

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @FDIM!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!


🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

@netlify
Copy link

netlify bot commented Nov 18, 2021

❌ Deploy Preview for typescript-eslint failed.

🔨 Explore the source changes: cd06e6a

🔍 Inspect the deploy log: https://app.netlify.com/sites/typescript-eslint/deploys/621783f450188400074b338d

@nx-cloud
Copy link

nx-cloud bot commented Nov 18, 2021

☁️ Nx Cloud Report

CI ran the following commands for commit cd06e6a. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 48 targets

Sent with 💌 from NxCloud.

@FDIM FDIM force-pushed the feat/space-before-blocks branch 7 times, most recently from 7d9b8d7 to 5f823df Compare November 20, 2021 20:47
@codecov
Copy link

codecov bot commented Nov 20, 2021

Codecov Report

Merging #4184 (cd06e6a) into main (823b945) will decrease coverage by 0.18%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #4184      +/-   ##
==========================================
- Coverage   94.62%   94.44%   -0.19%     
==========================================
  Files         149      155       +6     
  Lines        8072     8312     +240     
  Branches     2581     2647      +66     
==========================================
+ Hits         7638     7850     +212     
- Misses        239      261      +22     
- Partials      195      201       +6     
Flag Coverage Δ
unittest 94.44% <100.00%> (-0.19%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/eslint-plugin/src/configs/all.ts 100.00% <ø> (ø)
...ckages/eslint-plugin/src/util/getESLintCoreRule.ts 75.00% <ø> (ø)
...ges/eslint-plugin/src/rules/space-before-blocks.ts 100.00% <100.00%> (ø)
...gin-internal/src/rules/no-poorly-typed-ts-props.ts 88.88% <0.00%> (ø)
...lugin-internal/src/rules/plugin-test-formatting.ts 84.32% <0.00%> (ø)
...internal/src/rules/no-typescript-default-import.ts 100.00% <0.00%> (ø)
...plugin-internal/src/rules/prefer-ast-types-enum.ts 91.30% <0.00%> (ø)
...-internal/src/rules/no-typescript-estree-import.ts 87.50% <0.00%> (ø)

@bradzacher bradzacher added the enhancement: new base rule extension New base rule extension required to handle a TS specific case label Nov 23, 2021
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in general -- thanks for adding this in @FDIM! ❤️‍🔥

I'm not particularly familiar with extension rules yet so I'll defer to @bradzacher or another maintainer.

Just requesting changes on a few type system and API usage fixups.

@JoshuaKGoldberg JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Dec 18, 2021
Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all josh's comments are on point!
otherwise looks to be extended appropriately

@bradzacher bradzacher removed the awaiting response Issues waiting for a reply from the OP or another party label Jan 1, 2022
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the one thing on supporting older ESLint versions, otherwise 💯 !

Edit: and merging in from the main branch, if you don't mind 😄

@JoshuaKGoldberg
Copy link
Member

Btw there's no need to force push. We squash PR commits. Force pushing makes it a little harder to review in some cases not hit by this PR for us.

@bradzacher bradzacher added the awaiting response Issues waiting for a reply from the OP or another party label Jan 24, 2022
@FDIM
Copy link
Contributor Author

FDIM commented Feb 13, 2022

@JoshuaKGoldberg It should be ready again :D And sorry taking this long and for force pushing, got into this habit when working a small PRs for angular.

@JoshuaKGoldberg
Copy link
Member

I've always been more of a React dev myself... 😜

Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, still very close! 😄

But thanks for coming back to this! There are some introduced unit test failures due to merging in updated tests & documentation standards from main you'll want to clean up. Let me know if you don't have time and I can help. ❤️

@FDIM
Copy link
Contributor Author

FDIM commented Feb 13, 2022

@JoshuaKGoldberg few iterations later, everything should finally be good 😄

@bradzacher bradzacher removed the awaiting response Issues waiting for a reply from the OP or another party label Feb 14, 2022
Copy link
Member

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super, thanks @FDIM! ✨

@JoshuaKGoldberg JoshuaKGoldberg merged commit 208b6d0 into typescript-eslint:main Feb 24, 2022
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request 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 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement: new base rule extension New base rule extension required to handle a TS specific case
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing spaces around name interface and type
3 participants