Radix rule checks Number.parseInt(), window.Number.parseInt(), global…

[david-cannady]

….Number.parseInt()

PR checklist

• Addresses an existing issue: radix rule does not enforce requirement for Number.parseInt() calls. #3895
• bugfix
  • Includes tests
• Documentation update

Overview of change:

radix rule now enforced on calls to Number.parseInt(), window.Number.parseInt(), and global.Number.parseInt()

Is there anything you'd like reviewers to focus on?

CHANGELOG.md entry:

[palantirtech]

Thanks for your interest in palantir/tslint, @david-cannady! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

[david-cannady]

Test failures don't appear to be the result of any changes that I've made.

[giladgray]

pushing a new commit should resolve build errors

[giladgray]

please split this absolute unit of a condition into multiple variables, one for each top-level || clause.
so it reads like if (isParseInt || isGlobalParseInt || isNumberParseInt) instead of this madness which is quite hard to validate correctness now that you've added more || clauses.

[david-cannady]

Sorry for the lateness of this update. This should be a lot easier to read and validate. However, some CI unrelated tests are failing.

[giladgray]

@david-cannady please merge latest master to fix tests

[giladgray]

this seems like it would ban myCustomClass.parseInt(arg), which could be a perfectly valid use of a non-library function called parseInt. true?

please add a test for this case.

[david-cannady]

isPropertyAccessParseInt() will return true for that case yes. However, in conjunction with isPropertyAccessOfProperty() and isPropertyAccessOfIdentifier(), the check is constrained to specific instances. A test already exists for this:
parser.parseInt("123"); on line 6 of the radix rule test.

[giladgray]

nice