prevent-abbreviations: event_ is suffixed with underscore

[felixfbecker]

Why is event_ suffixed with underscore? 🤔

Please rename the variable `e`. Suggested names are: `error`, `event_`. A more descriptive name will do too.

[fisker]

Can you check your code? I guess event is used somewhere. See #579

[fisker]

This is expected, unless you didn't use event.

[felixfbecker]

I checked the code and there is no shadowed variable event anywhere in the scope. Here's an example where the rule complained:

https://github.com/sourcegraph/sourcegraph/blob/2b86b78820bf291c611bfea3493fcbab012d60bb/web/src/user/settings/accessTokens/UserSettingsCreateAccessTokenPage.tsx#L76

There is an eventLogger in scope, but not event

Could this be reopened?

[fisker]

There is a global event, because you use https://github.com/sourcegraph/sourcegraph/blob/2b86b78820bf291c611bfea3493fcbab012d60bb/.eslintrc.js#L6

[fisker]

Actually, I thought about this before, do we really need check scope up?

[felixfbecker]

Ah, that makes sense. Hmm.

Could the suffixing only be done if the same-name variable is actually used in the scope? If it's not used, it shouldn't be necessary to give it a different name

[felixfbecker]

I.e. only suffix in a case like this:

let error
try {
  foo()
} catch (error_) {
  error = error_
}

but not in this case:

let error
try {
  foo()
} catch (error) {
  console.error(error)
}

[fisker]

Yes, it could be done, but @bmish bring the no-shadow rule up today #749 (comment) .

I'm not sure what should we do

[felixfbecker]

I don't know how ESLint works, but could the rule detect whether no-shadow is used? Alternatively, have an option so the user can allow shadowing specifically

[fisker]

I think we don't read use configs in any other rule.

BTW: what's your opinion about child scopes?

try {

} catch (x) {
//       ^ we have a rule fix this to error, currently we are fixing to `error`, not `error_`, should we check the child scope?
	try {
	
	} catch (error) {
	
	}
}

[felixfbecker]

Fine to fix to error, unless inside the second catch there is a reference to x (which there isn't).

[fisker]

Yep, I prefer error too, but again, against no-shadow(with option all).

[fisker]

Let's close this issue for now, we can discuss the avoidCapture logic later.