Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

⭐ feat(rules/number-literal-format): add fix #4496

Merged
merged 5 commits into from
Feb 5, 2019
Merged

⭐ feat(rules/number-literal-format): add fix #4496

merged 5 commits into from
Feb 5, 2019

Conversation

ColCh
Copy link
Contributor

@ColCh ColCh commented Feb 3, 2019

PR checklist

Overview of change:

Added fix for number-format rule

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

tricky test cases, corner cases

CHANGELOG.md entry:

[new-fixer] number-literal-format now includes auto fix

@ColCh
⭐ feat(rules/number-literal-format): add fix
aa548ff
@ColCh
Merge branch 'master' into 3970-number-format-autofix
45aaa7b 
* master: (60 commits)
  Added tslint-brunch to the list of 3rd party tools (palantir#4251)
  Switch to tslint-plugin-prettier, clean up rule options config syntax (palantir#4488)
  Enable grouped-imports for ordered-imports rule in tslint:all config (palantir#4420)
  Ordered imports grouping (palantir#4134)
  trailing-comma: check for a closing parenthesis (palantir#4457)
  Update index.md (palantir#4473)
  [bugfix] `no-unsafe-any`: allow implicitly downcasting `any` to `unknown` (palantir#4442)
  Add v5.12.1 changelog
  Bump version to 5.12.1
  Fix quotemark avoid-template issues (palantir#4408)
  Skip linting JSON files entirely (palantir#4001)
  Fix strict-type-predicate for unknown (palantir#4444)
  restrict increment-decrement fixer while fixing the postfix unary expressions (palantir#4415)
  Mention file names in script parse failures (palantir#4397)
  Revert breaking change to tslint:recommended, update tslint:latest (palantir#4404)
  Fix quotemark avoid-template issues (palantir#4408)
  Bump tslint dev dependency to 5.12.0 (palantir#4452)
  Skip linting JSON files entirely (palantir#4001)
  Fix strict-type-predicate for unknown (palantir#4444)
  [README] Update link for Webstorm (palantir#4450)
  ...
@palantirtech
Copy link
Member

Thanks for your interest in palantir/tslint, @ColCh! 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.

@ColCh
Copy link
Contributor Author

ColCh commented Feb 3, 2019

Signed

JoshuaKGoldberg
JoshuaKGoldberg suggested changes Feb 3, 2019
View reviewed changes
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes are a little meaty, so asking for clarification on the tests before diving in there 😄

test/rules/number-literal-format/test.ts.lint Outdated Show resolved Hide resolved
@ColCh
Copy link
Contributor Author

ColCh commented Feb 4, 2019

@JoshuaKGoldberg Please take a look again :)

JoshuaKGoldberg
JoshuaKGoldberg suggested changes Feb 4, 2019
View reviewed changes
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks great! This is some tricky code and I appreciated the inline comments 😁. Just a few smaller inline things to touch up.

src/rules/numberLiteralFormatRule.ts Outdated Show resolved Hide resolved
src/rules/numberLiteralFormatRule.ts Outdated Show resolved Hide resolved
src/rules/numberLiteralFormatRule.ts Outdated Show resolved Hide resolved
@ColCh
Copy link
Contributor Author

ColCh commented Feb 5, 2019

Applied proposed changes. Please revisit

JoshuaKGoldberg
JoshuaKGoldberg approved these changes Feb 5, 2019
View reviewed changes
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic, thanks @ColCh! 💯

@JoshuaKGoldberg JoshuaKGoldberg merged commit 44febf9 into palantir:master Feb 5, 2019
@ColCh
Copy link
Contributor Author

ColCh commented Feb 5, 2019

@JoshuaKGoldberg Thank you for review! 👍

@rgant
Copy link
Contributor

rgant commented Sep 11, 2019 •
edited
Loading

I just had this fixer fail on a number with leading and trailing zeros:

-            rate: 0.022000,
+            rate: 22000,

Commenting so I can find this again when I have time to setup the environment, setup tests, and hopefully provide a fix.

@JoshuaKGoldberg
Copy link
Contributor

@rgant oh! It'd be swell if you could file a bug report for that!

@rgant
Copy link
Contributor

rgant commented Sep 11, 2019

I will, but because I'm using Angular I'm behind in tslint versions so it doesn't seem great to file a bug that would hopefully just be responded to with "try to reproduce the latest version". The playground doesn't appear to have the ability to run the fixer so I need to setup the env. Sorry for creating noise here.

@rgant rgant mentioned this pull request Sep 12, 2019
Closed
@snyk-bot snyk-bot mentioned this pull request Oct 11, 2019
Merged
@snyk-bot snyk-bot mentioned this pull request Oct 26, 2019
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@JoshuaKGoldberg JoshuaKGoldberg JoshuaKGoldberg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

number-literal-format autofix
4 participants
@ColCh @palantirtech @rgant @JoshuaKGoldberg