Merge pull request #3610 from GauthamGoli/arg-parser

cli : Add basic arg parser and use it for parsing `brew audit` args

Library/Homebrew/cli_parser.rb

@@ -0,0 +1,55 @@
+require "optparse"
+require "ostruct"
+
+module Homebrew
+  module CLI
+    class Parser
+      def initialize(&block)
+        @parser = OptionParser.new
+        @parsed_args = OpenStruct.new
+        instance_eval(&block)
+      end
+
+      def switch(*names, description: nil)
+        description = option_to_description(*names) if description.nil?
+        @parser.on(*names, description) do
+          names.each do |name|
+            @parsed_args["#{option_to_name(name)}?"] = true
+          end
+        end
+      end
+
+      def comma_array(name, description: nil)
+        description = option_to_description(name) if description.nil?
+        @parser.on(name, OptionParser::REQUIRED_ARGUMENT, Array, description) do |list|
+          @parsed_args[option_to_name(name)] = list
+        end
+      end
+
+      def flag(name, description: nil, required: false)
+        if required
+          option_required = OptionParser::REQUIRED_ARGUMENT
+        else
+          option_required = OptionParser::OPTIONAL_ARGUMENT
+        end
+        description = option_to_description(name) if description.nil?
+        @parser.on(name, description, option_required) do |option_value|
+          @parsed_args[option_to_name(name)] = option_value
+        end
+      end
+
+      def option_to_name(name)
+        name.sub(/\A--?/, "").tr("-", "_")
+      end
+
+      def option_to_description(*names)
+        names.map { |name| name.sub(/\A--?/, "").tr("-", " ") }.sort.last
+      end
+
+      def parse(cmdline_args = ARGV)
+        @parser.parse!(cmdline_args)
+        @parsed_args
+      end
+    end
+  end
+end

Library/Homebrew/dev-cmd/audit.rb

@@ -49,20 +49,34 @@ require "cmd/style"
 require "date"
 require "missing_formula"
 require "digest"
+require "cli_parser"
 
 module Homebrew
   module_function
 
   def audit
-    inject_dump_stats!(FormulaAuditor, /^audit_/) if ARGV.switch? "D"
+    args = Homebrew::CLI::Parser.new do
+      switch      "--strict"
+      switch      "--online"
+      switch      "--new-formula"
+      switch      "--fix"
+      switch      "--display-cop-names"
+      switch      "--display-filename"
+      switch      "-D", "--audit-debug", description: "Activates debugging and profiling"
+      comma_array "--only"
+      comma_array "--except"
+      comma_array "--only-cops"
+      comma_array "--except-cops"
+    end.parse
+
     Homebrew.auditing = true
+    inject_dump_stats!(FormulaAuditor, /^audit_/) if args.audit_debug?
 
     formula_count = 0
     problem_count = 0
-
+    new_formula = args.new_formula?
-    new_formula = ARGV.include? "--new-formula"
+    strict = new_formula || args.strict?
-    strict = new_formula || ARGV.include?("--strict")
+    online = new_formula || args.online?
-    online = new_formula || ARGV.include?("--online")
 
     ENV.activate_extensions!
     ENV.setup_build_environment
@@ -75,35 +89,36 @@ module Homebrew
       files = ARGV.resolved_formulae.map(&:path)
     end
 
-    only_cops = ARGV.value("only-cops").to_s.split(",")
+    only_cops = args.only_cops
-    except_cops = ARGV.value("except-cops").to_s.split(",")
+    except_cops = args.except_cops
 
-    if !only_cops.empty? && !except_cops.empty?
+    if only_cops && except_cops
       odie "--only-cops and --except-cops cannot be used simultaneously!"
-    elsif (!only_cops.empty? || !except_cops.empty?) && (strict || ARGV.value("only"))
+    elsif (only_cops || except_cops) && (strict || args.only)
       odie "--only-cops/--except-cops and --strict/--only cannot be used simultaneously"
     end
 
-    options = { fix: ARGV.flag?("--fix"), realpath: true }
+    options = { fix: args.fix?, realpath: true }
 
-    if !only_cops.empty?
+    if only_cops
       options[:only_cops] = only_cops
-      ARGV.push("--only=style")
+      args.only = ["style"]
-    elsif new_formula
+    elsif args.new_formula?
       nil
     elsif strict
       options[:except_cops] = [:NewFormulaAudit]
-    elsif !except_cops.empty?
+    elsif except_cops
       options[:except_cops] = except_cops
     elsif !strict
       options[:only_cops] = [:FormulaAudit]
     end
 
+    options[:display_cop_names] = args.display_cop_names?
     # Check style in a single batch run up front for performance
     style_results = check_style_json(files, options)
 
     ff.sort.each do |f|
-      options = { new_formula: new_formula, strict: strict, online: online }
+      options = { new_formula: new_formula, strict: strict, online: online, only: args.only, except: args.except }
       options[:style_offenses] = style_results.file_offenses(f.path)
       fa = FormulaAuditor.new(f, options)
       fa.audit
@@ -113,7 +128,7 @@ module Homebrew
       formula_count += 1
       problem_count += fa.problems.size
       problem_lines = fa.problems.map { |p| "* #{p.chomp.gsub("\n", "\n    ")}" }
-      if ARGV.include? "--display-filename"
+      if args.display_filename?
         puts problem_lines.map { |s| "#{f.path}: #{s}" }
       else
         puts "#{f.full_name}:", problem_lines.map { |s| "  #{s}" }
@@ -177,6 +192,9 @@ class FormulaAuditor
     @new_formula = options[:new_formula]
     @strict = options[:strict]
     @online = options[:online]
+    @display_cop_names = options[:display_cop_names]
+    @only = options[:only]
+    @except = options[:except]
     # Accept precomputed style offense results, for efficiency
     @style_offenses = options[:style_offenses]
     # Allow the actual official-ness of a formula to be overridden, for testing purposes
@@ -188,9 +206,8 @@ class FormulaAuditor
 
   def audit_style
     return unless @style_offenses
-    display_cop_names = ARGV.include?("--display-cop-names")
     @style_offenses.each do |offense|
-      problem offense.to_s(display_cop_name: display_cop_names)
+      problem offense.to_s(display_cop_name: @display_cop_names)
     end
   end
 
@@ -767,17 +784,17 @@ class FormulaAuditor
   end
 
   def audit
-    only_audits = ARGV.value("only").to_s.split(",")
+    only_audits = @only
-    except_audits = ARGV.value("except").to_s.split(",")
+    except_audits = @except
-    if !only_audits.empty? && !except_audits.empty?
+    if only_audits && except_audits
       odie "--only and --except cannot be used simultaneously!"
     end
 
     methods.map(&:to_s).grep(/^audit_/).each do |audit_method_name|
       name = audit_method_name.gsub(/^audit_/, "")
-      if !only_audits.empty?
+      if only_audits
         next unless only_audits.include?(name)
-      elsif !except_audits.empty?
+      elsif except_audits
         next if except_audits.include?(name)
       end
       send(audit_method_name)

Library/Homebrew/test/cli_parser_spec.rb

@@ -0,0 +1,60 @@
+require_relative "../cli_parser"
+
+describe Homebrew::CLI::Parser do
+  describe "test switch options" do
+    subject(:parser) {
+      described_class.new do
+        switch "-v", "--verbose", description: "Flag for verbosity"
+        switch "--more-verbose",  description: "Flag for higher verbosity"
+      end
+    }
+
+    it "parses short option" do
+      args = parser.parse(["-v"])
+      expect(args).to be_verbose
+    end
+
+    it "parses a single valid option" do
+      args = parser.parse(["--verbose"])
+      expect(args).to be_verbose
+    end
+
+    it "parses a valid option along with few unnamed args" do
+      args = %w[--verbose unnamed args]
+      parser.parse(args)
+      expect(args).to eq %w[unnamed args]
+    end
+
+    it "parses a single option and checks other options to be nil" do
+      args = parser.parse(["--verbose"])
+      expect(args).to be_verbose
+      expect(args.more_verbose?).to be nil
+    end
+
+    it "raises an exception when an invalid option is passed" do
+      expect { parser.parse(["--random"]) }.to raise_error(OptionParser::InvalidOption, /--random/)
+    end
+  end
+
+  describe "test long flag options" do
+    subject(:parser) {
+      described_class.new do
+        flag        "--filename", description: "Name of the file", required: true
+        comma_array "--files",    description: "Comma separated filenames"
+      end
+    }
+    it "parses a long flag option with its argument" do
+      args = parser.parse(["--filename=random.txt"])
+      expect(args.filename).to eq "random.txt"
+    end
+
+    it "raises an exception when a flag's required arg is not passed" do
+      expect { parser.parse(["--filename"]) }.to raise_error(OptionParser::MissingArgument, /--filename/)
+    end
+
+    it "parses a comma array flag option" do
+      args = parser.parse(["--files=random1.txt,random2.txt"])
+      expect(args.files).to eq %w[random1.txt random2.txt]
+    end
+  end
+end