From d921e94a2b72aaa6b6232a13c045d62578e29dfe Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Fri, 5 Jun 2020 17:53:23 -0400 Subject: [PATCH 1/4] rubocop: separate args for shell commands Use `system "foo", "bar"` instead of `system "foo bar"`. Also applies to `Utils.popen_read` and `Utils.popen_write` commands. RuboCop can automatically fix these problems. --- Library/Homebrew/dev-cmd/audit.rb | 9 -- Library/Homebrew/rubocops/lines.rb | 49 +++++++ Library/Homebrew/test/rubocops/lines_spec.rb | 144 +++++++++++++++++++ 3 files changed, 193 insertions(+), 9 deletions(-) diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 9210c975f9..084fdbab74 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -844,15 +844,6 @@ module Homebrew # TODO: check could be in RuboCop problem "`env :userpaths` in formulae is deprecated" if line.include?("env :userpaths") - if line =~ /system ((["'])[^"' ]*(?:\s[^"' ]*)+\2)/ - bad_system = Regexp.last_match(1) - unless %w[| < > & ; *].any? { |c| bad_system.include? c } - good_system = bad_system.gsub(" ", "\", \"") - # TODO: check could be in RuboCop - problem "Use `system #{good_system}` instead of `system #{bad_system}` " - end - end - # TODO: check could be in RuboCop problem "`#{Regexp.last_match(1)}` is now unnecessary" if line =~ /(require ["']formula["'])/ diff --git a/Library/Homebrew/rubocops/lines.rb b/Library/Homebrew/rubocops/lines.rb index faa5f6ff80..d8708c2362 100644 --- a/Library/Homebrew/rubocops/lines.rb +++ b/Library/Homebrew/rubocops/lines.rb @@ -252,6 +252,55 @@ module RuboCop end end + class ShellCommands < FormulaCop + def audit_formula(_node, _class_node, _parent_class_node, body_node) + # Match shell commands separated by spaces in the same string + shell_cmd_with_spaces_regex = /[^"' ]*(?:\s[^"' ]*)+/ + + popen_commands = [ + :popen_read, + :safe_popen_read, + :popen_write, + :safe_popen_write, + ] + + shell_metacharacters = %w[> >> < << | ; & && || *] + + find_every_method_call_by_name(body_node, :system).each do |method| + # Continue if a shell metacharacter is present + next if shell_metacharacters.any? { |meta| string_content(parameters(method).first).include?(meta) } + + next unless match = regex_match_group(parameters(method).first, shell_cmd_with_spaces_regex) + + good_args = match[0].gsub(" ", "\", \"") + offending_node(parameters(method).first) + problem "Separate `system` commands into `\"#{good_args}\"`" + end + + popen_commands.each do |command| + find_instance_method_call(body_node, "Utils", command) do |method| + index = parameters(method).first.hash_type? ? 1 : 0 + + # Continue if a shell metacharacter is present + next if shell_metacharacters.any? { |meta| string_content(parameters(method)[index]).include?(meta) } + + next unless match = regex_match_group(parameters(method)[index], shell_cmd_with_spaces_regex) + + good_args = match[0].gsub(" ", "\", \"") + offending_node(parameters(method)[index]) + problem "Separate `Utils.#{command}` commands into `\"#{good_args}\"`" + end + end + end + + def autocorrect(node) + lambda do |corrector| + good_args = node.source.gsub(" ", "\", \"") + corrector.replace(node.source_range, good_args) + end + end + end + class Miscellaneous < FormulaCop def audit_formula(_node, _class_node, _parent_class_node, body_node) # FileUtils is included in Formula diff --git a/Library/Homebrew/test/rubocops/lines_spec.rb b/Library/Homebrew/test/rubocops/lines_spec.rb index 9c92e47850..395c3de540 100644 --- a/Library/Homebrew/test/rubocops/lines_spec.rb +++ b/Library/Homebrew/test/rubocops/lines_spec.rb @@ -574,6 +574,150 @@ describe RuboCop::Cop::FormulaAudit::ShellVariables do end end +describe RuboCop::Cop::FormulaAudit::ShellCommands do + subject(:cop) { described_class.new } + + context "When auditing shell commands" do + it "system arguments should be separated" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + system "foo bar" + ^^^^^^^^^ Separate `system` commands into `\"foo\", \"bar\"` + end + end + RUBY + end + + it "system arguments with string interpolation should be separated" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + system "\#{bin}/foo bar" + ^^^^^^^^^^^^^^^^ Separate `system` commands into `\"\#{bin}/foo\", \"bar\"` + end + end + RUBY + end + + it "system arguments with metacharacters should not be separated" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + def install + system "foo bar > baz" + end + end + RUBY + end + + it "only the first system argument should be separated" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + def install + system "foo", "bar baz" + end + end + RUBY + end + + it "Utils.popen arguments should not be separated" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + def install + Utils.popen("foo bar") + end + end + RUBY + end + + it "Utils.popen_read arguments should be separated" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.popen_read("foo bar") + ^^^^^^^^^ Separate `Utils.popen_read` commands into `\"foo\", \"bar\"` + end + end + RUBY + end + + it "Utils.safe_popen_read arguments should be separated" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.safe_popen_read("foo bar") + ^^^^^^^^^ Separate `Utils.safe_popen_read` commands into `\"foo\", \"bar\"` + end + end + RUBY + end + + it "Utils.popen_write arguments should be separated" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.popen_write("foo bar") + ^^^^^^^^^ Separate `Utils.popen_write` commands into `\"foo\", \"bar\"` + end + end + RUBY + end + + it "Utils.safe_popen_write arguments should be separated" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.safe_popen_write("foo bar") + ^^^^^^^^^ Separate `Utils.safe_popen_write` commands into `\"foo\", \"bar\"` + end + end + RUBY + end + + it "Utils.popen_read arguments with string interpolation should be separated" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.popen_read("\#{bin}/foo bar") + ^^^^^^^^^^^^^^^^ Separate `Utils.popen_read` commands into `\"\#{bin}/foo\", \"bar\"` + end + end + RUBY + end + + it "Utils.popen_read arguments with metacharacters should not be separated" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + def install + Utils.popen_read("foo bar > baz") + end + end + RUBY + end + + it "only the first Utils.popen_read argument should be separated" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + def install + Utils.popen_read("foo", "bar baz") + end + end + RUBY + end + + it "Utils.popen_read arguments should be separated following a shell variable" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.popen_read({ "SHELL" => "bash"}, "foo bar") + ^^^^^^^^^ Separate `Utils.popen_read` commands into `\"foo\", \"bar\"` + end + end + RUBY + end + end +end + describe RuboCop::Cop::FormulaAudit::Miscellaneous do subject(:cop) { described_class.new } From 35a8c336902d9a18be81d16981da35724e94e19b Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Tue, 30 Jun 2020 10:18:49 -0400 Subject: [PATCH 2/4] Update shell metacharacter list --- Library/Homebrew/rubocops/lines.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/rubocops/lines.rb b/Library/Homebrew/rubocops/lines.rb index d8708c2362..ca62165b9d 100644 --- a/Library/Homebrew/rubocops/lines.rb +++ b/Library/Homebrew/rubocops/lines.rb @@ -264,7 +264,7 @@ module RuboCop :safe_popen_write, ] - shell_metacharacters = %w[> >> < << | ; & && || *] + shell_metacharacters = %w[> < < | ; : & * $ ? : ~ + @ !` ( ) [ ]] find_every_method_call_by_name(body_node, :system).each do |method| # Continue if a shell metacharacter is present From 0786003fe9d3aef49e62b329d2ed7795d5f871cc Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Sun, 5 Jul 2020 12:12:36 -0400 Subject: [PATCH 3/4] Add tests for autocorrect --- Library/Homebrew/rubocops/lines.rb | 4 +- Library/Homebrew/test/rubocops/lines_spec.rb | 105 +++++++++++++++++++ 2 files changed, 107 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/rubocops/lines.rb b/Library/Homebrew/rubocops/lines.rb index ca62165b9d..0626fc346b 100644 --- a/Library/Homebrew/rubocops/lines.rb +++ b/Library/Homebrew/rubocops/lines.rb @@ -267,7 +267,7 @@ module RuboCop shell_metacharacters = %w[> < < | ; : & * $ ? : ~ + @ !` ( ) [ ]] find_every_method_call_by_name(body_node, :system).each do |method| - # Continue if a shell metacharacter is present + # Only separate when no shell metacharacters are present next if shell_metacharacters.any? { |meta| string_content(parameters(method).first).include?(meta) } next unless match = regex_match_group(parameters(method).first, shell_cmd_with_spaces_regex) @@ -281,7 +281,7 @@ module RuboCop find_instance_method_call(body_node, "Utils", command) do |method| index = parameters(method).first.hash_type? ? 1 : 0 - # Continue if a shell metacharacter is present + # Only separate when no shell metacharacters are present next if shell_metacharacters.any? { |meta| string_content(parameters(method)[index]).include?(meta) } next unless match = regex_match_group(parameters(method)[index], shell_cmd_with_spaces_regex) diff --git a/Library/Homebrew/test/rubocops/lines_spec.rb b/Library/Homebrew/test/rubocops/lines_spec.rb index 395c3de540..370bea30a9 100644 --- a/Library/Homebrew/test/rubocops/lines_spec.rb +++ b/Library/Homebrew/test/rubocops/lines_spec.rb @@ -715,6 +715,111 @@ describe RuboCop::Cop::FormulaAudit::ShellCommands do end RUBY end + + it "separates shell commands in system" do + source = <<~RUBY + class Foo < Formula + def install + system "foo bar" + end + end + RUBY + + corrected_source = <<~RUBY + class Foo < Formula + def install + system "foo", "bar" + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(corrected_source) + end + + it "separates shell commands with string interpolation in system" do + source = <<~RUBY + class Foo < Formula + def install + system "\#{foo}/bar baz" + end + end + RUBY + + corrected_source = <<~RUBY + class Foo < Formula + def install + system "\#{foo}/bar", "baz" + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(corrected_source) + end + + it "separates shell commands in Utils.popen_read" do + source = <<~RUBY + class Foo < Formula + def install + Utils.popen_read("foo bar") + end + end + RUBY + + corrected_source = <<~RUBY + class Foo < Formula + def install + Utils.popen_read("foo", "bar") + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(corrected_source) + end + + it "separates shell commands with string interpolation in Utils.popen_read" do + source = <<~RUBY + class Foo < Formula + def install + Utils.popen_read("\#{foo}/bar baz") + end + end + RUBY + + corrected_source = <<~RUBY + class Foo < Formula + def install + Utils.popen_read("\#{foo}/bar", "baz") + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(corrected_source) + end + + it "separates shell commands following a shell variable in Utils.popen_read" do + source = <<~RUBY + class Foo < Formula + def install + Utils.popen_read({ "SHELL" => "bash" }, "foo bar") + end + end + RUBY + + corrected_source = <<~RUBY + class Foo < Formula + def install + Utils.popen_read({ "SHELL" => "bash" }, "foo", "bar") + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(corrected_source) + end end end From ae0d37e911f2ad0c97f9f9a1687304ce22664571 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Sun, 5 Jul 2020 13:57:49 -0400 Subject: [PATCH 4/4] Transfer to FormulaAuditStrict --- Library/Homebrew/rubocops/lines.rb | 98 ++-- Library/Homebrew/test/rubocops/lines_spec.rb | 498 +++++++++---------- 2 files changed, 298 insertions(+), 298 deletions(-) diff --git a/Library/Homebrew/rubocops/lines.rb b/Library/Homebrew/rubocops/lines.rb index 0626fc346b..93c9cd3ba9 100644 --- a/Library/Homebrew/rubocops/lines.rb +++ b/Library/Homebrew/rubocops/lines.rb @@ -252,55 +252,6 @@ module RuboCop end end - class ShellCommands < FormulaCop - def audit_formula(_node, _class_node, _parent_class_node, body_node) - # Match shell commands separated by spaces in the same string - shell_cmd_with_spaces_regex = /[^"' ]*(?:\s[^"' ]*)+/ - - popen_commands = [ - :popen_read, - :safe_popen_read, - :popen_write, - :safe_popen_write, - ] - - shell_metacharacters = %w[> < < | ; : & * $ ? : ~ + @ !` ( ) [ ]] - - find_every_method_call_by_name(body_node, :system).each do |method| - # Only separate when no shell metacharacters are present - next if shell_metacharacters.any? { |meta| string_content(parameters(method).first).include?(meta) } - - next unless match = regex_match_group(parameters(method).first, shell_cmd_with_spaces_regex) - - good_args = match[0].gsub(" ", "\", \"") - offending_node(parameters(method).first) - problem "Separate `system` commands into `\"#{good_args}\"`" - end - - popen_commands.each do |command| - find_instance_method_call(body_node, "Utils", command) do |method| - index = parameters(method).first.hash_type? ? 1 : 0 - - # Only separate when no shell metacharacters are present - next if shell_metacharacters.any? { |meta| string_content(parameters(method)[index]).include?(meta) } - - next unless match = regex_match_group(parameters(method)[index], shell_cmd_with_spaces_regex) - - good_args = match[0].gsub(" ", "\", \"") - offending_node(parameters(method)[index]) - problem "Separate `Utils.#{command}` commands into `\"#{good_args}\"`" - end - end - end - - def autocorrect(node) - lambda do |corrector| - good_args = node.source.gsub(" ", "\", \"") - corrector.replace(node.source_range, good_args) - end - end - end - class Miscellaneous < FormulaCop def audit_formula(_node, _class_node, _parent_class_node, body_node) # FileUtils is included in Formula @@ -604,6 +555,55 @@ module RuboCop end end end + + class ShellCommands < FormulaCop + def audit_formula(_node, _class_node, _parent_class_node, body_node) + # Match shell commands separated by spaces in the same string + shell_cmd_with_spaces_regex = /[^"' ]*(?:\s[^"' ]*)+/ + + popen_commands = [ + :popen_read, + :safe_popen_read, + :popen_write, + :safe_popen_write, + ] + + shell_metacharacters = %w[> < < | ; : & * $ ? : ~ + @ !` ( ) [ ]] + + find_every_method_call_by_name(body_node, :system).each do |method| + # Only separate when no shell metacharacters are present + next if shell_metacharacters.any? { |meta| string_content(parameters(method).first).include?(meta) } + + next unless match = regex_match_group(parameters(method).first, shell_cmd_with_spaces_regex) + + good_args = match[0].gsub(" ", "\", \"") + offending_node(parameters(method).first) + problem "Separate `system` commands into `\"#{good_args}\"`" + end + + popen_commands.each do |command| + find_instance_method_call(body_node, "Utils", command) do |method| + index = parameters(method).first.hash_type? ? 1 : 0 + + # Only separate when no shell metacharacters are present + next if shell_metacharacters.any? { |meta| string_content(parameters(method)[index]).include?(meta) } + + next unless match = regex_match_group(parameters(method)[index], shell_cmd_with_spaces_regex) + + good_args = match[0].gsub(" ", "\", \"") + offending_node(parameters(method)[index]) + problem "Separate `Utils.#{command}` commands into `\"#{good_args}\"`" + end + end + end + + def autocorrect(node) + lambda do |corrector| + good_args = node.source.gsub(" ", "\", \"") + corrector.replace(node.source_range, good_args) + end + end + end end end end diff --git a/Library/Homebrew/test/rubocops/lines_spec.rb b/Library/Homebrew/test/rubocops/lines_spec.rb index 370bea30a9..95ad56c311 100644 --- a/Library/Homebrew/test/rubocops/lines_spec.rb +++ b/Library/Homebrew/test/rubocops/lines_spec.rb @@ -574,255 +574,6 @@ describe RuboCop::Cop::FormulaAudit::ShellVariables do end end -describe RuboCop::Cop::FormulaAudit::ShellCommands do - subject(:cop) { described_class.new } - - context "When auditing shell commands" do - it "system arguments should be separated" do - expect_offense(<<~RUBY) - class Foo < Formula - def install - system "foo bar" - ^^^^^^^^^ Separate `system` commands into `\"foo\", \"bar\"` - end - end - RUBY - end - - it "system arguments with string interpolation should be separated" do - expect_offense(<<~RUBY) - class Foo < Formula - def install - system "\#{bin}/foo bar" - ^^^^^^^^^^^^^^^^ Separate `system` commands into `\"\#{bin}/foo\", \"bar\"` - end - end - RUBY - end - - it "system arguments with metacharacters should not be separated" do - expect_no_offenses(<<~RUBY) - class Foo < Formula - def install - system "foo bar > baz" - end - end - RUBY - end - - it "only the first system argument should be separated" do - expect_no_offenses(<<~RUBY) - class Foo < Formula - def install - system "foo", "bar baz" - end - end - RUBY - end - - it "Utils.popen arguments should not be separated" do - expect_no_offenses(<<~RUBY) - class Foo < Formula - def install - Utils.popen("foo bar") - end - end - RUBY - end - - it "Utils.popen_read arguments should be separated" do - expect_offense(<<~RUBY) - class Foo < Formula - def install - Utils.popen_read("foo bar") - ^^^^^^^^^ Separate `Utils.popen_read` commands into `\"foo\", \"bar\"` - end - end - RUBY - end - - it "Utils.safe_popen_read arguments should be separated" do - expect_offense(<<~RUBY) - class Foo < Formula - def install - Utils.safe_popen_read("foo bar") - ^^^^^^^^^ Separate `Utils.safe_popen_read` commands into `\"foo\", \"bar\"` - end - end - RUBY - end - - it "Utils.popen_write arguments should be separated" do - expect_offense(<<~RUBY) - class Foo < Formula - def install - Utils.popen_write("foo bar") - ^^^^^^^^^ Separate `Utils.popen_write` commands into `\"foo\", \"bar\"` - end - end - RUBY - end - - it "Utils.safe_popen_write arguments should be separated" do - expect_offense(<<~RUBY) - class Foo < Formula - def install - Utils.safe_popen_write("foo bar") - ^^^^^^^^^ Separate `Utils.safe_popen_write` commands into `\"foo\", \"bar\"` - end - end - RUBY - end - - it "Utils.popen_read arguments with string interpolation should be separated" do - expect_offense(<<~RUBY) - class Foo < Formula - def install - Utils.popen_read("\#{bin}/foo bar") - ^^^^^^^^^^^^^^^^ Separate `Utils.popen_read` commands into `\"\#{bin}/foo\", \"bar\"` - end - end - RUBY - end - - it "Utils.popen_read arguments with metacharacters should not be separated" do - expect_no_offenses(<<~RUBY) - class Foo < Formula - def install - Utils.popen_read("foo bar > baz") - end - end - RUBY - end - - it "only the first Utils.popen_read argument should be separated" do - expect_no_offenses(<<~RUBY) - class Foo < Formula - def install - Utils.popen_read("foo", "bar baz") - end - end - RUBY - end - - it "Utils.popen_read arguments should be separated following a shell variable" do - expect_offense(<<~RUBY) - class Foo < Formula - def install - Utils.popen_read({ "SHELL" => "bash"}, "foo bar") - ^^^^^^^^^ Separate `Utils.popen_read` commands into `\"foo\", \"bar\"` - end - end - RUBY - end - - it "separates shell commands in system" do - source = <<~RUBY - class Foo < Formula - def install - system "foo bar" - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - system "foo", "bar" - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "separates shell commands with string interpolation in system" do - source = <<~RUBY - class Foo < Formula - def install - system "\#{foo}/bar baz" - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - system "\#{foo}/bar", "baz" - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "separates shell commands in Utils.popen_read" do - source = <<~RUBY - class Foo < Formula - def install - Utils.popen_read("foo bar") - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - Utils.popen_read("foo", "bar") - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "separates shell commands with string interpolation in Utils.popen_read" do - source = <<~RUBY - class Foo < Formula - def install - Utils.popen_read("\#{foo}/bar baz") - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - Utils.popen_read("\#{foo}/bar", "baz") - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - - it "separates shell commands following a shell variable in Utils.popen_read" do - source = <<~RUBY - class Foo < Formula - def install - Utils.popen_read({ "SHELL" => "bash" }, "foo bar") - end - end - RUBY - - corrected_source = <<~RUBY - class Foo < Formula - def install - Utils.popen_read({ "SHELL" => "bash" }, "foo", "bar") - end - end - RUBY - - new_source = autocorrect_source(source) - expect(new_source).to eq(corrected_source) - end - end -end - describe RuboCop::Cop::FormulaAudit::Miscellaneous do subject(:cop) { described_class.new } @@ -1345,3 +1096,252 @@ describe RuboCop::Cop::FormulaAuditStrict::MakeCheck do include_examples "formulae exist", described_class::MAKE_CHECK_ALLOWLIST end + +describe RuboCop::Cop::FormulaAuditStrict::ShellCommands do + subject(:cop) { described_class.new } + + context "When auditing shell commands" do + it "system arguments should be separated" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + system "foo bar" + ^^^^^^^^^ Separate `system` commands into `\"foo\", \"bar\"` + end + end + RUBY + end + + it "system arguments with string interpolation should be separated" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + system "\#{bin}/foo bar" + ^^^^^^^^^^^^^^^^ Separate `system` commands into `\"\#{bin}/foo\", \"bar\"` + end + end + RUBY + end + + it "system arguments with metacharacters should not be separated" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + def install + system "foo bar > baz" + end + end + RUBY + end + + it "only the first system argument should be separated" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + def install + system "foo", "bar baz" + end + end + RUBY + end + + it "Utils.popen arguments should not be separated" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + def install + Utils.popen("foo bar") + end + end + RUBY + end + + it "Utils.popen_read arguments should be separated" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.popen_read("foo bar") + ^^^^^^^^^ Separate `Utils.popen_read` commands into `\"foo\", \"bar\"` + end + end + RUBY + end + + it "Utils.safe_popen_read arguments should be separated" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.safe_popen_read("foo bar") + ^^^^^^^^^ Separate `Utils.safe_popen_read` commands into `\"foo\", \"bar\"` + end + end + RUBY + end + + it "Utils.popen_write arguments should be separated" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.popen_write("foo bar") + ^^^^^^^^^ Separate `Utils.popen_write` commands into `\"foo\", \"bar\"` + end + end + RUBY + end + + it "Utils.safe_popen_write arguments should be separated" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.safe_popen_write("foo bar") + ^^^^^^^^^ Separate `Utils.safe_popen_write` commands into `\"foo\", \"bar\"` + end + end + RUBY + end + + it "Utils.popen_read arguments with string interpolation should be separated" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.popen_read("\#{bin}/foo bar") + ^^^^^^^^^^^^^^^^ Separate `Utils.popen_read` commands into `\"\#{bin}/foo\", \"bar\"` + end + end + RUBY + end + + it "Utils.popen_read arguments with metacharacters should not be separated" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + def install + Utils.popen_read("foo bar > baz") + end + end + RUBY + end + + it "only the first Utils.popen_read argument should be separated" do + expect_no_offenses(<<~RUBY) + class Foo < Formula + def install + Utils.popen_read("foo", "bar baz") + end + end + RUBY + end + + it "Utils.popen_read arguments should be separated following a shell variable" do + expect_offense(<<~RUBY) + class Foo < Formula + def install + Utils.popen_read({ "SHELL" => "bash"}, "foo bar") + ^^^^^^^^^ Separate `Utils.popen_read` commands into `\"foo\", \"bar\"` + end + end + RUBY + end + + it "separates shell commands in system" do + source = <<~RUBY + class Foo < Formula + def install + system "foo bar" + end + end + RUBY + + corrected_source = <<~RUBY + class Foo < Formula + def install + system "foo", "bar" + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(corrected_source) + end + + it "separates shell commands with string interpolation in system" do + source = <<~RUBY + class Foo < Formula + def install + system "\#{foo}/bar baz" + end + end + RUBY + + corrected_source = <<~RUBY + class Foo < Formula + def install + system "\#{foo}/bar", "baz" + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(corrected_source) + end + + it "separates shell commands in Utils.popen_read" do + source = <<~RUBY + class Foo < Formula + def install + Utils.popen_read("foo bar") + end + end + RUBY + + corrected_source = <<~RUBY + class Foo < Formula + def install + Utils.popen_read("foo", "bar") + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(corrected_source) + end + + it "separates shell commands with string interpolation in Utils.popen_read" do + source = <<~RUBY + class Foo < Formula + def install + Utils.popen_read("\#{foo}/bar baz") + end + end + RUBY + + corrected_source = <<~RUBY + class Foo < Formula + def install + Utils.popen_read("\#{foo}/bar", "baz") + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(corrected_source) + end + + it "separates shell commands following a shell variable in Utils.popen_read" do + source = <<~RUBY + class Foo < Formula + def install + Utils.popen_read({ "SHELL" => "bash" }, "foo bar") + end + end + RUBY + + corrected_source = <<~RUBY + class Foo < Formula + def install + Utils.popen_read({ "SHELL" => "bash" }, "foo", "bar") + end + end + RUBY + + new_source = autocorrect_source(source) + expect(new_source).to eq(corrected_source) + end + end +end