diff --git a/CHANGELOG.md b/CHANGELOG.md index ae5a7fcb0e91..82bcfea876c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ * Added ability to download shared rubocop config files from remote urls. ([@ptrippett][]) * [#1601](https://github.com/bbatsov/rubocop/issues/1601): Add `IgnoreEmptyMethods` config parameter for `Lint/UnusedMethodArgument` and `IgnoreEmptyBlocks` config parameter for `Lint/UnusedBlockArgument` cops. ([@alexdowad][]) * [#1729](https://github.com/bbatsov/rubocop/issues/1729): `Style/MethodDefParentheses` supports new 'require_no_parentheses_except_multiline' style. ([@alexdowad][]) +* [#2173](https://github.com/bbatsov/rubocop/issues/2173): `Style/AlignParameters` also checks parameter alignment for method definitions. ([@alexdowad][]) ### Bug Fixes diff --git a/lib/rubocop/cop/style/align_parameters.rb b/lib/rubocop/cop/style/align_parameters.rb index d18cfcbf9ad2..b4924d4caa03 100644 --- a/lib/rubocop/cop/style/align_parameters.rb +++ b/lib/rubocop/cop/style/align_parameters.rb @@ -3,23 +3,33 @@ module RuboCop module Cop module Style - # Here we check if the parameters on a multi-line method call are - # aligned. + # Here we check if the parameters on a multi-line method call or + # definition are aligned. class AlignParameters < Cop include AutocorrectAlignment - - MSG = 'Align the parameters of a method call if they span ' \ - 'more than one line.' + include OnMethodDef def on_send(node) _receiver, method, *args = *node return if method == :[]= - return if args.size <= 1 + return if args.size < 2 + + check_alignment(args, base_column(node, args)) + end + def on_method_def(node, _method_name, args, _body) + args = args.children + return if args.size < 2 check_alignment(args, base_column(node, args)) end + def message(node) + type = node.parent.send_type? ? 'call' : 'definition' + "Align the parameters of a method #{type} if they span " \ + 'more than one line.' + end + private def fixed_indentation? @@ -38,7 +48,9 @@ def base_column(node, args) end def target_method_lineno(node) - if node.loc.selector + if node.def_type? || node.defs_type? + node.loc.keyword.line + elsif node.loc.selector node.loc.selector.line else # l.(1) has no selector, so we use the opening parenthesis instead diff --git a/spec/rubocop/cop/style/align_parameters_spec.rb b/spec/rubocop/cop/style/align_parameters_spec.rb index 9e60a0b6ef5f..5e3ef336e259 100644 --- a/spec/rubocop/cop/style/align_parameters_spec.rb +++ b/spec/rubocop/cop/style/align_parameters_spec.rb @@ -190,6 +190,94 @@ expect(cop.offenses).to be_empty end + context 'method definitions' do + it 'registers an offense for parameters with single indent' do + inspect_source(cop, ['def method(a,', + ' b)', + 'end']) + expect(cop.offenses.size).to eq 1 + expect(cop.offenses.first.to_s).to match(/method definition/) + end + + it 'registers an offense for parameters with double indent' do + inspect_source(cop, ['def method(a,', + ' b)', + 'end']) + expect(cop.offenses.size).to eq 1 + end + + it 'accepts parameter lists on a single line' do + inspect_source(cop, ['def method(a, b)', + 'end']) + expect(cop.offenses).to be_empty + end + + it 'accepts proper indentation' do + inspect_source(cop, ['def method(a,', + ' b)', + 'end']) + expect(cop.offenses).to be_empty + end + + it 'accepts the first parameter being on a new row' do + inspect_source(cop, ['def method(', + ' a,', + ' b)', + 'end']) + expect(cop.offenses).to be_empty + end + + it 'accepts a method definition without parameters' do + inspect_source(cop, ['def method', + 'end']) + expect(cop.offenses).to be_empty + end + + it "doesn't get confused by splat" do + inspect_source(cop, ['def func2(a,', + ' *b,', + ' c)', + 'end']) + expect(cop.offenses.size).to eq 1 + expect(cop.highlights).to eq(['*b']) + end + + it 'auto-corrects alignment' do + new_source = autocorrect_source(cop, ['def method(a,', + ' b)', + 'end']) + expect(new_source).to eq(['def method(a,', + ' b)', + 'end'].join("\n")) + end + + context 'defining self.method' do + it 'registers an offense for parameters with single indent' do + inspect_source(cop, ['def self.method(a,', + ' b)', + 'end']) + expect(cop.offenses.size).to eq 1 + expect(cop.offenses.first.to_s).to match(/method definition/) + end + + it 'accepts proper indentation' do + inspect_source(cop, ['def self.method(a,', + ' b)', + 'end']) + expect(cop.offenses).to be_empty + end + + it 'auto-corrects alignment' do + new_source = autocorrect_source(cop, ['def self.method(a,', + ' b)', + 'end']) + expect(new_source).to eq(['def self.method(a,', + ' b)', + 'end'].join("\n")) + end + end + end + context 'assigned methods' do it 'accepts the first parameter being on a new row' do inspect_source(cop, [' assigned_value = match(', @@ -388,6 +476,94 @@ end end + context 'method definitions' do + it 'registers an offense for parameters aligned to first param' do + inspect_source(cop, ['def method(a,', + ' b)', + 'end']) + expect(cop.offenses.size).to eq 1 + expect(cop.offenses.first.to_s).to match(/method definition/) + end + + it 'registers an offense for parameters with double indent' do + inspect_source(cop, ['def method(a,', + ' b)', + 'end']) + expect(cop.offenses.size).to eq 1 + end + + it 'accepts parameter lists on a single line' do + inspect_source(cop, ['def method(a, b)', + 'end']) + expect(cop.offenses).to be_empty + end + + it 'accepts proper indentation' do + inspect_source(cop, ['def method(a,', + ' b)', + 'end']) + expect(cop.offenses).to be_empty + end + + it 'accepts the first parameter being on a new row' do + inspect_source(cop, ['def method(', + ' a,', + ' b)', + 'end']) + expect(cop.offenses).to be_empty + end + + it 'accepts a method definition without parameters' do + inspect_source(cop, ['def method', + 'end']) + expect(cop.offenses).to be_empty + end + + it "doesn't get confused by splat" do + inspect_source(cop, ['def func2(a,', + ' *b,', + ' c)', + 'end']) + expect(cop.offenses).not_to be_empty + expect(cop.highlights).to include '*b' + end + + it 'auto-corrects alignment' do + new_source = autocorrect_source(cop, ['def method(a,', + ' b)', + 'end']) + expect(new_source).to eq(['def method(a,', + ' b)', + 'end'].join("\n")) + end + + context 'defining self.method' do + it 'registers an offense for parameters aligned to first param' do + inspect_source(cop, ['def self.method(a,', + ' b)', + 'end']) + expect(cop.offenses.size).to eq 1 + expect(cop.offenses.first.to_s).to match(/method definition/) + end + + it 'accepts proper indentation' do + inspect_source(cop, ['def self.method(a,', + ' b)', + 'end']) + expect(cop.offenses).to be_empty + end + + it 'auto-corrects alignment' do + new_source = autocorrect_source(cop, ['def self.method(a,', + ' b)', + 'end']) + expect(new_source).to eq(['def self.method(a,', + ' b)', + 'end'].join("\n")) + end + end + end + context 'assigned methods' do context 'with IndentationWidth:Width set to 4' do let(:indentation_width) { 4 }