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): [unified-signatures] type comparison and exported nodes #839

Merged
merged 16 commits into from Aug 19, 2019

Conversation

uniqueiniquity
Copy link
Contributor

@uniqueiniquity uniqueiniquity commented Aug 12, 2019

Currently, the unified-signatures rule doesn't handle exported function declarations because of the way export is handled as a distinct AST node for ESTree.
Here, we update the rule to handle this case.

EDIT: this PR also fixes type comparison in typesAreEqual. Currently, the type properties of the type annotations are checked for equality, so any two array types as return types or parameter types are treated the same. Here, we use TSLint's approach in comparing the actual text of the type. It's not perfect, but it's definitely better.

@typescript-eslint
Copy link
Contributor

Thanks for the PR, @uniqueiniquity!

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

@uniqueiniquity
Copy link
Contributor Author

cc: @armanio123 @a-tarasyuk

@codecov
Copy link

codecov bot commented Aug 13, 2019

Codecov Report

Merging #839 into master will decrease coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #839      +/-   ##
==========================================
- Coverage   94.17%   94.13%   -0.04%     
==========================================
  Files         112      112              
  Lines        4840     4843       +3     
  Branches     1344     1347       +3     
==========================================
+ Hits         4558     4559       +1     
- Misses        161      163       +2     
  Partials      121      121
Impacted Files Coverage Δ
...ages/eslint-plugin/src/rules/unified-signatures.ts 94.47% <100%> (-1.15%) ⬇️

@bradzacher bradzacher added the bug Something isn't working label Aug 13, 2019
@uniqueiniquity uniqueiniquity changed the title fix(eslint-plugin): warn on exported signatures that should be unified fix(eslint-plugin): fixes for type comparison and exported nodes in unified-signatures Aug 14, 2019
@bradzacher bradzacher changed the title fix(eslint-plugin): fixes for type comparison and exported nodes in unified-signatures fix(eslint-plugin): [unified-signatures] type comparison and exported nodes Aug 16, 2019
Copy link
Member

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

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

Thanks!

@JamesHenry JamesHenry merged commit 580eceb into typescript-eslint:master Aug 19, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants