fix 'no-magic-number' ignores parseInt radix parameter #3536
Conversation
|
Thanks for your interest in palantir/tslint, @mateuszwitkowski! 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. |
src/rules/noMagicNumbersRule.ts
Outdated
| @@ -70,6 +71,10 @@ export class Rule extends Lint.Rules.AbstractRule { | |||
| class NoMagicNumbersWalker extends Lint.AbstractWalker<Set<string>> { | |||
| public walk(sourceFile: ts.SourceFile) { | |||
| const cb = (node: ts.Node): void => { | |||
| if (isCallExpression(node) && isIdentifier(node.expression) && node.expression.text === "parseInt") { | |||
| return; | |||
There was a problem hiding this comment.
you probably want to return node.arguments.length === 0 ? undefined : cb(node.arguments[0]);
Otherwise magic numbers in the first argument are not checked.
You can also add a test to ensure it works as expected:
parseInt(foo === 4711 ? bar : baz, 10);
~~~~ [error message]
And another test to ensure it doesn't crash if there are no arguments at all:
parseInt();| @@ -1,3 +1,4 @@ | |||
| parseInt('123', 10); | |||
There was a problem hiding this comment.
consider adding tests with other numbers: 2, 8, 16
There was a problem hiding this comment.
Thank you for the review. I've updated PR based on your hints.
|
Thanks @mateuszwitkowski |
PR checklist
Overview of change:
Enhancement of 'no-magic-number' based on issue #3514
CHANGELOG.md entry:
[enhancement]: 'no-magic-numbers' ignores parseInt radix parameter