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

feat: Catch more cases in no-constant-condition #15613

Merged
merged 1 commit into from Mar 10, 2022

Conversation

captbaritone
Copy link
Contributor

@captbaritone captbaritone commented Feb 16, 2022

Identify undefined and Boolean(somethingConstant) as both being constant.

Prerequisites checklist

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

[X] Changes an existing rule (template)

What changes did you make? (Give an overview)

Expanded no-constant-condition to catch two types of additional cases:

if(undefined) {}

if(Boolean(1)) {}

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

@eslint-github-bot eslint-github-bot bot added triage An ESLint team member will look at this issue soon bug ESLint is working incorrectly labels Feb 16, 2022
@mdjermanovic mdjermanovic changed the title fix: Catch more cases in no-constant-condition feat: Catch more cases in no-constant-condition Feb 16, 2022
@mdjermanovic mdjermanovic added enhancement This change enhances an existing feature of ESLint 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 bug ESLint is working incorrectly triage An ESLint team member will look at this issue soon labels Feb 16, 2022
@eslint-github-bot eslint-github-bot bot added the bug ESLint is working incorrectly label Feb 16, 2022
@captbaritone
Copy link
Contributor Author

captbaritone commented Feb 16, 2022

I've updated the commit message to mark this as a feat: in line with @mdjermanovic's change of the title of the PR.

Edit Strange, it looks like @eslintbot readded the bug tag which @mdjermanovic just removed...

@mdjermanovic mdjermanovic removed the bug ESLint is working incorrectly label Feb 16, 2022
@mdjermanovic
Copy link
Member

mdjermanovic commented Feb 16, 2022

Edit Strange, it looks like @eslintbot readded the bug tag which @mdjermanovic just removed...

I'm not sure why, both PR and commit tags are now feat:.

lib/rules/no-constant-condition.js Outdated Show resolved Hide resolved
lib/rules/no-constant-condition.js Outdated Show resolved Hide resolved
@captbaritone
Copy link
Contributor Author

captbaritone commented Feb 25, 2022

Updated.

@captbaritone
Copy link
Contributor Author

captbaritone commented Mar 3, 2022

cc @mdjermanovic for visibility (no hurry if you've already seen my update)

@nzakas
Copy link
Member

nzakas commented Mar 9, 2022

@captbaritone are you still working on this?

@captbaritone
Copy link
Contributor Author

captbaritone commented Mar 9, 2022

Oh, thanks for pinging. I missed @mdjermanovic's last reply. Yes. Still working on this.

@captbaritone
Copy link
Contributor Author

captbaritone commented Mar 9, 2022

Updated.

@captbaritone
Copy link
Contributor Author

captbaritone commented Mar 9, 2022

I think the lint failure is failing on the main branch:

docs/user-guide/configuring/configuration-files.md: 361: MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]

@mdjermanovic
Copy link
Member

mdjermanovic commented Mar 9, 2022

I think the lint failure is failing on the main branch

It's fixed now, I'll close & reopen to re-run checks on top of the latest main.

@mdjermanovic mdjermanovic reopened this Mar 9, 2022
@mdjermanovic mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Mar 9, 2022
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Code LGTM!

Can you also update the documentation for this rule with a few examples?

lib/rules/no-constant-condition.js Outdated Show resolved Hide resolved
Identify `undefined` and `Boolean(somethingConstant)` as both being constant.
@captbaritone
Copy link
Contributor Author

captbaritone commented Mar 10, 2022

I've fixed the docblock and added two examples to the docs.

Copy link
Member

@mdjermanovic mdjermanovic left a comment

LGTM, thanks for contributing!

@mdjermanovic mdjermanovic merged commit 8933fe7 into eslint:main Mar 10, 2022
14 checks passed
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Mar 18, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.10.0` -> `8.11.0`](https://renovatebot.com/diffs/npm/eslint/8.10.0/8.11.0) |

---

### Release Notes

<details>
<summary>eslint/eslint</summary>

### [`v8.11.0`](https://github.com/eslint/eslint/releases/v8.11.0)

[Compare Source](eslint/eslint@v8.10.0...v8.11.0)

#### Features

-   [`800bd25`](eslint/eslint@800bd25) feat: add `destructuredArrayIgnorePattern` option in `no-unused-vars` ([#&#8203;15649](eslint/eslint#15649)) (Nitin Kumar)
-   [`8933fe7`](eslint/eslint@8933fe7) feat: Catch `undefined` and `Boolean()` in no-constant-condition ([#&#8203;15613](eslint/eslint#15613)) (Jordan Eldredge)
-   [`f90fd9d`](eslint/eslint@f90fd9d) feat: Add ESLint favicon to the HTML report document ([#&#8203;15671](eslint/eslint#15671)) (Mahdi Hosseinzadeh)
-   [`57b8a57`](eslint/eslint@57b8a57) feat: `valid-typeof` always ban `undefined` ([#&#8203;15635](eslint/eslint#15635)) (Zzzen)

#### Bug Fixes

-   [`6814922`](eslint/eslint@6814922) fix: escaping for square brackets in ignore patterns ([#&#8203;15666](eslint/eslint#15666)) (Milos Djermanovic)
-   [`c178ce7`](eslint/eslint@c178ce7) fix: extend the autofix range in comma-dangle to ensure the last element ([#&#8203;15669](eslint/eslint#15669)) (Milos Djermanovic)

#### Documentation

-   [`c481cec`](eslint/eslint@c481cec) docs: add fast-eslint-8 to atom integrations (userguide) ([#&#8203;15695](eslint/eslint#15695)) (db developer)
-   [`d2255db`](eslint/eslint@d2255db) docs: Add clarification about `eslint-enable` ([#&#8203;15680](eslint/eslint#15680)) (dosisod)
-   [`8b9433c`](eslint/eslint@8b9433c) docs: add object pattern to first section of computed-property-spacing ([#&#8203;15679](eslint/eslint#15679)) (Milos Djermanovic)
-   [`de800c3`](eslint/eslint@de800c3) docs: link to minimatch docs added.  ([#&#8203;15688](eslint/eslint#15688)) (Gaurav Tewari)
-   [`8f675b1`](eslint/eslint@8f675b1) docs: sort-imports add single named import example ([#&#8203;15675](eslint/eslint#15675)) (Arye Eidelman)

#### Chores

-   [`385c9ad`](eslint/eslint@385c9ad) chore: rm trailing space in docs ([#&#8203;15689](eslint/eslint#15689)) (唯然)

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

 **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1217
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
srijan-deepsource pushed a commit to deepsourcelabs/eslint that referenced this pull request May 30, 2022
…int#15613)

Identify `undefined` and `Boolean(somethingConstant)` as both being constant.
srijan-deepsource added a commit to deepsourcelabs/eslint that referenced this pull request May 30, 2022
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Sep 7, 2022
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Sep 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint 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