This was reported by a linter error? I think the original location was more appropriate.

It was. It was unreachable code - if this was ever compiled to a let in the result rather than a var, the resulting TS wouldn't have run correctly. :D

let declarations after a return statement are unreachable and are therefore never scoped, unlike a var which would be hoisted with the function definition.

So you can say the linter found a bug waiting to happen here.

Ah I see; nice!. Perhaps it would be better to just make it a var declaration again.

If we do that it'll have to be wrapped:

/* tslint:disable:no-var-keyword */
// This is for caching the result of getSymbolDisplayBuilder. Do not access directly.
var _displayBuilder: SymbolDisplayBuilder;
/* tslint:enable:no-var-keyword */

Is it worth it?

No one is going to want that amount of verbosity to disable things, especially one liners

I thought that was the point? You're not supposed to disable the linter.

Yeah there is an interesting push and pull in this sort of thing. You can make the syntax to disable things as gross as possible to disincentivize disabling, on the other hand you might accept that disabling things is a fact of life and it shouldn't look like an atrocity.

This is a good example of what I was mentioning to @DanielRosenwasser as far as how I only fixed the easy 80-90% cases as the rest that require non-trivial rewrites will generate some amount of discussion on whether we feel some particular situation warrants an exception.

My opinion is just make the change to satisfy the linter. While I see the argument for the 'define as close to usage as possible' it's not hard to use GoToDef occasionally vs adding gross comments.

this seems like the kind of place you want to mix quotes instead of using escapes (except for the hilarious inversion of ' and " on each side of the ||)

There's a number of places where that's true... but quote consistency is there for quote consistency, not for convenience, I suppose.

Yeah I saw a bunch more and didn't bother commenting again. I don't feel super strongly here but I always found mixing ' and " handy for cases like this but I can live without it.

I've actually been wondering this for awhile - since template strings are a thing in JS now, why don't linters have an option to prefer those? I think they're a bit more flexible and generally easier to keep consistent with. Plus, it reduces what you have as far as string literals go from quotes and backticks to just backticks. (Since you're likely using backticks for interpolation anyway!)

@weswigham TSLint supports string templates for the quote rule -- it does this by simply ignoring the contents of template strings. So a "double" quote rule really means only allow double quotes or string templates.

No, I mean, why not forbid all quotes (single and double)? Template string literals can function as string literals handily, and it further unifies what string literals get used. Not really relevant here, I was just wondering why no linting tools seemed to support this option.

@weswigham that's a valid suggestion. filed that feature request here: palantir/tslint#539

I'd still prefer to be able to mix quotes. I think it would be preferable for TSLint to support "double quotes unless the string contains a double quote."

Wouldn't be a problem if you used backticks for all the quotes. Until you needed to include backticks. 😄

But no really - try not to mix quotes. It makes sense not to mix quotes if you consider searching for strings in the source code. Knowing the exact quotes used lets you (plaintext) search with much higher precision - knowing you can always Ctrl-Shift-F "Joe's "Error" Message" (over 'Joe's "Error" Message') is way easier than not knowing if it's in one of many possible string formats. Generally speaking it's worth the effort of needing to toss in a \ in your string literals every so often.