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

fix(eslint-plugin): [type-annotation-spacing] report excess spaces as error #1013

Closed
wants to merge 5 commits into from
Closed

fix(eslint-plugin): [type-annotation-spacing] report excess spaces as error #1013

wants to merge 5 commits into from

Conversation

darcien
Copy link

@darcien darcien commented Sep 27, 2019

Fixes #1001

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @darcien!

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.

@bradzacher bradzacher added the bug Something isn't working label Sep 27, 2019
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.

code itself looks good, but we should probably handle extra spacing on both sides of the delimiter.

@darcien
Copy link
Author

darcien commented Oct 14, 2019

code itself looks good, but we should probably handle extra spacing on both sides of the delimiter.

Aaah right, I'll update it.

@darcien
Copy link
Author

darcien commented Oct 14, 2019

For the tests, I'm not sure how many test cases I should add though.

If I added too many, the test will be failing.

Something like this:

Summary of all failing tests
 FAIL  tests/rules/type-annotation-spacing.test.ts
  ● Test suite failed to run

    TypeScript diagnostics (customize using `[jest-config].globals.ts-jest.diagnostics` option):
    tests/rules/type-annotation-spacing.test.ts:1044:11 - error TS2322: Type 'string' is not assignable to type '"expectedSpaceAfter" | "expectedSpaceBefore" | "unexpectedSpaceBefore"'.

    1044           messageId: 'unexpectedSpaceBefore',
                   ~~~~~~~~~

      ../../node_modules/@typescript-eslint/experimental-utils/dist/ts-eslint/RuleTester.d.ts:21:5
        21     messageId: TMessageIds;
               ~~~~~~~~~
        The expected type comes from property 'messageId' which is declared here on type 'TestCaseError<"expectedSpaceAfter" | "expectedSpaceBefore" | "unexpectedSpaceBefore">'

@bradzacher
Copy link
Member

It's a really weird error, I know - it's a weird way that typescript reports the error.
That type error usually occurs because there is something wrong with the test you've added - either there's a typo in a prop name, a missing property, or similar.
I usually narrow it down by commenting out tests one-by-one until I find out which one has the error.

@bradzacher bradzacher added the awaiting response Issues waiting for a reply from the OP or another party label Nov 12, 2019
@armano2
Copy link
Member

armano2 commented Dec 29, 2019

@darcien i did some testing and this issue with types got solved (is no longer present in experimental-utils/typescript)

after running this rule it seems to try to remove new lines after : (in our case this is conflicting with prettier)

      node:
        | TSESTree.VariableDeclarator
        | TSESTree.Parameter
        | TSESTree.ClassProperty,

becomes:

      node: | TSESTree.VariableDeclarator
        | TSESTree.Parameter
        | TSESTree.ClassProperty,

@bradzacher
Copy link
Member

bradzacher commented Dec 29, 2019

in our case this is conflicting with prettier

We should probably add the prettier/@typescript-eslint config to our codebase.
Prettier is not expected to play nicely with formatting rules, and as such this conflict is fine.

@bradzacher bradzacher added the stale PRs or Issues that are at risk of being or have been closed due to inactivity for a prolonged period label Apr 13, 2020
@bradzacher bradzacher closed this Apr 13, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
awaiting response Issues waiting for a reply from the OP or another party bug Something isn't working stale PRs or Issues that are at risk of being or have been closed due to inactivity for a prolonged period
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[type-annotation-spacing] Number of spaces aren't enforced
3 participants