Merge pull request #20030 from Homebrew/rubocop-messages

audits: quote and reword problem description text

Author: MikeMcQuaid

Library/Homebrew/formula_auditor.rb

@@ -114,7 +114,7 @@ def audit_file
                 ln -s #{formula.path.to_s.gsub(formula.tap.path, "..")} #{alias_name}
             EOS
           else
-            problem "Formula has other versions so create an alias named #{alias_name}."
+            problem "Formula has other versions so create an alias named '#{alias_name}'."
           end
         end
 
@@ -153,7 +153,7 @@ def audit_synced_versions_formulae
           next if synced_formula == name
 
           if (synced_version = Formulary.factory(synced_formula).version) != version
-            problem "Version of `#{synced_formula}` (#{synced_version}) should match version of `#{name}` (#{version})"
+            problem "Version of #{synced_formula} (#{synced_version}) should match version of #{name} (#{version})"
           end
         end
 
@@ -188,7 +188,7 @@ def audit_name
       return if formula.core_formula?
       return unless Formula.core_names.include?(name)
 
-      problem "Formula name conflicts with existing core formula."
+      problem "Formula name conflicts with an existing formula in homebrew/core."
     end
 
     PERMITTED_LICENSE_MISMATCHES = {
@@ -226,7 +226,7 @@ def audit_license
           problem <<~EOS
             Formula #{formula.name} contains incompatible licenses: #{incompatible_licenses}.
             Formulae in homebrew/core must either use a Debian Free Software Guidelines license
-            or be released into the public domain. See #{Formatter.url("https://docs.brew.sh/License-Guidelines")}
+            or be released into the public domain: #{Formatter.url("https://docs.brew.sh/License-Guidelines")}
           EOS
         end
 
@@ -327,7 +327,7 @@ def audit_deps
               end
             end
 
-            problem "Dependency '#{dep}' does not define option #{opt.name.inspect}"
+            problem "Dependency '#{dep}' does not define option: #{opt.name.inspect}"
           end
 
           problem "Don't use 'git' as a dependency (it's always available)" if @new_formula && dep.name == "git"
@@ -448,7 +448,7 @@ def audit_conflicts
 
         if T.must(tap).formula_renames.key?(conflict.name) || T.must(tap).aliases.include?(conflict.name)
           problem "Formula conflict should be declared using " \
-                  "canonical name (#{conflicting_formula.name}) instead of #{conflict.name}"
+                  "canonical name (#{conflicting_formula.name}) instead of '#{conflict.name}'"
         end
 
         reverse_conflict_found = T.let(false, T::Boolean)
@@ -457,7 +457,7 @@ def audit_conflicts
           if T.must(tap).formula_renames.key?(reverse_conflict.name) ||
              T.must(tap).aliases.include?(reverse_conflict.name)
             problem "Formula #{conflicting_formula.name} conflict should be declared using " \
-                    "canonical name (#{reverse_conflict_formula.name}) instead of #{reverse_conflict.name}"
+                    "canonical name (#{reverse_conflict_formula.name}) instead of '#{reverse_conflict.name}'"
           end
 
           reverse_conflict_found ||= reverse_conflict_formula == formula
@@ -651,7 +651,7 @@ def audit_github_repository_archived
       metadata = SharedAudits.github_repo_data(user, repo)
       return if metadata.nil?
 
-      problem "GitHub repo is archived" if metadata["archived"]
+      problem "GitHub repository is archived" if metadata["archived"]
     end
 
     def audit_gitlab_repository_archived
@@ -663,7 +663,7 @@ def audit_gitlab_repository_archived
       metadata = SharedAudits.gitlab_repo_data(user, repo)
       return if metadata.nil?
 
-      problem "GitLab repo is archived" if metadata["archived"]
+      problem "GitLab repository is archived" if metadata["archived"]
     end
 
     def audit_github_repository
@@ -712,7 +712,7 @@ def get_repo_data(regex)
     end
 
     def audit_specs
-      problem "Head-only (no stable download)" if head_only?(formula)
+      problem "HEAD-only (no stable download)" if head_only?(formula)
 
       %w[Stable HEAD].each do |name|
         spec_name = name.downcase.to_sym
@@ -759,7 +759,7 @@ def audit_specs
 
       if formula.head && @versioned_formula &&
          !formula.tap&.audit_exception(:versioned_head_spec_allowlist, formula.name)
-        problem "Versioned formulae should not have a `HEAD` spec"
+        problem "Versioned formulae should not have a `head` spec"
       end
 
       stable = formula.stable
@@ -771,7 +771,7 @@ def audit_specs
 
       stable_version_string = version.to_s
       if stable_version_string.start_with?("HEAD")
-        problem "Stable: non-HEAD version name (#{stable_version_string}) should not begin with HEAD"
+        problem "Stable: non-HEAD version (#{stable_version_string}) should not begin with `HEAD`"
       end
 
       stable_url_version = Version.parse(stable.url)
@@ -790,7 +790,7 @@ def audit_specs
         return if formula.tap&.audit_exception :unstable_allowlist, formula.name, version_prefix
         return if formula.tap&.audit_exception :unstable_devel_allowlist, formula.name, version_prefix
 
-        problem "Stable version URLs should not contain #{matched}"
+        problem "Stable: version URLs should not contain `#{matched}`"
       when %r{download\.gnome\.org/sources}, %r{ftp\.gnome\.org/pub/GNOME/sources}i
         version_prefix = stable.version.major_minor
         return if formula.tap&.audit_exception :gnome_devel_allowlist, formula.name, version_prefix
@@ -800,11 +800,11 @@ def audit_specs
         return if stable_url_version >= Version.new("40.0")
         return if stable_url_minor_version.even?
 
-        problem "#{stable.version} is a development release"
+        problem "Stable: version (#{stable.version}) is a development release"
       when %r{isc.org/isc/bind\d*/}i
         return if stable_url_minor_version.even?
 
-        problem "#{stable.version} is a development release"
+        problem "Stable: version (#{stable.version}) is a development release"
 
       when %r{https?://gitlab\.com/([\w-]+)/([\w-]+)}
         owner = T.must(Regexp.last_match(1))
@@ -845,7 +845,7 @@ def audit_stable_version
       if !newest_committed[:version].nil? &&
          current_version < newest_committed[:version] &&
          current_version_scheme == previous_committed[:version_scheme]
-        problem "stable version should not decrease (from #{newest_committed[:version]} to #{current_version})"
+        problem "Stable: version should not decrease (from #{newest_committed[:version]} to #{current_version})"
       end
     end
 
@@ -867,14 +867,14 @@ def audit_revision
          !current_revision.zero? &&
          current_revision == newest_committed[:revision] &&
          current_revision == previous_committed[:revision]
-        problem "'revision #{current_revision}' should be removed"
+        problem "`revision #{current_revision}` should be removed"
       elsif current_version == previous_committed[:version] &&
             !previous_committed[:revision].nil? &&
             current_revision < previous_committed[:revision]
-        problem "revision should not decrease (from #{previous_committed[:revision]} to #{current_revision})"
+        problem "`revision` should not decrease (from #{previous_committed[:revision]} to #{current_revision})"
       elsif newest_committed[:revision] &&
             current_revision > (newest_committed[:revision] + 1)
-        problem "revisions should only increment by 1"
+        problem "`revision` should only increment by 1"
       end
     end
 
@@ -891,10 +891,10 @@ def audit_version_scheme
       return if previous_committed[:version_scheme].nil?
 
       if current_version_scheme < previous_committed[:version_scheme]
-        problem "version_scheme should not decrease (from #{previous_committed[:version_scheme]} " \
+        problem "`version_scheme` should not decrease (from #{previous_committed[:version_scheme]} " \
                 "to #{current_version_scheme})"
       elsif current_version_scheme > (previous_committed[:version_scheme] + 1)
-        problem "version_schemes should only increment by 1"
+        problem "`version_scheme` should only increment by 1"
       end
     end
 
@@ -935,7 +935,7 @@ def audit_text
       bin_names.each do |name|
         shell_commands.each do |cmd|
           if text.to_s.match?(/test do.*#{cmd}[(\s]+['"]#{Regexp.escape(name)}[\s'"]/m)
-            problem %Q(fully scope test #{cmd} calls, e.g. #{cmd} "\#{bin}/#{name}")
+            problem %Q(Fully scope test `#{cmd}` calls, e.g.: #{cmd} "\#{bin}/#{name}")
           end
         end
       end

Library/Homebrew/resource_auditor.rb

@@ -63,30 +63,30 @@ def audit_download_strategy
       url_strategy = DownloadStrategyDetector.detect(url)
 
       if (using == :git || url_strategy == GitDownloadStrategy) && specs[:tag] && !specs[:revision]
-        problem "Git should specify :revision when a :tag is specified."
+        problem "Git should specify `revision:` when a `tag:` is specified."
       end
 
       return unless using
 
       if using == :cvs
         mod = specs[:module]
 
-        problem "Redundant :module value in URL" if mod == name
+        problem "Redundant `module:` value in URL" if mod == name
 
         if url.match?(%r{:[^/]+$})
           mod = url.split(":").last
 
           if mod == name
             problem "Redundant CVS module appended to URL"
           else
-            problem "Specify CVS module as `:module => \"#{mod}\"` instead of appending it to the URL"
+            problem "Specify CVS module as `module: \"#{mod}\"` instead of appending it to the URL"
           end
         end
       end
 
       return if url_strategy != DownloadStrategyDetector.detect("", using)
 
-      problem "Redundant :using value in URL"
+      problem "Redundant `using:` value in URL"
     end
 
     def audit_checksum
@@ -125,7 +125,7 @@ def audit_resource_name_matches_pypi_package_name_in_url
 
       return if name.casecmp(pypi_package_name).zero?
 
-      problem "resource name should be `#{pypi_package_name}` to match the PyPI package name"
+      problem "`resource` name should be '#{pypi_package_name}' to match the PyPI package name"
     end
 
     def audit_urls
@@ -166,12 +166,12 @@ def audit_urls
             remote_exists = Utils::Git.remote_exists?(url)
             attempts += 1
           end
-          problem "The URL #{url} is not a valid git URL" unless remote_exists
+          problem "The URL #{url} is not a valid Git URL" unless remote_exists
         elsif strategy <= SubversionDownloadStrategy
           next unless DevelopmentTools.subversion_handles_most_https_certificates?
           next unless Utils::Svn.available?
 
-          problem "The URL #{url} is not a valid svn URL" unless Utils::Svn.remote_exists? url
+          problem "The URL #{url} is not a valid SVN URL" unless Utils::Svn.remote_exists? url
         end
       end
     end
@@ -188,7 +188,7 @@ def audit_head_branch
                     .match(%r{ref: refs/heads/(.*?)\s+HEAD})&.to_a&.second
       return if branch.blank? || branch == specs[:branch]
 
-      problem "Use `branch: \"#{branch}\"` to specify the default branch"
+      problem "Specify the default branch as `branch: \"#{branch}\"`"
     end
 
     def problem(text)

Library/Homebrew/rubocops/caveats.rb

@@ -30,7 +30,7 @@ class Caveats < FormulaCop
         def audit_formula(_formula_nodes)
           caveats_strings.each do |n|
             if regex_match_group(n, /\bsetuid\b/i)
-              problem "Don't recommend `setuid` in the caveats, suggest `sudo` instead."
+              problem "Instead of recommending `setuid` in the caveats, suggest `sudo`."
             end
 
             problem "Don't use ANSI escape codes in the caveats." if regex_match_group(n, /\e/)

Library/Homebrew/rubocops/checksum.rb

@@ -28,17 +28,17 @@ def audit_sha256(checksum)
           return if checksum.nil?
 
           if regex_match_group(checksum, /^$/)
-            problem "sha256 is empty"
+            problem "`sha256` is empty"
             return
           end
 
           if string_content(checksum).size != 64 && regex_match_group(checksum, /^\w*$/)
-            problem "sha256 should be 64 characters"
+            problem "`sha256` should be 64 characters"
           end
 
           return unless regex_match_group(checksum, /[^a-f0-9]+/i)
 
-          add_offense(T.must(@offensive_source_range), message: "sha256 contains invalid characters")
+          add_offense(T.must(@offensive_source_range), message: "`sha256` contains invalid characters")
         end
       end
 
@@ -54,7 +54,7 @@ def audit_formula(formula_nodes)
             next if checksum.nil?
             next unless regex_match_group(checksum, /[A-F]+/)
 
-            add_offense(@offensive_source_range, message: "sha256 should be lowercase") do |corrector|
+            add_offense(@offensive_source_range, message: "`sha256` should be lowercase") do |corrector|
               correction = T.must(@offensive_node).source.downcase
               corrector.insert_before(T.must(@offensive_node).source_range, correction)
               corrector.remove(T.must(@offensive_node).source_range)

Library/Homebrew/rubocops/class.rb

@@ -23,7 +23,7 @@ def audit_formula(formula_nodes)
           parent_class = class_name(parent_class_node)
           return unless DEPRECATED_CLASSES.include?(parent_class)
 
-          problem "#{parent_class} is deprecated, use Formula instead" do |corrector|
+          problem "`#{parent_class}` is deprecated, use `Formula` instead" do |corrector|
             corrector.replace(parent_class_node.source_range, "Formula")
           end
         end
@@ -49,14 +49,14 @@ def audit_formula(formula_nodes)
             p1, p2 = params
             if (match = string_content(p1).match(%r{(/usr/local/(s?bin))}))
               offending_node(p1)
-              problem "use \#{#{match[2]}} instead of #{match[1]} in #{node}" do |corrector|
+              problem "Use `\#{#{match[2]}}` instead of `#{match[1]}` in `#{node}`" do |corrector|
                 corrector.replace(p1.source_range, p1.source.sub(match[1], "\#{#{match[2]}}"))
               end
             end
 
             if node == :shell_output && node_equals?(p2, 0)
               offending_node(p2)
-              problem "Passing 0 to shell_output() is redundant" do |corrector|
+              problem "Passing 0 to `shell_output` is redundant" do |corrector|
                 corrector.remove(range_with_surrounding_comma(range_with_surrounding_space(range: p2.source_range,
                                                                                            side:  :left)))
               end

Library/Homebrew/rubocops/components_order.rb

@@ -39,7 +39,7 @@ def audit_formula(formula_nodes)
 
             if on_method_blocks.length > 1
               @offensive_node = on_method_blocks.second
-              problem "there can only be one `#{on_method}` block in a formula."
+              problem "There can only be one `#{on_method}` block in a formula."
             end
 
             check_on_system_block_content(component_precedence_list, on_method_blocks.first)
@@ -112,7 +112,7 @@ def audit_formula(formula_nodes)
 
             on_system_blocks.each do |on_method, blocks|
               if blocks.length > 1
-                problem "there can only be one `#{on_method}` block in a resource block."
+                problem "There can only be one `#{on_method}` block in a resource block."
                 next
               end
             end

Library/Homebrew/rubocops/components_redundancy.rb

@@ -32,7 +32,7 @@ def audit_formula(formula_nodes)
 
               url_args = arg.keys.each.map(&:value)
               if method_called?(body_node, :sha256) && url_args.include?(:tag) && url_args.include?(:revision)
-                problem "Do not use both sha256 and tag/revision."
+                problem "Do not use both `sha256` and `tag:`/`revision:`."
               end
             end
           end

Library/Homebrew/rubocops/dependency_order.rb

@@ -108,8 +108,8 @@ def verify_order_in_source(ordered)
 
             offending_node(node_1)
 
-            problem "dependency \"#{dependency_name(node_1)}\" (line #{l1}) should be put before dependency " \
-                    "\"#{dependency_name(node_2)}\" (line #{l2})" do |corrector|
+            problem "`dependency \"#{dependency_name(node_1)}\"` (line #{l1}) should be put before " \
+                    "`dependency \"#{dependency_name(node_2)}\"` (line #{l2})" do |corrector|
               indentation = " " * (start_column(node_2) - line_start_column(node_2))
               line_breaks = "\n"
               corrector.insert_before(node_2.source_range,