Don't flag raised to a power negative numeric literals. #3770
Conversation
Removing the parentheses would change the meaning of the expressions.
amogil
force-pushed
the
bug-numeric-literals
branch
from
e22d7c2 to
408d2ef
Compare
|
Hi
For example 2**(-2)$ rubocop
Inspecting 1 file
.
1 file inspected, no offenses detectedI think the above code should be detected by RuboCop. What do you think? |
|
Fixed. Sorry, the difference between |
| @@ -126,8 +128,17 @@ def only_closing_paren_before_comma?(node) | |||
| line_range.source =~ /^\s*\)\s*,/ | |||
| end | |||
|
|
|||
| def disallowed_literal?(node) | |||
| node.literal? && !ALLOWED_LITERALS.include?(node.type) | |||
| def disallowed_literal?(begin_node, node) | |||
There was a problem hiding this comment.
begin_node? This name sounds confusing to me.
There was a problem hiding this comment.
I've used the same convention as in other places in the file. See check, check_unary, call_chain_starts_with_int? methods.
That was wrong?
| !raised_to_power_negative_numeric?(begin_node, node) | ||
| end | ||
|
|
||
| def raised_to_power_negative_numeric?(begin_node, node) |
| end | ||
|
|
||
| def raised_to_power_negative_numeric?(begin_node, node) | ||
| return false unless node.int_type? || node.float_type? |
There was a problem hiding this comment.
Probably we should add a numeric_type? method to the underlying class.
|
|
||
| def raised_to_power_negative_numeric?(begin_node, node) | ||
| return false unless node.int_type? || node.float_type? | ||
| return false if node.children.first >= 0 || begin_node.parent.nil? |
There was a problem hiding this comment.
node.children.first >= 0? I don't immediately understand what you're checking for here. I'm guessing it's whether the base number is positive of negative but it's really hard to infer from the code you've written.
| return false unless node.int_type? || node.float_type? | ||
| return false if node.children.first >= 0 || begin_node.parent.nil? | ||
|
|
||
| begin_node.parent.children[begin_node.sibling_index + 1] == :** |
There was a problem hiding this comment.
Same here.
Adding a few locals would certainly improve the readability of your code.
True. There can be a lint cop for this. |
- Numeric type detection moved into Node class. - raised_to_power_negative_numeric? was refactored to improve readability. - One more spec.
There was a bug and I've fixed it. Other comments were fixed too, except one about |
|
OK, the naming there is not that important. |
Currently
(-2)**2is flagged as "Don't use parentheses around a literal" offense. But removing the parentheses would change the meaning:(-2)**2 #=> 4-2**2 #=> -4Style/RedundantParenthesesfixed by adding the exception for raised to a power negative numeric literals.rubocop -V #=> 0.46.0 (using Parser 2.3.3.1, running on ruby 2.2.2 x86_64-darwin14)