Tslint: Undeprecate no-use-before-declare

[ikokostya]

Feature request

Is your feature request related to a problem? Please describe.

Rule no-use-before-declare was deprecated in the PR https://github.com/palantir/tslint/pull/4695, but as @karol-majewski noticed TypeScript compiler can't detect some errors.

For example, the following code:

const consumer = () => {
  console.log(variable);
};

consumer();

const variable = 'foo';

doesn't produce any error (TypeScript playground), but causes runtime error.

While deprecated no-use-before-declare rule detects this error:

variable 'variable' used before declaration (no-use-before-declare)
  1 | const consumer = () => {
> 2 |     console.log(variable);
    |                ^
  3 | };
  4 | 
  5 | consumer();

Describe the solution you'd like

Undeprecate no-use-before-declare.

Describe alternatives you've considered

File issue to TypeScript project as parallel process.

[JoshuaKGoldberg]

~https://github.com/palantir/tslint/pull/4695#issuecomment-489457888~

~> The rule has been somewhat marked as deprecation for a while (#4666); if you'd like to see it un-deprecated or generally re-implemented please file a new issue.~

~It's reasonable that you'd want this rule to not be deprecated, but there are blockers from us enabling it. #2551 shows an example of a false positive. It's not clear whether & how we'd want to try to overcome them. Comments on an existing PR thread aren't very discoverable.~

~Also, keep #4534 in mind - TSLint itself is being deprecated soon.~

Please ignore, I need to read issues more thoroughly before closing them.

[ikokostya]

Because example from issue #2551 is not isolated and contains some project specific entities, I tried to test code from replies:

https://github.com/palantir/tslint/issues/2551#issuecomment-300904638
https://github.com/palantir/tslint/issues/2551#issuecomment-424837231

export function foo ({ param: p }: { param: any }) {
}

declare let user1: any;

const { email: user1Email, name: user1Name }: { email: string, name: string } = user1;

There are no errors with no-use-before-declare rule and latest version of tslint 5.18.0 for me.

[JoshuaKGoldberg]

@adidahiya do you have more context on why the rule was deprecated? I'm not seeing anything linked by #3520.

Edit: for future reference, it's been mentioned as slow as far back as #1261.

[JoshuaKGoldberg]

If we take the code from the OP and move consumer(); to _after_ variable, the rule still gives a complaint. This is the false positive mentioned in the other discussions.

const consumer = () => {
  console.log(variable);
};

const variable = "foo";

consumer();

https://github.com/palantir/tslint/pull/4695#issuecomment-509858306
False positive result is not a reason for removing rule. By this logic any buggy rule can be removed.

Well, no, there's a structural difference between a rule that has an _implementation_ bug verses a rule that has a _seemingly-unfixable structural_ bug. It would be quite a lot of work to make the rule understand that placing consumer(); before variable _is_ a bug but variable before consumer(); is not. The rule would likely have to do something like converting the source file into a graph and checking for cycles in the identifiers and references. That's out of scope for the core ruleset.

If the rule is to be enabled, I think it would be a requirement to clearly mention its shortcomings in the docs per this thread. Thoughts?

[ikokostya]

If the rule is to be enabled, I think it would be a requirement to clearly mention its shortcomings in the docs per this thread. Thoughts?

I think it's good tradeoff. In original topic @OliverJAsh mentioned about it.

[adidahiya]

This should be covered by the eslint rule: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/no-use-before-define.md

Closing as per deprecation timeline in #4534