Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

How to make a rule that disables implicit truthy conversion from function to boolean #112

Closed
ziriax opened this issue Apr 18, 2016 · 7 comments
Milestone

Comments

@ziriax
Copy link

ziriax commented Apr 18, 2016

We have a large typescript application, and sometimes we have code like

if (shouldFireNuclearMissiles)...

while we should have written

if (shouldFireNuclearMissiles())...

(pun intended).

So forgetting to evaluate a function results in a truthy value.

Is it possible to write a rule that forbids this kind of implicit function-to-boolean conversions?

Or does this require forking typescript?

Thanks,
Peter

@HamletDRC
Copy link
Member

This would be quite a nice rule.

A couple things to consider:

  • tslint analyzes one file at a time and does not look at imports or /// <ref imports. This means that if "shouldFireNuclearMissiles" is defined in some other file then tslint will consider it an 'any' type. This means that, for the most part, tslint will have no idea what type your function is
  • tslint does not have a reliable type checker. Even if "shouldFireNuclearMissles" is a locally defined function, tslint will see it as an 'any' type. Again, this means the rule will not work that well.

We could make this rule, but honestly it would miss about 95% of the violations. That isn't much safety.

@ziriax
Copy link
Author

ziriax commented Apr 19, 2016

Thanks for the fast reply!

I forked and modified the Typescript compiler for checking this. I was amazed how cleanly it was written, so it was very easy to hack, it took less than 2 hours!.

And indeed, we found some bugs in our large code base :-)

But I'm pretty sure the Typescript team will not want this kind of checks, as they deviate too much from Javascript?

Personally I hate automatic truthy and falsy conversions in large projects, the comfort they bring does not outweigh the dangers.

@joshbw
Copy link

joshbw commented Apr 19, 2016

I don’t think it would hurt to submit the idea on the TS project and see what they think. Personally I’d love to see this as part of a TSStrictMode option where the devs could tell the compiler to be much more pedantic about potential issues

From: Peter Verswyvelen [mailto:[email protected]]
Sent: Tuesday, April 19, 2016 6:42 AM
To: Microsoft/tslint-microsoft-contrib [email protected]
Subject: Re: [Microsoft/tslint-microsoft-contrib] How to make a rule that disables implicit truthy conversion from function to boolean (#112)

Thanks for the fast reply!

I forked and modified the Typescript compiler for checking this. I was amazed how cleanly it was written, so it was very easy to hack, it took less than 2 hours!.

And indeed, we found some bugs in our large code base :-)

But I'm pretty sure the Typescript team will not want this kind of checks, as they deviate too much from Javascript?

Personally I hate automatic truthy and falsy conversions in large projects, the comfort they bring does not outweigh the dangers.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHubhttps://github.com//issues/112#issuecomment-211927477

@myitcv
Copy link

myitcv commented Apr 19, 2016

This is covered in palantir/tslint#253

@ziriax
Copy link
Author

ziriax commented Apr 19, 2016

Great!

However, the feedback I got from @HamletDRC seems to indicate that in most cases this is useless since the node type will be infered to be any instead of boolean, for the reasons he described?

@reversi-fun
Copy link

reversi-fun commented Apr 22, 2016

@ziriax and @joshbw ,Hang in there.

Implementation of that new Tsllint rule not easy than hack of tsc-complier.

(a) TSLint use TSC-Complier with "compilerOptions.noResolve = true".
SO function'S type seems "Any" or "undefined" by TSLint-rules.
(b) TSC-Compiler read only one sourceFile at 1 TSLint-Roule, not read any other SourceFile sutch as function defined in.

for (a); I have been challenged to workaround, performance has become very bad.
Issues:[ScopedSymbolTrackingWalker.typeChecker returns undefinedhttps://github.com//issues/94]

for (b);TSLint-Contributors says that No-Good "search" for the many file, and tsconfig.json read in TSLint-task, to be compiles.

I hope that the better solution is created.
I'm Sorry for My broken English.

@HamletDRC
Copy link
Member

I am closing this issue here because it will be part of TSLint when they start linting entire programs as a whole.

@HamletDRC HamletDRC added this to the Pre 2.0.9 milestone Jul 13, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants