Add custom cops to Chefstyle

.gitignore

@@ -7,3 +7,4 @@
 /pkg/
 /spec/reports/
 /tmp/
+/test.rb # used for quick testing of cops

Rakefile

@@ -12,7 +12,7 @@ task :vendor do
   cp(src.join("default.yml"), dst.join("upstream.yml"))
 
   require "rubocop"
-  require "yaml"
+  require "yaml" unless defined?(YAML)
   cfg = RuboCop::Cop::Cop.all.inject({}) { |acc, cop| acc[cop.cop_name] = { "Enabled" => false }; acc }
   File.open(dst.join("disable_all.yml"), "w") { |fh| fh.write cfg.to_yaml }
 
@@ -27,7 +27,7 @@ RuboCop::RakeTask.new(:style) do |task|
 end
 
 begin
-  require "yard"
+  require "yard" unless defined?(YARD)
   YARD::Rake::YardocTask.new(:docs)
 rescue LoadError
   puts "yard is not available. bundle install first to make sure all dependencies are installed."

chefstyle.gemspec

@@ -21,5 +21,6 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency "bundler"
   spec.add_development_dependency "rake", ">= 12.0"
   spec.add_development_dependency "rspec"
+  spec.add_development_dependency "pry"
   spec.add_dependency("rubocop", Chefstyle::RUBOCOP_VERSION)
 end

config/chefstyle.yml

@@ -631,4 +631,18 @@ Style/CommentedKeyword:
 
 # make sure we catch this Ruby 3.0 breaking change now
 Lint/DeprecatedOpenSSLConstant:
-  Enabled: true
+  Enabled: true
+
+ChefRuby/Ruby27KeywordArgumentWarnings:
+  Description: Pass options to shell_out helpers without the brackets to avoid Ruby 2.7 deprecation warnings.
+  Enabled: true
+  VersionAdded: '1.3.0'
+
+ChefRuby/UnlessDefinedRequire:
+  Description: Workaround rubygems slow requires by only running require if the class isn't already defined
+  Enabled: true
+  VersionAdded: '1.3.0'
+  Exclude:
+    - '**/spec/**/*.rb'
+    - '**/omnibus/**/*.rb'
+    - '**/tasks/*.rb'

lib/chefstyle.rb

@@ -22,9 +22,17 @@ class ConfigLoader
 # Chefstyle patches the RuboCop tool to set a new default configuration that
 # is vendored in the Chefstyle codebase.
 module Chefstyle
-  # @return [String] the absolute path to the main RuboCop configuration YAML
-  #   file
+  # @return [String] the absolute path to the main RuboCop configuration YAML file
   def self.config
     RuboCop::ConfigLoader::DEFAULT_FILE
   end
 end
+
+require_relative "rubocop/chef"
+
+# Chef custom cops
+Dir.glob(File.dirname(__FILE__) + "/rubocop/cop/chef/**/*.rb") do |file|
+  next if File.directory?(file)
+
+  require_relative file # not actually relative but require_relative is faster
+end

lib/rubocop/chef.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+module RuboCop
+  # RuboCop Chef project namespace
+  module Chef
+    PROJECT_ROOT   = Pathname.new(__dir__).parent.parent.expand_path.freeze
+    CONFIG_DEFAULT = PROJECT_ROOT.join("config", "chefstyle.yml").freeze
+    CONFIG         = YAML.load(CONFIG_DEFAULT.read).freeze
+
+    private_constant(*constants(false))
+  end
+end

lib/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings.rb

@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+#
+# Copyright:: 2020, Chef Software, Inc.
+# Author:: Tim Smith (<tsmith@chef.io>)
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+module RuboCop
+  module Cop
+    module Chef
+      module ChefRuby
+        # Pass options to shell_out helpers without the brackets to avoid Ruby 2.7 deprecation warnings.
+        #
+        # @example
+        #
+        #   # bad
+        #   shell_out!('hostnamectl status', { returns: [0, 1] })
+        #   shell_out('hostnamectl status', { returns: [0, 1] })
+        #
+        #   # good
+        #   shell_out!('hostnamectl status', returns: [0, 1])
+        #   shell_out('hostnamectl status', returns: [0, 1])
+        #
+        class Ruby27KeywordArgumentWarnings < Cop
+          MSG = "Pass options to shell_out helpers without the brackets to avoid Ruby 2.7 deprecation warnings."
+
+          def_node_matcher :positional_shellout?, <<-PATTERN
+            (send nil? {:shell_out :shell_out!} ... $(hash ... ))
+          PATTERN
+
+          def on_send(node)
+            positional_shellout?(node) do |h|
+              add_offense(h, location: :expression, message: MSG, severity: :refactor) if h.braces?
+            end
+          end
+
+          def autocorrect(node)
+            lambda do |corrector|
+              # @todo when we drop ruby 2.4 support we can convert this to just delete_prefix delete_suffix
+              corrector.replace(node.loc.expression, node.loc.expression.source.gsub(/^{/, "").gsub(/}$/, ""))
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/chef/ruby/unless_defined_require.rb

@@ -0,0 +1,119 @@
+# frozen_string_literal: true
+#
+# Copyright:: Chef Software, Inc.
+# Author:: Tim Smith (<tsmith@chef.io>)
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+module RuboCop
+  module Cop
+    module Chef
+      module ChefRuby
+        # Rubygems is VERY slow to require gems even if they've already been loaded. To work around this
+        # wrap your require statement with an `if defined?()` check.
+        #
+        class UnlessDefinedRequire < Base
+          extend RuboCop::Cop::AutoCorrector
+
+          MSG = "Workaround rubygems slow requires by only running require if the class isn't already defined"
+
+          REQUIRE_TO_CLASS = {
+            "addressable/uri" => "Addressable::URI",
+            "appscript" => "Appscript",
+            "base64" => "Base64",
+            "benchmark" => "Benchmark",
+            "cgi" => "CGI",
+            "chef-utils" => "ChefUtils",
+            "csv" => "CSV",
+            "digest" => "Digest",
+            "digest/md5" => "Digest::MD5",
+            "digest/sha1" => "Digest::SHA1",
+            "digest/sha2" => "Digest::SHA2",
+            "droplet_kit" => "DropletKit",
+            "erb" => "Erb",
+            "erubis" => "Erubis",
+            "etc" => "Etc",
+            "excon" => "Excon",
+            "ffi_yajl" => "FFI_Yajl",
+            "ffi" => "FFI",
+            "fileutils" => "FileUtils",
+            "find" => "Find.find",
+            "forwardable" => "Forwardable",
+            "ipaddr" => "IPAddr",
+            "json" => "JSON",
+            "mime/types" => "MIME::Types",
+            "mixlib/archive" => "Mixlib::Archive",
+            "mixlib/cli" => "Mixlib::CLI",
+            "mixlib/config" => "Mixlib::Config",
+            "mixlib/shellout" => "Mixlib::ShellOut",
+            "multi_json" => "MultiJson",
+            "net/http" => "Net::HTTP",
+            "net/ssh" => "Net::SSH",
+            "netaddr" => "NetAddr",
+            "ohai" => "Ohai::System",
+            "open-uri" => "OpenURI",
+            "openssl" => "OpenSSL",
+            "optparse" => "OptionParser",
+            "ostruct" => "OpenStruct",
+            "pathname" => "Pathname",
+            "pp" => "PP",
+            "rack" => "Rack",
+            "rbconfig" => "RbConfig",
+            "retryable" => "Retryable",
+            "rexml/document" => "REXML::Document",
+            "rubygems" => "Gem",
+            "rubygems/package" => "Gem::Package",
+            "securerandom" => "SecureRandom",
+            "set" => "Set",
+            "shellwords" => "Shellwords",
+            "singleton" => "Singleton",
+            "socket" => "Socket",
+            "sslshake" => "SSLShake",
+            "stringio" => "StringIO",
+            "tempfile" => "Tempfile",
+            "thor" => "Thor",
+            "time" => "Time",
+            "timeout" => "Timeout",
+            "tmpdir" => "Dir.mktmpdir",
+            "tomlrb" => "Tomlrb",
+            "uri" => "URI",
+            "webrick" => "WEBrick",
+            "win32/registry" => "Win32::Registry",
+            "win32ole" => "WIN32OLE",
+            "winrm" => "WinRM::Connection",
+            "yaml" => "YAML",
+            "yard" => "YARD",
+            "zip" => "Zip",
+            "zlib" => "Zlib",
+          }.freeze
+
+          def_node_matcher :require?, <<-PATTERN
+            (send nil? :require (str $_) )
+          PATTERN
+
+          def on_send(node)
+            require?(node) do |r|
+              next if node.parent && node.parent.conditional? # catch both if and unless
+              next unless REQUIRE_TO_CLASS[r]
+
+              add_offense(node.loc.expression, message: MSG, severity: :refactor) do |corrector|
+                corrector.replace(node.loc.expression, "#{node.source} unless defined?(#{REQUIRE_TO_CLASS[r]})")
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+end