Skip to content

Commit

Permalink
Add custom cops to Chefstyle
Browse files Browse the repository at this point in the history
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 <tsmith@chef.io>
  • Loading branch information
tas50 committed Aug 20, 2020
1 parent 560b01f commit a8bc18c
Show file tree
Hide file tree
Showing 8 changed files with 217 additions and 5 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@
/pkg/
/spec/reports/
/tmp/
/test.rb # used for quick testing of cops
4 changes: 2 additions & 2 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand All @@ -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."
Expand Down
1 change: 1 addition & 0 deletions chefstyle.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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
16 changes: 15 additions & 1 deletion config/chefstyle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
12 changes: 10 additions & 2 deletions lib/chefstyle.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
11 changes: 11 additions & 0 deletions lib/rubocop/chef.rb
Original file line number Diff line number Diff line change
@@ -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
58 changes: 58 additions & 0 deletions lib/rubocop/cop/chef/ruby/ruby_27_keyword_argument_warnings.rb
Original file line number Diff line number Diff line change
@@ -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
119 changes: 119 additions & 0 deletions lib/rubocop/cop/chef/ruby/unless_defined_require.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit a8bc18c

Please sign in to comment.