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

Enable Style/CommandLiteral cop #51741

Closed
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
4 changes: 4 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,10 @@ Style/OpenStructUse:
Style/ArrayIntersect:
Enabled: true

Style/CommandLiteral:
Enabled: true
EnforcedStyle: percent_x

Performance/BindCall:
Enabled: true

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def structure_dump(filename, extra_flags)

def structure_load(filename, extra_flags)
flags = extra_flags.join(" ") if extra_flags
`sqlite3 #{flags} #{db_config.database} < "#{filename}"`
%x(sqlite3 #{flags} #{db_config.database} < "#{filename}")
end

private
Expand Down
4 changes: 2 additions & 2 deletions activerecord/test/cases/adapters/sqlite3/sqlite_rake_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ def setup
"database" => @database
}

`sqlite3 #{@database} 'CREATE TABLE bar(id INTEGER)'`
`sqlite3 #{@database} 'CREATE TABLE foo(id INTEGER)'`
%x(sqlite3 #{@database} 'CREATE TABLE bar(id INTEGER)')
%x(sqlite3 #{@database} 'CREATE TABLE foo(id INTEGER)')
end

def test_structure_dump
Expand Down
2 changes: 1 addition & 1 deletion guides/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ The editing files for the Guides rebuild reside in `stylesrc` and use SCSS to im

## Building the Guides in Development

To generate new guides into static files, type `rake guides:generate` from inside the `guides` folder. If you make changes to the HTML or ERB, you'll need to remove the "output" directory before running this command. The master SCSS files (style.scss, highlight.scss) will compile as part of this process.
To generate new guides into static files, type `rake guides:generate` from inside the `guides` folder. If you make changes to the HTML or ERB, you'll need to remove the "output" directory before running this command. The master SCSS files (style.scss, highlight.scss) will compile as part of this process.

## FAQ

Expand Down
2 changes: 1 addition & 1 deletion guides/rails_guides.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
env_flag = ->(name) { "1" == env_value[name] }

version = env_value["RAILS_VERSION"]
edge = `git rev-parse HEAD`.strip unless version
edge = %x(git rev-parse HEAD).strip unless version

RailsGuides::Generator.new(
edge: edge,
Expand Down
2 changes: 1 addition & 1 deletion railties/lib/rails/api/task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ def api_dir

class EdgeTask < RepoTask
def rails_version
"main@#{`git rev-parse HEAD`[0, 7]}"
"main@#{%x(git rev-parse HEAD)[0, 7]}"
end

def badge_version
Expand Down
10 changes: 5 additions & 5 deletions railties/lib/rails/generators/app_base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -521,21 +521,21 @@ def using_bun?
def node_version
if using_node?
ENV.fetch("NODE_VERSION") do
`node --version`[/\d+\.\d+\.\d+/]
%x(node --version)[/\d+\.\d+\.\d+/]
rescue
NODE_LTS_VERSION
end
end
end

def dockerfile_yarn_version
using_node? and `yarn --version`[/\d+\.\d+\.\d+/]
using_node? and %x(yarn --version)[/\d+\.\d+\.\d+/]
rescue
"latest"
end

def dockerfile_bun_version
using_bun? and `bun --version`[/\d+\.\d+\.\d+/]
using_bun? and %x(bun --version)[/\d+\.\d+\.\d+/]
rescue
BUN_VERSION
end
Expand Down Expand Up @@ -745,13 +745,13 @@ def keep_file(destination)
end

def user_default_branch
@user_default_branch ||= `git config init.defaultbranch`
@user_default_branch ||= %x(git config init.defaultbranch)
end

def git_init_command
return "git init" if user_default_branch.strip.present?

git_version = `git --version`[/\d+\.\d+\.\d+/]
git_version = %x(git --version)[/\d+\.\d+\.\d+/]

if Gem::Version.new(git_version) >= Gem::Version.new("2.28.0")
"git init -b main"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ def author
if skip_git?
@author = default
else
@author = `git config user.name`.chomp rescue default
@author = %x(git config user.name).chomp rescue default
end
end

Expand All @@ -419,7 +419,7 @@ def email
if skip_git?
@email = default
else
@email = `git config user.email`.chomp rescue default
@email = %x(git config user.email).chomp rescue default
end
end

Expand Down
2 changes: 1 addition & 1 deletion railties/lib/rails/tasks/yarn.rake
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace :yarn do
end

yarn_flags =
if `yarn --version`.start_with?("1")
if %x(yarn --version).start_with?("1")
"--no-progress --frozen-lockfile"
else
"--immutable"
Expand Down
4 changes: 2 additions & 2 deletions railties/test/application/bin_setup_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def test_bin_setup
assert_equal 5, File.size("log/test.log")
assert_not File.exist?("tmp/restart.txt")

`bin/setup 2>&1`
%x(bin/setup 2>&1)
assert_equal 0, File.size("log/test.log")
assert_equal '["schema_migrations", "ar_internal_metadata", "articles"]', list_tables.call
assert File.exist?("tmp/restart.txt")
Expand All @@ -35,7 +35,7 @@ def test_bin_setup_output

app_file "db/schema.rb", ""

output = `bin/setup 2>&1`
output = %x(bin/setup 2>&1)

# Ignore line that's only output by Bundler < 1.14
output.sub!(/^Resolving dependencies\.\.\.\n/, "")
Expand Down
2 changes: 1 addition & 1 deletion railties/test/application/console_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def test_sandbox_when_sandbox_is_disabled
config.disable_sandbox = true
RUBY

output = `#{app_path}/bin/rails console --sandbox`
output = %x(#{app_path}/bin/rails console --sandbox)

assert_includes output, "sandbox mode is disabled"
assert_equal 1, $?.exitstatus
Expand Down
4 changes: 2 additions & 2 deletions railties/test/application/rake/migrations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ class TwoMigration < ActiveRecord::Migration::Current
rails "generate", "model", "user", "username:string", "password:string"
rails "generate", "migration", "add_email_to_users", "email:string"
rails "db:migrate"
`rm db/migrate/*email*.rb`
%x(rm db/migrate/*email*.rb)

output = rails("db:migrate:status")
assert_match(/up\s+\d{14}\s+Create users/, output)
Expand Down Expand Up @@ -509,7 +509,7 @@ class TwoMigration < ActiveRecord::Migration::Current
rails "generate", "model", "user", "username:string", "password:string"
rails "generate", "migration", "add_email_to_users", "email:string"
rails "db:migrate"
`rm db/migrate/*email*.rb`
%x(rm db/migrate/*email*.rb)

output = rails("db:migrate:status")

Expand Down
2 changes: 1 addition & 1 deletion railties/test/application/rake_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ class Hello
end
RUBY

output = Dir.chdir(app_path) { `bin/rails do_something RAILS_ENV=production` }
output = Dir.chdir(app_path) { %x(bin/rails do_something RAILS_ENV=production) }
assert_equal "Answer: 42\n", output.lines.last
end

Expand Down
4 changes: 2 additions & 2 deletions railties/test/application/runner_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def test_no_minitest_loaded_in_production_mode
p $LOADED_FEATURES.grep(/minitest/)
SCRIPT
assert_match "[]", Dir.chdir(app_path) {
`RAILS_ENV=production bin/rails runner "bin/print_features.rb"`
%x(RAILS_ENV=production bin/rails runner "bin/print_features.rb")
}
end

Expand Down Expand Up @@ -88,7 +88,7 @@ def test_should_run_stdin
puts User.count
SCRIPT

assert_match "42", Dir.chdir(app_path) { `cat bin/count_users.rb | bin/rails runner -` }
assert_match "42", Dir.chdir(app_path) { %x(cat bin/count_users.rb | bin/rails runner -) }
end

def test_with_hook
Expand Down
32 changes: 16 additions & 16 deletions railties/test/application/test_runner_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ class PostTest < ActiveSupport::TestCase
RUBY

Dir.chdir(app_path) do
`ruby -Itest test/models/post_test.rb`.tap do |output|
%x(ruby -Itest test/models/post_test.rb).tap do |output|
assert_match "PostTest", output
assert_no_match "is already defined in", output
end
Expand Down Expand Up @@ -977,7 +977,7 @@ def test_pass_TEST_env_on_rake_test
create_test_file :models, "post", pass: false
# This specifically verifies TEST for backwards compatibility with rake test
# as `bin/rails test` already supports running tests from a single file more cleanly.
output = Dir.chdir(app_path) { `bin/rake test TEST=test/models/post_test.rb` }
output = Dir.chdir(app_path) { %x(bin/rake test TEST=test/models/post_test.rb) }

assert_match "PostTest", output, "passing TEST= should run selected test"
assert_no_match "AccountTest", output, "passing TEST= should only run selected test"
Expand All @@ -986,7 +986,7 @@ def test_pass_TEST_env_on_rake_test

def test_pass_rake_options
create_test_file :models, "account"
output = Dir.chdir(app_path) { `bin/rake --rakefile Rakefile --trace=stdout test` }
output = Dir.chdir(app_path) { %x(bin/rake --rakefile Rakefile --trace=stdout test) }

assert_match "1 runs, 1 assertions", output
assert_match "Execute test", output
Expand All @@ -995,21 +995,21 @@ def test_pass_rake_options
def test_rails_db_create_all_restores_db_connection
create_test_file :models, "account"
rails "db:create:all", "db:migrate"
output = Dir.chdir(app_path) { `echo ".tables" | rails dbconsole` }
output = Dir.chdir(app_path) { %x(echo ".tables" | rails dbconsole) }
assert_match "ar_internal_metadata", output, "tables should be dumped"
end

def test_rails_db_create_all_restores_db_connection_after_drop
create_test_file :models, "account"
rails "db:create:all" # create all to avoid warnings
rails "db:drop:all", "db:create:all", "db:migrate"
output = Dir.chdir(app_path) { `echo ".tables" | rails dbconsole` }
output = Dir.chdir(app_path) { %x(echo ".tables" | rails dbconsole) }
assert_match "ar_internal_metadata", output, "tables should be dumped"
end

def test_rake_passes_TESTOPTS_to_minitest
create_test_file :models, "account"
output = Dir.chdir(app_path) { `bin/rake test TESTOPTS=-v` }
output = Dir.chdir(app_path) { %x(bin/rake test TESTOPTS=-v) }
assert_match "AccountTest#test_truth", output, "passing TESTOPTS= should be sent to the test runner"
end

Expand All @@ -1020,7 +1020,7 @@ def test_running_with_ruby_gets_test_env_by_default

file = create_test_for_env("test")
results = Dir.chdir(app_path) {
`ruby -Ilib:test #{file}`.each_line.filter_map { |line| JSON.parse(line) if line.start_with?("{") }
%x(ruby -Ilib:test #{file}).each_line.filter_map { |line| JSON.parse(line) if line.start_with?("{") }
}
assert_equal 1, results.length
failures = results.first["failures"]
Expand All @@ -1037,7 +1037,7 @@ def test_running_with_ruby_can_set_env_via_cmdline

file = create_test_for_env("development")
results = Dir.chdir(app_path) {
`RAILS_ENV=development ruby -Ilib:test #{file}`.each_line.
%x(RAILS_ENV=development ruby -Ilib:test #{file}).each_line.
filter_map { |line| JSON.parse(line) if line.start_with?("{") }
}
assert_equal 1, results.length
Expand All @@ -1050,22 +1050,22 @@ def test_running_with_ruby_can_set_env_via_cmdline

def test_rake_passes_multiple_TESTOPTS_to_minitest
create_test_file :models, "account"
output = Dir.chdir(app_path) { `bin/rake test TESTOPTS='-v --seed=1234'` }
output = Dir.chdir(app_path) { %x(bin/rake test TESTOPTS='-v --seed=1234') }
assert_match "AccountTest#test_truth", output, "passing TEST= should run selected test"
assert_match "seed=1234", output, "passing TEST= should run selected test"
end

def test_rake_runs_multiple_test_tasks
create_test_file :models, "account"
create_test_file :controllers, "accounts_controller"
output = Dir.chdir(app_path) { `bin/rake test:models test:controllers TESTOPTS='-v'` }
output = Dir.chdir(app_path) { %x(bin/rake test:models test:controllers TESTOPTS='-v') }
assert_match "AccountTest#test_truth", output
assert_match "AccountsControllerTest#test_truth", output
end

def test_rake_db_and_test_tasks_parses_args_correctly
create_test_file :models, "account"
output = Dir.chdir(app_path) { `bin/rake db:migrate test:models TESTOPTS='-v' && echo ".tables" | rails dbconsole` }
output = Dir.chdir(app_path) { %x(bin/rake db:migrate test:models TESTOPTS='-v' && echo ".tables" | rails dbconsole) }
assert_match "AccountTest#test_truth", output
assert_match "ar_internal_metadata", output
end
Expand All @@ -1076,7 +1076,7 @@ def test_rake_runs_tests_before_other_tasks_when_specified
puts "echo"
end
RUBY
output = Dir.chdir(app_path) { `bin/rake test echo` }
output = Dir.chdir(app_path) { %x(bin/rake test echo) }
assert_equal "echo", output.split("\n").last
end

Expand All @@ -1087,7 +1087,7 @@ def test_rake_exits_on_failure
puts "echo"
end
RUBY
output = Dir.chdir(app_path) { `bin/rake test echo` }
output = Dir.chdir(app_path) { %x(bin/rake test echo) }
assert_no_match "echo", output
assert_not_predicate $?, :success?
end
Expand Down Expand Up @@ -1211,7 +1211,7 @@ class DummyTest < ApplicationSystemTestCase
end
RUBY

output = Dir.chdir(app_path) { `bin/rake test` }
output = Dir.chdir(app_path) { %x(bin/rake test) }
assert_match "0 runs, 0 assertions, 0 failures, 0 errors, 0 skips", output
end

Expand All @@ -1227,7 +1227,7 @@ class DummyTest < ApplicationSystemTestCase
end
RUBY

output = Dir.chdir(app_path) { `bin/rake test TEST=test/system/dummy_test.rb` }
output = Dir.chdir(app_path) { %x(bin/rake test TEST=test/system/dummy_test.rb) }
assert_match "1 runs, 1 assertions, 0 failures, 0 errors, 0 skips", output
end

Expand All @@ -1248,7 +1248,7 @@ def test_can_exclude_files_from_being_tested_via_default_rails_command_by_settin
def test_can_exclude_files_from_being_tested_via_rake_task_by_setting_DEFAULT_TEST_EXCLUDE_env_var
create_test_file "smoke", "smoke_foo"

output = Dir.chdir(app_path) { `DEFAULT_TEST_EXCLUDE="test/smoke/**/*_test.rb" bin/rake test` }
output = Dir.chdir(app_path) { %x(DEFAULT_TEST_EXCLUDE="test/smoke/**/*_test.rb" bin/rake test) }
assert_match "0 runs, 0 assertions, 0 failures, 0 errors, 0 skips", output
end

Expand Down
2 changes: 1 addition & 1 deletion railties/test/commands/credentials_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ class Rails::Command::CredentialsTest < ActiveSupport::TestCase
assert_match %r/git diff driver/i, run_edit_command

Dir.chdir(app_path) do
assert_equal "bin/rails credentials:diff", `git config --get 'diff.rails_credentials.textconv'`.strip
assert_equal "bin/rails credentials:diff", %x(git config --get 'diff.rails_credentials.textconv').strip
end

assert_no_match %r/git diff driver/i, run_edit_command
Expand Down
2 changes: 1 addition & 1 deletion railties/test/commands/db_system_change_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class Rails::Command::DbSystemChangeTest < ActiveSupport::TestCase
end

test "change can be forced" do
output = `cd #{app_path}; bin/rails db:system:change --to=postgresql --force`
output = %x(cd #{app_path}; bin/rails db:system:change --to=postgresql --force)

assert_match "force config/database.yml", output
assert_match "gsub Gemfile", output
Expand Down
2 changes: 1 addition & 1 deletion railties/test/commands/runner_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class Rails::RunnerTest < ActiveSupport::TestCase
teardown :teardown_app

def test_rails_runner_with_stdin
command_output = `echo "puts 'Hello world'" | #{app_path}/bin/rails runner -`
command_output = %x(echo "puts 'Hello world'" | #{app_path}/bin/rails runner -)

assert_equal <<~OUTPUT, command_output
Hello world
Expand Down
2 changes: 1 addition & 1 deletion railties/test/engine/commands_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def teardown

def test_help_command_work_inside_engine
output = capture(:stderr) do
in_plugin_context(plugin_path) { `bin/rails --help` }
in_plugin_context(plugin_path) { %x(bin/rails --help) }
end
assert_no_match "NameError", output
end
Expand Down
6 changes: 3 additions & 3 deletions railties/test/engine/test_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ class Rails::Engine::TestTest < ActiveSupport::TestCase
test "automatically synchronize test schema" do
in_plugin_context(plugin_path) do
# In order to confirm that migration files are loaded, generate multiple migration files.
`bin/rails generate model user name:string;
%x(bin/rails generate model user name:string;
bin/rails generate model todo name:string;
RAILS_ENV=development bin/rails db:migrate`
RAILS_ENV=development bin/rails db:migrate)

output = `bin/rails test test/models/bukkits/user_test.rb`
output = %x(bin/rails test test/models/bukkits/user_test.rb)
assert_includes(output, "0 runs, 0 assertions, 0 failures, 0 errors, 0 skips")
end
end
Expand Down