Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New: Unsafe integer rule #12889

Closed
wants to merge 1 commit into from
Closed

Conversation

jdoubleu
Copy link

@jdoubleu jdoubleu commented Feb 8, 2020

Prerequisites checklist

  • I have read the contributing guidelines.
  • The team has reached consensus on the changes proposed in this pull request. If not, I understand that the evaluation process will begin with this pull request and won't be merged until the team has reached consensus.

What is the purpose of this pull request? (put an "X" next to an item)

  • Documentation update
  • Bug fix (template)
  • New rule (template)
  • Changes an existing rule (template)
  • Add autofixing to a rule
  • Add a CLI option
  • Add something to the core
  • Other, please explain:

New Rule

Please describe what the rule should do:
Due to IEEE-754 double precision, integers with a value higher or equal to 2^53 cannot properly be represented in JavaScript and are therefore considered unsafe.

What category of rule is this? (place an "X" next to just one item)

  • Warns about a potential error (problem)
  • Suggests an alternate way of doing something (suggestion)
  • Enforces code style (layout)
  • Other (please specify:)

Provide 2-3 code examples that this rule will warn about:

/*eslint no-unsafe-integer: "error"*/

var num1 = 9223372036854775807; // INT64_MAX_NUMBER
var num2 = 0x7FFFFFFFFFFFFFFF; // INT64_MAX_NUMBER in hex
parseInt("9007199254740992");

Why should this rule be included in ESLint (instead of a plugin)?
It is a very minimal and simple rule which does not produce much overhead. Although I'd guess this rule does not apply to all code bases, when detected it could save a few minutes and quickly resolve confusion about JavaScript's Number object.

What changes did you make? (Give an overview)

  1. Add new rule definition in lib/rules/,
  2. add tests for that rule,
  3. add documentation for the rule.

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

Documentation: Rule Description.

@jsf-clabot
Copy link

jsf-clabot commented Feb 8, 2020

CLA assistant check
All committers have signed the CLA.

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Feb 8, 2020
@kaicataldo kaicataldo added evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion feature This change adds a new feature to ESLint rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels Feb 8, 2020
@jdoubleu
Copy link
Author

This rule is missing checking for constant calculations (e.g. 2 ** 80.

However I'll wait for the discussion at #11279 before updating this PR.

@jdoubleu
Copy link
Author

Closed in favor of #11279

@jdoubleu jdoubleu closed this May 21, 2020
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Nov 18, 2020
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Nov 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion feature This change adds a new feature to ESLint rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants