Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Include actual types for restrict-plus-operands. #4635

Merged
merged 4 commits into from Apr 16, 2019

Conversation

mprobst
Copy link
Contributor

@mprobst mprobst commented Apr 3, 2019

It can be difficult for users to find out why exactly they received the
"restrict-plus-operands" lint error. In particular, users often end up
with any types leaking into their code (e.g. in tests), and are then
puzzled why this triggers.

To fix, this change includes a textual representation of the type in the
error message, e.g.:

[Operands of '+' operation must either be both strings or both numbers, but found 5 + undefined[]]

The type representation isn't always super pretty (e.g. the
undefined[] bit above), but should still be helpful, and also matches
TS compiler's representation of these.

PR checklist

  • bugfix
    • Includes tests

CHANGELOG.md entry:

[enhancement] Improve error message for restrict-plus-operands rule.

@mprobst
Copy link
Contributor Author

mprobst commented Apr 3, 2019

@bowenni can you take a look at this?

@mprobst
Copy link
Contributor Author

mprobst commented Apr 3, 2019

I think the test failure is an infrastructure failure, but I cannot retry because I don't have permissions.

@JoshuaKGoldberg
Copy link
Contributor

I love this idea! Marked as accepting PRs.

In the future @mprobst, please file an issue describing your requested changes before sending a PR that non-trivially changes TSLint. This one happens to be immediately acceptable but most aren't.

Copy link
Contributor

@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.

Thanks for sending this in! Just one touchup on the failure text requested.

src/rules/restrictPlusOperandsRule.ts Show resolved Hide resolved
test/rules/restrict-plus-operands/test.ts.lint Outdated Show resolved Hide resolved
It can be difficult for users to find out why exactly they received the
"restrict-plus-operands" lint error. In particular, users often end up
with `any` types leaking into their code (e.g. in tests), and are then
puzzled why this triggers.

To fix, this change includes a textual representation of the type in the
error message, e.g.:

    [Operands of '+' operation must either be both strings or both numbers, but found 5 + undefined[]]

The type representation isn't always super pretty (e.g. the
`undefined[]` bit above), but should still be helpful, and also matches
TS compiler's representation of these.
@mprobst
Copy link
Contributor Author

mprobst commented Apr 7, 2019

In the future @mprobst, please file an issue describing your requested changes before sending a PR that non-trivially changes TSLint. This one happens to be immediately acceptable but most aren't.

Will do.

Copy link
Contributor

@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.

Fantastic, thanks so much @mprobst! 🙌

Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

thanks @mprobst @JoshuaKGoldberg! I'll fix the build on master if it fails there.

@adidahiya adidahiya merged commit 3dc40d0 into palantir:master Apr 16, 2019
@rrdelaney rrdelaney mentioned this pull request Mar 10, 2020
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants