Skip to content

Commit

Permalink
Rubocop: Enable most SpaceInside cops (#15201)
Browse files Browse the repository at this point in the history
Enabled:
* SpaceInsideArrayLiteralBrackets
* SpaceInsideParens
* SpaceInsidePercentLiteralDelimiters
* SpaceInsideStringInterpolation
* Add enforced style for SpaceInsideStringInterpolation

Enabled without offenses:
* SpaceInsideArrayPercentLiteral
* Layout/SpaceInsideRangeLiteral
* Layout/SpaceInsideReferenceBrackets
  • Loading branch information
roaksoax authored Jul 20, 2023
1 parent 0f86955 commit cf67cb1
Show file tree
Hide file tree
Showing 59 changed files with 185 additions and 181 deletions.
32 changes: 18 additions & 14 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,24 @@ Layout/SpaceBeforeFirstArg:
Enabled: true
Layout/SpaceBeforeSemicolon:
Enabled: true
Layout/SpaceInsideArrayLiteralBrackets:
Enabled: true
EnforcedStyle: no_space
EnforcedStyleForEmptyBrackets: no_space
Layout/SpaceInsideArrayPercentLiteral:
Enabled: true # no offenses
Layout/SpaceInsideParens:
Enabled: true
EnforcedStyle: no_space
Layout/SpaceInsidePercentLiteralDelimiters:
Enabled: true
Layout/SpaceInsideRangeLiteral:
Enabled: true # no offenses
Layout/SpaceInsideReferenceBrackets:
Enabled: true # no offenses
Layout/SpaceInsideStringInterpolation:
Enabled: true
EnforcedStyle: no_space

##### Need review #####
Layout/AccessModifierIndentation:
Expand Down Expand Up @@ -213,21 +231,7 @@ Layout/SpaceAfterNot:
Enabled: false
Layout/SpaceInLambdaLiteral:
Enabled: false
Layout/SpaceInsideArrayLiteralBrackets:
Enabled: false
Layout/SpaceInsideArrayPercentLiteral:
Enabled: false
Layout/SpaceInsideBlockBraces:
Enabled: false
Layout/SpaceInsideHashLiteralBraces:
Enabled: false
Layout/SpaceInsideParens:
Enabled: false
Layout/SpaceInsidePercentLiteralDelimiters:
Enabled: false
Layout/SpaceInsideRangeLiteral:
Enabled: false
Layout/SpaceInsideReferenceBrackets:
Enabled: false
Layout/SpaceInsideStringInterpolation:
Enabled: false
6 changes: 3 additions & 3 deletions lib/bootstrap/patches/jar_dependencies.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@

require "jar_dependencies"

def require_jar( *args )
def require_jar(*args)
return nil unless Jars.require?
result = Jars.require_jar( *args )
result = Jars.require_jar(*args)
if result.is_a? String
# JAR_DEBUG=1 will now show theses
Jars.debug { "--- jar coordinate #{args[0..-2].join( ':' )} already loaded with version #{result} - omit version #{args[-1]}" }
Jars.debug { "--- jar coordinate #{args[0..-2].join(':')} already loaded with version #{result} - omit version #{args[-1]}" }
Jars.debug { " try to load from #{caller.join("\n\t")}" }
return false
end
Expand Down
2 changes: 1 addition & 1 deletion lib/bootstrap/rspec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

# Bundler + gemspec already setup $LOAD_PATH << '.../lib'
# but since we load specs from 2 locations we need to hook up these:
[ LogStash::Environment::LOGSTASH_HOME, LogStash::Environment::LOGSTASH_CORE ].each do |path|
[LogStash::Environment::LOGSTASH_HOME, LogStash::Environment::LOGSTASH_CORE].each do |path|
spec_path = File.join(path, "spec")
$LOAD_PATH.unshift(spec_path) unless $LOAD_PATH.include?(spec_path)
end
Expand Down
8 changes: 4 additions & 4 deletions lib/pluginmanager/generate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

class LogStash::PluginManager::Generate < LogStash::PluginManager::Command

TYPES = [ "input", "filter", "output", "codec" ]
TYPES = ["input", "filter", "output", "codec"]

option "--type", "TYPE", "Type of the plugin {input, filter, codec, output}s", :required => true
option "--name", "PLUGIN", "Name of the new plugin", :required => true
Expand Down Expand Up @@ -58,7 +58,7 @@ def create_scaffold(source, target)

def transform_r(source, target)
Dir.entries(source).each do |entry|
next if [ ".", ".." ].include?(entry)
next if [".", ".."].include?(entry)
source_entry = File.join(source, entry)
target_entry = File.join(target, entry)

Expand Down Expand Up @@ -98,8 +98,8 @@ def options

def get_git_info
git = OpenStruct.new
git.author = %x{ git config --get user.name }.strip rescue "your_username"
git.email = %x{ git config --get user.email }.strip rescue "your_username@example.com"
git.author = %x{git config --get user.name}.strip rescue "your_username"
git.email = %x{git config --get user.email}.strip rescue "your_username@example.com"
git
end

Expand Down
2 changes: 1 addition & 1 deletion lib/pluginmanager/pack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def execute
def delete_target_file?
return true if overwrite?
puts("File #{target_file} exist, do you want to overwrite it? (Y/N)")
( "y" == STDIN.gets.strip.downcase ? true : false)
("y" == STDIN.gets.strip.downcase ? true : false)
end

def validate_target_file
Expand Down
2 changes: 1 addition & 1 deletion lib/pluginmanager/unpack.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def validate_cache_location
cache_location = LogStash::Environment::CACHE_PATH
if File.exist?(cache_location)
puts("Directory #{cache_location} is going to be overwritten, do you want to continue? (Y/N)")
override = ( "y" == STDIN.gets.strip.downcase ? true : false)
override = ("y" == STDIN.gets.strip.downcase ? true : false)
if override
FileUtils.rm_rf(cache_location)
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def initialize(cmd, options)

def to_s
hash = to_hash[:hot_threads]
report = "#{I18n.t("logstash.web_api.hot_threads.title", :hostname => hash[:hostname], :time => hash[:time], :top_count => @thread_dump.top_count )} \n"
report = "#{I18n.t("logstash.web_api.hot_threads.title", :hostname => hash[:hostname], :time => hash[:time], :top_count => @thread_dump.top_count)} \n"
report << '=' * STRING_SEPARATOR_LENGTH
report << "\n"
hash[:threads].each do |thread|
Expand Down
4 changes: 2 additions & 2 deletions logstash-core/lib/logstash/api/modules/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ def node
:vertices => as_boolean(params.fetch("vertices", false))}
payload = node.pipeline(pipeline_id, opts)
halt(404) if payload.empty?
respond_with(:pipelines => { pipeline_id => payload } )
respond_with(:pipelines => { pipeline_id => payload })
end

get "/pipelines" do
opts = {:graph => as_boolean(params.fetch("graph", false)),
:vertices => as_boolean(params.fetch("vertices", false))}
payload = node.pipelines(opts)
respond_with(:pipelines => payload )
respond_with(:pipelines => payload)
end

get "/?:filter?" do
Expand Down
2 changes: 1 addition & 1 deletion logstash-core/lib/logstash/config/source_loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def fetch
duplicate_ids = find_duplicate_ids(pipeline_configs)

if duplicate_ids.any?
logger.debug("Fetching pipelines with duplicate ids", duplicate_ids.each { |k, v| v.collect(&:pipeline_id) } )
logger.debug("Fetching pipelines with duplicate ids", duplicate_ids.each { |k, v| v.collect(&:pipeline_id) })
return FailedFetch.new("Found duplicate ids in your source: #{duplicate_ids.keys.sort.join(", ")}")
end

Expand Down
4 changes: 2 additions & 2 deletions logstash-core/lib/logstash/dependency_report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
require 'securerandom'

class LogStash::DependencyReport < Clamp::Command
option [ "--csv" ], "OUTPUT_PATH", "The path to write the dependency report in csv format.",
option ["--csv"], "OUTPUT_PATH", "The path to write the dependency report in csv format.",
:required => true, :attribute_name => :output_path

OTHER_DEPENDENCIES = [
Expand All @@ -40,7 +40,7 @@ def execute
tmp_dir = java.lang.System.getProperty("java.io.tmpdir")
ruby_output_path = File.join(tmp_dir, SecureRandom.uuid)
# Write a CSV with just the ruby stuff
CSV.open(ruby_output_path, "wb", :headers => [ "name", "version", "url", "license", "copyright", "sourceURL" ], :write_headers => true) do |csv|
CSV.open(ruby_output_path, "wb", :headers => ["name", "version", "url", "license", "copyright", "sourceURL"], :write_headers => true) do |csv|
puts "Finding gem dependencies"
gems.each { |d| csv << d }
puts "Finding gem embedded java/jar dependencies"
Expand Down
2 changes: 1 addition & 1 deletion logstash-core/lib/logstash/elasticsearch_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def initialize(settings, logger)
if password.is_a?(LogStash::Util::Password)
password = password.value
end
@client_args[:transport_options] = { :headers => { "Authorization" => 'Basic ' + Base64.encode64( "#{username}:#{password}" ).chomp } }
@client_args[:transport_options] = { :headers => { "Authorization" => 'Basic ' + Base64.encode64("#{username}:#{password}").chomp } }
end

@client = Elasticsearch::Client.new(@client_args)
Expand Down
2 changes: 1 addition & 1 deletion logstash-core/lib/logstash/modules/kibana_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def initialize(settings, client = nil) # allow for test mock injection
if password.is_a?(LogStash::Util::Password)
password = password.value
end
@http_options[:headers]['Authorization'] = 'Basic ' + Base64.encode64( "#{username}:#{password}" ).chomp
@http_options[:headers]['Authorization'] = 'Basic ' + Base64.encode64("#{username}:#{password}").chomp
end

# e.g. {"name":"Elastics-MacBook-Pro.local","version":{"number":"6.0.0-beta1","build_hash":"41e69","build_number":15613,"build_snapshot":true}..}
Expand Down
2 changes: 1 addition & 1 deletion logstash-core/lib/logstash/plugins/registry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ def load_available_plugins
logger.debug("Executing hooks", :name => plugin_context.name, :type => plugin_context.type, :hooks_file => plugin_context.hooks_file)
plugin_context.execute_hooks!
rescue => e
logger.error("error occured when loading plugins hooks file", :name => plugin_context.name, :type => plugin_context.type, :exception => e.message, :stacktrace => e.backtrace )
logger.error("error occured when loading plugins hooks file", :name => plugin_context.name, :type => plugin_context.type, :exception => e.message, :stacktrace => e.backtrace)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion logstash-core/lib/logstash/util/thread_dump.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
module LogStash
module Util
class ThreadDump
SKIPPED_THREADS = [ "Finalizer", "Reference Handler", "Signal Dispatcher" ].freeze
SKIPPED_THREADS = ["Finalizer", "Reference Handler", "Signal Dispatcher"].freeze
THREADS_COUNT_DEFAULT = 10.freeze
IGNORE_IDLE_THREADS_DEFAULT = true.freeze

Expand Down
38 changes: 19 additions & 19 deletions logstash-core/spec/conditionals_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ def multi_receive(events)
CONFIG

sample_one("foo" => 123, "bar" => 123) do
expect(subject.get("tags") ).to include("woot")
expect(subject.get("tags")).to include("woot")
end
end

Expand Down Expand Up @@ -248,7 +248,7 @@ def multi_receive(events)
}
CONFIG

sample_one("foo" => "foo", "somelist" => [ "one", "two" ], "foobar" => "foobar", "greeting" => "hello world", "tags" => [ "fancypantsy" ]) do
sample_one("foo" => "foo", "somelist" => ["one", "two"], "foobar" => "foobar", "greeting" => "hello world", "tags" => ["fancypantsy"]) do
# verify the original exists
expect(subject.get("tags")).to include("fancypantsy")

Expand All @@ -263,8 +263,8 @@ def multi_receive(events)

describe "operators" do
conditional "[message] == 'sample'" do
sample_one("sample") { expect(subject.get("tags") ).to include("success") }
sample_one("different") { expect(subject.get("tags") ).to include("failure") }
sample_one("sample") { expect(subject.get("tags")).to include("success") }
sample_one("different") { expect(subject.get("tags")).to include("failure") }
end

conditional "'sample' == [message]" do
Expand All @@ -281,30 +281,30 @@ def multi_receive(events)
end

conditional "[message] != 'sample'" do
sample_one("sample") { expect(subject.get("tags") ).to include("failure") }
sample_one("different") { expect(subject.get("tags") ).to include("success") }
sample_one("sample") { expect(subject.get("tags")).to include("failure") }
sample_one("different") { expect(subject.get("tags")).to include("success") }
end

conditional "[message] < 'sample'" do
sample_one("apple") { expect(subject.get("tags") ).to include("success") }
sample_one("zebra") { expect(subject.get("tags") ).to include("failure") }
sample_one("apple") { expect(subject.get("tags")).to include("success") }
sample_one("zebra") { expect(subject.get("tags")).to include("failure") }
end

conditional "[message] > 'sample'" do
sample_one("zebra") { expect(subject.get("tags") ).to include("success") }
sample_one("apple") { expect(subject.get("tags") ).to include("failure") }
sample_one("zebra") { expect(subject.get("tags")).to include("success") }
sample_one("apple") { expect(subject.get("tags")).to include("failure") }
end

conditional "[message] <= 'sample'" do
sample_one("apple") { expect(subject.get("tags") ).to include("success") }
sample_one("zebra") { expect(subject.get("tags") ).to include("failure") }
sample_one("sample") { expect(subject.get("tags") ).to include("success") }
sample_one("apple") { expect(subject.get("tags")).to include("success") }
sample_one("zebra") { expect(subject.get("tags")).to include("failure") }
sample_one("sample") { expect(subject.get("tags")).to include("success") }
end

conditional "[message] >= 'sample'" do
sample_one("zebra") { expect(subject.get("tags") ).to include("success") }
sample_one("sample") { expect(subject.get("tags") ).to include("success") }
sample_one("apple") { expect(subject.get("tags") ).to include("failure") }
sample_one("zebra") { expect(subject.get("tags")).to include("success") }
sample_one("sample") { expect(subject.get("tags")).to include("success") }
sample_one("apple") { expect(subject.get("tags")).to include("failure") }
end

conditional "[message] == 5" do
Expand Down Expand Up @@ -357,9 +357,9 @@ def multi_receive(events)
end

conditional "[message] =~ /sample/" do
sample_one("apple") { expect(subject.get("tags") ).to include("failure") }
sample_one("sample") { expect(subject.get("tags") ).to include("success") }
sample_one("some sample") { expect(subject.get("tags") ).to include("success") }
sample_one("apple") { expect(subject.get("tags")).to include("failure") }
sample_one("sample") { expect(subject.get("tags")).to include("success") }
sample_one("some sample") { expect(subject.get("tags")).to include("success") }
end

conditional "[message] !~ /sample/" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def registerIfNot(setting)

before :all do
registerIfNot(LogStash::Setting::Boolean.new("xpack.monitoring.enabled", false))
registerIfNot(LogStash::Setting::ArrayCoercible.new("xpack.monitoring.elasticsearch.hosts", String, [ "http://localhost:9200" ] ))
registerIfNot(LogStash::Setting::ArrayCoercible.new("xpack.monitoring.elasticsearch.hosts", String, ["http://localhost:9200"]))
registerIfNot(LogStash::Setting::NullableString.new("xpack.monitoring.elasticsearch.username", "logstash_TEST system"))
registerIfNot(LogStash::Setting::NullableString.new("xpack.monitoring.elasticsearch.username", "logstash_TEST system"))
end
Expand Down
6 changes: 3 additions & 3 deletions logstash-core/spec/logstash/config/mixin_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ def initialize(params)
plugin_class.new(
"oneString" => "${notExistingVar:foo}",
"oneBoolean" => "${notExistingVar:true}",
"oneArray" => [ "first array value", "${notExistingVar:foo}", "${notExistingVar:}", "${notExistingVar: }", "${notExistingVar:foo bar}" ],
"oneArray" => ["first array value", "${notExistingVar:foo}", "${notExistingVar:}", "${notExistingVar: }", "${notExistingVar:foo bar}"],
"oneHash" => { "key" => "${notExistingVar:foo}" }
)
end
Expand Down Expand Up @@ -556,7 +556,7 @@ def initialize(params)
plugin_class.new(
"oneString" => "${FunString:foo}",
"oneBoolean" => "${FunBool:false}",
"oneArray" => [ "first array value", "${FunString:foo}" ],
"oneArray" => ["first array value", "${FunString:foo}"],
"oneHash" => { "key1" => "${FunString:foo}", "key2" => "${FunString} is ${FunBool}", "key3" => "${FunBool:false} or ${funbool:false}" },
"nestedHash" => { "level1" => { "key1" => "http://${FunString}:8080/blah.txt" } },
"nestedArray" => { "level1" => [{ "key1" => "http://${FunString}:8080/blah.txt" }, { "key2" => "http://${FunString}:8080/foo.txt" }] },
Expand All @@ -568,7 +568,7 @@ def initialize(params)
skip("This test fails on Windows, tracked in https://github.com/elastic/logstash/issues/10454")
expect(subject.oneString).to(be == "fancy")
expect(subject.oneBoolean).to(be_truthy)
expect(subject.oneArray).to(be == [ "first array value", "fancy" ])
expect(subject.oneArray).to(be == ["first array value", "fancy"])
expect(subject.oneHash).to(be == { "key1" => "fancy", "key2" => "fancy is true", "key3" => "true or false" })
expect(subject.nestedHash).to(be == { "level1" => { "key1" => "http://fancy:8080/blah.txt" } })
expect(subject.nestedArray).to(be == { "level1" => [{ "key1" => "http://fancy:8080/blah.txt" }, { "key2" => "http://fancy:8080/foo.txt" }] })
Expand Down
Loading

0 comments on commit cf67cb1

Please sign in to comment.