diff --git a/CHANGELOG.md b/CHANGELOG.md index 3dc304d200c..8b16da72a39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,9 @@ * [#8422](https://github.com/rubocop-hq/rubocop/issues/8422): Fix an error for `Lint/SelfAssignment` when using or-assignment for constant. ([@koic][]) * [#8423](https://github.com/rubocop-hq/rubocop/issues/8423): Fix an error for `Style/SingleArgumentDig` when without a receiver. ([@koic][]) * [#8424](https://github.com/rubocop-hq/rubocop/issues/8424): Fix an error for `Lint/IneffectiveAccessModifier` when there is `begin...end` before a method definition. ([@koic][]) +* [#8006](https://github.com/rubocop-hq/rubocop/issues/8006): Fix line length calculation for `Style/IfUnlessModifier` to correctly take into account code before the if condition when considering conversation to a single-line form. ([@dsavochkin][]) +* [#8283](https://github.com/rubocop-hq/rubocop/issues/8283): Fix line length calculation for `Style/IfUnlessModifier` to correctly take into account a comment on the first line when considering conversation to a single-line form. ([@dsavochkin][]) +* [#8226](https://github.com/rubocop-hq/rubocop/issues/8226): Fix `Style/IfUnlessModifier` to add parentheses when converting if-end condition inside an array or a hash to a single-line form. ([@dsavochkin][]) ### Changes @@ -4736,3 +4739,4 @@ [@knejad]: https://github.com/knejad [@iamravitejag]: https://github.com/iamravitejag [@volfgox]: https://github.com/volfgox +[@dsavochkin]: https://github.com/dmytro-savochkin diff --git a/lib/rubocop/cop/mixin/statement_modifier.rb b/lib/rubocop/cop/mixin/statement_modifier.rb index 6712b185bed..9d4947b75f4 100644 --- a/lib/rubocop/cop/mixin/statement_modifier.rb +++ b/lib/rubocop/cop/mixin/statement_modifier.rb @@ -36,14 +36,41 @@ def non_eligible_condition?(condition) def modifier_fits_on_single_line?(node) return true unless max_line_length - length_in_modifier_form(node, node.condition) <= max_line_length + length_in_modifier_form(node) <= max_line_length end - def length_in_modifier_form(node, cond) + def length_in_modifier_form(node) keyword = node.loc.keyword - indentation = keyword.source_line[/^\s*/] - line_length("#{indentation}#{node.body.source} #{keyword.source} " \ - "#{cond.source}") + prefix = keyword.source_line[0...keyword.column] + expression = to_modifier_form(node) + line_length("#{prefix}#{expression}") + end + + def to_modifier_form(node) + expression = [node.body.source, + node.keyword, + node.condition.source].compact.join(' ') + parenthesized = parenthesize?(node) ? "(#{expression})" : expression + [parenthesized, first_line_comment(node)].compact.join(' ') + end + + def first_line_comment(node) + comment = + processed_source.find_comment { |c| c.loc.line == node.loc.line } + + comment ? comment.loc.expression.source : nil + end + + def parenthesize?(node) + # Parenthesize corrected expression if changing to modifier-if form + # would change the meaning of the parent expression + # (due to the low operator precedence of modifier-if) + parent = node.parent + return false if parent.nil? + return true if parent.assignment? || parent.operator_keyword? + return true if %i[array pair].include?(parent.type) + + node.parent.send_type? && !node.parent.parenthesized? end def max_line_length diff --git a/lib/rubocop/cop/style/if_unless_modifier.rb b/lib/rubocop/cop/style/if_unless_modifier.rb index 72a1c3e1f7c..551f2393169 100644 --- a/lib/rubocop/cop/style/if_unless_modifier.rb +++ b/lib/rubocop/cop/style/if_unless_modifier.rb @@ -148,26 +148,6 @@ def another_statement_on_same_line?(node) sibling.source_range.first_line == line_no end - def parenthesize?(node) - # Parenthesize corrected expression if changing to modifier-if form - # would change the meaning of the parent expression - # (due to the low operator precedence of modifier-if) - parent = node.parent - return false if parent.nil? - return true if parent.assignment? || parent.operator_keyword? - - node.parent.send_type? && !node.parent.parenthesized? - end - - def to_modifier_form(node) - expression = [node.body.source, - node.keyword, - node.condition.source, - first_line_comment(node)].compact.join(' ') - - parenthesize?(node) ? "(#{expression})" : expression - end - def to_normal_form(node) indentation = ' ' * node.source_range.column <<~RUBY.chomp diff --git a/spec/rubocop/cop/style/if_unless_modifier_spec.rb b/spec/rubocop/cop/style/if_unless_modifier_spec.rb index 5bcb14da619..5ed4b8eb785 100644 --- a/spec/rubocop/cop/style/if_unless_modifier_spec.rb +++ b/spec/rubocop/cop/style/if_unless_modifier_spec.rb @@ -609,4 +609,146 @@ def f RUBY end end + + context 'when if-end condition is assigned to a variable' do + context 'with variable being on the same line' do + let(:source) do + <<~RUBY + x = if a + #{'b' * body_length} + end + RUBY + end + + context 'when it is short enough to fit on a single line' do + let(:body_length) { 69 } + + it 'corrects it to the single-line form' do + corrected = autocorrect_source(source) + expect(corrected).to eq "x = (#{'b' * body_length} if a)\n" + end + end + + context 'when it is not short enough to fit on a single line' do + let(:body_length) { 70 } + + it 'accepts it in the multiline form' do + expect_no_offenses(source) + end + end + end + + context 'with variable being on the previous line' do + let(:source) do + <<~RUBY + x = + if a + #{'b' * body_length} + end + RUBY + end + + context 'when it is short enough to fit on a single line' do + let(:body_length) { 71 } + + it 'corrects it to the single-line form' do + corrected = autocorrect_source(source) + expect(corrected).to eq "x =\n (#{'b' * body_length} if a)\n" + end + end + + context 'when it is not short enough to fit on a single line' do + let(:body_length) { 72 } + + it 'accepts it in the multiline form' do + expect_no_offenses(source) + end + end + end + end + + context 'when if-end condition is an element of an array' do + let(:source) do + <<~RUBY + [ + if a + #{'b' * body_length} + end + ] + RUBY + end + + context 'when short enough to fit on a single line' do + let(:body_length) { 71 } + + it 'corrects it to the single-line form' do + corrected = autocorrect_source(source) + expect(corrected).to eq "[\n (#{'b' * body_length} if a)\n]\n" + end + end + + context 'when not short enough to fit on a single line' do + let(:body_length) { 72 } + + it 'accepts it in the multiline form' do + expect_no_offenses(source) + end + end + end + + context 'when if-end condition is a value in a hash' do + let(:source) do + <<~RUBY + { + x: if a + #{'b' * body_length} + end + } + RUBY + end + + context 'when it is short enough to fit on a single line' do + let(:body_length) { 68 } + + it 'corrects it to the single-line form' do + corrected = autocorrect_source(source) + expect(corrected).to eq "{\n x: (#{'b' * body_length} if a)\n}\n" + end + end + + context 'when it is not short enough to fit on a single line' do + let(:body_length) { 69 } + + it 'accepts it in the multiline form' do + expect_no_offenses(source) + end + end + end + + context 'when if-end condition has a first line comment' do + let(:source) do + <<~RUBY + if foo # #{'c' * comment_length} + bar + end + RUBY + end + + context 'when it is short enough to fit on a single line' do + let(:comment_length) { 67 } + + it 'corrects it to the single-line form' do + corrected = autocorrect_source(source) + expect(corrected).to eq "bar if foo # #{'c' * comment_length}\n" + end + end + + context 'when it is not short enough to fit on a single line' do + let(:comment_length) { 68 } + + it 'accepts it in the multiline form' do + expect_no_offenses(source) + end + end + end end