Skip to content

The assertion-arguments rule should validate message argument type #167

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

Closed
sindresorhus opened this issue Jan 28, 2017 · 6 comments · Fixed by #314
Closed

The assertion-arguments rule should validate message argument type #167

sindresorhus opened this issue Jan 28, 2017 · 6 comments · Fixed by #314
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted

Comments

@sindresorhus
Copy link
Member

sindresorhus commented Jan 28, 2017

Issuehunt badges

This currently passes t.deepEqual({}, {}, {});, which can make for some hard to debug issues. @gajus reported such issue on Gitter.

We should ensure that if message is specified, it's a string. We could implement inline literal validation first, as that's pretty easy.

We should also detect the type if it's a variable defined in the same scope:

const message = 'foo';

...

t.deepEqual({}, {}, message);

@jfmengels Many rules could use some kind of naive type inference. Maybe eslint-ast-utils could add something like that?


Note: This issue has a bounty, so it's expected that you are an experienced programmer and that you give it your best effort if you intend to tackle this. Don't forget, if applicable, to add tests, docs (double-check for typos). And don't be sloppy. Review your own diff multiple times and try to find ways to improve and simplify your code. Instead of asking too many questions, present solutions. The point of an issue bounty is to reduce my workload, not give me more. Include a 🦄 in your PR description to indicate that you've read this. Thanks for helping out 🙌 - @sindresorhus


IssueHunt Summary

gmartigny gmartigny has been rewarded.

Backers (Total: $60.00)

Submitted pull Requests


Tips

@gajus
Copy link
Contributor

gajus commented Jan 28, 2017

Sorry forgot to raise an issue for this. Thank you, @sindresorhus .

@sindresorhus sindresorhus changed the title assertion-arguments rule should validate message argument type The assertion-arguments rule should validate message argument type Jul 14, 2018
@issuehunt-oss
Copy link

issuehunt-oss bot commented Sep 17, 2019

@issuehunt has funded $60.00 to this issue.


@issuehunt-oss issuehunt-oss bot added the 💵 Funded on Issuehunt This issue has been funded on Issuehunt label Sep 17, 2019
@GMartigny
Copy link
Contributor

Should this be under a new option or by default ?
I can imagine a case where user use an object with a toString function, but I guess this is extra rare.

@novemberborn
Copy link
Member

Should this be under a new option or by default ?

Default, I think.

AVA already validates this, so I think it's unlikely anybody will see their linter rules fail, tests would have already been failing. I don't think we need a major release for this. @sindresorhus what do you reckon?

@sindresorhus
Copy link
Member Author

Agreed

@issuehunt-oss
Copy link

issuehunt-oss bot commented Dec 24, 2020

@sindresorhus has rewarded $54.00 to @gmartigny. See it on IssueHunt

  • 💰 Total deposit: $60.00
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $6.00

@issuehunt-oss issuehunt-oss bot added 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt and removed 💵 Funded on Issuehunt This issue has been funded on Issuehunt labels Dec 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants