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

no-constant-condition false positive on string compare #12225

Closed
tiansh opened this issue Sep 5, 2019 · 5 comments
Closed

no-constant-condition false positive on string compare #12225

tiansh opened this issue Sep 5, 2019 · 5 comments
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 bug ESLint is working incorrectly rule Relates to ESLint's core rules

Comments

@tiansh
Copy link

tiansh commented Sep 5, 2019

Check out Demo Page for more details

Tell us about your environment

  • ESLint Version: 6.3.0

Please show your full configuration:

Configuration
{
    "parserOptions": {
        "ecmaVersion": 11,
        "sourceType": "script",
        "ecmaFeatures": {}
    },
    "rules": {
        "constructor-super": 2,
        "for-direction": 2,
        "getter-return": 2,
        "no-async-promise-executor": 2,
        "no-case-declarations": 2,
        "no-class-assign": 2,
        "no-compare-neg-zero": 2,
        "no-cond-assign": 2,
        "no-const-assign": 2,
        "no-constant-condition": 2,
        "no-control-regex": 2,
        "no-debugger": 2,
        "no-delete-var": 2,
        "no-dupe-args": 2,
        "no-dupe-class-members": 2,
        "no-dupe-keys": 2,
        "no-duplicate-case": 2,
        "no-empty": 2,
        "no-empty-character-class": 2,
        "no-empty-pattern": 2,
        "no-ex-assign": 2,
        "no-extra-boolean-cast": 2,
        "no-extra-semi": 2,
        "no-fallthrough": 2,
        "no-func-assign": 2,
        "no-global-assign": 2,
        "no-inner-declarations": 2,
        "no-invalid-regexp": 2,
        "no-irregular-whitespace": 2,
        "no-misleading-character-class": 2,
        "no-mixed-spaces-and-tabs": 2,
        "no-new-symbol": 2,
        "no-obj-calls": 2,
        "no-octal": 2,
        "no-prototype-builtins": 2,
        "no-redeclare": 2,
        "no-regex-spaces": 2,
        "no-self-assign": 2,
        "no-shadow-restricted-names": 2,
        "no-sparse-arrays": 2,
        "no-this-before-super": 2,
        "no-undef": 2,
        "no-unexpected-multiline": 2,
        "no-unreachable": 2,
        "no-unsafe-finally": 2,
        "no-unsafe-negation": 2,
        "no-unused-labels": 2,
        "no-unused-vars": 2,
        "no-useless-catch": 2,
        "no-useless-escape": 2,
        "no-with": 2,
        "require-atomic-updates": 2,
        "require-yield": 2,
        "use-isnan": 2,
        "valid-typeof": 2
    },
    "env": {
        "browser": true
    }
}

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

/*eslint no-constant-condition: "error"*/
function isDate(ty, tm, td) {
  const [y, m, d] = new Date().toISOString().split(/[-T]/).map(x => +x);
  if ('' + [y, m, d] === '' + [ty, tm, td]) {
    return true;
  }
  return false;
}
console.log(isDate(2019,9,5));

What did you expect to happen?
No error

What actually happened? Please include the actual, raw output from ESLint.

4:7 - Unexpected constant condition. (no-constant-condition)

@tiansh tiansh added bug ESLint is working incorrectly triage An ESLint team member will look at this issue soon labels Sep 5, 2019
@aladdin-add aladdin-add added accepted There is consensus among the team that this change meets the criteria for inclusion rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels Sep 5, 2019
@tickct
Copy link
Contributor

tickct commented Sep 16, 2019

I am going to take a look at this.

@mdjermanovic
Copy link
Member

A workaround in this particular situation could be to use the equivalent String() (or .toString() or .join()) instead of '' +:

/*eslint no-constant-condition: "error"*/
function isDate(ty, tm, td) {
  const [y, m, d] = new Date().toISOString().split(/[-T]/).map(x => Number(x));
  if (String([y, m, d]) === String([ty, tm, td])) {
    return true;
  }
  return false;
}
console.log(isDate(2019,9,5));

The rule, in general, doesn't assume that a function always returns the same value for the same arguments (even constants), so any function call inside a condition makes the whole condition valid by the logic of this rule.

@eslint-deprecated eslint-deprecated bot added the auto closed The bot closed this issue label Dec 5, 2019
@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that accepted issues failing to be implemented after 90 days tend to
never be implemented, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@kaicataldo
Copy link
Member

Reopening this since it's accepted and we have a PR.

@kaicataldo kaicataldo reopened this Dec 7, 2019
tickct added a commit to tickct/eslint that referenced this issue Dec 24, 2019
@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that accepted issues failing to be implemented after 90 days tend to
never be implemented, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@mdjermanovic mdjermanovic reopened this Jan 8, 2020
@mdjermanovic mdjermanovic removed the auto closed The bot closed this issue label Jan 8, 2020
montmanu pushed a commit to montmanu/eslint that referenced this issue Mar 4, 2020
) (eslint#12307)

* Fix: no-constant-condition doesn't introspect arrays (fixes eslint#12225)

* Update: no-constant-condition allows for narrower scope of varriable arrays

* Update: no-constant condition now requires direct parent to be binary expression

* Update: no-contant-contidtion now checks parent type before operator, unit tests include more paths
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Jul 27, 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 Jul 27, 2020
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 bug ESLint is working incorrectly rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

5 participants