Skip to content

False "duplicate ternary conditions" #9

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

Open
crystalfp opened this issue Aug 16, 2021 · 7 comments · Fixed by #10
Open

False "duplicate ternary conditions" #9

crystalfp opened this issue Aug 16, 2021 · 7 comments · Fixed by #10
Assignees
Labels
bug Something isn't working needs info This thing needs more info

Comments

@crystalfp
Copy link

I installed the plugin and received plenty of error in my Typescript project. For example the following lines are marked as "Duplicate ternary conditions" when really there is a single ternary:

error: Duplicate ternary conditions 'header.date' (ternary/no-unreachable) at ...:
header.date = header.date ? normalizeDate(header.date, header.language) : dateISO8601();

error: Duplicate ternary conditions 'sts' (ternary/no-unreachable) at ...:
ctx.body = sts ? {msg: "Error", error: sts} : {msg: "Updated"};

Or even this, where there are two independent near ternaries:

error: Duplicate ternary conditions 'sts' (ternary/no-unreachable) at ...:185:30:
  183 |                 const sts = await resources.updateProjectFields(id, ctx.request.body as Partial<Project>);
  184 |                 ctx.body = JSON.stringify(sts ? {msg: "Error", error: sts} : {msg: "Updated"});
> 185 |                 ctx.status = sts ? HttpStatusCode.NOT_FOUND : HttpStatusCode.OK;

Seems the plugin raises this error when the same variable is used as ternary condition and inside the alternatives.

I don't see how, but if these are legitimate errors, then the rule documentation needs an update.
Thanks for clarifying!
mario

@GrayedFox
Copy link
Owner

Hi there Mario, thanks for using this plugin and opening an issue, I will have a look into this and get back to you. From what I can see your ternary conditions look fine so it might be a problem with how I am passing objects, or when the ternary operator is repeated inside one of the ternary conditions. Will get back to you latest by next week!

Until then it might be worth configuring the no-unreachable rule as a warning or turning it off temporarily. Apologies for any inconvenience caused.

@GrayedFox
Copy link
Owner

GrayedFox commented Sep 1, 2021

Hi @crystalfp I got around to testing these. Under the default rules I cannot get an error to trigger using the samples you provided using JavaScript:

    'header.date = header.date ? normalizeDate(header.date, header.language) : dateISO8601();',
    'ctx.body = JSON.stringify(sts ? {msg: "Error", error: sts} : {msg: "Updated"});',
    'ctx.status = sts ? HttpStatusCode.NOT_FOUND : HttpStatusCode.OK;',

I tested this by adding those three lines to the valid tests part of the no-unreachable tests file.

To help me debug the issue, could you:

  • find out if TypeScript is somewhere interfering with ESLint? In theory the plugin should also support typescript files, here is a guide to getting ESLint to work with TS
  • share the exact config you are using for eslint-plugin-ternary (i.e. default, recommended, or custom rules)

I am also pushing an update today since I found a bug with the depth option not having been implemented, but I haven't touched the no-unreachable rule so I can't imagine updating will fix it for you. Still, v2.0.0 of the plugin is out today so you try updating just in case :)

@GrayedFox GrayedFox self-assigned this Sep 1, 2021
@GrayedFox GrayedFox added the needs info This thing needs more info label Sep 1, 2021
@crystalfp
Copy link
Author

Thanks @GrayedFox ! I will do soon after fixing another annoying & pervasive problem ( no more supported). In any case, I happy eslint my Typescript code after configuring the right parser.

@crystalfp
Copy link
Author

@GrayedFox I tested the latest version, the false positive continues for ternary/no-unreachable.

Configuration: Typescript file, eslint.yaml attached below.
Examples:

error: Duplicate ternary conditions 'count' (ternary/no-unreachable) at prepare\database.ts:356:40:
  354 |                                 for(const term of doc.kw) {
  355 |                                         const count = documentsContainingTerm.get(term);
> 356 |                                         documentsContainingTerm.set(term, count ? count + 1 : 1);
      |                                                                           ^
  357 |                                 }
  358 |                                 return doc;
  359 |                         }))

error: Duplicate ternary conditions 'mt === 12' (ternary/no-unreachable) at common\NormalizeDate.ts:158:38:
  157 |                 if(mt >= 0) {
> 158 |                         return checkAndConvertDate(dt[3], mt === 12 ? 9 : mt + 1, dt[1]);
      |                                                           ^
  159 |                 }
parser: '@typescript-eslint/parser'

plugins:
    - node
    - promise
    - import
    - '@typescript-eslint'
    - no-useless-assign
    - eslint-comments
    - dollar-sign
    - optimize-regex
    - const-case
    - jquery
    - jest
    - jest-formatting
    - unicorn
    - security
    - array-plural
    - no-unsafe-regex
    - deprecate
    - deprecation
    - sonarjs
    - regexp
    - '@alasdair/max-len'
    - ternary

extends:
    - 'eslint:recommended'
    - 'plugin:eslint-comments/recommended'
    - 'plugin:promise/recommended'
    - 'plugin:node/recommended'
    - 'plugin:import/errors'
    - 'plugin:import/warnings'
    - 'plugin:import/typescript'
    - 'plugin:@typescript-eslint/recommended'
    - 'plugin:@typescript-eslint/recommended-requiring-type-checking'
    - 'plugin:jquery/slim'
    - 'plugin:jquery/deprecated'
    - 'plugin:jest/recommended'
    - 'plugin:jest/style'
    - 'plugin:jest-formatting/recommended'
    - 'plugin:unicorn/recommended'
    - 'plugin:security/recommended'
    - 'plugin:sonarjs/recommended'
    - 'plugin:regexp/recommended'
    - 'plugin:ternary/recommended'

parserOptions:
    ecmaVersion: 2021
    ecmaFeatures:
        impliedStrict: true
    project:
        - ./client/tsconfig.json
        - ./server/tsconfig.json
        - ./tests/tsconfig.json
        - ./plugins/tsconfig.json
        - ./client-widgets/tsconfig.json
        - ./prepare/tsconfig.json
        - ./common/tsconfig.json
    warnOnUnsupportedTypeScriptVersion: false

reportUnusedDisableDirectives: true

settings:
    import/parsers: {'@typescript-eslint/parser': [.ts, .tsx]}
    import/resolver: {typescript: {}}
    import/extensions: [.js, .ts]
    import/ignore: [node_modules]

env:
    es2021: true
    node: true
    shared-node-browser: true
    jest: true
    jest/globals: true

rules:
    node/no-deprecated-api: error
    node/exports-style: [error, exports]
    node/no-unsupported-features/node-builtins: [error, {version: '>=15'}]
    node/no-unsupported-features/es-builtins: [error, {version: '>=15'}]
    node/no-unsupported-features/es-syntax: [off, {version: '>=15'}]
    node/no-missing-import:
        - off
        - allowModules: [marked, jquery, knockout]
          tryExtensions: [.js, .json]
    node/handle-callback-err: error
    node/global-require: error
    node/no-process-exit: off

    '@typescript-eslint/consistent-type-assertions': [warn, {assertionStyle: 'as'}]
    '@typescript-eslint/array-type': [warn, {default: array, readonly: array}]

    '@alasdair/max-len/max-len':
        - warn
        - code: 115
          ignoreTrailingComments: true
          ignoreUrls: true
          ignoreStrings: true
          ignoreTemplateLiterals: true
          ignoreRegExpLiterals: true

    no-dupe-class-members: off
    '@typescript-eslint/no-dupe-class-members': error
    no-buffer-constructor: error
    no-redeclare: [off, {builtinGlobals: true}]
    '@typescript-eslint/no-redeclare': [error, {builtinGlobals: true}]
    no-unused-vars: off
    '@typescript-eslint/no-unused-vars': error
    no-empty-function: off
    '@typescript-eslint/no-empty-function': warn
    no-useless-constructor: off
    '@typescript-eslint/no-useless-constructor': warn
    lines-between-class-members: off
    '@typescript-eslint/lines-between-class-members': off

    no-undef: error
    no-extend-native: error
    no-sequences: error
    no-new: error
    no-bitwise: error
    no-unsafe-negation: [warn, {enforceForOrderingRelations: true}]
    quotes: [off, double, {avoidEscape: true}]
    '@typescript-eslint/quotes': [warn, double, {avoidEscape: true}]
    eqeqeq: [error, always]
    strict: [error, global]
    no-loop-func: off
    '@typescript-eslint/no-loop-func': error
    no-unused-expressions: off
    '@typescript-eslint/no-unused-expressions': warn
    max-params: [warn, 4]
    max-len: [off, 113]
    space-before-function-paren: off
    '@typescript-eslint/space-before-function-paren':
        - error
        - anonymous: never
          named: never
          asyncArrow: always
    space-before-blocks: warn
    no-shadow: off
    '@typescript-eslint/no-shadow':
        - error
        - hoist: all
          builtinGlobals: true
          allow: [$, event, self]
    comma-spacing: off
    '@typescript-eslint/comma-spacing': [error, {before: false, after: true}]
    keyword-spacing: off
    '@typescript-eslint/keyword-spacing':
        - warn
        - before: true
          after: false
          overrides:
            else: {after: true}
            return: {after: true}
            try: {after: true}
            catch: {after: false}
            case: {after: true}
            const: {after: true}
            throw: {after: true}
            let: {after: true}
            do: {after: true}
            of: {after: true}
            as: {after: true}
            finally: {after: true}
            from: {after: true}
            import: {after: true}
            export: {after: true}
            default: {after: true}
    no-trailing-spaces: warn
    id-length: [warn, {exceptions: [$, i, j, k, x, y, w, h, g]}]
    prefer-const: warn
    for-direction: error
    no-template-curly-in-string: error
    consistent-return: [error, {treatUndefinedAsUnspecified: false}]
    no-unmodified-loop-condition: error
    array-bracket-spacing: [warn, never]
    object-curly-spacing: off
    '@typescript-eslint/object-curly-spacing': ['warn']
    brace-style: off
    '@typescript-eslint/brace-style': [warn, stroustrup, {allowSingleLine: true}]
    spaced-comment: [warn, always, {markers: [':', '-', '+', '::', '/']}]
    no-var: error
    block-scoped-var: error
    yoda: error
    camelcase: [warn, {properties: never}]
    comma-dangle: [off, never]
    '@typescript-eslint/comma-dangle': [off, {arrays: only-multiline, objects: only-multiline}]
    max-depth: [warn, 8]
    arrow-parens: error
    no-confusing-arrow: [error, {allowParens: false}]
    dot-location: [error, property]
    no-else-return: error
    no-throw-literal: off
    '@typescript-eslint/no-throw-literal': error
    require-await: off
    '@typescript-eslint/require-await': error
    no-return-await: off
    '@typescript-eslint/return-await': error
    dot-notation: off
    '@typescript-eslint/dot-notation': off
    eol-last: [error, always]
    newline-per-chained-call: [error, {ignoreChainWithDepth: 3}]
    nonblock-statement-body-position: [warn, beside]
    space-infix-ops: off
    '@typescript-eslint/space-infix-ops': warn
    semi-spacing: error
    operator-assignment: [error, always]
    object-shorthand: [error, properties, {avoidQuotes: true}]
    no-process-exit: off
    no-negated-condition: warn
    no-constant-condition: [error, {checkLoops: false}]
    prefer-destructuring:
        - error
        - VariableDeclarator: {array: false, object: true}
          AssignmentExpression: {array: false, object: false}
        - enforceForRenamedProperties: false
    no-extra-parens: off
    '@typescript-eslint/no-extra-parens': [warn, functions]
    no-invalid-this: off
    '@typescript-eslint/no-invalid-this': [error, {capIsConstructor: false}]
    prefer-template: warn
    semi: off
    '@typescript-eslint/semi': [error, always]
    '@typescript-eslint/explicit-function-return-type': [warn, {allowExpressions: true}]
    '@typescript-eslint/method-signature-style': warn
    '@typescript-eslint/no-implicit-any-catch': warn
    '@typescript-eslint/prefer-includes': warn
    '@typescript-eslint/prefer-nullish-coalescing': warn
    '@typescript-eslint/prefer-optional-chain': warn
    '@typescript-eslint/no-base-to-string': warn
    '@typescript-eslint/non-nullable-type-assertion-style': warn
    '@typescript-eslint/no-unnecessary-boolean-literal-compare': warn
    '@typescript-eslint/prefer-readonly': warn
    '@typescript-eslint/prefer-readonly-parameter-types': off
    '@typescript-eslint/no-confusing-void-expression': [off, {ignoreArrowShorthand: true}]
    '@typescript-eslint/explicit-member-accessibility':
        - warn
        - accessibility: 'explicit'
          overrides:
            constructors: 'no-public'
            properties: 'explicit'
            parameterProperties: 'explicit'
            methods: 'no-public'
            accessors: 'no-public'
    no-multiple-empty-lines: [warn, {max: 2, maxEOF: 1}]
    prefer-arrow-callback: warn
    array-callback-return: [error, {allowImplicit: true}]
    init-declarations: off
    '@typescript-eslint/init-declarations': off
    func-call-spacing: off
    '@typescript-eslint/func-call-spacing': warn
    default-param-last: off
    '@typescript-eslint/default-param-last': warn

    optimize-regex/optimize-regex: off

    const-case/uppercase: off

    import/no-unresolved: off
    import/no-named-as-default: error
    import/no-named-as-default-member: error
    import/default: error
    import/no-deprecated: warn

    jest-formatting/padding-around-expect-groups: warn
    jest/no-disabled-tests: off

    array-plural/array-plural:
        - warn
        - allows:
            - ignore
            - array
            - list
            - match
            - count
            - table

    unicorn/no-keyword-prefix: [warn, {checkProperties: false}]
    unicorn/prefer-array-some: warn
    unicorn/prefer-default-parameters: warn
    unicorn/prefer-array-index-of: warn
    unicorn/prefer-regexp-test: warn
    unicorn/consistent-destructuring: warn
    unicorn/prefer-string-starts-ends-with: warn
    '@typescript-eslint/prefer-string-starts-ends-with': warn
    unicorn/numeric-separators-style: [error, number: {onlyIfContainsSeparator: true, minimumDigits: 3}]
    unicorn/no-console-spaces: warn
    unicorn/prefer-string-replace-all: warn
    unicorn/prevent-abbreviations:
        - warn
        - replacements:
            len: false
            params: false
            doc: false
            pkg: false
            ctx: false
            i: false
            j: false
            idx: false
            args: false
            dir: false
          checkFilenames: false

    promise/catch-or-return: [error, {allowFinally: true}]

    dollar-sign/dollar-sign: [warn, ignoreProperties]
    no-useless-assign/no-useless-assign: warn

    deprecate/function: warn
    deprecate/member-expression: warn
    deprecate/import: warn
    deprecation/deprecation: warn

    no-unsafe-regex/no-unsafe-regex: error

    '@typescript-eslint/member-delimiter-style': warn

    no-loss-of-precision: off
    '@typescript-eslint/no-loss-of-precision': error
    no-magic-numbers: [off, {ignoreArrayIndexes: true, ignore: [0]}]
    '@typescript-eslint/no-magic-numbers': [off, {ignoreArrayIndexes: true, ignore: [0]}]
    promise/always-return: off
    linebreak-style: [off, windows]
    no-tabs: off
    one-var: [off, always]
    no-mixed-spaces-and-tabs: off
    no-multi-spaces: off
    no-plusplus: [off, {allowForLoopAfterthoughts: true}]
    no-console: off
    jquery/no-class: 0
    jquery/no-hide: 0
    jquery/no-css: 0
    jquery/no-toggle: 0
    jquery/no-animate: 0
    jquery/no-sizzle: 0
    jquery/no-show: 0
    jquery/no-is: 0
    unicorn/filename-case: [off, {case: camelCase}]
    node/no-missing-require: off
    node/no-unpublished-require: off
    node/no-unpublished-import: off
    unicorn/throw-new-error: 0
    unicorn/new-for-builtins: 0
    unicorn/no-process-exit: 0
    unicorn/prefer-query-selector: 0
    unicorn/consistent-function-scoping: 0
    unicorn/no-for-loop: 0
    unicorn/no-useless-undefined: 0
    unicorn/no-array-reduce: 0
    security/detect-non-literal-fs-filename: 0
    security/detect-object-injection: 0
    unicorn/prefer-node-protocol: 0
    '@typescript-eslint/interface-name-prefix': 0
    indent: [off, tab]
    '@typescript-eslint/indent': [off, tab]
    no-use-before-define: off
    '@typescript-eslint/no-use-before-define': off
    '@typescript-eslint/no-non-null-assertion': off
    sonarjs/cognitive-complexity: [off, 40]
    sonarjs/no-duplicate-string: [off, 6]
    sonarjs/elseif-without-else: 0
    sonarjs/no-nested-switch: 0
    unicorn/better-regex: 0

globals:
    requirejs: readonly
    __dirname: readonly
    document: readonly
    fetch: readonly
    window: readonly
    Request: readonly
    Headers: readonly
    localStorage: readonly
    NodeJS: readonly
    JQuery: readonly
    CodeMirror: readonly
    URLSearchParams: readonly
    navigator: readonly

@GrayedFox
Copy link
Owner

Hello again Mario thanks for posting that. I can see you are using the ternary plugin with the recommended settings (i.e. you have not tweaked the rules). When I have time I will try and reproduce this result using ESLint, the typescript eslint parser, and just the ternary plugin installed.

Just to be super clear: the files you are linting are the typescript files before they have been transpiled into JavaScript, right? Because I am pretty sure there is no point linting the files output from the TypeScript parser (not super familiar with TS but I assume that's how it works). Thanks again for your input.

@crystalfp
Copy link
Author

Yes, I use ESlint on the typescript files.

@brookjordan
Copy link

I am getting the issue shown in the errors above. That is, the ternary/no-unreachable rule is also giving me errors for duplicate ternary conditions, which I don’t want to check for.

@GrayedFox GrayedFox added the bug Something isn't working label Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs info This thing needs more info
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants