Skip to content

Always-false-condition in editorServices.ts #17892

Closed

Description

The condition on line 1663 in editorServices.ts:
https://github.com/Microsoft/TypeScript/blob/eef7d8bd3d974528464c7968deab78b747568c06/src/server/editorServices.ts#L1650-L1666

is always false as the called method closeExternalProject always returns void. That way the if-block with shouldRefreshInferredProjects = true; is never executed (for more information see the linter rule detecting this).

Activity

  1. mhegazy commented on Aug 18, 2017

    @mhegazy
    Contributor

    thanks for the report. a PR would be appreciated.

  2. RyanCavanaugh commented on Aug 19, 2017

    @RyanCavanaugh
    Member

    Mohamed Hegazy (@mhegazy) seems like using a void value in a truthiness position is hyper-sketchy - thoughts on making that an error in TS?

  3. mhegazy commented on Aug 21, 2017

    @mhegazy
    Contributor

    thoughts on making that an error in TS?

    i thought you had experimented with this a while back; I think it is a check worth adding, baring any breaks that we might find from RWC tests.

  4. RyanCavanaugh commented on Aug 22, 2017

    @RyanCavanaugh
    Member

    I had a test for always-truthy values that attempted to find bugs like if (x.f) (in lieu of if (x.f()) {) but the problem was that non-strictNullChecks .d.ts file gave tons of false positives. Checking for void values in control flow test positions should be a home run

  5. mhegazy commented on Aug 22, 2017

    @mhegazy
    Contributor

    i think the void check should only flag errors. seems like a low risk-high yield change to make.

  6. added a commit that references this issue on Aug 31, 2017
    ada1d1e
  7. added a commit that references this issue on Sep 7, 2017
    7b12b79
  8. locked and limited conversation to collaborators on Jun 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    BugA bug in TypeScriptFixedA PR has been merged for this issueHelp WantedYou can do this

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions