From a8bc18c6ae507717df18f5d559fe9f064c3ec3f5 Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Thu, 20 Aug 2020 08:35:33 -0700 Subject: [PATCH 01/11] Add custom cops to Chefstyle There are many things we need to do project wide that are a lot easier to accomplish with rubocop cops. This copies of the scaffolding to do that from Cookstyle and add the shellout cop from cookstyle as well as a new cop. Signed-off-by: Tim Smith --- .gitignore | 1 + Rakefile | 4 +- chefstyle.gemspec | 1 + config/chefstyle.yml | 16 ++- lib/chefstyle.rb | 12 +- lib/rubocop/chef.rb | 11 ++ .../ruby/ruby_27_keyword_argument_warnings.rb | 58 +++++++++ .../cop/chef/ruby/unless_defined_require.rb | 119 ++++++++++++++++++ 8 files changed, 217 insertions(+), 5 deletions(-) create mode 100755 lib/rubocop/chef.rb create mode 100644 lib/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings.rb create mode 100644 lib/rubocop/cop/chef/ruby/unless_defined_require.rb diff --git a/.gitignore b/.gitignore index 0cb6eeb..3458d42 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ /pkg/ /spec/reports/ /tmp/ +/test.rb # used for quick testing of cops \ No newline at end of file diff --git a/Rakefile b/Rakefile index f409242..2916741 100644 --- a/Rakefile +++ b/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." diff --git a/chefstyle.gemspec b/chefstyle.gemspec index 082a45d..f229346 100644 --- a/chefstyle.gemspec +++ b/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 diff --git a/config/chefstyle.yml b/config/chefstyle.yml index 6377f1e..ef05f05 100644 --- a/config/chefstyle.yml +++ b/config/chefstyle.yml @@ -631,4 +631,18 @@ Style/CommentedKeyword: # make sure we catch this Ruby 3.0 breaking change now Lint/DeprecatedOpenSSLConstant: - Enabled: true \ No newline at end of file + 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' \ No newline at end of file diff --git a/lib/chefstyle.rb b/lib/chefstyle.rb index fa6dd53..be32937 100644 --- a/lib/chefstyle.rb +++ b/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 diff --git a/lib/rubocop/chef.rb b/lib/rubocop/chef.rb new file mode 100755 index 0000000..0d8db2c --- /dev/null +++ b/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 diff --git a/lib/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings.rb b/lib/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings.rb new file mode 100644 index 0000000..48e086e --- /dev/null +++ b/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 () +# +# 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 diff --git a/lib/rubocop/cop/chef/ruby/unless_defined_require.rb b/lib/rubocop/cop/chef/ruby/unless_defined_require.rb new file mode 100644 index 0000000..f67644f --- /dev/null +++ b/lib/rubocop/cop/chef/ruby/unless_defined_require.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true +# +# Copyright:: Chef Software, Inc. +# Author:: Tim Smith () +# +# 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 From cbc5a09692691fad7101fcf56a55a9828cf54172 Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Thu, 20 Aug 2020 10:45:41 -0700 Subject: [PATCH 02/11] Update config/chefstyle.yml Signed-off-by: Tim Smith Co-authored-by: pete higgins --- config/chefstyle.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/chefstyle.yml b/config/chefstyle.yml index ef05f05..f7c5a37 100644 --- a/config/chefstyle.yml +++ b/config/chefstyle.yml @@ -639,10 +639,10 @@ ChefRuby/Ruby27KeywordArgumentWarnings: VersionAdded: '1.3.0' ChefRuby/UnlessDefinedRequire: - Description: Workaround rubygems slow requires by only running require if the class isn't already defined + Description: Workaround RubyGems' slow requires by only running require if the constant isn't already defined Enabled: true VersionAdded: '1.3.0' Exclude: - '**/spec/**/*.rb' - '**/omnibus/**/*.rb' - - '**/tasks/*.rb' \ No newline at end of file + - '**/tasks/*.rb' From 9197f78d2bc107f945c4bc87ffb66ccb3a924f6b Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Thu, 20 Aug 2020 10:57:02 -0700 Subject: [PATCH 03/11] Refactor Ruby27KeywordArgumentWarnings Use the new cop format and simplify how we extract the value Signed-off-by: Tim Smith --- .../ruby/ruby_27_keyword_argument_warnings.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings.rb b/lib/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings.rb index 48e086e..5691c41 100644 --- a/lib/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings.rb +++ b/lib/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true # -# Copyright:: 2020, Chef Software, Inc. +# Copyright:: Chef Software, Inc. # Author:: Tim Smith () # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -32,7 +32,10 @@ module ChefRuby # shell_out!('hostnamectl status', returns: [0, 1]) # shell_out('hostnamectl status', returns: [0, 1]) # - class Ruby27KeywordArgumentWarnings < Cop + class Ruby27KeywordArgumentWarnings < Base + include RuboCop::Chef::CookbookHelpers + extend RuboCop::Cop::AutoCorrector + MSG = "Pass options to shell_out helpers without the brackets to avoid Ruby 2.7 deprecation warnings." def_node_matcher :positional_shellout?, <<-PATTERN @@ -41,14 +44,11 @@ class Ruby27KeywordArgumentWarnings < Cop def on_send(node) positional_shellout?(node) do |h| - add_offense(h, location: :expression, message: MSG, severity: :refactor) if h.braces? - end - end + next unless h.braces? - 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(/}$/, "")) + add_offense(h.loc.expression, message: MSG, severity: :refactor) do |corrector| + corrector.replace(h.loc.expression, h.loc.expression.source[1..-2]) + end end end end From 2eb1a64c3dc5d41f727bb855c6e20ff451473005 Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Thu, 20 Aug 2020 11:44:53 -0700 Subject: [PATCH 04/11] Remove extra include of cookbook helpers Removed in cookstyle as well where it's not being used. Signed-off-by: Tim Smith --- lib/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings.rb b/lib/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings.rb index 5691c41..7f958fc 100644 --- a/lib/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings.rb +++ b/lib/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings.rb @@ -33,7 +33,6 @@ module ChefRuby # shell_out('hostnamectl status', returns: [0, 1]) # class Ruby27KeywordArgumentWarnings < Base - include RuboCop::Chef::CookbookHelpers extend RuboCop::Cop::AutoCorrector MSG = "Pass options to shell_out helpers without the brackets to avoid Ruby 2.7 deprecation warnings." From 7f25976867255715de8587778f6d09c7df0ce01a Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Fri, 21 Aug 2020 13:04:22 -0700 Subject: [PATCH 05/11] Update how we detect Time and add a few more requires Signed-off-by: Tim Smith --- lib/rubocop/cop/chef/ruby/unless_defined_require.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/rubocop/cop/chef/ruby/unless_defined_require.rb b/lib/rubocop/cop/chef/ruby/unless_defined_require.rb index f67644f..426afa2 100644 --- a/lib/rubocop/cop/chef/ruby/unless_defined_require.rb +++ b/lib/rubocop/cop/chef/ruby/unless_defined_require.rb @@ -45,6 +45,7 @@ class UnlessDefinedRequire < Base "erubis" => "Erubis", "etc" => "Etc", "excon" => "Excon", + "faraday" => "Faraday", "ffi_yajl" => "FFI_Yajl", "ffi" => "FFI", "fileutils" => "FileUtils", @@ -61,6 +62,7 @@ class UnlessDefinedRequire < Base "net/http" => "Net::HTTP", "net/ssh" => "Net::SSH", "netaddr" => "NetAddr", + "nokogiri" => "Nokogiri", "ohai" => "Ohai::System", "open-uri" => "OpenURI", "openssl" => "OpenSSL", @@ -83,7 +85,7 @@ class UnlessDefinedRequire < Base "stringio" => "StringIO", "tempfile" => "Tempfile", "thor" => "Thor", - "time" => "Time", + "time" => "Time.zone_offset", "timeout" => "Timeout", "tmpdir" => "Dir.mktmpdir", "tomlrb" => "Tomlrb", From 2e5adefe82f1978f3a7310b67afadf25f5fee468 Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Fri, 21 Aug 2020 13:04:41 -0700 Subject: [PATCH 06/11] Add a new cop to remove require rubygems in gemspecs There is no reason to do this. Signed-off-by: Tim Smith --- config/chefstyle.yml | 13 +++-- .../cop/chef/ruby/gemspec_require_rubygems.rb | 48 +++++++++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 lib/rubocop/cop/chef/ruby/gemspec_require_rubygems.rb diff --git a/config/chefstyle.yml b/config/chefstyle.yml index f7c5a37..45b1665 100644 --- a/config/chefstyle.yml +++ b/config/chefstyle.yml @@ -642,7 +642,12 @@ ChefRuby/UnlessDefinedRequire: Description: Workaround RubyGems' slow requires by only running require if the constant isn't already defined Enabled: true VersionAdded: '1.3.0' - Exclude: - - '**/spec/**/*.rb' - - '**/omnibus/**/*.rb' - - '**/tasks/*.rb' + Include: + - 'lib/**/*' + +ChefRuby/GemspecRequireRubygems: + Description: Rubygems does not need to be required in a Gemspec + Enabled: true + VersionAdded: '1.3.0' + Include: + - '**/*.gemspec' \ No newline at end of file diff --git a/lib/rubocop/cop/chef/ruby/gemspec_require_rubygems.rb b/lib/rubocop/cop/chef/ruby/gemspec_require_rubygems.rb new file mode 100644 index 0000000..ae757d7 --- /dev/null +++ b/lib/rubocop/cop/chef/ruby/gemspec_require_rubygems.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true +# +# Copyright:: Chef Software, Inc. +# Author:: Tim Smith () +# +# 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 GemspecRequireRubygems < Base + extend RuboCop::Cop::AutoCorrector + include RangeHelp + + MSG = "Rubygems does not need to be required in a Gemspec" + + def_node_matcher :require_rubygems?, <<-PATTERN + (send nil? :require (str "rubygems") ) + PATTERN + + def on_send(node) + require_rubygems?(node) do |r| + node = node.parent if node.parent.conditional? # make sure we identify conditionals on the require + add_offense(node.loc.expression, message: MSG, severity: :refactor) do |corrector| + corrector.remove(range_with_surrounding_space(range: node.loc.expression, side: :left)) + end + end + end + end + end + end + end +end From 0957063982dbd6c040b858937f4f87f7889864c3 Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Fri, 21 Aug 2020 15:44:07 -0700 Subject: [PATCH 07/11] Add specs Signed-off-by: Tim Smith --- .expeditor/verify.pipeline.yml | 20 ++++---- Gemfile | 11 +++++ Rakefile | 36 +++++++++++++- chefstyle.gemspec | 4 -- .../ruby_27_keyword_argument_warnings_spec.rb | 49 +++++++++++++++++++ spec/shared/autocorrect_behavior.rb | 8 +++ spec/spec_helper.rb | 24 +++++++++ 7 files changed, 137 insertions(+), 15 deletions(-) create mode 100644 spec/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings_spec.rb create mode 100755 spec/shared/autocorrect_behavior.rb create mode 100755 spec/spec_helper.rb diff --git a/.expeditor/verify.pipeline.yml b/.expeditor/verify.pipeline.yml index f3aecf9..be96224 100644 --- a/.expeditor/verify.pipeline.yml +++ b/.expeditor/verify.pipeline.yml @@ -11,42 +11,42 @@ expeditor: steps: -- label: run-chefstyle-ruby-2.4 +- label: run-chefstyle-and-specs-ruby-2.4 command: - - .expeditor/run_linux_tests.sh rake style + - .expeditor/run_linux_tests.sh rake expeditor: executor: docker: image: ruby:2.4-buster -- label: run-chefstyle-ruby-2.5 +- label: run-chefstyle-and-specs-ruby-2.5 command: - - .expeditor/run_linux_tests.sh rake style + - .expeditor/run_linux_tests.sh rake expeditor: executor: docker: image: ruby:2.5-buster -- label: run-chefstyle-ruby-2.6 +- label: run-chefstyle-and-specs-ruby-2.6 command: - - .expeditor/run_linux_tests.sh rake style + - .expeditor/run_linux_tests.sh rake expeditor: executor: docker: image: ruby:2.6-buster -- label: run-chefstyle-ruby-2.7 +- label: run-chefstyle-and-specs-ruby-2.7 command: - - .expeditor/run_linux_tests.sh rake style + - .expeditor/run_linux_tests.sh rake expeditor: executor: docker: image: ruby:2.7-buster -- label: run-chefstyle-windows +- label: run-chefstyle-and-specs-windows command: - bundle install --jobs=7 --retry=3 --without docs debug - - bundle exec rake style + - bundle exec rake expeditor: executor: docker: diff --git a/Gemfile b/Gemfile index f5788f3..aed4cdc 100644 --- a/Gemfile +++ b/Gemfile @@ -3,3 +3,14 @@ source "https://rubygems.org" # Specify your gem's dependencies in chefstyle.gemspec gemspec + +group :debug do + gem "pry" + gem "pry-byebug" + gem "pry-stack_explorer", "~> 0.4.0" # 0.5.0 drops support for Ruby < 2.6 +end + +group :development do + gem "rake", ">= 12.0" + gem "rspec", ">= 3.4" +end \ No newline at end of file diff --git a/Rakefile b/Rakefile index 2916741..f68b35e 100644 --- a/Rakefile +++ b/Rakefile @@ -26,6 +26,40 @@ RuboCop::RakeTask.new(:style) do |task| task.options << "--display-cop-names" end +require "rspec/core/rake_task" +RSpec::Core::RakeTask.new(:spec) do |spec| + spec.pattern = FileList["spec/cop/**/*.rb"] +end + +desc "Run RSpec with code coverage" +task :coverage do + ENV["COVERAGE"] = "true" + Rake::Task["spec"].execute +end + +desc "Ensure that all cops are defined in the chefstyle.yml config" +task :validate_config do + require "chefstyle" + require "yaml" unless defined?(YAML) + status = 0 + config = YAML.load_file("config/chefstyle.yml") + + puts "Checking that all cops are defined in config/chefstyle.yml:" + + RuboCop::Cop::Chef.constants.each do |dep| + RuboCop::Cop::Chef.const_get(dep).constants.each do |cop| + unless config["#{dep}/#{cop}"] + puts "Error: #{dep}/#{cop} not found in config/chefstyle.yml" + status = 1 + end + end + end + + puts "All Cops found in the config. Good work." if status == 0 + + exit status +end + begin require "yard" unless defined?(YARD) YARD::Rake::YardocTask.new(:docs) @@ -39,4 +73,4 @@ task :console do ARGV.clear IRB.start end -task default: %i{build install} +task default: %i{style spec validate_config} diff --git a/chefstyle.gemspec b/chefstyle.gemspec index f229346..043f325 100644 --- a/chefstyle.gemspec +++ b/chefstyle.gemspec @@ -18,9 +18,5 @@ Gem::Specification.new do |spec| spec.executables = %w{chefstyle} spec.require_paths = ["lib"] - 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 diff --git a/spec/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings_spec.rb b/spec/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings_spec.rb new file mode 100644 index 0000000..3403fc0 --- /dev/null +++ b/spec/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true +# +# Copyright:: Chef Software, Inc. +# Author:: Tim Smith () +# +# 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. +# + +require "spec_helper" + +describe RuboCop::Cop::Chef::ChefRuby::Ruby27KeywordArgumentWarnings, :config do + subject(:cop) { described_class.new(config) } + + it "registers an offense when passing a hash with brackets to shell_out" do + expect_offense(<<~RUBY) + shell_out('hostnamectl status', {returns: [0, 1]}) + ^^^^^^^^^^^^^^^^^ Pass options to shell_out helpers without the brackets to avoid Ruby 2.7 deprecation warnings. + RUBY + + expect_correction(<<~RUBY) + shell_out('hostnamectl status', returns: [0, 1]) + RUBY + end + + it "registers an offense when passing a hash with brackets to shell_out!" do + expect_offense(<<~RUBY) + shell_out!('hostnamectl status', {returns: [0, 1]}) + ^^^^^^^^^^^^^^^^^ Pass options to shell_out helpers without the brackets to avoid Ruby 2.7 deprecation warnings. + RUBY + + expect_correction(<<~RUBY) + shell_out!('hostnamectl status', returns: [0, 1]) + RUBY + end + + it "doesn't register an offense when properly passing options to the helpers" do + expect_no_offenses("shell_out!('hostnamectl status', returns: [0, 1])") + end +end diff --git a/spec/shared/autocorrect_behavior.rb b/spec/shared/autocorrect_behavior.rb new file mode 100755 index 0000000..f652b09 --- /dev/null +++ b/spec/shared/autocorrect_behavior.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true +RSpec.shared_examples "autocorrect" do |original, corrected| + it "autocorrects `#{original}` to `#{corrected}`" do + autocorrected = autocorrect_source(original, "spec/foo_spec.rb") + + expect(autocorrected).to eql(corrected) + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb new file mode 100755 index 0000000..21de65f --- /dev/null +++ b/spec/spec_helper.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true +require "rubocop" +require "rubocop/rspec/support" + +module SpecHelper + ROOT = Pathname.new(__dir__).parent.freeze +end + +spec_helper_glob = File.expand_path("{support,shared}/*.rb", __dir__) +Dir.glob(spec_helper_glob).map(&method(:require)) + +RSpec.configure do |config| + # Basic configuration + config.run_all_when_everything_filtered = true + config.filter_run(:focus) + config.order = :random + + config.include RuboCop::RSpec::ExpectOffense +end + +$LOAD_PATH.unshift(File.join(File.dirname(__FILE__), "..", "lib")) +$LOAD_PATH.unshift(File.dirname(__FILE__)) + +require "chefstyle" From d7771b88cd73e039946ae27d1b492d2708d2995f Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Fri, 21 Aug 2020 16:17:49 -0700 Subject: [PATCH 08/11] Add more specs and update to resolve nil bug Signed-off-by: Tim Smith --- .../cop/chef/ruby/gemspec_require_rubygems.rb | 8 ++-- .../ruby/gemspec_require_rubygems_spec.rb | 41 +++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 spec/rubocop/cop/chef/ruby/gemspec_require_rubygems_spec.rb diff --git a/lib/rubocop/cop/chef/ruby/gemspec_require_rubygems.rb b/lib/rubocop/cop/chef/ruby/gemspec_require_rubygems.rb index ae757d7..636cec1 100644 --- a/lib/rubocop/cop/chef/ruby/gemspec_require_rubygems.rb +++ b/lib/rubocop/cop/chef/ruby/gemspec_require_rubygems.rb @@ -20,14 +20,12 @@ 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. - # + # Rubygems does not need to be required in a Gemspec. It's already loaded out of the box in Ruby now. class GemspecRequireRubygems < Base extend RuboCop::Cop::AutoCorrector include RangeHelp - MSG = "Rubygems does not need to be required in a Gemspec" + MSG = "Rubygems does not need to be required in a Gemspec. It's already loaded out of the box in Ruby now." def_node_matcher :require_rubygems?, <<-PATTERN (send nil? :require (str "rubygems") ) @@ -35,7 +33,7 @@ class GemspecRequireRubygems < Base def on_send(node) require_rubygems?(node) do |r| - node = node.parent if node.parent.conditional? # make sure we identify conditionals on the require + node = node.parent if node.parent && node.parent.conditional? # make sure we identify conditionals on the require add_offense(node.loc.expression, message: MSG, severity: :refactor) do |corrector| corrector.remove(range_with_surrounding_space(range: node.loc.expression, side: :left)) end diff --git a/spec/rubocop/cop/chef/ruby/gemspec_require_rubygems_spec.rb b/spec/rubocop/cop/chef/ruby/gemspec_require_rubygems_spec.rb new file mode 100644 index 0000000..df0929c --- /dev/null +++ b/spec/rubocop/cop/chef/ruby/gemspec_require_rubygems_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true +# +# Copyright:: Chef Software, Inc. +# Author:: Tim Smith () +# +# 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. +# + +require "spec_helper" + +describe RuboCop::Cop::Chef::ChefRuby::GemspecRequireRubygems, :config do + subject(:cop) { described_class.new(config) } + + it "registers an offense when requiring rubygems" do + expect_offense(<<~RUBY) + require "rubygems" + ^^^^^^^^^^^^^^^^^^ Rubygems does not need to be required in a Gemspec. It's already loaded out of the box in Ruby now. + RUBY + + expect_correction("\n") + end + + it "registers an offense when requiring rubygems with a conditional" do + expect_offense(<<~RUBY) + require "rubygems" unless defined?(Gem) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Rubygems does not need to be required in a Gemspec. It's already loaded out of the box in Ruby now. + RUBY + + expect_correction("\n") + end +end From 9b59b9015a6408faec248f9df2909cf63c4d2ebd Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Fri, 21 Aug 2020 16:27:05 -0700 Subject: [PATCH 09/11] Add spec for UnlessDefinedRequire Signed-off-by: Tim Smith --- .../chef/ruby/unless_defined_require_spec.rb | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 spec/rubocop/cop/chef/ruby/unless_defined_require_spec.rb diff --git a/spec/rubocop/cop/chef/ruby/unless_defined_require_spec.rb b/spec/rubocop/cop/chef/ruby/unless_defined_require_spec.rb new file mode 100644 index 0000000..607a001 --- /dev/null +++ b/spec/rubocop/cop/chef/ruby/unless_defined_require_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true +# +# Copyright:: Chef Software, Inc. +# Author:: Tim Smith () +# +# 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. +# + +require "spec_helper" + +describe RuboCop::Cop::Chef::ChefRuby::UnlessDefinedRequire, :config do + subject(:cop) { described_class.new(config) } + + context "with a require known to the cop" do + it "registers an offense without an if defined? check" do + expect_offense(<<~RUBY) + require 'digest/md5' + ^^^^^^^^^^^^^^^^^^^^ Workaround rubygems slow requires by only running require if the class isn't already defined + RUBY + + expect_correction("require 'digest/md5' unless defined?(Digest::MD5)\n") + end + + it "doesn't register an offense when using if defined?" do + expect_no_offenses("require 'digest/md5' unless defined?(Digest::MD5)") + end + + it "doesn't register an offense when using another conditional" do + expect_no_offenses("require 'digest/md5' if something?") + end + end + + context "with a require unknown to the cop" do + it "doesn't register an offense with an if defined? check" do + expect_no_offenses("require 'foo/bar' unless defined?(Foo::Bar)") + end + + it "doesn't register an offense without an if defined? check" do + expect_no_offenses("require 'foo/bar'") + end + end +end From e8fbb08a8884da31310e4bd0008de2bc700629be Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Fri, 21 Aug 2020 16:28:42 -0700 Subject: [PATCH 10/11] Fix a typo Signed-off-by: Tim Smith --- config/chefstyle.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/chefstyle.yml b/config/chefstyle.yml index 45b1665..de60018 100644 --- a/config/chefstyle.yml +++ b/config/chefstyle.yml @@ -554,7 +554,7 @@ Style/RescueModifier: Style/AsciiComments: Enabled: false -# Parens around ternaries often make them more readable and reduces cognitive load over operator precidence +# Parens around ternaries often make them more readable and reduces cognitive load over operator precedence Style/TernaryParentheses: Enabled: false From dcdace60f849bc18fd5be60f79f0db1a1603e9cd Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Fri, 21 Aug 2020 16:30:18 -0700 Subject: [PATCH 11/11] Remove path manipulation for rspec Signed-off-by: Tim Smith --- spec/spec_helper.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 21de65f..a692fc8 100755 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -18,7 +18,4 @@ module SpecHelper config.include RuboCop::RSpec::ExpectOffense end -$LOAD_PATH.unshift(File.join(File.dirname(__FILE__), "..", "lib")) -$LOAD_PATH.unshift(File.dirname(__FILE__)) - require "chefstyle"