Forbid explicit step in slice

[orsinium]

Rule request

Thesis

# bad
a[1:4:]
# good
a[1:4]

# bad
a[1::]
# good
a[1:]

# bad
a[1:4:1]
# good
a[1:4]

# bad
a[1::1]
# good
a[1:]

# bad
a[::1]
# good
a.copy()

Case with a[::] will be covered in #1011

Reasoning

Consistency

[skarzi]

I would like to prepare some PR adding visitor and violation for that, but after doing quick experiments, I concluded that for 1st and 2nd case the AST looks identical (I have checked it on python 3.7.4), so probably the only way to implement this violation is to use BaseTokenVisitor, which can be quite complicated, however if you agree, I can give it a try :)

Also I am not sure about the last example, because it will work only with list and other objects that has .copy(), but e.g. tuple is immutable so it doesn't have such method, so suggesting users to use .copy() instead of a[::1] can be incorrect and misleading, however we can still suggest to use a[:]

[orsinium]

I concluded that for 1st and 2nd case the AST looks identical

Let's for the beginning cover remaining cases. It's better than nothing.

tuple is immutable so it doesn't have such method

So, why could we want to use [::] on it if it is immutable? Also, you can always use copy.copy function for custom collections (than does nothing for tuples as well, btw).

however we can still suggest to use a[:]

As you said, we can't say the difference between a[:] and a[::]. So we can't suggest using one instead of others.

[skarzi]

So, why could we want to use [::] on it if it is immutable? Also, you can always use copy.copy function for custom collections (than does nothing for tuples as well, btw).

True, it doesn't make much sense when we are talking about immutable objects, I just wanted to give some example that .copy() is not a good solution, however I agree that copy.copy() is.

As you said, we can't say the difference between a[:] and a[::]. So we can't suggest using one instead of others.

If we decide to write token visitor instead of AST, it will be possible to distinguish cases like 1st and 2nd ones, but I need to do some research and experiments to fully confirm that.

[sobolevn]

In case we cannot work with [::] and [:] even with tokens, then we can wait for 0.15 with libcst support.

[skarzi]

I have checked if it's possible to implement this violation with libast and it seems to be doable, so I will try to implement it in following week

[skarzi]

When do you plan to merge #1147 to master?
I am asking, because this PR introduces few features I'd like to use in my PR.

[sobolevn]

@skarzi after 0.14 is released. I hope to finish it in two weeks.