Skip to content

Create a new plugin for ensuring a single space before a '=>' #175

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

Merged
merged 3 commits into from
Feb 9, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 'detects extra space before arrow' 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 'detects extra space before arrow' 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