Enable the 'scopelint' and 'unconvert' linters

[nfnt]

What this PR does / why we need it:
'scopelint' checks for unpinned variables in 'range'-loops. This helps to avoid subtle bugs.
'unconvert' checks for unnecessary type conversions.

[porridge]

Isn't the table test case a false-positive? kyoh86/scopelint#4

[zen-dog]

Not necessarily part of this PR, but wdyt of normalizing this test?

Suggested change

	for i, test := range tests {
	for _, tt := range tests {
	...

[nfnt]

I'd normalize to test, tt is non-descriptive ;)

[zen-dog]

Stop fighting Go conventions 👅 . Just accept and move on 😂

[nfnt]

Someone using tt in a Wiki doesn't make this a convention 😉

[zen-dog]

Still, resisting?

[zen-dog]

I guess this is needed once we decide to run tests in parallel. 🚢

[nfnt]

@porridge Yes, these are false positives. I'm okay with that, rather have some false positives than subtle bugs.

[porridge]

I think it is fair to say that if we have a scope where two things can be called `t` (the testing.T object and the test struct), then calling one `t` and the other `tt` could be a source of confusion. In that case I'd also prefer `test` rather than `tt`.

[zen-dog]

then calling one t and the other tt could be a source of confusion.

...and the resistance is growing 😂 Again, you could argue that using i/j/k as loop indexes is stupid but that's the convention. And in Go's case using for _, tt := range tests gives you 1M+ search results in Github so accept the convention 😆

[porridge]

I agree about i/j/k, though if used in anything much different than matrix multiplication I'd say the code needs refactoring :-P I don't think there is a generally accepted convention about naming variables t, tt, ttt, tttt though :-D GitHub search is not perfect. "for _, test := range tests" yields 14M+ results :-)

[nfnt]

Let's not have this discussion here, but in other channels. This PR is only adding new linters. Let's discuss naming elsewhere and enforce code consistency in follow-up PRs.

[kensipe]

/lgtm