Tslint: prefer-const when variable is used in catch/finally

[selfrefactor]

Bug Report

• __TSLint version__: 5.9.1
• __TypeScript version__: 2.8.0-insiders.20180127
• __Running TSLint via__: CLI

TypeScript code being linted

https://github.com/selfrefactor/word-definition/blob/2264b65c6dfef8ebfb7f2496b33c6f896af21ea4/src/german/index.ts#L24

// code snippet
async function german(word: string){
  try{
    var {$, $$, browser, page} = await initPuppeteer()
  }catch(e){
    await browser.close()
  }
}

It's pretty long so I post link https://github.com/selfrefactor/word-definition/blob/master/tslint.json

with tslint.json configuration:

"prefer-const": true,

Actual behavior

Identifier '$$' is never reassigned; use 'const' instead of 'var'.
Identifier 'browser' is never reassigned; use 'const' instead of 'var'

Expected behavior

I expect tslint to acknowledge that browser is needed in catch phrase and don't warn me.

There is this other issue, that even if browser warning is rightfully ignored, then I will still get warning for variables $ and $$ which are used in the body of the function.

So the rule should ignore $ and $$ as they are assigned together with browser and page, which are needed in catch/finally phrase.

[suchanlee]

The rule for prefer-const is as follows:

Requires that variable declarations use const instead of let and var if possible.

In this case, the linter is correct in saying that you can use const instead because it's never reassigned, which is correct. The linter (or the compiler in this case) isn't responsible for _how_ the variable is used.

The problem you're saying is that because it's block-scoped, you don't have access to it in your catch block. I think the correct fix here would be to use let to initialize variables outside of your try/catch, try to initialize them in your try block.

[ajafff]

Wait a minute @suchanlee. This is a legitimate bug report.

The rule for prefer-const is as follows:

Requires that variable declarations use const instead of let and var if possible.

In the code above it is not possible to change var to let.

correct fix here would be to use let to initialize variables outside of your try/catch

The correct fix for this bug is not reporting an error. It's not the scope of this rule to forbid var.
In addition that would require moving all declarations around and adding type annotations. Or you just move that one declaration out of the tryBlock and need to split up the destructuring.

In this case, the linter is correct in saying that you can use const instead because it's never reassigned, which is correct. The linter (or the compiler in this case) isn't responsible for how the variable is used.

What is that even supposed to mean?
"We did the best we can to detect the variable is never reassigned. Unfortunately we were not smart enough to handle edge cases that annoy you with false positives. BUT we didn't advertise an autofix, so you need to destroy your code yourself. Don't blame us, it was you who wrote bad code in the first place." (not meant to blame anyone, in fact I was the last one to refactor this rule and I didn't thing about this case back then)

Btw. there are a few more cases where var is valid, but const is not.

---

@selfrefactor FWIW I have a working implementation of prefer-const as core rule of the Wotan linter runtime. The list of available rules also states what's better than in the original TSLint rule.

[selfrefactor]

@ajafff Spot on!

Thanks for the info.

[suchanlee]

@selfrefactor @ajafff I apologize if I came across as rude. I was actually thinking about this last night and had planned on reopening this today. As @ajafff stated, yes I now think that TSLint should catch this case and report it as such. Again, sorry for the confusion -- I'm just ramping on to the TSLint project and learning about what are and are not its responsibilities. And thank you @ajafff for taking the time to comment on it and point out my mistake!

[JoshuaKGoldberg]

💀 _It's time!_ 💀

TSLint is deprecated and no longer accepting pull requests other than security fixes. See #4534. ☠️
We recommend you instead use typescript-eslint to lint your TypeScript code with ESLint. ✅

👋 It was a pleasure open sourcing with you!

[JoshuaKGoldberg]

🤖 Beep boop! 👉 TSLint is deprecated 👈 _(#4534)_ and you should switch to typescript-eslint! 🤖

🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋