Skip to content

Commit

Permalink
Create a new plugin for ensuring a single space before a '=>'
Browse files Browse the repository at this point in the history
This will trigger a warning only for resources with single parameters
such as:

```
file { 'foo':
  ensure⎵⎵=> file,
}
```
  • Loading branch information
gerases committed Feb 9, 2024
1 parent 1be24a2 commit 0e8da67
Show file tree
Hide file tree
Showing 3 changed files with 200 additions and 1 deletion.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ Any flag that can be specified on the command line can also be specified in the
Or to specify an allowlist of allowed checks, include a line like:

```
--only-checks=trailing_whitespace,hard_tabs,duplicate_params,double_quoted_strings,unquoted_file_mode,only_variable_string,variables_not_enclosed,single_quote_string_with_variables,variable_contains_dash,ensure_not_symlink_target,unquoted_resource_title,relative_classname_inclusion,file_mode,resource_reference_without_title_capital,leading_zero,arrow_alignment,variable_is_lowercase,ensure_first_param,resource_reference_without_whitespace,file_ensure,trailing_comma,leading_zero
--only-checks=trailing_whitespace,hard_tabs,duplicate_params,double_quoted_strings,unquoted_file_mode,only_variable_string,variables_not_enclosed,single_quote_string_with_variables,variable_contains_dash,ensure_not_symlink_target,unquoted_resource_title,relative_classname_inclusion,file_mode,resource_reference_without_title_capital,leading_zero,arrow_alignment,space_before_arrow,variable_is_lowercase,ensure_first_param,resource_reference_without_whitespace,file_ensure,trailing_comma,leading_zero
```

Please note that there is an important difference between reading options from the command line and reading options from a configuration file: In the former case the shell interprets one level of quotes. That does not happen in the latter case. So, it would make sense to quote some configuration values on the command line, like so:
Expand Down Expand Up @@ -253,6 +253,7 @@ For a complete list of checks, and how to resolve errors on each check, see the
* Should not exceed an 140-character line width.
* An exception has been made for `source => 'puppet://...'` lines as splitting these over multiple lines decreases the readability of the manifests.
* Should align arrows (`=>`) within blocks of attributes.
* Should contain at most a single space before an arrows(`=>`) where the parameter block contains exactly one parameter.

### Quoting

Expand Down
72 changes: 72 additions & 0 deletions lib/puppet-lint/plugins/check_whitespace/space_before_arrow.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# frozen_string_literal: true

# Check the manifest tokens for any arrows (=>) that have too much space
# before them in situations when a given resource has at most one line with
# such arrows. For example:

# file {
# # too much space after "foo"
# foo⎵⎵=>⎵'bar'
# }
#
# file {
# # too much space after 'bar'
# foo⎵=>⎵{ bar⎵⎵=>⎵'baz' }
# }
#

# Linting multi-parameter resources like this:
#
# package { 'xxx':
# foo => 'bar',
# bar => 'baz',
# }
#
# is handled by the "arrow_alignment") plugin.

PuppetLint.new_check(:space_before_arrow) do
def check
resource_indexes.each do |res_idx|
resource_tokens = res_idx[:tokens]
resource_tokens.reject! do |token|
Set[:COMMENT, :SLASH_COMMENT, :MLCOMMENT].include?(token.type)
end

first_arrow = resource_tokens.index { |r| r.type == :FARROW }
last_arrow = resource_tokens.rindex { |r| r.type == :FARROW }
next if first_arrow.nil?
next if last_arrow.nil?

# If this is a single line resource, skip it
next unless resource_tokens[first_arrow].line == resource_tokens[last_arrow].line

resource_tokens.select { |token| token.type == :FARROW }.each do |token|
prev_token = token.prev_token
next unless prev_token
next if prev_token.value == ' '

line = prev_token.line
column = prev_token.column

notify(
:warning,
message: "there should be a single space before '=>' on line #{line}, column #{column}",
line: line,
column: column,
token: prev_token,
)
end
end
end

def fix(problem)
token = problem[:token]

if token.type == :WHITESPACE
token.value = ' '
return
end

add_token(tokens.index(token), PuppetLint::Lexer::Token.new(:WHITESPACE, ' ', 0, 0))
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
require 'spec_helper'

describe 'space_before_arrow' do
let(:msg) { "there should be a single space before '=>' on line %d, column %d" }

context 'with code that should not trigger any warnings' do
context 'resource with multiple parameters on different lines' do
let(:code) do
<<-END
file { 'foo':
foo => bar,
bar => buzz,
}
END
end

it 'does not detect any problems' do
expect(problems).to be_empty
end
end

context 'resource with single param and normal spacing' do
let(:code) do
<<-END
file { 'foo':
foo => bar,
}
END
end

it 'does not detect any problems' do
expect(problems).to be_empty
end
end

context 'resource with multiple params and normal spacing' do
let(:code) do
<<-END
file { 'foo':
foo => { "bar" => "baz" },
}
END
end

it 'does not detect any problems' do
expect(problems).to be_empty
end
end
end

context 'resource with a single param and simple value with too much space before arrow' do
let(:code) do
<<-END
file { 'foo':
foo => bar,
}
END
end

context 'with fix disabled' do
it 'does not detect any problems' do
expect(problems.size).to eq(1)
end

it 'produces 1 warning' do
expect(problems).to contain_warning(msg % [2, 14]).on_line(2).in_column(14)
end
end

context 'with fix enabled' do
before(:each) do
PuppetLint.configuration.fix = true
end

after(:each) do
PuppetLint.configuration.fix = false
end

it 'detects the problem' do
expect(problems.size).to eq(1)
end

it 'fixes the manifest' do
expect(problems).to contain_fixed(msg % [2, 14])
end
end
end

context 'resource with a single param, a hash as value and bad spacing within the hash' do
let(:code) do
<<-END
file { 'foo':
foo => { "bar" => "baz" },
}
END
end

context 'with fix disabled' do
it 'does not detect any problems' do
expect(problems.size).to eq(1)
end

it 'produces a warning' do
expect(problems).to contain_warning(msg % [2, 25]).on_line(2).in_column(25)
end
end

context 'with fix enabled' do
before(:each) do
PuppetLint.configuration.fix = true
end

after(:each) do
PuppetLint.configuration.fix = false
end

it 'detects the problem' do
expect(problems.size).to eq(1)
end

it 'fixes the manifest' do
expect(problems).to contain_fixed(msg % [2, 25])
end
end
end
end

0 comments on commit 0e8da67

Please sign in to comment.