Rubocop: Lint/BinaryOperatorWithIdenticalOperands makes no sense with bitwise shift operators

[mvz]

Lint/BinaryOperatorWithIdenticalOperands reports an offense with the following code:

    bit_mask :IFunctionInfoFlags,
             is_method:      (1 << 0),
             is_constructor: (1 << 1),
             is_getter:      (1 << 2),
             is_setter:      (1 << 3),
             wraps_vfunc:    (1 << 4),
             throws:         (1 << 5)

an offense is flagged in the line is_constructor: (1 << 1),.

While for most binary operators, the result of foo [op] foo can be replaced by a constant or foo regardless of the value of foo, this is not the case for the shift operators: 1 << 1 has a completely different result from ['a'] << ['a'].

Expected behavior

No offense is flagged in the above code

Actual behavior

RuboCop flags an offense, e.g.:

lib/ffi-gobject_introspection/lib.rb:67:31: W: Lint/BinaryOperatorWithIdenticalOperands: Binary operator << has identical operands.
             is_constructor: (1 << 1),
                              ^^^^^^

Steps to reproduce the problem

Put the code at the top in a ruby file and run rubocop on it.

RuboCop version

$ bundle exec rubocop -V
0.89.0 (using Parser 2.7.1.4, rubocop-ast 0.3.0, running on ruby 2.7.1 x86_64-linux)

[marcandre]

Agreed, we could lookout for that exact case 1 << 1 and ignore it.

[marcandre]

I've relaxed the checks for any integers and binary operators; I doubt there will be many invalid uses of 42 >> 42 in the wild...

[marcandre]

Fixed. Thanks for raising this issue 👍

[mvz]

Thanks, @marcandre!

[mathieujobin]

same applies to ** operator

rubocop now complains about foo ** foo which is perfectly fine

[bbatsov]

I see @marcandre covered **, but I think it's ignored only for literals currently.

[marcandre]

rubocop now complains about foo ** foo which is perfectly fine

Right... Is this a theoretical issue or do you actually have such a need? I'm curious to use cases.

[mathieujobin]

it { expect( batch_job.reduce(0) { |val, item_val| val + item_val**item_val } ).to eq 287 }

[mathieujobin]

@marcandre maybe it isn't a bad thing that rubocop catch those and we can add them to our ignore list if it make sense.

I'm sure happy to see foo || foo being reported to me as I didn't know we had those in our code base...

[marcandre]

@marcandre maybe it isn't a bad thing that rubocop catch those and we can add them to our ignore list if it make sense.

Cool. I must admit I still have no idea what your it example is trying to achieve. If at least instead of 287 it was written 2 ** 2 + 3 ** 3 + 4 ** 4 😅 Still, it looks like could be rewritten as it { expect(batch_job).to =~ [2, 3, 4] }?

[mathieujobin]

true that, honestly, I didn't wrote it, its part of the code base I am maintaining. and you are probably right, maybe its worth trashing ;)

[bbatsov]

Well, I guess this proves that the cop is helpful in its current form. :wink: