12.0.1: Enable default Rubocop rules (#613)

* Remove DisabledByDefault: true in rubocop, and fix + autofix lints.

* Add a CHANGELOG entry about Rubocop rules.

* Undo some weird rubocop auto-corrects

* Fix some rubocop mistakes (e.g. select->match? != grep in one case)

* Bump rubocop to fix broken HashSyntax rule, and some other lints

* Put a requirement on rubocop

* Fix Gemfile requirement groups

* Ignore vendor/ in rubocop

* 12.0.1

.circleci/config.yml

@@ -20,6 +20,7 @@ commands:
             bundle lock --add-platform x86_64-linux
       - ruby/install-deps:
           bundler-version: "2.3"
+          key: gems-v3
 
 
 jobs:

.rubocop.yml

@@ -1,8 +1,16 @@
 ---
 
+# Without this, CI might pickup nested dep's rubocop files in vendor/
+inherit_mode:
+  merge:
+    - Exclude
+
 AllCops:
-  DisabledByDefault: true
-  TargetRubyVersion: 3.0
+  NewCops: enable
+  TargetRubyVersion: 3.2.6
+  Exclude:
+    - spec/fixtures/**/*
+    - vendor/bundle/**/* # This is actually needed for CI, not for biblio itself
 
 
 Metrics/BlockLength:

CHANGELOG.md

@@ -9,11 +9,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+### Changed
+
+### Removed
+
+## [12.1.0] - 2025-01-30
+
+### Added
+
 - Populate Bibliothecary::Dependency#source field in all parsers. This makes the source field useful when consuming 
   from Bibliothecary, and removes a step from consumers having to populate this field themselves.
 
 ### Changed
 
+- Improved Rubocop rules to make future spec changes easier via Rubocop auto-correcting formatting violations.
+
 ### Removed
 
 ## [12.0.0] - 2025-01-27

Gemfile

@@ -1,9 +1,24 @@
+# frozen_string_literal: true
+
 source "https://rubygems.org"
 
 # Specify your gem's dependencies in bibliothecary.gemspec
 gemspec
 
+group :development do
+  gem "pry"
+end
+
+group :development, :test do
+  gem "rake", "~> 12.0"
+  gem "rubocop", "~> 1.71"
+  gem "rubocop-rails"
+  gem "rubocop-rake" # This is needed by packageurl-ruby, until it reclassifies it as a dev dependency.
+end
+
 group :test do
-  gem "simplecov"
   gem "codeclimate-test-reporter", "~> 1.0.0"
+  gem "rspec", "~> 3.0"
+  gem "simplecov"
+  gem "webmock"
 end

Rakefile

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require "bundler/gem_tasks"
 require "rspec/core/rake_task"
 

bibliothecary.gemspec

@@ -1,9 +1,12 @@
-# coding: utf-8
-lib = File.expand_path("../lib", __FILE__)
+# frozen_string_literal: true
+
+lib = File.expand_path("lib", __dir__)
 $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 require "bibliothecary/version"
 
 Gem::Specification.new do |spec|
+  spec.required_ruby_version = ">= 3.2.0"
+
   spec.name          = "bibliothecary"
   spec.version       = Bibliothecary::VERSION
   spec.authors       = ["Andrew Nesbitt"]
@@ -18,19 +21,14 @@ Gem::Specification.new do |spec|
   spec.executables   = spec.files.grep(%r{^bin/}) { |f| File.basename(f) }
   spec.require_paths = ["lib"]
 
-  spec.add_dependency "tomlrb", "~> 2.0"
+  spec.add_dependency "commander"
+  spec.add_dependency "deb_control"
   spec.add_dependency "librariesio-gem-parser"
   spec.add_dependency "ox", ">= 2.8.1"
-  spec.add_dependency "typhoeus"
-  spec.add_dependency "deb_control"
-  spec.add_dependency "sdl4r"
-  spec.add_dependency "commander"
   spec.add_dependency "packageurl-ruby"
+  spec.add_dependency "sdl4r"
+  spec.add_dependency "tomlrb", "~> 2.0"
+  spec.add_dependency "typhoeus"
 
-  spec.add_development_dependency "pry"
-  spec.add_development_dependency "rake", "~> 12.0"
-  spec.add_development_dependency "rspec", "~> 3.0"
-  spec.add_development_dependency "webmock"
-  spec.add_development_dependency "rubocop"
-  spec.add_development_dependency "rubocop-rails"
+  spec.metadata["rubygems_mfa_required"] = "true"
 end

bin/bibliothecary

@@ -1,6 +1,7 @@
 #!/usr/bin/env ruby
+# frozen_string_literal: true
 
-lib = File.expand_path("../../lib", __FILE__)
+lib = File.expand_path("../lib", __dir__)
 $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 
 require "bibliothecary/cli"

bin/console

@@ -1,4 +1,5 @@
 #!/usr/bin/env ruby
+# frozen_string_literal: true
 
 require "bundler/setup"
 require "bibliothecary"

lib/bibliothecary.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require "bibliothecary/version"
 require "bibliothecary/dependency"
 require "bibliothecary/analyser"
@@ -10,16 +12,16 @@
 require "find"
 require "tomlrb"
 
-Dir[File.expand_path("../bibliothecary/multi_parsers/*.rb", __FILE__)].each do |file|
+Dir[File.expand_path("bibliothecary/multi_parsers/*.rb", __dir__)].each do |file|
   require file
 end
-Dir[File.expand_path("../bibliothecary/parsers/*.rb", __FILE__)].each do |file|
+Dir[File.expand_path("bibliothecary/parsers/*.rb", __dir__)].each do |file|
   require file
 end
 
 module Bibliothecary
-  VERSION_OPERATORS = /[~^<>*"]/.freeze
-  INVALID_UTF8_ERROR_REGEXP = /invalid byte sequence/.freeze
+  VERSION_OPERATORS = /[~^<>*"]/
+  INVALID_UTF8_ERROR_REGEXP = /invalid byte sequence/
 
   def self.analyse(path, ignore_unparseable_files: true)
     runner.analyse(path, ignore_unparseable_files: ignore_unparseable_files)
@@ -86,6 +88,7 @@ def self.utf8_string(string)
   rescue ArgumentError => e
     # Bibliothecary doesn't need to analyze non-UTF8 files like binary files, so just return blank.
     return "" if e.message.match?(INVALID_UTF8_ERROR_REGEXP)
+
     raise e
   end
 

lib/bibliothecary/analyser.rb

@@ -1,10 +1,12 @@
-require_relative "./analyser/matchers.rb"
-require_relative "./analyser/determinations.rb"
-require_relative "./analyser/analysis.rb"
+# frozen_string_literal: true
+
+require_relative "analyser/matchers"
+require_relative "analyser/determinations"
+require_relative "analyser/analysis"
 
 module Bibliothecary
   module Analyser
-    def self.create_error_analysis(platform_name, relative_path, kind, message, location=nil)
+    def self.create_error_analysis(platform_name, relative_path, kind, message, location = nil)
       {
         platform: platform_name,
         path: relative_path,
@@ -53,11 +55,11 @@ def generic?
       end
 
       def platform_name
-        self.name.to_s.split("::").last.downcase
+        name.to_s.split("::").last.downcase
       end
 
-      def map_dependencies(hash, key, type, source=nil)
-        hash.fetch(key,[]).map do |name, requirement|
+      def map_dependencies(hash, key, type, source = nil)
+        hash.fetch(key, []).map do |name, requirement|
           Dependency.new(
             name: name,
             requirement: requirement,
@@ -76,7 +78,7 @@ def map_dependencies(hash, key, type, source=nil)
       def add_multi_parser(klass)
         raise "No mapping found! You should place the add_multi_parser call below def self.mapping." unless respond_to?(:mapping)
 
-        original_mapping = self.mapping
+        original_mapping = mapping
 
         define_singleton_method(:mapping) do
           original_mapping.merge(klass.mapping)

lib/bibliothecary/analyser/analysis.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Bibliothecary
   module Analyser
     module Analysis
@@ -42,25 +44,29 @@ def analyse_contents_from_info(info, options: {})
 
         dependencies_to_analysis(info, kind, dependencies)
       rescue Bibliothecary::FileParsingError => e
-        Bibliothecary::Analyser::create_error_analysis(platform_name, info.relative_path, kind, e.message, e.location)
+        Bibliothecary::Analyser.create_error_analysis(platform_name, info.relative_path, kind, e.message, e.location)
       end
       alias analyze_contents_from_info analyse_contents_from_info
 
       def dependencies_to_analysis(info, kind, dependencies)
-        dependencies = dependencies || [] # work around any legacy parsers that return nil
+        dependencies ||= [] # work around any legacy parsers that return nil
         if generic?
           grouped = dependencies.group_by { |dep| dep[:platform] }
           all_analyses = grouped.keys.map do |platform|
-            deplatformed_dependencies = grouped[platform].map { |d| d.delete(:platform); d }
-            Bibliothecary::Analyser::create_analysis(platform, info.relative_path, kind, deplatformed_dependencies)
+            deplatformed_dependencies = grouped[platform].map do |d|
+              d.delete(:platform)
+              d
+            end
+            Bibliothecary::Analyser.create_analysis(platform, info.relative_path, kind, deplatformed_dependencies)
           end
           # this is to avoid a larger refactor for the time being. The larger refactor
           # needs to make analyse_contents return multiple analysis, or add another
           # method that can return multiple and deprecate analyse_contents, perhaps.
           raise "File contains zero or multiple platforms, currently must have exactly one" if all_analyses.length != 1
+
           all_analyses.first
         else
-          Bibliothecary::Analyser::create_analysis(platform_name, info.relative_path, kind, dependencies)
+          Bibliothecary::Analyser.create_analysis(platform_name, info.relative_path, kind, dependencies)
         end
       end
 
@@ -81,8 +87,7 @@ def parse_file(filename, contents, options: {})
         # this comment, some of the parsers return [] or nil to mean an error
         # which is confusing to users.
         send(details[:parser], contents, options: options.merge(filename: filename))
-
-      rescue Exception => e # default is StandardError but C bindings throw Exceptions
+      rescue Exception => e # default is StandardError but C bindings throw Exceptions # rubocop:disable Lint/RescueException
         # the C xml parser also puts a newline at the end of the message
         location = e.backtrace_locations[0]
           .to_s
@@ -97,7 +102,7 @@ def related_paths(info, infos)
 
         kind = determine_kind_from_info(info)
         relate_to_kind = first_matching_mapping_details(info)
-          .fetch(:related_to, %w(manifest lockfile).reject { |k| k == kind })
+          .fetch(:related_to, %w[manifest lockfile].reject { |k| k == kind })
         dirname = File.dirname(info.relative_path)
 
         infos

lib/bibliothecary/analyser/determinations.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module Bibliothecary
   module Analyser
     module Determinations

lib/bibliothecary/analyser/matchers.rb

@@ -1,42 +1,42 @@
+# frozen_string_literal: true
+
 module Bibliothecary
   module Analyser
     module Matchers
       def match_filename(filename, case_insensitive: false)
         if case_insensitive
-          lambda { |path| path.downcase == filename.downcase || path.downcase.end_with?("/" + filename.downcase) }
+          ->(path) { path.downcase == filename.downcase || path.downcase.end_with?("/#{filename.downcase}") }
         else
-          lambda { |path| path == filename || path.end_with?("/" + filename) }
+          ->(path) { path == filename || path.end_with?("/#{filename}") }
         end
       end
 
       def match_filenames(*filenames)
         lambda do |path|
           filenames.any? { |f| path == f } ||
-            filenames.any? { |f| path.end_with?("/" + f) }
+            filenames.any? { |f| path.end_with?("/#{f}") }
         end
       end
 
       def match_extension(filename, case_insensitive: false)
         if case_insensitive
-          lambda { |path| path.downcase.end_with?(filename.downcase) }
+          ->(path) { path.downcase.end_with?(filename.downcase) }
         else
-          lambda { |path| path.end_with?(filename) }
+          ->(path) { path.end_with?(filename) }
         end
       end
 
       def mapping_entry_match?(matcher, details, info)
-        if matcher.call(info.relative_path)
-          # we only want to load contents if we don't have them already
-          # and there's a content_matcher method to use
-          return true if details[:content_matcher].nil?
-          # this is the libraries.io case where we won't load all .xml
-          # files (for example) just to look at their contents, we'll
-          # assume they are not manifests.
-          return false if info.contents.nil?
-          return send(details[:content_matcher], info.contents)
-        else
-          return false
-        end
+        return false unless matcher.call(info.relative_path)
+        # we only want to load contents if we don't have them already
+        # and there's a content_matcher method to use
+        return true if details[:content_matcher].nil?
+        # this is the libraries.io case where we won't load all .xml
+        # files (for example) just to look at their contents, we'll
+        # assume they are not manifests.
+        return false if info.contents.nil?
+
+        send(details[:content_matcher], info.contents)
       end
 
       # this is broken with contents=nil because it can't look at file

lib/bibliothecary/cli.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require "bibliothecary/version"
 require "bibliothecary"
 require "commander"
@@ -20,7 +22,7 @@ def run
           output = Bibliothecary.analyse(options.path)
           output.each do |file_contents|
             puts "#{file_contents[:path]} (#{file_contents[:platform]})"
-            file_contents[:dependencies].group_by{|d| d[:type] }.each do |type, deps|
+            file_contents[:dependencies].group_by { |d| d[:type] }.each do |type, deps|
               puts "  #{type}"
               deps.each do |dep|
                 puts "    #{dep[:name]} #{dep[:requirement]}"

lib/bibliothecary/configuration.rb

@@ -1,13 +1,8 @@
+# frozen_string_literal: true
+
 module Bibliothecary
   class Configuration
-    attr_accessor :ignored_dirs
-    attr_accessor :ignored_files
-    attr_accessor :carthage_parser_host
-    attr_accessor :clojars_parser_host
-    attr_accessor :mix_parser_host
-    attr_accessor :conda_parser_host
-    attr_accessor :swift_parser_host
-    attr_accessor :cabal_parser_host
+    attr_accessor :ignored_dirs, :ignored_files, :carthage_parser_host, :clojars_parser_host, :mix_parser_host, :conda_parser_host, :swift_parser_host, :cabal_parser_host
 
     def initialize
       @ignored_dirs = [".git", "node_modules", "bower_components", "vendor", "dist"]