Thanks for the PR, @skondrashov!

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.

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	6822a69
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/68166b5ec9ca820008470ef2
😎 Deploy Preview	https://deploy-preview-10182--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 97 (🔴 down 1 from production) Accessibility: 100 (no change from production) Best Practices: 100 (no change from production) SEO: 98 (no change from production) PWA: 80 (no change from production) View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

View your CI Pipeline Execution ↗ for commit 6822a69.

☁️ Nx Cloud last updated this comment at 2025-05-03 19:28:41 UTC

Sorry for force-push, wasn't sure how else to resolve the merge conflicts other than redoing the commit cause they didn't show up locally. Doesn't seem like it did anything to the merge conflict though, so I'm out of ideas.

I see I have problems from the perfectionist plugin in my files as well, but the plugin doesn't run locally when I use yarn lint, so I don't know what to do about that either.

Would be happy to address the 2 missing lines of coverage at some point if everything else gets figured out.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.90%. Comparing base (bca8a91) to head (6822a69).
Report is 1 commits behind head on main.

@@            Coverage Diff             @@
##             main   #10182      +/-   ##
==========================================
+ Coverage   90.84%   90.90%   +0.06%     
==========================================
  Files         497      498       +1     
  Lines       50320    50655     +335     
  Branches     8311     8335      +24     
==========================================
+ Hits        45714    46049     +335     
  Misses       4591     4591              
  Partials       15       15              

// setup
let str = Math.random() > 0.5 ? "string" : 1;
const num = Math.random() > 0.5 ? 1 : "1";
const bool = Math.random() > 0.5 ? true : 1;
const big = Math.random() > 0.5 ? BigInt(1) : 1;

(hint) - We often use declare to do this sort of setup for demo purposes, i.e.

// setup
declare let str: string | number;
declare const num: number | string;
// etc

It's a useful pattern for testing sometimes too, since you may run into surprising behavior when const t = true; has type true rather than type boolean. Nothing specifically requested, just letting you know about another tool for your kit in case it comes in handy during discussion on this PR 🙂

Does not do anything with Symbol() because everything weird I tried to do with symbols was already a typescript error.

Makes sense 👍

Does not do anything for some conversions to number which are already forbidden by typescript ("The left-hand side of an arithmetic operation must be of type 'any', 'number', 'bigint' or an enum type."), including:

"123" * 1;
"123" / 1;
"123" - 0;
"123" | 0;

Yep, also makes sense. And, even if TS did allow them, we don't need to stress about identifying every possible way for a value to be coerced, just the common patterns, and others can always be added in the future, too. Appreciate your close attention to detail on all these, though!

Does not cover ${"123"}, because that overlaps with the no-unnecessary-template-expression rule. Need feedback on what to do about that.

This is an astute observation. Is there anything this rule would flag that that rule doesn't flag? If not, I'd lean towards letting that separate rule handle this case. If you want you could even put a note in the docs that tells the user that template expressions intentionally aren't handled because they're already dealt with more thoroughly in the no-unnecessary-template-expression rule.

I went with the word "coercion" because it was chosen by @Josh-Cena in the original issue but I think it's a poor choice. I think "conversion" or even "type conversion idiom" might be the most precise, because .toString() for example doesn't rely on coercion, and while !!value and !value both coerce the type of value to a boolean, it's the property of the idiom !! that it can be used for type conversion which puts it under the umbrella of this rule. So I think no-unnecessary-type-conversion is a better name for this rule, given the things that I'm currently having it check for.

Hmm, I hear you. I personally think either one is fine. Maybe someone else will care more strongly 🤷

Might not have the best placement for the error scope, and might not have the best formatting for autofixes, could use feedback on those too.

I think for these reports, reporting the whole node is totally fine

declare const s: string;
s + '';
~~~~~~

!!(longExpressionThatsABoolean);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

But, I really like what you've done with the more granular approach.

!!(longExpressionThatsABoolean);
~~

So I'd say go for it! But if it becomes an implementation hassle, no stress either if you end up falling back to the typical strategy of letting the node dictate the report.

Sorry for force-push...

No worries! The real pain is just force pushing between reviews, not before a reviewer has looked at it at all 🙂.

..., wasn't sure how else to resolve the merge conflicts other than redoing the commit cause they didn't show up locally.

Oh, so I think the trouble that you're having is that you've used the main branch on your fork. The "usual" vanilla workflow would be

1. fork typescript-eslint. The main branch of your fork points to the HEAD of main of the original typescript-eslint at the time the fork was made.
2. Create a branch for your feature, work on there.
3. Whenever you need to synchronize upstream changes, use the github UI or CLI to synchronize the changes on typescript-eslint's main with your fork's main. Then merge those into your branch.

But you're in a slightly awkward scenario since your branch is on main. What I personally do (which differs from the above) is to set a second remote on your local repo, i.e. git remote add upstream git@github.com:typescript-eslint/typescript-eslint.git. Then you can git fetch --all && git checkout upstream/main (or similar) to make a branch from there, to merge changes into your PR branch. There's... (too) many ways to address this, good ol git, haha.

LMK if this helps or if you'd like some more help. (I can also push things directly to your branch to bail you out if needed 🤣). And no stress if you end up doing a force push again this time to get yourself in a clean state here!

I went with the word "coercion" because it was chosen by @Josh-Cena in the original issue but I think it's a poor choice. I think "conversion" or even "type conversion idiom" might be the most precise, because .toString() for example doesn't rely on coercion, and while !!value and !value both coerce the type of value to a boolean, it's the property of the idiom !! that it can be used for type conversion which puts it under the umbrella of this rule. So I think no-unnecessary-type-conversion is a better name for this rule, given the things that I'm currently having it check for.

Agreed; let's go with no-unnecessary-type-conversion instead.

👋 Just checking in @skondrashov - is this still something you have time and energy for?

I'll have time to finish up the changes probably early in the new year!

Awesome! I'll move this to draft then in the meantime, so we don't bug you. 🚀

Excited about this rule! I just saw a lot of Boolean('x' in globalThis) in a repo

I am salivating at how many existing violations were removed in this PR. 🤤