LoopOverLength check

[pfitzseb]

Fixes #337.

[non-Jedi]

Does check_loop_iter get called on comprehensions or only for loops?

[pfitzseb]

Ah, good call. Generators and for loops with multiple iterators now work:

[fredrikekre]

Shouldn't it check that the index is used for indexing? There is nothing wrong with having a loop from 1 to the length of something.

[pfitzseb]

Yes.

[pfitzseb]

Also fixed the wording:

[non-Jedi]

There's a bikeshedding discussion to be hadavoided on whether the linter should highlight the for-loop or the indexing location.

Thanks so much for tackling this @pfitzseb. Hopefully this is one of those little changes that nudge the whole ecosystem towards more care for correctness.

[pfitzseb]

I was thinking about this too, which is why both are highlighted now (turns out I never committed those changes):

[fredrikekre]

LGTM but don't know this code that well.

[pfitzseb]

The check is now disabled if we infer the array to be a Base.Array, for which this indexing method is valid.

[Seelengrab]

This looks great! Could this be extended to multidimensional arrays and encouraging axes, if that's not already a thing that I just missed?

[pfitzseb]

I've adjusted the message:

Currently only Base.length and Base.size are handled.

[fredrikekre]

I got a crash with:

ERROR: type Nothing has no field type
Stacktrace:
 [1] getproperty(x::Nothing, f::Symbol)
   @ Base ./Base.jl:42
 [2] check_incorrect_iter_spec(x::CSTParser.EXPR, body::CSTParser.EXPR, env::StaticLint.ExternalEnv)
   @ StaticLint ~/.julia/packages/StaticLint/ktd1a/src/linting/checks.jl:388
 [3] check_loop_iter(x::CSTParser.EXPR, env::StaticLint.ExternalEnv)
   @ StaticLint ~/.julia/packages/StaticLint/ktd1a/src/linting/checks.jl:357

[pfitzseb]

Should be fixed now.