From 4b0f24c57bf4a333de642b893dfd7c0f0bfddeb0 Mon Sep 17 00:00:00 2001 From: Lukas Audzevicius Date: Fri, 19 Jan 2024 16:18:15 +0000 Subject: [PATCH 1/3] (CAT-1688) Upgrade rubocop Following a recent team decision, we are implementing a Rubocop Upgrade, moving the version from 1.48.1 to 1.50.0. This should be the final version until Puppet 7 is unsupported. --- .rubocop.yml | 1 + .rubocop_todo.yml | 46 +++++++++++++++++++++++++++++++++++++++++++++- Gemfile | 2 +- 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index aa1cf03..8d47e55 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,6 +1,7 @@ inherit_from: .rubocop_todo.yml AllCops: + NewCops: enable Include: - 'lib/**/*.rb' - 'puppetfile-cli.rb' diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index d737759..de686af 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,11 +1,17 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2023-10-27 07:36:27 UTC using RuboCop version 1.48.1. +# on 2024-01-19 15:46:30 UTC using RuboCop version 1.50.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. +# Offense count: 2 +# This cop supports safe autocorrection (--autocorrect). +Lint/AmbiguousOperatorPrecedence: + Exclude: + - 'lib/puppetfile-resolver/puppetfile/parser/r10k_eval.rb' + # Offense count: 1 # This cop supports unsafe autocorrection (--autocorrect-all). Lint/DuplicateRequire: @@ -17,6 +23,20 @@ Lint/MissingSuper: Exclude: - 'lib/puppetfile-resolver/puppetfile/parser/errors.rb' +# Offense count: 2 +# This cop supports unsafe autocorrection (--autocorrect-all). +Lint/NonAtomicFileOperation: + Exclude: + - 'lib/puppetfile-resolver/cache/persistent.rb' + +# Offense count: 3 +# This cop supports unsafe autocorrection (--autocorrect-all). +Lint/OrAssignmentToConstant: + Exclude: + - 'lib/puppetfile-resolver/puppetfile/parser/r10k_eval.rb' + - 'lib/puppetfile-resolver/spec_searchers/forge_configuration.rb' + - 'lib/puppetfile-resolver/version.rb' + # Offense count: 2 # Configuration parameters: ExpectMatchingDefinition, CheckDefinitionPathHierarchy, CheckDefinitionPathHierarchyRoots, Regex, IgnoreExecutableScripts, AllowedAcronyms. # CheckDefinitionPathHierarchyRoots: lib, spec, test, src @@ -53,6 +73,18 @@ Style/CombinableLoops: Exclude: - 'lib/puppetfile-resolver/puppetfile/document.rb' +# Offense count: 1 +# This cop supports unsafe autocorrection (--autocorrect-all). +Style/ConcatArrayLiterals: + Exclude: + - 'lib/puppetfile-resolver/util.rb' + +# Offense count: 1 +# This cop supports safe autocorrection (--autocorrect). +Style/FileRead: + Exclude: + - 'puppetfile-cli.rb' + # Offense count: 1 # Configuration parameters: AllowedMethods. # AllowedMethods: respond_to_missing? @@ -66,6 +98,18 @@ Style/RedundantAssignment: Exclude: - 'lib/puppetfile-resolver/puppetfile/parser/r10k_eval/module/local.rb' +# Offense count: 1 +# This cop supports safe autocorrection (--autocorrect). +Style/RedundantConstantBase: + Exclude: + - 'puppetfile-cli.rb' + +# Offense count: 1 +# This cop supports safe autocorrection (--autocorrect). +Style/RedundantFreeze: + Exclude: + - 'lib/puppetfile-resolver/puppetfile/parser/r10k_eval/module/forge.rb' + # Offense count: 3 # This cop supports safe autocorrection (--autocorrect). Style/RedundantRegexpEscape: diff --git a/Gemfile b/Gemfile index af760ed..6c5bb1d 100644 --- a/Gemfile +++ b/Gemfile @@ -8,7 +8,7 @@ group :test do gem 'rspec-collection_matchers', '~> 1.0' gem 'rspec-its', '~> 1.0' - gem 'rubocop', '~> 1.48.1' + gem 'rubocop', '~> 1.50.0' gem 'rubocop-rspec', '~> 2.19' gem 'rubocop-performance', '~> 1.16' From 311358c47cb16a24c44c43042a973337668974be Mon Sep 17 00:00:00 2001 From: Lukas Audzevicius Date: Fri, 19 Jan 2024 16:29:48 +0000 Subject: [PATCH 2/3] Correct safe offenses --- .rubocop_todo.yml | 69 +------------------ lib/puppetfile-resolver/cache/persistent.rb | 2 +- .../models/module_dependency.rb | 4 +- .../models/module_specification.rb | 6 +- .../puppetfile/document.rb | 8 +-- .../puppetfile/git_module.rb | 5 +- .../puppetfile/parser/r10k_eval.rb | 10 +-- .../parser/r10k_eval/module/local.rb | 3 +- .../puppetfile/validation_errors.rb | 6 +- lib/puppetfile-resolver/resolver.rb | 3 +- .../spec_searchers/configuration.rb | 4 +- .../spec_searchers/git/gclone.rb | 1 - .../spec_searchers/git/github.rb | 6 +- .../spec_searchers/git/gitlab.rb | 6 +- lib/puppetfile-resolver/util.rb | 2 +- puppetfile-cli.rb | 4 +- 16 files changed, 26 insertions(+), 113 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index de686af..a4a09cf 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,23 +1,11 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2024-01-19 15:46:30 UTC using RuboCop version 1.50.2. +# on 2024-01-22 11:46:15 UTC using RuboCop version 1.50.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 2 -# This cop supports safe autocorrection (--autocorrect). -Lint/AmbiguousOperatorPrecedence: - Exclude: - - 'lib/puppetfile-resolver/puppetfile/parser/r10k_eval.rb' - -# Offense count: 1 -# This cop supports unsafe autocorrection (--autocorrect-all). -Lint/DuplicateRequire: - Exclude: - - 'lib/puppetfile-resolver/spec_searchers/git/gclone.rb' - # Offense count: 1 Lint/MissingSuper: Exclude: @@ -46,28 +34,6 @@ Naming/FileName: - 'lib/puppetfile-resolver.rb' - 'puppetfile-cli.rb' -# Offense count: 27 -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: EnforcedStyle. -# SupportedStyles: separated, grouped -Style/AccessorGrouping: - Exclude: - - 'lib/puppetfile-resolver/models/module_dependency.rb' - - 'lib/puppetfile-resolver/models/module_specification.rb' - - 'lib/puppetfile-resolver/puppetfile/document.rb' - - 'lib/puppetfile-resolver/puppetfile/git_module.rb' - - 'lib/puppetfile-resolver/puppetfile/validation_errors.rb' - - 'lib/puppetfile-resolver/resolver.rb' - - 'lib/puppetfile-resolver/spec_searchers/configuration.rb' - -# Offense count: 1 -# This cop supports safe autocorrection (--autocorrect). -# Configuration parameters: AllowedMethods, AllowedPatterns. -# AllowedMethods: ==, equal?, eql? -Style/ClassEqualityComparison: - Exclude: - - 'lib/puppetfile-resolver/util.rb' - # Offense count: 1 Style/CombinableLoops: Exclude: @@ -79,12 +45,6 @@ Style/ConcatArrayLiterals: Exclude: - 'lib/puppetfile-resolver/util.rb' -# Offense count: 1 -# This cop supports safe autocorrection (--autocorrect). -Style/FileRead: - Exclude: - - 'puppetfile-cli.rb' - # Offense count: 1 # Configuration parameters: AllowedMethods. # AllowedMethods: respond_to_missing? @@ -92,35 +52,8 @@ Style/OptionalBooleanParameter: Exclude: - 'lib/puppetfile-resolver/cache/base.rb' -# Offense count: 1 -# This cop supports safe autocorrection (--autocorrect). -Style/RedundantAssignment: - Exclude: - - 'lib/puppetfile-resolver/puppetfile/parser/r10k_eval/module/local.rb' - -# Offense count: 1 -# This cop supports safe autocorrection (--autocorrect). -Style/RedundantConstantBase: - Exclude: - - 'puppetfile-cli.rb' - # Offense count: 1 # This cop supports safe autocorrection (--autocorrect). Style/RedundantFreeze: Exclude: - 'lib/puppetfile-resolver/puppetfile/parser/r10k_eval/module/forge.rb' - -# Offense count: 3 -# This cop supports safe autocorrection (--autocorrect). -Style/RedundantRegexpEscape: - Exclude: - - 'lib/puppetfile-resolver/puppetfile/parser/r10k_eval.rb' - -# Offense count: 7 -# This cop supports unsafe autocorrection (--autocorrect-all). -# Configuration parameters: Mode. -Style/StringConcatenation: - Exclude: - - 'lib/puppetfile-resolver/cache/persistent.rb' - - 'lib/puppetfile-resolver/spec_searchers/git/github.rb' - - 'lib/puppetfile-resolver/spec_searchers/git/gitlab.rb' diff --git a/lib/puppetfile-resolver/cache/persistent.rb b/lib/puppetfile-resolver/cache/persistent.rb index b1eda50..e32abba 100644 --- a/lib/puppetfile-resolver/cache/persistent.rb +++ b/lib/puppetfile-resolver/cache/persistent.rb @@ -43,7 +43,7 @@ def persist(name, content_string) private def to_cache_name(name) - ::Digest::SHA256.hexdigest(name) + '.txt' + "#{::Digest::SHA256.hexdigest(name)}.txt" end end end diff --git a/lib/puppetfile-resolver/models/module_dependency.rb b/lib/puppetfile-resolver/models/module_dependency.rb index 2d27364..ea2af0d 100644 --- a/lib/puppetfile-resolver/models/module_dependency.rb +++ b/lib/puppetfile-resolver/models/module_dependency.rb @@ -3,9 +3,7 @@ module PuppetfileResolver module Models class ModuleDependency - attr_accessor :name - attr_accessor :owner - attr_accessor :version_requirement + attr_accessor :name, :owner, :version_requirement def initialize(options = {}) # Munge the name diff --git a/lib/puppetfile-resolver/models/module_specification.rb b/lib/puppetfile-resolver/models/module_specification.rb index ef80e86..8f1afaf 100644 --- a/lib/puppetfile-resolver/models/module_specification.rb +++ b/lib/puppetfile-resolver/models/module_specification.rb @@ -7,11 +7,7 @@ module PuppetfileResolver module Models class ModuleSpecification - attr_accessor :name - attr_accessor :owner - attr_accessor :version - attr_accessor :origin # Same as R10K module :type - attr_accessor :resolver_flags + attr_accessor :name, :owner, :version, :origin, :resolver_flags # Same as R10K module :type def initialize(options = {}) require 'semantic_puppet' diff --git a/lib/puppetfile-resolver/puppetfile/document.rb b/lib/puppetfile-resolver/puppetfile/document.rb index c402985..43e0a54 100644 --- a/lib/puppetfile-resolver/puppetfile/document.rb +++ b/lib/puppetfile-resolver/puppetfile/document.rb @@ -5,16 +5,12 @@ module PuppetfileResolver module Puppetfile class DocumentLocation - attr_accessor :start_line # Base 0 - attr_accessor :start_char # Base 0 - attr_accessor :end_line # Base 0 - attr_accessor :end_char # Base 0 + attr_accessor :start_line, :start_char, :end_line, :end_char # Base 0 # Base 0 # Base 0 # Base 0 end class Document - attr_accessor :forge_uri + attr_accessor :forge_uri, :content attr_reader :modules - attr_accessor :content def initialize(puppetfile_content) @content = puppetfile_content diff --git a/lib/puppetfile-resolver/puppetfile/git_module.rb b/lib/puppetfile-resolver/puppetfile/git_module.rb index 1df68c4..07abe6d 100644 --- a/lib/puppetfile-resolver/puppetfile/git_module.rb +++ b/lib/puppetfile-resolver/puppetfile/git_module.rb @@ -5,10 +5,7 @@ module PuppetfileResolver module Puppetfile class GitModule < BaseModule - attr_accessor :remote - attr_accessor :ref - attr_accessor :commit - attr_accessor :tag + attr_accessor :remote, :ref, :commit, :tag def initialize(title) super diff --git a/lib/puppetfile-resolver/puppetfile/parser/r10k_eval.rb b/lib/puppetfile-resolver/puppetfile/parser/r10k_eval.rb index ebd8934..6565d2d 100644 --- a/lib/puppetfile-resolver/puppetfile/parser/r10k_eval.rb +++ b/lib/puppetfile-resolver/puppetfile/parser/r10k_eval.rb @@ -65,19 +65,19 @@ def self.parse(puppetfile_contents) def self.post_process_flags!(document) flag_ranges = {} document.content.lines.each_with_index do |line, index| - if (matches = line.match(%r{^\s*# resolver:disable ([A-Za-z\/,]+)(?:\s|$)})) + if (matches = line.match(%r{^\s*# resolver:disable ([A-Za-z/,]+)(?:\s|$)})) flags_from_line(matches[1]).each do |flag| # Start a flag range if there isn't already one going next unless flag_ranges[flag].nil? flag_ranges[flag] = index end - elsif (matches = line.match(%r{# resolver:disable ([A-Za-z\/,]+)(?:\s|$)})) + elsif (matches = line.match(%r{# resolver:disable ([A-Za-z/,]+)(?:\s|$)})) flags_from_line(matches[1]).each do |flag| # Assert the flag if we're not already within a range next unless flag_ranges[flag].nil? assert_resolver_flag(document, flag, index, index) end - elsif (matches = line.match(%r{^\s*# resolver:enable ([A-Za-z\/,]+)(?:\s|$)})) + elsif (matches = line.match(%r{^\s*# resolver:enable ([A-Za-z/,]+)(?:\s|$)})) flags_from_line(matches[1]).each do |flag| # End a flag range if there isn't already one going next if flag_ranges[flag].nil? @@ -122,8 +122,8 @@ def self.assert_resolver_flag(document, flag, from_line, to_line) next if mod.location.start_line.nil? || mod.location.end_line.nil? # If the module doesn't span the range we're looking for (from_line --> to_line) ignore it - next unless mod.location.start_line >= from_line && mod.location.start_line <= to_line || - mod.location.end_line >= from_line && mod.location.end_line <= to_line + next unless (mod.location.start_line >= from_line && mod.location.start_line <= to_line) || + (mod.location.end_line >= from_line && mod.location.end_line <= to_line) mod.resolver_flags << flag unless mod.resolver_flags.include?(flag) end nil diff --git a/lib/puppetfile-resolver/puppetfile/parser/r10k_eval/module/local.rb b/lib/puppetfile-resolver/puppetfile/parser/r10k_eval/module/local.rb index 80dcb8c..b1060ed 100644 --- a/lib/puppetfile-resolver/puppetfile/parser/r10k_eval/module/local.rb +++ b/lib/puppetfile-resolver/puppetfile/parser/r10k_eval/module/local.rb @@ -15,8 +15,7 @@ def self.implements?(_name, args) end def self.to_document_module(title, _args) - mod = ::PuppetfileResolver::Puppetfile::LocalModule.new(title) - mod + ::PuppetfileResolver::Puppetfile::LocalModule.new(title) end end end diff --git a/lib/puppetfile-resolver/puppetfile/validation_errors.rb b/lib/puppetfile-resolver/puppetfile/validation_errors.rb index a0ae24d..cf6d3b9 100644 --- a/lib/puppetfile-resolver/puppetfile/validation_errors.rb +++ b/lib/puppetfile-resolver/puppetfile/validation_errors.rb @@ -3,8 +3,7 @@ module PuppetfileResolver module Puppetfile class DocumentValidationErrorBase - attr_accessor :message - attr_accessor :puppet_module + attr_accessor :message, :puppet_module def initialize(message, puppet_module) @message = message @@ -79,8 +78,7 @@ def puppetfile_modules # Resolution Validation Error classes for validating # a valid Puppetfile against a dependency resolution class DocumentResolutionErrorBase < DocumentValidationErrorBase - attr_accessor :puppet_module - attr_accessor :module_specification + attr_accessor :puppet_module, :module_specification def initialize(message, puppet_module, module_specification) super(message, puppet_module) diff --git a/lib/puppetfile-resolver/resolver.rb b/lib/puppetfile-resolver/resolver.rb index dc22e41..fdc0ae8 100644 --- a/lib/puppetfile-resolver/resolver.rb +++ b/lib/puppetfile-resolver/resolver.rb @@ -7,8 +7,7 @@ module PuppetfileResolver class Resolver - attr_reader :puppetfile - attr_reader :dependencies_to_resolve + attr_reader :puppetfile, :dependencies_to_resolve def initialize(puppetfile_document, puppet_version = nil) @puppetfile = puppetfile_document diff --git a/lib/puppetfile-resolver/spec_searchers/configuration.rb b/lib/puppetfile-resolver/spec_searchers/configuration.rb index b46b69e..a7e72e9 100644 --- a/lib/puppetfile-resolver/spec_searchers/configuration.rb +++ b/lib/puppetfile-resolver/spec_searchers/configuration.rb @@ -7,9 +7,7 @@ module PuppetfileResolver module SpecSearchers class Configuration - attr_reader :local - attr_reader :forge - attr_reader :git + attr_reader :local, :forge, :git def initialize @local = LocalConfiguration.new diff --git a/lib/puppetfile-resolver/spec_searchers/git/gclone.rb b/lib/puppetfile-resolver/spec_searchers/git/gclone.rb index 9522d47..371c34d 100644 --- a/lib/puppetfile-resolver/spec_searchers/git/gclone.rb +++ b/lib/puppetfile-resolver/spec_searchers/git/gclone.rb @@ -5,7 +5,6 @@ require 'puppetfile-resolver/util' require 'puppetfile-resolver/spec_searchers/common' require 'puppetfile-resolver/spec_searchers/git_configuration' -require 'puppetfile-resolver/util' require 'uri' module PuppetfileResolver diff --git a/lib/puppetfile-resolver/spec_searchers/git/github.rb b/lib/puppetfile-resolver/spec_searchers/git/github.rb index 4ca164e..9d1c6f4 100644 --- a/lib/puppetfile-resolver/spec_searchers/git/github.rb +++ b/lib/puppetfile-resolver/spec_searchers/git/github.rb @@ -19,11 +19,11 @@ def self.metadata(puppetfile_module, resolver_ui, config) end return nil if repo_url.nil? - metadata_url = 'https://raw.githubusercontent.com/' + repo_url + '/' + metadata_url = "https://raw.githubusercontent.com/#{repo_url}/" if puppetfile_module.ref - metadata_url += puppetfile_module.ref + '/' + metadata_url += "#{puppetfile_module.ref}/" elsif puppetfile_module.tag - metadata_url += puppetfile_module.tag + '/' + metadata_url += "#{puppetfile_module.tag}/" else # Default to master. Should it raise? metadata_url += 'master/' diff --git a/lib/puppetfile-resolver/spec_searchers/git/gitlab.rb b/lib/puppetfile-resolver/spec_searchers/git/gitlab.rb index ee66336..01ccdf1 100644 --- a/lib/puppetfile-resolver/spec_searchers/git/gitlab.rb +++ b/lib/puppetfile-resolver/spec_searchers/git/gitlab.rb @@ -21,11 +21,11 @@ def self.metadata(puppetfile_module, resolver_ui, config) # Example URL # https://gitlab.com/simp/pupmod-simp-crypto_policy/-/raw/0.1.4/metadata.json - metadata_url = 'https://gitlab.com/' + repo_url + '/-/raw/' + metadata_url = "https://gitlab.com/#{repo_url}/-/raw/" if puppetfile_module.ref - metadata_url += puppetfile_module.ref + '/' + metadata_url += "#{puppetfile_module.ref}/" elsif puppetfile_module.tag - metadata_url += puppetfile_module.tag + '/' + metadata_url += "#{puppetfile_module.tag}/" else # Default to master. Should it raise? metadata_url += 'master/' diff --git a/lib/puppetfile-resolver/util.rb b/lib/puppetfile-resolver/util.rb index 10972be..3104cfd 100644 --- a/lib/puppetfile-resolver/util.rb +++ b/lib/puppetfile-resolver/util.rb @@ -29,7 +29,7 @@ def self.static_ca_cert_file def self.net_http_get(uri, proxy = nil) uri = URI.parse(uri) unless uri.is_a?(URI) - http_options = { :use_ssl => uri.class == URI::HTTPS } + http_options = { :use_ssl => uri.instance_of?(URI::HTTPS) } # Because on Windows Ruby doesn't use the Windows certificate store which has up-to date # CA certs, we can't depend on someone setting the environment variable correctly. So use our # static CA PEM file if SSL_CERT_FILE is not set. diff --git a/puppetfile-cli.rb b/puppetfile-cli.rb index f079abe..d342b4e 100644 --- a/puppetfile-cli.rb +++ b/puppetfile-cli.rb @@ -72,9 +72,9 @@ def self.parse(options) end # Parse the Puppetfile into an object model -content = File.open(options[:path], 'rb') { |f| f.read } +content = File.binread(options[:path]) require 'puppetfile-resolver/puppetfile/parser/r10k_eval' -puppetfile = ::PuppetfileResolver::Puppetfile::Parser::R10KEval.parse(content) +puppetfile = PuppetfileResolver::Puppetfile::Parser::R10KEval.parse(content) # Make sure the Puppetfile is valid unless puppetfile.valid? From 6cb17920bfa669c1b9caeb4142a0ebb034b165b6 Mon Sep 17 00:00:00 2001 From: Lukas Audzevicius <97180854+LukasAud@users.noreply.github.com> Date: Tue, 23 Jan 2024 09:53:26 +0000 Subject: [PATCH 3/3] Update lib/puppetfile-resolver/models/module_specification.rb Co-authored-by: Jordan Breen <112936862+jordanbreen28@users.noreply.github.com> --- lib/puppetfile-resolver/models/module_specification.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puppetfile-resolver/models/module_specification.rb b/lib/puppetfile-resolver/models/module_specification.rb index 8f1afaf..32e6bbb 100644 --- a/lib/puppetfile-resolver/models/module_specification.rb +++ b/lib/puppetfile-resolver/models/module_specification.rb @@ -7,7 +7,7 @@ module PuppetfileResolver module Models class ModuleSpecification - attr_accessor :name, :owner, :version, :origin, :resolver_flags # Same as R10K module :type + attr_accessor :name, :owner, :version, :origin, :resolver_flags # origin attr same as R10K module :type def initialize(options = {}) require 'semantic_puppet'