Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Layout/AlignHash allow two options #6649

Merged
merged 4 commits into from
May 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,14 @@ Lint/AmbiguousBlockAssociation:
Exclude:
- 'spec/**/*.rb'

Layout/AlignHash:
EnforcedHashRocketStyle:
- key
- table
EnforcedColonStyle:
- key
- table

Lint/InterpolationCheck:
Exclude:
- 'spec/**/*.rb'
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

* Add support for subclassing using `Class.new` to `Lint/InheritException`. ([@houli][])
* [#6779](https://github.com/rubocop-hq/rubocop/issues/6779): Add new cop `Style/NegativeUnless` that checks for unless with negative condition. ([@tejasbubane][])
* [#6649](https://github.com/rubocop-hq/rubocop/pull/6649): Layout/AlignHash supports list of options. ([@stoivo][])


### Bug fixes

Expand Down Expand Up @@ -4027,3 +4029,4 @@
[@houli]: https://github.com/houli
[@lavoiesl]: https://github.com/lavoiesl
[@fwininger]: https://github.com/fwininger
[@stoivo]: https://github.com/stoivo
1 change: 1 addition & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ Layout/AlignHash:
Align the elements of a hash literal if they span more than
one line.
Enabled: true
AllowMultipleStyles: true
VersionAdded: '0.49'
# Alignment of entries using hash rocket as separator. Valid values are:
#
Expand Down
74 changes: 37 additions & 37 deletions lib/rubocop/ast/builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,45 +15,45 @@ module AST
# root_node = parser.parse(buffer)
class Builder < Parser::Builders::Default
NODE_MAP = {
and: AndNode,
alias: AliasNode,
args: ArgsNode,
array: ArrayNode,
block: BlockNode,
break: BreakNode,
case: CaseNode,
class: ClassNode,
def: DefNode,
defined?: DefinedNode,
defs: DefNode,
ensure: EnsureNode,
for: ForNode,
hash: HashNode,
if: IfNode,
irange: RangeNode,
erange: RangeNode,
kwsplat: KeywordSplatNode,
module: ModuleNode,
or: OrNode,
pair: PairNode,
regexp: RegexpNode,
resbody: ResbodyNode,
retry: RetryNode,
csend: SendNode,
send: SendNode,
str: StrNode,
dstr: StrNode,
xstr: StrNode,
sclass: SelfClassNode,
super: SuperNode,
zsuper: SuperNode,
sym: SymbolNode,
until: UntilNode,
and: AndNode,
alias: AliasNode,
args: ArgsNode,
array: ArrayNode,
block: BlockNode,
break: BreakNode,
case: CaseNode,
class: ClassNode,
def: DefNode,
defined?: DefinedNode,
defs: DefNode,
ensure: EnsureNode,
for: ForNode,
hash: HashNode,
if: IfNode,
irange: RangeNode,
erange: RangeNode,
kwsplat: KeywordSplatNode,
module: ModuleNode,
or: OrNode,
pair: PairNode,
regexp: RegexpNode,
resbody: ResbodyNode,
retry: RetryNode,
csend: SendNode,
send: SendNode,
str: StrNode,
dstr: StrNode,
xstr: StrNode,
sclass: SelfClassNode,
super: SuperNode,
zsuper: SuperNode,
sym: SymbolNode,
until: UntilNode,
until_post: UntilNode,
when: WhenNode,
while: WhileNode,
when: WhenNode,
while: WhileNode,
while_post: WhileNode,
yield: YieldNode
yield: YieldNode
}.freeze

# Generates {Node} from the given information.
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cached_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ def serialize_offense(offense)
begin_pos: offense.location.begin_pos,
end_pos: offense.location.end_pos
},
message: message(offense),
message: message(offense),
cop_name: offense.cop_name,
status: :uncorrected
status: :uncorrected
}
end

Expand Down
8 changes: 8 additions & 0 deletions lib/rubocop/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -543,8 +543,10 @@ def validate_enforced_styles(valid_cop_names)
styles.each do |style_name, style|
supported_key = RuboCop::Cop::Util.to_supported_styles(style_name)
valid = ConfigLoader.default_configuration[name][supported_key]

next unless valid
next if valid.include?(style)
next if validate_support_and_has_list(name, style, valid)

msg = "invalid #{style_name} '#{style}' for #{name} found in " \
"#{smart_loaded_path}\n" \
Expand All @@ -554,6 +556,12 @@ def validate_enforced_styles(valid_cop_names)
end
end

def validate_support_and_has_list(name, formats, valid)
ConfigLoader.default_configuration[name]['AllowMultipleStyles'] &&
formats.is_a?(Array) &&
formats.all? { |format| valid.include?(format) }
end

def reject_obsolete_cops_and_parameters
messages = [
obsolete_cops,
Expand Down
105 changes: 74 additions & 31 deletions lib/rubocop/cop/layout/align_hash.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ module Layout
# - always_inspect
# - always_ignore
# - ignore_implicit (without curly braces)
# - ignore_explicit (with curly braces)
#
# Alternatively you can specify multiple allowed styles. That's done by
# passing a list of styles to EnforcedStyles.
#
# @example EnforcedHashRocketStyle: key (default)
# # bad
Expand Down Expand Up @@ -198,48 +200,65 @@ def on_hash(node)
return if ignored_node?(node)
return if node.pairs.empty? || node.single_line?

return unless alignment_for_hash_rockets.checkable_layout?(node) &&
alignment_for_colons.checkable_layout?(node)
return unless alignment_for_hash_rockets
.any? { |a| a.checkable_layout?(node) } &&
alignment_for_colons
.any? { |a| a.checkable_layout?(node) }

check_pairs(node)
end

def autocorrect(node)
# We can't use the instance variable inside the lambda. That would
# just give each lambda the same reference and they would all get the
# last value of each. A local variable fixes the problem.
key_delta = column_deltas[:key] || 0
delta = column_deltas[alignment_for(node).first.class][node]
return if delta.nil?

if !node.value
correct_no_value(key_delta, node.source_range)
else
correct_key_value(key_delta, node.key.source_range,
node.value.source_range,
node.loc.operator)
end
correct_node(node, delta)
end

private

attr_accessor :offences_by
attr_accessor :column_deltas

private

def double_splat?(node)
node.children.last.is_a?(Symbol)
end

def check_pairs(node)
first_pair = node.pairs.first
self.column_deltas = alignment_for(first_pair)
.deltas_for_first_pair(first_pair, node)
add_offense(first_pair) unless good_alignment?
self.offences_by = {}
self.column_deltas = Hash.new { |hash, key| hash[key] = {} }

alignment_for(first_pair).each do |alignment|
delta = alignment.deltas_for_first_pair(first_pair, node)
check_delta delta, node: first_pair, alignment: alignment
end

node.children.each do |current|
self.column_deltas = alignment_for(current)
.deltas(first_pair, current)
add_offense(current) unless good_alignment?
alignment_for(current).each do |alignment|
delta = alignment.deltas(first_pair, current)
check_delta delta, node: current, alignment: alignment
end
end

add_offences
end

def add_offences
_format, offences = offences_by.min_by { |_, v| v.length }
(offences || []).each do |offence|
add_offense offence
end
end

def check_delta(delta, node:, alignment:)
offences_by[alignment.class] ||= []
return if good_alignment? delta

column_deltas[alignment.class][node] = delta
offences_by[alignment.class].push(node)
end

def ignore_hash_argument?(node)
case cop_config['EnforcedLastArgumentHashStyle']
when 'always_inspect' then false
Expand Down Expand Up @@ -267,16 +286,31 @@ def alignment_for_colons
new_alignment('EnforcedColonStyle')
end

def correct_node(node, delta)
# We can't use the instance variable inside the lambda. That would
# just give each lambda the same reference and they would all get the
# last value of each. A local variable fixes the problem.

if !node.value
correct_no_value(delta[:key] || 0, node.source_range)
else
correct_key_value(delta, node.key.source_range,
node.value.source_range,
node.loc.operator)
end
end

def correct_no_value(key_delta, key)
->(corrector) { adjust(corrector, key_delta, key) }
end

def correct_key_value(key_delta, key, value, separator)
def correct_key_value(delta, key, value, separator)
# We can't use the instance variable inside the lambda. That would
# just give each lambda the same reference and they would all get the
# last value of each. Some local variables fix the problem.
separator_delta = column_deltas[:separator] || 0
value_delta = column_deltas[:value] || 0
separator_delta = delta[:separator] || 0
value_delta = delta[:value] || 0
key_delta = delta[:key] || 0

key_column = key.column
key_delta = -key_column if key_delta < -key_column
Expand All @@ -289,11 +323,20 @@ def correct_key_value(key_delta, key, value, separator)
end

def new_alignment(key)
case cop_config[key]
when 'key' then KeyAlignment.new
when 'table' then TableAlignment.new
when 'separator' then SeparatorAlignment.new
else raise "Unknown #{key}: #{cop_config[key]}"
formats = cop_config[key]
formats = [formats] if formats.is_a? String

formats.uniq.map do |format|
case format
when 'key'
KeyAlignment.new
when 'table'
TableAlignment.new
when 'separator'
SeparatorAlignment.new
else
raise "Unknown #{key}: #{formats}"
end
end
end

Expand All @@ -306,7 +349,7 @@ def adjust(corrector, delta, range)
end
end

def good_alignment?
def good_alignment?(column_deltas)
column_deltas.values.all?(&:zero?)
end
end
Expand Down
12 changes: 6 additions & 6 deletions lib/rubocop/cop/layout/closing_parenthesis_indentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ def check_for_elements(node, elements)

add_offense(right_paren,
location: right_paren,
message: message(correct_column,
left_paren,
right_paren))
message: message(correct_column,
left_paren,
right_paren))
end

def check_for_no_elements(node)
Expand All @@ -137,9 +137,9 @@ def check_for_no_elements(node)
@column_delta = correct_column - right_paren.column
add_offense(right_paren,
location: right_paren,
message: message(correct_column,
left_paren,
right_paren))
message: message(correct_column,
left_paren,
right_paren))
end

def expected_column(left_paren, elements)
Expand Down
10 changes: 5 additions & 5 deletions lib/rubocop/cop/lint/ambiguous_operator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ class AmbiguousOperator < Cop
include ParserDiagnostic

AMBIGUITIES = {
'+' => { actual: 'positive number', possible: 'addition' },
'-' => { actual: 'negative number', possible: 'subtraction' },
'*' => { actual: 'splat', possible: 'multiplication' },
'&' => { actual: 'block', possible: 'binary AND' },
'**' => { actual: 'keyword splat', possible: 'exponent' }
'+' => { actual: 'positive number', possible: 'addition' },
'-' => { actual: 'negative number', possible: 'subtraction' },
'*' => { actual: 'splat', possible: 'multiplication' },
'&' => { actual: 'block', possible: 'binary AND' },
'**' => { actual: 'keyword splat', possible: 'exponent' }
}.each do |key, hash|
hash[:operator] = key
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/mixin/configurable_naming.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module ConfigurableNaming

FORMATS = {
snake_case: /^@{0,2}[\da-z_]+[!?=]?$/,
camelCase: /^@{0,2}_?[a-z][\da-zA-Z]+[!?=]?$/
camelCase: /^@{0,2}_?[a-z][\da-zA-Z]+[!?=]?$/
}.freeze
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/mixin/configurable_numbering.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ module ConfigurableNumbering
include ConfigurableFormatting

FORMATS = {
snake_case: /(?:[a-z_]|_\d+)$/,
normalcase: /(?:_\D*|[A-Za-z]\d*)$/,
snake_case: /(?:[a-z_]|_\d+)$/,
normalcase: /(?:_\D*|[A-Za-z]\d*)$/,
non_integer: /[A-Za-z_]$/
}.freeze
end
Expand Down