fix(eslint-plugin): [no-base-to-string] handle intersection types

[bogdanb]

Fixes #2162 [no-base-to-string] complains about intersection types

[typescript-eslint]

Thanks for the PR, @bogdanb!

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.

[codecov]

Codecov Report

Merging #2170 into master will decrease coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2170      +/-   ##
==========================================
- Coverage   93.47%   93.39%   -0.08%     
==========================================
  Files         171      174       +3     
  Lines        7813     7889      +76     
  Branches     2229     2253      +24     
==========================================
+ Hits         7303     7368      +65     
- Misses        244      247       +3     
- Partials      266      274       +8     

Flag	Coverage Δ
#unittest	93.39% <100.00%> (-0.08%)	⬇️

Impacted Files	Coverage Δ
...kages/eslint-plugin/src/rules/no-base-to-string.ts	98.46% <100.00%> (+0.15%)	⬆️
...int-plugin/src/rules/require-array-sort-compare.ts	88.23% <0.00%> (-11.77%)	⬇️
...s/eslint-plugin/src/rules/no-unused-expressions.ts	94.11% <0.00%> (-5.89%)	⬇️
packages/typescript-estree/src/convert.ts	98.61% <0.00%> (-0.28%)	⬇️
...plugin/src/rules/explicit-module-boundary-types.ts	88.96% <0.00%> (-0.27%)	⬇️
packages/eslint-plugin/src/rules/return-await.ts	95.77% <0.00%> (-0.18%)	⬇️
...lugin/src/rules/no-confusing-non-null-assertion.ts	82.60% <0.00%> (ø)
...es/eslint-plugin/src/rules/no-loss-of-precision.ts	91.66% <0.00%> (ø)
...ages/eslint-plugin/src/rules/ban-tslint-comment.ts	100.00% <0.00%> (ø)
...kages/eslint-plugin/src/rules/naming-convention.ts	88.82% <0.00%> (+0.03%)	⬆️
... and 1 more

[bogdanb]

Hi guys! I’m not sure exactly what’s going on with the code coverage test. Either I’m not understanding its report, or I missed something about how the tests work. I expected better coverage from those tests. Any hints?

(Also, I’d appreciate some hints on how to lint a local project using the patched code instead of the ones from npm.)

[bradzacher]

Codecov comments sometimes do weird things.
What you want to look at is the codecov UI

https://codecov.io/gh/typescript-eslint/typescript-eslint/pull/2170/diff?src=pr&el=tree#diff-cGFja2FnZXMvZXNsaW50LXBsdWdpbi9zcmMvcnVsZXMvbm8tYmFzZS10by1zdHJpbmcudHM=

[bradzacher]

thanks for your work!

logic itself looks fine.
Just need to get the test coverage higher please.

[bogdanb]

@bradzacher Hi Brad! Is there a way I could get something like that codecov diff locally? It does not look the way I expected from the tests I added, so I’d like to run a lot of small tests to figure out what’s going on, and I don’t want to spam the pull request with a dozen tiny commits just to get that diff.

[bradzacher]

yup! just run the tests and it'll output the coverage to a folder in the plugin root. There'll be some HTML files in there, drag n drop the index file onto chrome and you can view it and click around to see roughly the same output that you see on codecov.

[bogdanb]

OK, I got the patch coverage to 100%, mostly by simplifying the logic. I couldn’t figure out a way to generate an intersection type where some of the sub-types have Usefulness.Sometimes.

By the way, unless I’m mistaken the rule currently accepts code like this:

function(v: unknown) { return `${v}`}
function<A>(v: A) { return `${v}`}

I think both of those should be rejected. I could insert an issue & pull request for the first one, at least. (The second one needs more care, to allow things like .) I wanted to finish this one first, and also get a second opinion.

By the way, I see a message that the branch is out-of-date with the base branch, which offers me a button to update the branch. Should I trust that it does the right thing? Should I merge the changes by hand? Should I ignore it?

[bradzacher]

The "Update branch" button will simply merge master into your branch.
It will do the right thing. If there are merge conflicts, it will list them all and give you a UI to resolve them.

It's safe to use it.

---

I would definitely consider raising an issue for unknown and untyped generics.

[bradzacher]

LGTM - thanks for your contribution!