[Fix rubocop#3256] Highlight closing brace in Multiline...BraceLayout…

… cops The closing brace is what the cops are reporting, so that's what we should highlight. It's easy to misunderstand the reports otherwise.
jonas054 · Jul 22, 2016 · 33ea0d4 · 33ea0d4
1 parent ff4867c
commit 33ea0d4
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,6 +23,7 @@
 * Extend `Style/MethodMissing` cop to check for the conditions in the style guide. ([@drenmi][])
 * [#3325](https://github.com/bbatsov/rubocop/issues/3325): Drop support for MRI 1.9.3. ([@drenmi][])
 * Add support for MRI 2.4. ([@dvandersluis][])
+* [#3256](https://github.com/bbatsov/rubocop/issues/3256): Highlight the closing brace in `Style/Multiline...BraceLayout` cops. ([@jonas054][])
 
 ## 0.41.2 (2016-07-07)
 

diff --git a/lib/rubocop/cop/mixin/multiline_literal_brace_layout.rb b/lib/rubocop/cop/mixin/multiline_literal_brace_layout.rb
@@ -60,24 +60,24 @@ def last_element_trailing_comma_range(node)
       def handle_new_line(node)
         return unless closing_brace_on_same_line?(node)
 
-        add_offense(node, :expression, self.class::ALWAYS_NEW_LINE_MESSAGE)
+        add_offense(node, :end, self.class::ALWAYS_NEW_LINE_MESSAGE)
       end
 
       def handle_same_line(node)
         return if closing_brace_on_same_line?(node)
 
-        add_offense(node, :expression, self.class::ALWAYS_SAME_LINE_MESSAGE)
+        add_offense(node, :end, self.class::ALWAYS_SAME_LINE_MESSAGE)
       end
 
       def handle_symmetrical(node)
         if opening_brace_on_same_line?(node)
           return if closing_brace_on_same_line?(node)
 
-          add_offense(node, :expression, self.class::SAME_LINE_MESSAGE)
+          add_offense(node, :end, self.class::SAME_LINE_MESSAGE)
         else
           return unless closing_brace_on_same_line?(node)
 
-          add_offense(node, :expression, self.class::NEW_LINE_MESSAGE)
+          add_offense(node, :end, self.class::NEW_LINE_MESSAGE)
         end
       end
 

diff --git a/spec/rubocop/cop/style/multiline_method_call_brace_layout_spec.rb b/spec/rubocop/cop/style/multiline_method_call_brace_layout_spec.rb
@@ -37,7 +37,7 @@
   end
 
   context 'when EnforcedStyle is new_line' do
-    let(:cop_config) { { 'EnforcedStyle' => 'symmetrical' } }
+    let(:cop_config) { { 'EnforcedStyle' => 'new_line' } }
 
     it 'still ignores single-line calls' do
       inspect_source(cop, 'puts("Hello world!")')

diff --git a/spec/support/multiline_literal_brace_layout_examples.rb b/spec/support/multiline_literal_brace_layout_examples.rb
@@ -100,11 +100,13 @@ def construct(*args)
       end
 
       it 'detects closing brace on different line from last element' do
-        inspect_source(cop, construct(false, true))
+        src = construct(false, true)
+        inspect_source(cop, src)
 
         expect(cop.offenses.size).to eq(1)
-        expect(cop.offenses.first.line).to eq(1)
-        expect(cop.highlights).to eq([braces(false, true)])
+        expect(cop.offenses.first.line)
+          .to eq(src.length - (suffix.empty? ? 0 : 1))
+        expect(cop.highlights).to eq([close])
         expect(cop.messages).to eq([described_class::SAME_LINE_MESSAGE])
       end
 
@@ -133,11 +135,13 @@ def construct(*args)
       end
 
       it 'detects closing brace on same line as last element' do
-        inspect_source(cop, construct(true, false))
+        src = construct(true, false)
+        inspect_source(cop, src)
 
         expect(cop.offenses.size).to eq(1)
-        expect(cop.offenses.first.line).to eq(1)
-        expect(cop.highlights).to eq([braces(true, false)])
+        expect(cop.offenses.first.line)
+          .to eq(src.length - (suffix.empty? ? 0 : 1))
+        expect(cop.highlights).to eq([close])
         expect(cop.messages).to eq([described_class::NEW_LINE_MESSAGE])
       end
 
@@ -166,21 +170,24 @@ def construct(*args)
       end
 
       it 'detects closing brace on same line as last element' do
-        inspect_source(cop, construct(false, false))
+        src = construct(false, false)
+        inspect_source(cop, src)
 
         expect(cop.offenses.size).to eq(1)
-        expect(cop.offenses.first.line).to eq(1)
-        expect(cop.highlights).to eq([braces(false, false)])
+        expect(cop.offenses.first.line)
+          .to eq(src.length - (suffix.empty? ? 0 : 1))
+        expect(cop.highlights).to eq([close])
         expect(cop.messages).to eq([described_class::ALWAYS_NEW_LINE_MESSAGE])
       end
 
       it 'detects closing brace on same line as last multiline element' do
-        inspect_source(cop, construct(false, a, make_multi(multi), false))
+        src = construct(false, a, make_multi(multi), false)
+        inspect_source(cop, src)
 
         expect(cop.offenses.size).to eq(1)
-        expect(cop.offenses.first.line).to eq(1)
-        expect(cop.highlights)
-          .to eq([braces(false, a, make_multi(multi), false)])
+        expect(cop.offenses.first.line)
+          .to eq(src.length - (suffix.empty? ? 0 : 1))
+        expect(cop.highlights).to eq([close])
         expect(cop.messages).to eq([described_class::ALWAYS_NEW_LINE_MESSAGE])
       end
 
@@ -208,11 +215,13 @@ def construct(*args)
       end
 
       it 'detects closing brace on same line as last element' do
-        inspect_source(cop, construct(true, false))
+        src = construct(true, false)
+        inspect_source(cop, src)
 
         expect(cop.offenses.size).to eq(1)
-        expect(cop.offenses.first.line).to eq(1)
-        expect(cop.highlights).to eq([braces(true, false)])
+        expect(cop.offenses.first.line)
+          .to eq(src.length - (suffix.empty? ? 0 : 1))
+        expect(cop.highlights).to eq([close])
         expect(cop.messages).to eq([described_class::ALWAYS_NEW_LINE_MESSAGE])
       end
 
@@ -241,21 +250,24 @@ def construct(*args)
       end
 
       it 'detects closing brace on different line from last element' do
-        inspect_source(cop, construct(false, true))
+        src = construct(false, true)
+        inspect_source(cop, src)
 
         expect(cop.offenses.size).to eq(1)
-        expect(cop.offenses.first.line).to eq(1)
-        expect(cop.highlights).to eq([braces(false, true)])
+        expect(cop.offenses.first.line)
+          .to eq(src.length - (suffix.empty? ? 0 : 1))
+        expect(cop.highlights).to eq([close])
         expect(cop.messages).to eq([described_class::ALWAYS_SAME_LINE_MESSAGE])
       end
 
       it 'detects closing brace on different line from multiline element' do
-        inspect_source(cop, construct(false, a, make_multi(multi), true))
+        src = construct(false, a, make_multi(multi), true)
+        inspect_source(cop, src)
 
         expect(cop.offenses.size).to eq(1)
-        expect(cop.offenses.first.line).to eq(1)
-        expect(cop.highlights)
-          .to eq([braces(false, a, make_multi(multi), true)])
+        expect(cop.offenses.first.line)
+          .to eq(src.length - (suffix.empty? ? 0 : 1))
+        expect(cop.highlights).to eq([close])
         expect(cop.messages).to eq([described_class::ALWAYS_SAME_LINE_MESSAGE])
       end
 
@@ -284,11 +296,12 @@ def construct(*args)
       end
 
       it 'detects closing brace on different line from last element' do
-        inspect_source(cop, construct(true, true))
-
+        src = construct(true, true)
+        inspect_source(cop, src)
         expect(cop.offenses.size).to eq(1)
-        expect(cop.offenses.first.line).to eq(1)
-        expect(cop.highlights).to eq([braces(true, true)])
+        expect(cop.offenses.first.line)
+          .to eq(src.length - (suffix.empty? ? 0 : 1))
+        expect(cop.highlights).to eq([close])
         expect(cop.messages).to eq([described_class::ALWAYS_SAME_LINE_MESSAGE])
       end