Add more tests and cleanup comments

Author: martinemde

.rubocop.yml

@@ -41,9 +41,6 @@ Style/Documentation:
 Style/SignalException:
   Enabled: false
 
-Style/FrozenStringLiteralComment:
-  Enabled: false
-
 Style/MultilineBlockChain:
   Enabled: false
 

lib/query_packwerk.rb

@@ -5,12 +5,12 @@
 require 'parse_packwerk'
 require 'rubocop'
 
-#
 # QueryPackwerk is a tool for querying Packwerk violations.
 #
 # It is built on top of ParsePackwerk, and provides a Ruby-friendly API
 # for querying package.yml and package_todo.yml files.
 #
+# See also: https://github.com/rubyatscale/parse_packwerk
 module QueryPackwerk
   autoload :Console, 'query_packwerk/console'
   autoload :ConsoleHelpers, 'query_packwerk/console_helpers'
@@ -24,8 +24,8 @@ module QueryPackwerk
   autoload :Version, 'query_packwerk/version'
 
   extend T::Sig
-  # TODO: module_function isn't playing nicely with Sorbet
-  extend self # rubocop:todo Style/ModuleFunction
+
+  module_function
 
   sig { params(name: String).returns(T.nilable(QueryPackwerk::Package)) }
   def package(name)

lib/query_packwerk/cli.rb

@@ -1,4 +1,5 @@
 # typed: strict
+# frozen_string_literal: true
 
 require 'thor'
 

lib/query_packwerk/console_helpers.rb

@@ -1,8 +1,10 @@
 # typed: true
+# frozen_string_literal: true
 
 module QueryPackwerk
   module ConsoleHelpers
     extend T::Sig
+    include Kernel
 
     sig { returns(String) }
     def inspect

lib/query_packwerk/packages.rb

@@ -61,9 +61,17 @@ def empty?
       @original_collection.empty?
     end
 
-    sig { params(arg0: T.proc.params(arg0: QueryPackwerk::Package).returns(T::Boolean)).returns(T::Boolean) }
-    def any?(&block)
-      @original_collection.any?(&block)
+    sig do
+      params(
+        blk: T.nilable(T.proc.params(arg0: QueryPackwerk::Package).returns(T::Boolean))
+      ).returns(T::Boolean)
+    end
+    def any?(&blk)
+      if blk.nil?
+        !empty?
+      else
+        @original_collection.any?(&blk)
+      end
     end
 
     # You can query for packages rather than violations to get a broader view, and
@@ -78,11 +86,12 @@ def violations
 
     sig { returns(String) }
     def inspect
-      [
-        "#<#{self.class.name} [",
-        to_a.map(&:inspect).join("\n"),
-        ']>'
-      ].join("\n")
+      arr = to_a.map(&:inspect)
+      if arr.empty?
+        "#<#{self.class.name} []>"
+      else
+        "#<#{self.class.name} [\n#{arr.join("\n")}\n]>"
+      end
     end
   end
 end

lib/query_packwerk/query_interface.rb

@@ -36,8 +36,6 @@ def each(&block)
     end
 
     # Should be overridden, the base of most of the queries.
-    #
-    # TODO: Consider refactoring to `abstract` interface
     sig { overridable.returns(T::Array[T.untyped]) }
     def original_collection
       []

lib/query_packwerk/violation.rb

@@ -88,7 +88,7 @@ def files
 
     # Whether or not the files containing violations match any provided globs
     #
-    # See also: https://ruby-doc.org/core-2.7.6/File.html#method-c-fnmatch
+    # See Ruby documentation for File.fnmatch for more details.
     sig { params(globs: T.any(String, Regexp)).returns(T::Boolean) }
     def includes_files?(*globs)
       globs.any? do |glob|
@@ -272,9 +272,6 @@ def deconstruct_keys(keys)
         **runtime_keys(keys)
       }
 
-      # all_values[:is_active_record] = active_record? if !keys || keys.include?(:is_active_record)
-      # all_values[:is_constant] = active_record? if !keys || keys.include?(:is_constant)
-
       keys.nil? ? all_values : all_values.slice(*T.unsafe(keys))
     end
 

lib/query_packwerk/violations.rb

@@ -263,11 +263,12 @@ def excluding_files(*file_globs)
 
     sig { returns(String) }
     def inspect
-      [
-        "#<#{self.class.name} [",
-        to_a.map(&:inspect).join("\n"),
-        ']>'
-      ].join("\n")
+      arr = to_a.map(&:inspect)
+      if arr.empty?
+        "#<#{self.class.name} []>"
+      else
+        "#<#{self.class.name} [\n#{arr.join("\n")}\n]>"
+      end
     end
 
     private

spec/query_packwerk/packages_spec.rb

@@ -0,0 +1,54 @@
+# typed: false
+# frozen_string_literal: true
+
+RSpec.describe QueryPackwerk::Packages do
+  include_context 'pseudo packs'
+
+  describe '#where' do
+    it 'can query packages' do
+      packages = described_class.where(owner: QueryPackwerk::Packages::UNOWNED)
+      expect(packages).to be_a(described_class)
+      expect(packages.map(&:owner).uniq).to eq([QueryPackwerk::Packages::UNOWNED])
+    end
+  end
+
+  describe '#empty? and #any?' do
+    it 'returns true for empty? when there are no packages' do
+      empty_packages = described_class.new([])
+      expect(empty_packages.empty?).to be true
+      expect(empty_packages.any?).to be false
+    end
+
+    it 'returns false for empty? when there are packages' do
+      expect(described_class.all.empty?).to be false
+      expect(described_class.all.any?).to be true
+    end
+
+    it 'supports any? with a block' do
+      packages = described_class.all
+      expect(packages.any? { |p| p.owner == QueryPackwerk::Packages::UNOWNED }).to be true
+      expect(packages.any? { |p| p.name == 'nonexistent' }).to be false
+    end
+  end
+
+  describe '#violations' do
+    it 'returns violations for all packages in the collection' do
+      packages = described_class.all
+      violations = packages.violations
+      expect(violations).to be_a(QueryPackwerk::Violations)
+      expect(violations.count).to be > 0
+    end
+  end
+
+  describe '#inspect' do
+    it 'returns a string representation of an empty collection' do
+      packages = described_class.new([])
+      expect(packages.inspect).to eq("#<#{described_class.name} []>")
+    end
+
+    it 'returns a string representation of the packages' do
+      packages = described_class.all
+      expect(packages.inspect).to eq("#<#{described_class.name} [\n#{described_class.all.map(&:inspect).join("\n")}\n]>")
+    end
+  end
+end

spec/query_packwerk/violations_spec.rb

@@ -31,6 +31,18 @@
     end
   end
 
+  describe '#inspect' do
+    it 'returns a string representation of an empty collection' do
+      violations = described_class.new([])
+      expect(violations.inspect).to eq("#<#{described_class.name} []>")
+    end
+
+    it 'returns a string representation of a collection with violations' do
+      violations = described_class.all
+      expect(violations.inspect).to eq("#<#{described_class.name} [\n#{described_class.all.map(&:inspect).join("\n")}\n]>")
+    end
+  end
+
   describe '#anonymous_sources_with_locations' do
     it 'can get anonymized sources with their file locations' do
       # { constant => { violating code shape => [where it happened] } }