Skip to content

Add prefer-math-min-max#2432

Merged
sindresorhus merged 26 commits into
sindresorhus:mainfrom
axetroy:prefer-math-min-max
Aug 23, 2024
Merged

Add prefer-math-min-max#2432
sindresorhus merged 26 commits into
sindresorhus:mainfrom
axetroy:prefer-math-min-max

Conversation

@axetroy
Copy link
Copy Markdown
Contributor

@axetroy axetroy commented Aug 21, 2024

Close #2373

Comment thread docs/rules/prefer-math-min-max.md Outdated
@@ -0,0 +1,56 @@
# Prefer `Math.min()` and `Math.max()` over ternary expressions for simple comparisons
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Suggested change
# Prefer `Math.min()` and `Math.max()` over ternary expressions for simple comparisons
# Prefer `Math.min()` and `Math.max()` over ternaries for simple comparisons

Comment thread docs/rules/prefer-math-min-max.md Outdated
<!-- end auto-generated rule header -->
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->

Enforce the use of `Math.min()` and `Math.max()` instead of ternary expressions for simple comparisons. This makes the code more readable.
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is repeating the title.

Comment thread rules/prefer-math-min-max.js Outdated
@fisker
Copy link
Copy Markdown
Collaborator

fisker commented Aug 21, 2024

Two edge cases will be broken

  1. need parens
(0,foo) > 10 ? 10 : (0,foo)
  1. need insert space before
function a() {
  return+foo > 10 ? 10 : +foo
}

Comment thread rules/prefer-math-min-max.js Outdated
Comment thread rules/prefer-math-min-max.js Outdated
@axetroy

This comment was marked as outdated.

@axetroy
Copy link
Copy Markdown
Contributor Author

axetroy commented Aug 21, 2024

Two edge cases will be broken

1. need parens
(0,foo) > 10 ? 10 : (0,foo)
2. need insert space before
function a() {
  return+foo > 10 ? 10 : +foo
}

Resolve the edge case in 87815e9 and 48de8a4

Comment thread rules/prefer-math-min-max.js Outdated
@axetroy
Copy link
Copy Markdown
Contributor Author

axetroy commented Aug 22, 2024

Code coverage 100%, reached

@axetroy axetroy requested review from fisker and sindresorhus August 22, 2024 06:50
Comment thread rules/prefer-math-min-max.js Outdated
Copy link
Copy Markdown
Collaborator

@fisker fisker left a comment

Choose a reason for hiding this comment

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

Good job

@yoyo837
Copy link
Copy Markdown

yoyo837 commented Dec 3, 2024

prefer-math-min-max does not consider Implicit Type Conversion.

col.width > 0 ? col.width : 0

conver to

Math.max(col.width, 0)

It broke the logic.

@sindresorhus
Copy link
Copy Markdown
Owner

It's impossible to account for that without types. And I don't think it should account for that regardless. Implicit type conversion is an anti-pattern. If you want to do it, I suggest disabling the rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rule proposal: prefer-math-min-max

4 participants