diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index feed61c..3591ab1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -12,3 +12,6 @@ jobs: contents: read uses: rubyatscale/shared-config/.github/workflows/ci.yml@main # zizmor: ignore[unpinned-uses,secrets-inherit] internal reusable workflow tracked at @main by convention so shared-config updates propagate automatically; its ci.yml reads SLACK_WEBHOOK_URL but declares no workflow_call secrets, so inherit is the only way to pass it secrets: inherit + with: + # With pks, spec/query_packwerk/pks_spec.rb checks the references that the specs use against what pks lists + test-command: bin/install-pks && PATH="$PWD/tmp/pks:$PATH" bundle exec rspec diff --git a/README.md b/README.md index 4ebad31..a2943a6 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # QueryPackwerk QueryPackwerk is a Ruby gem for querying and analyzing [Packwerk](https://github.com/Shopify/packwerk) violations in Ruby applications. -It provides a friendly API for exploring `package.yml` and `package_todo.yml` files, making it easier to manage module boundaries and dependencies in your codebase. +It provides a friendly API for exploring `package.yml` and `package_todo.yml` files, and the references between packs that [pks](https://github.com/rubyatscale/pks) lists, making it easier to manage module boundaries and dependencies in your codebase. ## Installation @@ -80,10 +80,33 @@ query_packwerk:001:0> Violations.all ``` -If you change any of the package.yml or package_todo.yml files, you can reload the running console: +Explore references, whether or not they break a rule: + +package_todo.yml files record only the references that break a pack's rules. To list every reference from one pack to another pack's constants, including those that a declared dependency or a public constant allows, QueryPackwerk runs [pks](https://github.com/rubyatscale/pks) 0.7.0 or later. It runs `bin/pks` if your app has one, or else `pks` on your `PATH`. See pks's [installation instructions](https://github.com/rubyatscale/pks/blob/main/INSTALLATION.md). + +```ruby +# Get all references to a pack's constants +query_packwerk:001:0> references_for("pack_name") + +# Get the 'shape' of the references to a pack that break no rule +query_packwerk:001:0> References.where(producing_pack: "packs/pack_name", allowed: true).anonymous_source_counts + +# Get the references to a pack that break a rule no package_todo.yml records, which `pks check` reports +query_packwerk:001:0> References.where(producing_pack: "packs/pack_name", unrecorded: true) + +# Get all references from a pack to other packs' constants +query_packwerk:001:0> references_from("pack_name") + +# Get all references +query_packwerk:001:0> References.all +``` + +References have the same methods as violations, like `sources_with_locations` and `anonymous_source_counts`. Each one is where pks finds it, so its source is found even where the code names a constant relatively, as `Bar` inside `module Foo`. A reference that pks infers from an association, as `has_many :tacos` does for `Taco`, has the association as its source. pks gives no positions in ERB templates, so there a reference's sources are wherever the template names the constant. Finding sources parses each file they're in, which for a pack that thousands of files reference takes a while, and keeps each one in memory, about 200 KB for a file of 100 lines, until `reload!`. + +If you change your code or any of the package.yml or package_todo.yml files, you can reload the running console: ```ruby -# Reset the cache +# Reset the cache, so the next query of references runs pks again query_packwerk:001:0> reload! ``` @@ -102,6 +125,9 @@ consumers = QueryPackwerk.consumers("my_pack") # Get a count of all violation types counts = QueryPackwerk.violation_counts_for("my_pack") + +# Get how other packs use your pack, whether or not they break a rule +QueryPackwerk.references_for("my_pack").anonymous_source_counts ``` ## Examples @@ -124,6 +150,8 @@ QueryPackwerk.anonymous_violation_counts_for("my_pack", threshold: 3) After checking out the repo, run `bin/setup` to install dependencies. Then, run `rake spec` to run the tests. You can also run `bin/console` for an interactive prompt that will allow you to experiment. +The specs for references use what pks lists for an app they write, and one spec checks that against pks itself. It runs where pks is on your `PATH`, as in CI. To install the version it checks against, run `bin/install-pks` and add `tmp/pks` to your `PATH`. + To install this gem onto your local machine, run `bundle exec rake install`. To release a new version, bump `VERSION` in `lib/query_packwerk/version.rb` in your PR and run `bundle install` so `Gemfile.lock` matches. When the PR is merged and CI passes on `main`, the gem is tagged and published to RubyGems automatically. Merges that don't change the version don't publish anything. ## Contributing diff --git a/bin/install-pks b/bin/install-pks new file mode 100755 index 0000000..5042a35 --- /dev/null +++ b/bin/install-pks @@ -0,0 +1,34 @@ +#!/usr/bin/env bash +# Installs pks in tmp/pks, as CI does, so pks_spec.rb checks the specs' listing against pks. +# Keep version at QueryPackwerk::Pks::MINIMUM_VERSION, with each archive's sha256. +set -euo pipefail + +version=0.7.0 +case "$(uname -sm)" in + 'Linux x86_64') + target=x86_64-unknown-linux-gnu + sha256=51b8fb321a463769d59b99a1be2832fca035c92729e0bc26ecaca88c0dc9baa7 + ;; + 'Darwin arm64') + target=aarch64-apple-darwin + sha256=0c360f17e1d4e617711d9dce5869e2034841fa584a911b1d7b831ce536ed500e + ;; + *) + echo "bin/install-pks has no pks $version for $(uname -sm)" >&2 + exit 1 + ;; +esac + +archive="tmp/pks/pks-$target.tar.xz" +mkdir -p tmp/pks +curl --proto '=https' --tlsv1.2 -fsSL -o "$archive" \ + "https://github.com/rubyatscale/pks/releases/download/v$version/pks-$target.tar.xz" + +actual=$(shasum -a 256 "$archive" 2> /dev/null || sha256sum "$archive") +if [ "${actual%% *}" != "$sha256" ]; then + echo "$archive's sha256 is ${actual%% *}, not $sha256" >&2 + exit 1 +fi + +tar -xJf "$archive" -C tmp/pks --strip-components 1 "pks-$target/pks" +tmp/pks/pks --version diff --git a/lib/query_packwerk.rb b/lib/query_packwerk.rb index 66e2746..d94eade 100644 --- a/lib/query_packwerk.rb +++ b/lib/query_packwerk.rb @@ -9,15 +9,21 @@ # 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. +# for querying package.yml and package_todo.yml files. It also queries every +# reference between packages, allowed or not, which pks lists. # # See also: https://github.com/rubyatscale/parse_packwerk module QueryPackwerk autoload :Console, 'query_packwerk/console' autoload :ConsoleHelpers, 'query_packwerk/console_helpers' autoload :QueryInterface, 'query_packwerk/query_interface' + autoload :ConstantUse, 'query_packwerk/constant_use' + autoload :ConstantUses, 'query_packwerk/constant_uses' autoload :Violations, 'query_packwerk/violations' autoload :Violation, 'query_packwerk/violation' + autoload :References, 'query_packwerk/references' + autoload :Reference, 'query_packwerk/reference' + autoload :Pks, 'query_packwerk/pks' autoload :Packages, 'query_packwerk/packages' autoload :Package, 'query_packwerk/package' autoload :RuleRewriter, 'query_packwerk/rule_rewriter' @@ -30,7 +36,7 @@ module QueryPackwerk sig { params(name: String).returns(T.nilable(QueryPackwerk::Package)) } def package(name) - Packages.where(name: name).first + Packages.where(name: full_name(name)).first end # Get all violations where other packages access code from this package @@ -48,7 +54,7 @@ def todos_for(pack_name) end # Get where the violations occurred (where other packages access this package) - sig { params(pack_name: String).returns(T::Hash[String, T::Array[String]]) } + sig { params(pack_name: String).returns(T::Hash[String, T::Array[[String, String]]]) } def violation_sources_for(pack_name) violations_for(pack_name).sources_with_locations end @@ -77,17 +83,38 @@ def consumers(pack_name, threshold: 0) violations_for(pack_name).consumers(threshold: threshold) end + # Get all references from other packages to this package's constants, whether + # or not they break the packages' rules (i.e., how others use this package). + # These come from pks, which must be installed. + sig { params(pack_name: String).returns(QueryPackwerk::References) } + def references_for(pack_name) + QueryPackwerk::References.where(producing_pack: full_name(pack_name)) + end + + # Get all references from this package to other packages' constants, whether + # or not they break the packages' rules (i.e., how this package uses others). + # These come from pks too. + sig { params(pack_name: String).returns(QueryPackwerk::References) } + def references_from(pack_name) + QueryPackwerk::References.where(consuming_pack: full_name(pack_name)) + end + # In case anyone is still using shorthand for pack names sig { params(pack_name: String).returns(String) } def full_name(pack_name) - return pack_name if pack_name.match?(%r{\A(packs|components)/}) + return pack_name if pack_name.match?(%r{\A(packs|components)/}) || ParsePackwerk.find(pack_name) "packs/#{pack_name}" end - sig { void } + # nil rather than void, which the console shows as T::Private::Types::Void::VOID + sig { returns(NilClass) } def reload! + # ParsePackwerk keeps the packs it reads, along with their package_todo.yml files + ParsePackwerk.bust_cache! Packages.reload! Violations.reload! + References.reload! + nil end end diff --git a/lib/query_packwerk/console_helpers.rb b/lib/query_packwerk/console_helpers.rb index 4d8bb74..164c243 100644 --- a/lib/query_packwerk/console_helpers.rb +++ b/lib/query_packwerk/console_helpers.rb @@ -25,6 +25,10 @@ def help(type = nil) help_violation when 'violations' help_violations + when 'reference' + help_reference + when 'references' + help_references when 'package' help_package when 'packages' @@ -44,6 +48,8 @@ def help_help help # This help help_violation # Help for QueryPackwerk::Violation help_violations # Help for QueryPackwerk::Violations + help_reference # Help for QueryPackwerk::Reference + help_references # Help for QueryPackwerk::References help_package # Help for QueryPackwerk::Package help_packages # Help for QueryPackwerk::Packages HELP @@ -63,7 +69,7 @@ def help_violation .sources_with_locations # Returns array of [file:line, source] pairs for each violation .source_counts # Returns hash of how often each violation source occurs .anonymous_sources # Returns array of violation sources with arguments anonymized - .anonymous_sources_with_locations # Returns array of [file:line, source] pairs for each violation with arguments anonymized + .anonymous_sources_with_locations # Returns hash of anonymized source => [file:line] .count # Returns total number of violation occurrences HELP end @@ -78,16 +84,66 @@ def help_violations .sources # Returns hash of constant => source strings for all violations .sources_with_locations # Returns hash of constant => [file:line, source] pairs .source_counts # Returns hash of constant => {source => count} with occurrence counts - .sources_with_contexts # Returns hash of constant => [file:line, source] pairs for each violation with context + .sources_with_contexts # Returns hash of constant => {anonymized source => [file:line with the lines around it]} .anonymous_sources # Returns hash of constant => anonymized sources - .anonymous_sources_with_locations # Returns array of [file:line, source] pairs for each violation with arguments anonymized - .consumers(threshold) # Returns hash of consuming package names and violation counts - .producers(threshold) # Returns hash of producing package names and violation counts + .anonymous_sources_with_locations # Returns hash of constant => {anonymized source => [file:line]} + .consumers(threshold: 0) # Returns hash of consuming package names and violation counts + .producers(threshold: 0) # Returns hash of producing package names and violation counts .including_files(globs) # Returns new Violations filtered to include specific files .excluding_files(globs) # Returns new Violations filtered to exclude specific files HELP end + sig { returns(NilClass) } + def help_reference + puts <<~HELP + QueryPackwerk::Reference (lib/query_packwerk/reference.rb) + ------------------------------------------------ + .constant_name # Returns the referenced constant's name + .file # Returns the file that makes the reference + .line # Returns the line of the reference, or nil in an ERB template + .column # Returns the column of the reference in bytes from 0, or nil in an ERB template + .location # Returns file:line, or the file for an ERB template + .producing_pack # Returns the Package that defines the constant + .consuming_pack # Returns the Package whose file makes the reference + .violations # Returns the rules the reference breaks, with each one's type and whether it's strict and recorded + .violation_types # Returns the types of the rules the reference breaks + .unrecorded_violation_types # Returns the types of those rules that no package_todo.yml records, which pks check reports + .allowed? # Returns whether the reference breaks none of the rules + .sources # Returns array of AST nodes representing the reference + .sources_with_locations # Returns array of [file:line, source] pairs + .source_counts # Returns hash of how often each source occurs + .anonymous_sources # Returns array of sources with arguments anonymized + .anonymous_sources_with_locations # Returns hash of anonymized source => [file:line] + .count # Returns how many sources make the reference, which is more than one only in an ERB template + HELP + end + + sig { returns(NilClass) } + def help_references + puts <<~HELP + QueryPackwerk::References (lib/query_packwerk/references.rb) + -------------------------------------------------- + Every reference from a package to another package's constants, whether or not it breaks their rules. + These come from pks 0.7.0 or later (https://github.com/rubyatscale/pks), in bin/pks or on your PATH. + + References.all # Returns all references + References.where(conditions) # Returns new References filtered by conditions, as allowed: true, violation_types: 'privacy' or unrecorded: true + .raw_sources # Returns hash of constant => AST nodes for all references + .sources # Returns hash of constant => source strings for all references + .sources_with_locations # Returns hash of constant => [file:line, source] pairs + .source_counts # Returns hash of constant => {source => count} with occurrence counts + .sources_with_contexts # Returns hash of constant => {anonymized source => [file:line with the lines around it]} + .anonymous_sources # Returns hash of constant => anonymized sources + .anonymous_sources_with_locations # Returns hash of constant => {anonymized source => [file:line]} + .anonymous_source_counts # Returns hash of constant => {anonymized source => count} + .consumers(threshold: 0) # Returns hash of consuming package names and reference counts + .producers(threshold: 0) # Returns hash of producing package names and reference counts + .including_files(globs) # Returns new References filtered to include specific files + .excluding_files(globs) # Returns new References filtered to exclude specific files + HELP + end + sig { returns(NilClass) } def help_package puts <<~HELP @@ -104,6 +160,8 @@ def help_package .directory # Returns the package directory path .todos # Returns Violations where this package is the consumer .violations # Returns Violations where this package is the producer + .incoming_references # Returns References from other packages to this package (needs pks) + .outgoing_references # Returns References from this package to other packages (needs pks) .consumers # Returns Packages that consume this package .consumer_names # Returns array of names of consuming packages .consumer_counts # Returns hash of consumer package names and counts @@ -141,7 +199,12 @@ def help_query_packwerk Package Dependencies (how this package uses others): todos_for(pack_name) # Get all todos where this package accesses others + Package References, whether or not they break a rule (needs pks): + references_for(pack_name) # Get all references from others to this package + references_from(pack_name) # Get all references from this package to others + Violations.all # Get all violations + References.all # Get all references (needs pks) HELP end end diff --git a/lib/query_packwerk/constant_use.rb b/lib/query_packwerk/constant_use.rb new file mode 100644 index 0000000..afc6b3f --- /dev/null +++ b/lib/query_packwerk/constant_use.rb @@ -0,0 +1,241 @@ +# typed: strict +# frozen_string_literal: true + +module QueryPackwerk + # A pack's use of a constant that another pack defines, such as a Violation + # or a Reference. Shows the code that uses the constant: its sources, each the + # constant with any calls chained on it. Subclasses say which files the code + # is in and how to find the sources in each. + class ConstantUse + extend T::Sig + extend T::Helpers + + abstract! + + ALL_CAPS = /\A[A-Z_]+\z/ + RUNTIME_KEYS = %i[is_active_record is_constant].freeze + + sig { abstract.returns(String) } + def class_name; end + + sig { abstract.returns(T::Array[String]) } + def files; end + + sig { abstract.returns(QueryPackwerk::Package) } + def producing_pack; end + + sig { abstract.returns(QueryPackwerk::Package) } + def consuming_pack; end + + # The sources in one of the files, with the calls chained on each + sig { abstract.params(file_name: String).returns(T::Array[RuboCop::AST::Node]) } + def full_sources_in(file_name); end + + # The keys that where and pattern matching can ask for, with how to read each + sig { abstract.returns(T::Hash[Symbol, T.proc.params(arg0: T.untyped).returns(T.untyped)]) } + def query_keys; end + + sig { params(file_cache: QueryPackwerk::FileCache).void } + def initialize(file_cache:) + @file_cache = file_cache + @cache_loaded = T.let(false, T::Boolean) + end + + sig { params(headers: T::Boolean).void } + def load_cache!(headers: false) + return true if @cache_loaded + + @file_cache.load!(*T.unsafe(files), headers: headers) + @cache_loaded = true + end + + sig { params(cache: QueryPackwerk::FileCache).void } + def set_cache!(cache) + # Each query sets the cache again, and a cache it already has is still loaded + return if cache.equal?(@file_cache) + + @cache_loaded = false + @file_cache = cache + end + + sig { returns(Integer) } + def file_count + files.size + end + + # Whether or not any of its files match any provided globs + # + # 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| + files.any? do |file_name| + glob.is_a?(Regexp) ? glob.match?(file_name) : File.fnmatch?(glob, file_name) + end + end + end + + # All sources and their receiver chains across all of its files + sig { returns(T::Array[RuboCop::AST::Node]) } + def sources + load_cache! + + files.flat_map do |file_name| + full_sources_in(file_name) + end + end + + # Adds additional file and line number information to each source + sig { returns(T::Array[[String, String]]) } + def sources_with_locations + load_cache! + + files.flat_map do |file_name| + full_sources_in(file_name).map { |s| ["#{file_name}:#{s.loc.line}", s.source] } + end + end + + # Frequency of which each source occurs + sig { returns(T::Hash[String, Integer]) } + def source_counts + load_cache! + + sources = files.flat_map do |file_name| + full_sources_in(file_name).map(&:source) + end + + sources.tally + end + + # Sources that have had their arguments anonymized + sig { returns(T::Array[String]) } + def anonymous_sources + load_cache! + + files.flat_map do |file_name| + full_sources_in(file_name).map { |s| @file_cache.anonymize_arguments(s.source) } + end + end + + sig { returns(T::Hash[String, T::Array[String]]) } + def anonymous_sources_with_locations + load_cache! + + file_sources = files.flat_map do |file_name| + full_sources_in(file_name).map do |s| + ["#{file_name}:#{s.loc.line}", @file_cache.anonymize_arguments(s.source)] + end + end + + anonymous_source_groups = Hash.new { |h, source| h[source] = [] } + + file_sources.each_with_object(anonymous_source_groups) do |(location, source), groups| + groups[source] << location + end + end + + sig { params(start_offset: Integer, end_offset: Integer).returns(T::Hash[String, T::Array[String]]) } + def sources_with_contexts(start_offset: 3, end_offset: 3) + load_cache! + + file_sources = files.flat_map do |file_name| + lines = @file_cache.get_file(file_name).lines + + full_sources_in(file_name).map do |s| + line_number = s.loc.line + start_pos = [line_number - start_offset, 1].max + end_pos = [line_number + end_offset, lines.size].min + + location = "#{file_name}:#{s.loc.line} (L#{start_pos}..#{end_pos})" + context = lines[(start_pos - 1)...end_pos] + full_context = unindent((context || ['']).join) + + [@file_cache.anonymize_arguments(s.source), "> #{location}\n\n#{full_context}"] + end + end + + anonymous_source_groups = Hash.new { |h, source| h[source] = [] } + + file_sources.each_with_object(anonymous_source_groups) do |(anonymous_source, full_source), groups| + groups[anonymous_source] << full_source + end + end + + # Like the above source frequency, except grouped by method "shape" rather than + # exact arguments + sig { returns(T::Hash[String, Integer]) } + def anonymous_source_counts + anonymous_sources.tally + end + + # True count of uses, as a file can use the constant more than once. + sig { returns(Integer) } + def count + files.sum do |file_name| + full_sources_in(file_name).size + end + end + + # Reads only the keys asked for, since where asks every use in a collection + sig do + params(keys: T.nilable(T::Array[Symbol])).returns(T::Hash[Symbol, T.untyped]) + end + def deconstruct_keys(keys) + readers = query_keys + values = (keys || readers.keys).each_with_object({}) do |key, key_values| + reader = readers[key] + key_values[key] = reader.call(self) if reader + end + + runtime_values = runtime_keys(keys) + values.merge(keys.nil? ? runtime_values : runtime_values.slice(*T.unsafe(keys))) + end + + sig do + params(keys: T.nilable(T::Array[Symbol])).returns(T::Hash[Symbol, T.untyped]) + end + def runtime_keys(keys) + return {} unless defined?(Rails) + # These load the constant, so only when asked for + return {} unless keys.nil? || keys.intersect?(RUNTIME_KEYS) + + runtime_values = {} + + return { is_active_record: false, is_constant: false } unless Kernel.const_defined?(class_name) + + if keys.nil? || keys.include?(:is_active_record) + constant = Kernel.const_get(class_name) # rubocop:disable Sorbet/ConstantsFromStrings + + value = @file_cache.set( + :is_active_record, + key: class_name, + # Class#< gives nil, not false, for a class that isn't related + value: constant.is_a?(Class) && (constant < ApplicationRecord || false) + ) + + runtime_values[:is_active_record] = value + end + + if keys.nil? || keys.include?(:is_constant) + value = @file_cache.set( + :is_constant, + key: class_name, + value: class_name.split('::').last&.match?(ALL_CAPS) + ) + + runtime_values[:is_constant] = value + end + + runtime_values + end + + private + + sig { params(string: String).returns(String) } + def unindent(string) + # Multi-line match, this is intentional + min_space = string.scan(/^\s*/).min_by(&:length) + string.gsub(/^#{min_space}/, '') + end + end +end diff --git a/lib/query_packwerk/constant_uses.rb b/lib/query_packwerk/constant_uses.rb new file mode 100644 index 0000000..19c7b15 --- /dev/null +++ b/lib/query_packwerk/constant_uses.rb @@ -0,0 +1,275 @@ +# typed: strict +# frozen_string_literal: true + +require 'coderay' + +module QueryPackwerk + # The base of Violations and References, which are collections of ConstantUse + # objects, with methods that group their sources by the constant they use, + # and that say which packs use and define those constants. + class ConstantUses + extend T::Sig + extend T::Generic + extend T::Helpers + + abstract! + + Elem = type_member { { upper: QueryPackwerk::ConstantUse } } + + include Enumerable + include QueryInterface + + class << self + extend T::Sig + + # package_todo.yml and pks name a constant from the top, as ::Foo::Bar, so + # `constant_name: 'Foo::Bar'` would find nothing unless it's qualified too + sig do + params( + query_params: T.untyped, + query_fn: T.nilable(T.proc.params(arg0: T.untyped).returns(T::Boolean)) + ).returns(T.untyped) + end + def where(**query_params, &query_fn) + query_params[:constant_name] = qualify(query_params[:constant_name]) if query_params.key?(:constant_name) + + super + end + + private + + sig { params(name: T.untyped).returns(T.untyped) } + def qualify(name) + case name + when String then name.start_with?('::') ? name : "::#{name}" + when Array then name.map { |each_name| qualify(each_name) } + else name + end + end + end + + sig { override.returns(T::Array[Elem]) } + attr_reader :original_collection + + sig { returns(QueryPackwerk::FileCache) } + attr_reader :file_cache + + sig do + params( + original_collection: T::Array[Elem], + file_cache: QueryPackwerk::FileCache + ).void + end + def initialize(original_collection, file_cache: QueryPackwerk::FileCache.new) + @original_collection = original_collection + @file_cache = file_cache + + @original_collection.each do |use| + use.set_cache!(file_cache) + end + + @cache_loaded = T.let(false, T::Boolean) + @sources_loaded = T.let(false, T::Boolean) + end + + sig { void } + def load_cache! + return true if @cache_loaded + + warn "Prepopulating AST cache with #{file_count} files: " + start_time = Time.now + + @original_collection.each(&:load_cache!) + + finish_time = Time.now - start_time + warn '', "AST cache loaded in #{finish_time}" + @cache_loaded = true + end + + sig { void } + def load_sources! + return true if @sources_loaded + + unless @cache_loaded + load_cache! + warn + end + + warn "Prepopulating sources cache with #{count} #{self.class.name.to_s.split('::').last.to_s.downcase}" + start_time = Time.now + + # No dot for each use: parsing, which load_cache! dots, is the slow part + total_sources_loaded = @original_collection.sum { |use| use.sources.size } + + finish_time = Time.now - start_time + warn "Loaded #{total_sources_loaded} full sources in #{finish_time}" + + @sources_loaded = true + end + + sig { returns(Integer) } + def file_count + # Once for each file, which is parsed once however many uses are in it + @original_collection.flat_map(&:files).uniq.size + end + + sig { returns(T::Boolean) } + def empty? + @original_collection.empty? + end + + sig { returns(T::Boolean) } + def any? + !empty? + end + + # Gets all sources and their receiving chains grouped by the constant they use. + sig { returns(T::Hash[String, T::Array[RuboCop::AST::Node]]) } + def raw_sources + load_sources! + + deep_merge_groups(@original_collection) do |v| + [v.class_name, v.sources] + end + end + + # Gets all sources and their receiving chains grouped by the constant they use. + sig { returns(T::Hash[String, T::Array[String]]) } + def sources + load_sources! + + deep_merge_groups(@original_collection) { |v| [v.class_name, v.sources.map(&:source)] }.transform_values(&:uniq) + end + + # In addition to the above also provide the file location and line number along with the + # source. + sig { returns(T::Hash[String, T::Array[[String, String]]]) } + def sources_with_locations + load_sources! + + deep_merge_groups(@original_collection) { |v| [v.class_name, v.sources_with_locations] } + end + + # Instead of returning all instances of the source, count how often each occurs, with the option to + # provide a threshold to remove lower-occurring items. + sig { params(threshold: Integer).returns(T::Hash[String, T::Hash[String, Integer]]) } + def source_counts(threshold: 0) + load_sources! + + deep_merge_counts(@original_collection, threshold:) { |v| [v.class_name, v.source_counts] } + end + + # "Anonymize" the arguments of sources by replacing all arguments with underscores to get a look + # at the "shape" of a function rather than its exact call (i.e. `test(1, 2, 3)` becomes `test(_, _, _)`). + # + # This also removes extra spacing, line-breaks, cbase constant sigils, and other extra information to + # give a clearer view of a call's "shape". + sig { returns(T::Hash[String, T::Array[String]]) } + def anonymous_sources + load_sources! + + deep_merge_groups(@original_collection) { |v| [v.class_name, v.anonymous_sources] }.transform_values(&:uniq) + end + + sig { returns(T::Hash[String, T::Hash[String, T::Array[String]]]) } + def anonymous_sources_with_locations + load_sources! + + deep_merge_hash_groups(@original_collection) { |v| [v.class_name, v.anonymous_sources_with_locations] } + end + + sig do + params(start_offset: Integer, end_offset: Integer).returns(T::Hash[String, T::Hash[String, T::Array[String]]]) + end + def sources_with_contexts(start_offset: 3, end_offset: 3) + load_sources! + + deep_merge_hash_groups(@original_collection) do |v| + [v.class_name, v.sources_with_contexts(start_offset:, end_offset:)] + end + end + + sig { params(start_offset: Integer, end_offset: Integer).returns(String) } + def sources_with_contexts_report(start_offset: 3, end_offset: 3) + contexts = sources_with_contexts(start_offset:, end_offset:) + output = +'' + + contexts.each do |violated_constant, anonymized_sources| + heavy_underline = '=' * violated_constant.size + output << "#{violated_constant}\n#{heavy_underline}\n\n" + + anonymized_sources.each do |anonymized_source, full_contexts| + light_underline = '-' * anonymized_source.size + output << "#{anonymized_source}\n#{light_underline}\n\n" + + full_contexts.each do |context| + output << highlight_ruby(context) + output << "\n\n" + end + end + end + + output + end + + # Like the above source counts, but uses anonymized sources to give a clearer look at how often each + # "shape" of a method is called across a set of violations or references. + sig { params(threshold: Integer).returns(T::Hash[String, T::Hash[String, Integer]]) } + def anonymous_source_counts(threshold: 0) + load_sources! + + deep_merge_counts(@original_collection, threshold:) { |v| [v.class_name, v.anonymous_source_counts] } + end + + # Find which packs consume the constants these use + sig { params(threshold: Integer).returns(T::Hash[String, Integer]) } + def consumers(threshold: 0) + tallies = @original_collection.map { |v| v.consuming_pack.name }.tally + threshold_filter_sort(tallies, threshold:) + end + + # Find which packs produce the constants these use + sig { params(threshold: Integer).returns(T::Hash[String, Integer]) } + def producers(threshold: 0) + tallies = @original_collection.map { |v| v.producing_pack.name }.tally + threshold_filter_sort(tallies, threshold:) + end + + # Filter for those in files that match one of the provided file globs + sig { params(file_globs: T.any(String, Regexp)).returns(T.self_type) } + def including_files(*file_globs) + filtered = @original_collection.select do |use| + T.unsafe(use).includes_files?(*file_globs) # Sorbet hates splats + end + + self.class.new(filtered, file_cache: @file_cache) + end + + # Filter for those not in files that match one of the provided file globs + sig { params(file_globs: T.any(String, Regexp)).returns(T.self_type) } + def excluding_files(*file_globs) + filtered = @original_collection.reject do |use| + T.unsafe(use).includes_files?(*file_globs) # Sorbet hates splats + end + + self.class.new(filtered, file_cache: @file_cache) + end + + sig { returns(String) } + def inspect + arr = to_a.map(&:inspect) + if arr.empty? + "#<#{self.class.name} []>" + else + "#<#{self.class.name} [\n#{arr.join("\n")}\n]>" + end + end + + private + + sig { params(string: String).returns(String) } + def highlight_ruby(string) + CodeRay.encode(string, :ruby, :terminal) + end + end +end diff --git a/lib/query_packwerk/file_cache.rb b/lib/query_packwerk/file_cache.rb index 0951f11..e0f9394 100644 --- a/lib/query_packwerk/file_cache.rb +++ b/lib/query_packwerk/file_cache.rb @@ -25,8 +25,8 @@ def initialize # { [file_name, const_name] => [AST const nodes with full receiver chains] } @full_source_cache = T.let({}, T::Hash[T::Array[String], T::Array[RuboCop::AST::Node]]) - # { [file_name, const_name] => [anonymized sources] } - @anonymized_source_cache = T.let({}, T::Hash[T::Array[String], T::Array[String]]) + # { file_name => { line => [AST const and send nodes that start on it] } } + @line_cache = T.let({}, T::Hash[String, T::Hash[Integer, T::Array[RuboCop::AST::Node]]]) @anonymized_args_cache = T.let({}, T::Hash[String, String]) @@ -41,6 +41,9 @@ def load!(*file_names, headers: true) warn "Prepopulating AST cache with #{file_count} files: " if headers file_names.each do |f| + # A dot for each file parsed, rather than for each use of one + next if @file_ast_cache.key?(f) + get_file_ast(f) $stderr.print '.' end @@ -68,8 +71,9 @@ def get_file_ast(file_name) source = processed_source(ruby_source(file_name)) warn_unparseable(file_name, source) unless source.valid_syntax? - # The AST is also nil for source with no code, like a template that's only HTML - source.ast || ast_from('x') + # The AST is also nil for source with no code, like a template that's only HTML. + # `nil` stands in, as it has no constant or call that a reference's position could match. + source.ast || ast_from('nil') end end @@ -78,7 +82,7 @@ def get_file(file_name) @file_cache[file_name] ||= if (RUBY_FILE.match?(file_name) || ERB_FILE.match?(file_name)) && File.exist?(file_name) File.read(file_name) else - 'x' + '' end end @@ -93,7 +97,8 @@ def get_all_const_occurrences(file_name:, class_name:) relative_const_node = ast_from(class_name.delete_prefix('::')) @file_const_cache[const_key] = get_file_ast(file_name).each_node.select do |node| - next false unless node.const_type? + # Not the `Foo` in `Foo::Bar`, which packwerk and pks count only as a use of `Foo::Bar` + next false unless node.const_type? && !node.parent&.const_type? node == absolute_const_node || node == relative_const_node end @@ -114,6 +119,26 @@ def get_full_sources(file_name:, class_name:) end end + # Gets the full source of the reference that pks finds at a line and column. + # pks counts the column in bytes, where the parser counts characters. A + # reference that pks infers from an association, as `has_many :tacos` gives + # `::Taco`, is at the call, so the call is its source. + sig do + params(file_name: String, line: Integer, column: Integer).returns(T.nilable(RuboCop::AST::Node)) + end + def get_full_source_at(file_name:, line:, column:) + source_line = get_file_ast(file_name).source_range.source_buffer.source_lines[line - 1] + return unless source_line + + # Converted once, rather than for each node, as on a line with thousands of references + character_column = source_line.byteslice(0, column).to_s.length + nodes = nodes_by_line(file_name).fetch(line, []).select { |node| node.source_range.column == character_column } + + # The outermost constant, as `Foo::Bar` rather than the `Foo` in it + node = nodes.find(&:const_type?) || nodes.find(&:send_type?) + node && get_full_receiver_chain(node) + end + # Cleans up and anonymizes a source sig { params(source: String).returns(String) } def anonymize_arguments(source) @@ -122,25 +147,14 @@ def anonymize_arguments(source) .delete("\n").squeeze(' ').delete_prefix('::') end - # Get the full receiver chains on a constant, and anonymize their arguments - sig { params(file_name: String, class_name: String).returns(T::Array[String]) } - def get_full_anonymous_sources(file_name:, class_name:) - const_key = [file_name, class_name] - - return T.must(@anonymized_source_cache[const_key]) if @anonymized_source_cache.key?(const_key) + private - @anonymized_source_cache[const_key] = get_full_sources( - file_name: file_name, - class_name: class_name - ).map do |node| - get_full_receiver_chain(node) - .source - .then { |s| anonymize_arguments(s) } - end + # Constants and calls by the line they start on, outermost first, as `each_node` visits them + sig { params(file_name: String).returns(T::Hash[Integer, T::Array[RuboCop::AST::Node]]) } + def nodes_by_line(file_name) + @line_cache[file_name] ||= get_file_ast(file_name).each_node(:const, :send).group_by { |node| node.loc.line } end - private - # Blanks out everything in an ERB template except its Ruby code, so line numbers still match the template sig { params(file_name: String).returns(String) } def ruby_source(file_name) @@ -160,7 +174,7 @@ def warn_unparseable(file_name, source) error = source.diagnostics.find { |diagnostic| diagnostic.level == :error } location = error ? "#{file_name}:#{error.location.line} (#{error.message})" : file_name - warn "Could not parse #{location}, so its violations are not included" + warn "Could not parse #{location}, so its sources are not included" end # Turns a string into a Ruby AST @@ -170,8 +184,9 @@ def ast_from(string) end # We can find a constant, but by going up its parents we can find the full call chain. - # We check if each parent is a receiver of the child, giving us method calls and - # arguments on a constant as well as where it occurred. + # We check that each parent is a call on the child, giving us method calls and + # arguments on a constant as well as where it occurred. The receiver must be the + # child itself, not an equal node, as the first `Foo` is to the second in `Foo.bar(Foo)`. sig { params(node: RuboCop::AST::Node).returns(RuboCop::AST::Node) } def get_full_receiver_chain(node) return node unless node.const_type? @@ -179,7 +194,7 @@ def get_full_receiver_chain(node) current_node = T.let(node, RuboCop::AST::Node) while (parent_node = current_node.parent) - break unless parent_node.receiver == current_node + break unless parent_node.call_type? && parent_node.receiver.equal?(current_node) current_node = parent_node end diff --git a/lib/query_packwerk/package.rb b/lib/query_packwerk/package.rb index 5c89382..5b20d09 100644 --- a/lib/query_packwerk/package.rb +++ b/lib/query_packwerk/package.rb @@ -81,6 +81,20 @@ def violations end alias incoming_violations violations + # Returns references from other packages to this package's constants, + # whether or not they break the packages' rules. These come from pks. + sig { returns(QueryPackwerk::References) } + def incoming_references + QueryPackwerk::References.where(producing_pack: name) + end + + # Returns references from this package to other packages' constants, + # whether or not they break the packages' rules. These come from pks. + sig { returns(QueryPackwerk::References) } + def outgoing_references + QueryPackwerk::References.where(consuming_pack: name) + end + # Returns all packages that consume (depend on) this package sig { returns(QueryPackwerk::Packages) } def consumers diff --git a/lib/query_packwerk/packages.rb b/lib/query_packwerk/packages.rb index c6cbd8d..7f9299a 100644 --- a/lib/query_packwerk/packages.rb +++ b/lib/query_packwerk/packages.rb @@ -45,7 +45,7 @@ def where(**query_params, &query_fn) QueryPackwerk::Packages.new(super) end - sig { void } + sig { returns(NilClass) } def reload! @all = nil end diff --git a/lib/query_packwerk/pks.rb b/lib/query_packwerk/pks.rb new file mode 100644 index 0000000..41c8ebb --- /dev/null +++ b/lib/query_packwerk/pks.rb @@ -0,0 +1,74 @@ +# typed: strict +# frozen_string_literal: true + +require 'json' +require 'open3' + +module QueryPackwerk + # Runs pks (https://github.com/rubyatscale/pks), which lists every reference + # from one pack to a constant that another pack defines, whether or not it + # breaks the packs' rules. package_todo.yml files record only those that do. + module Pks + extend T::Sig + + class Error < StandardError; end + + # The first version with `list-references` + MINIMUM_VERSION = Gem::Version.new('0.7.0') + INSTALLATION_URL = 'https://github.com/rubyatscale/pks/blob/main/INSTALLATION.md' + + class << self + extend T::Sig + + # The references in the app, as `pks list-references` prints them: + # https://github.com/rubyatscale/pks/blob/main/schema/list-references-output.json + sig { returns(T::Array[T::Hash[String, T.untyped]]) } + def list_references + pks = executable + stdout, stderr, status = Open3.capture3(pks, 'list-references') + raise Error, "`#{pks} list-references` failed:\n#{stderr}" unless status.success? + + # Warnings, as about a file that pks can't parse + $stderr.print(stderr) + # pks prints UTF-8, whatever the locale. Freezing shares the strings of each file and constant. + JSON.parse(stdout.force_encoding(Encoding::UTF_8), freeze: true).fetch('references') + end + + # bin/pks, as in an app that pins a version of pks, or else pks on the PATH + sig { returns(String) } + def executable + # A DotSlash file saved from a release isn't executable at first. Running + # pks on the PATH instead would ignore the version the app pins. + raise Error, "bin/pks isn't executable. To run it, `chmod +x bin/pks`" if File.file?('bin/pks') && !File.executable?('bin/pks') + + candidates = ['bin', *ENV.fetch('PATH', '').split(File::PATH_SEPARATOR)].map do |directory| + File.expand_path('pks', directory) + end + executable = candidates.find { |candidate| File.file?(candidate) && File.executable?(candidate) } + unless executable + raise Error, "query_packwerk finds references with pks, which isn't in bin/pks or on your PATH. " \ + "To install it, see #{INSTALLATION_URL}" + end + + check_version(executable) + executable + end + + private + + # Checked each time pks runs, since checking out another branch can change bin/pks + sig { params(executable: String).void } + def check_version(executable) + # Only stdout, as a wrapper may warn on stderr first + stdout, stderr, status = Open3.capture3(executable, '--version') + version = stdout[/^pks (\S+)/, 1] + raise Error, "Could not get the version of #{executable}:\n#{stdout}#{stderr}" unless status.success? && version && Gem::Version.correct?(version) + + if Gem::Version.new(version) < MINIMUM_VERSION + raise Error, "query_packwerk finds references with `pks list-references`, which needs pks #{MINIMUM_VERSION} " \ + "or later, but #{executable} is pks #{version}. To upgrade it, see #{INSTALLATION_URL}" + end + end + end + end +end diff --git a/lib/query_packwerk/query_interface.rb b/lib/query_packwerk/query_interface.rb index a50330e..0853267 100644 --- a/lib/query_packwerk/query_interface.rb +++ b/lib/query_packwerk/query_interface.rb @@ -48,7 +48,19 @@ def size end alias length size - alias count size + + # Not an alias of size, so that an item or a block counts what it matches, as in Enumerable + sig do + params( + args: T.untyped, + block: T.nilable(T.proc.params(arg0: Elem).returns(T.untyped)) + ).returns(Integer) + end + def count(*args, &block) + return size if args.empty? && block.nil? + + T.unsafe(original_collection).count(*args, &block) + end # Extend the class with class extensions whenever this module is # included so we get both singleton and instance methods we need for @@ -99,13 +111,19 @@ def where(**query_params, &_query_fn) query_keys = query_params.keys all_values = all - accepted_keys = all_values.first.deconstruct_keys(nil).keys + # Keys are checked against the first value, and with none, nothing can match + first_value = all_values.first + return [] unless first_value + + accepted_keys = first_value.deconstruct_keys(nil).keys invalid_keys = query_keys - accepted_keys raise ArgumentError, "The following keys are invalid for querying: #{invalid_keys.join(', ')}" if invalid_keys.any? all_values.select do |value| - next yield(value) if block_given? + # A block narrows what the keys find, rather than taking their place + next false if block_given? && !yield(value) + next true if query_keys.empty? object_params = value.deconstruct_keys(query_keys) diff --git a/lib/query_packwerk/reference.rb b/lib/query_packwerk/reference.rb new file mode 100644 index 0000000..1767c1b --- /dev/null +++ b/lib/query_packwerk/reference.rb @@ -0,0 +1,166 @@ +# typed: strict +# frozen_string_literal: true + +module QueryPackwerk + # A reference from one pack to a constant that another pack defines, as pks + # lists it, whether or not it breaks the packs' rules. Unlike a Violation, + # which package_todo.yml records by file, it's at one place in one file, so + # its source is the code there, except in ERB, where pks gives no place. + class Reference < ConstantUse + extend T::Sig + + # A rule of the packs that a reference breaks + class Violation < T::Struct + # dependency, privacy, visibility, layer or folder_privacy + const :type, String + + # Whether the pack that enforces the rule is strict about it + const :strict, T::Boolean + + # Whether a package_todo.yml records the violation. `pks check` fails on + # one that isn't recorded. + const :recorded, T::Boolean + end + + sig { override.returns(QueryPackwerk::Package) } + attr_reader :producing_pack + + sig { override.returns(QueryPackwerk::Package) } + attr_reader :consuming_pack + + sig { returns(String) } + attr_reader :file + + # Counted from 1, and nil in ERB, where pks keeps no source positions + sig { returns(T.nilable(Integer)) } + attr_reader :line + + # Counted in bytes from 0, and nil when line is + sig { returns(T.nilable(Integer)) } + attr_reader :column + + sig { returns(T::Array[QueryPackwerk::Reference::Violation]) } + attr_reader :violations + + sig do + params( + constant_name: String, + file: String, + line: T.nilable(Integer), + column: T.nilable(Integer), + producing_pack: QueryPackwerk::Package, + consuming_pack: QueryPackwerk::Package, + violations: T::Array[QueryPackwerk::Reference::Violation], + file_cache: QueryPackwerk::FileCache + ).void + end + def initialize( + constant_name:, + file:, + line:, + column:, + producing_pack:, + consuming_pack:, + violations:, + file_cache: QueryPackwerk::FileCache.new + ) + super(file_cache:) + @constant_name = constant_name + @file = file + @line = line + @column = column + @producing_pack = producing_pack + @consuming_pack = consuming_pack + @violations = violations + end + + # Fully qualified, as `::Foo::Bar`, though the code may name it + # relatively, as `Bar` inside `module Foo` + sig { override.returns(String) } + def class_name + @constant_name + end + alias constant_name class_name + + sig { override.returns(T::Array[String]) } + def files + [file] + end + + # Whether the reference breaks none of the packs' rules + sig { returns(T::Boolean) } + def allowed? + violations.empty? + end + + sig { returns(T::Array[String]) } + def violation_types + violations.map(&:type) + end + + # The types of the rules it breaks that no package_todo.yml records, which `pks check` reports + sig { returns(T::Array[String]) } + def unrecorded_violation_types + violations.reject(&:recorded).map(&:type) + end + + sig { returns(String) } + def location + line ? "#{file}:#{line}" : file + end + + # The code at the reference's line and column. A reference that pks infers + # from an association is at the call, as `has_many :tacos` is for `::Taco`. + # In ERB, pks gives no position and lists a template's references to a + # constant once, so they're wherever the template names it. + sig { override.params(file_name: String).returns(T::Array[RuboCop::AST::Node]) } + def full_sources_in(file_name) + line = @line + column = @column + return @file_cache.get_full_sources(file_name: file_name, class_name: class_name) unless line && column + + [@file_cache.get_full_source_at(file_name: file_name, line: line, column: column)].compact + end + + QUERY_KEYS = T.let( + { + constant_name: lambda(&:class_name), + pack_name: ->(reference) { reference.producing_pack.name }, + file: lambda(&:file), + + # Which of the packs' rules the reference breaks, if any + allowed: lambda(&:allowed?), + violation_types: lambda(&:violation_types), + privacy: ->(reference) { reference.violation_types.include?('privacy') }, + dependency: ->(reference) { reference.violation_types.include?('dependency') }, + + # Which of those no package_todo.yml records + unrecorded: ->(reference) { reference.unrecorded_violation_types.any? }, + unrecorded_violation_types: lambda(&:unrecorded_violation_types), + + # Which pack defines the constant, and which pack's code uses it + consuming_pack: ->(reference) { reference.consuming_pack.name }, + producing_pack: ->(reference) { reference.producing_pack.name }, + + producing_owner: ->(reference) { reference.producing_pack.owner }, + consuming_owner: ->(reference) { reference.consuming_pack.owner }, + + # As for violations, "owner" is the producing pack's + owner: ->(reference) { reference.producing_pack.owner }, + owned: ->(reference) { reference.producing_pack.owner != Packages::UNOWNED } + }.freeze, + T::Hash[Symbol, T.proc.params(arg0: QueryPackwerk::Reference).returns(T.untyped)] + ) + + sig { override.returns(T::Hash[Symbol, T.proc.params(arg0: QueryPackwerk::Reference).returns(T.untyped)]) } + def query_keys + QUERY_KEYS + end + + sig { returns(String) } + def inspect + rules = allowed? ? 'allowed' : violation_types.join(', ') + "#<#{self.class.name} #{consuming_pack.name} -> #{class_name} (#{rules}) at #{location}>" + end + end +end diff --git a/lib/query_packwerk/references.rb b/lib/query_packwerk/references.rb new file mode 100644 index 0000000..89cad3a --- /dev/null +++ b/lib/query_packwerk/references.rb @@ -0,0 +1,82 @@ +# typed: strict +# frozen_string_literal: true + +module QueryPackwerk + # A collection of the references between packs that pks lists. It includes + # the references that break none of the packs' rules, which package_todo.yml + # files leave out, and has the same methods as Violations. + class References < ConstantUses + extend T::Sig + extend T::Generic + + Elem = type_member { { fixed: QueryPackwerk::Reference } } + + @all = T.let(nil, T.nilable(QueryPackwerk::References)) + + class << self + extend T::Sig + + # Every reference from one pack to a constant that another pack defines. + # The first call runs pks, which reads every file in the app. + sig { returns(QueryPackwerk::References) } + def all + return @all if @all + + file_cache = QueryPackwerk::FileCache.new + packages = T.let({}, T::Hash[String, QueryPackwerk::Package]) + pack = ->(name) { packages[name] ||= package(name) } + + references = Pks.list_references.map do |reference| + QueryPackwerk::Reference.new( + constant_name: reference.fetch('constant_name'), + file: reference.fetch('file'), + line: reference.fetch('line'), + column: reference.fetch('column'), + producing_pack: pack.call(reference.fetch('defining_pack_name')), + consuming_pack: pack.call(reference.fetch('referencing_pack_name')), + violations: reference.fetch('violations').map do |violation| + QueryPackwerk::Reference::Violation.new( + type: violation.fetch('violation_type'), + strict: violation.fetch('strict'), + recorded: violation.fetch('recorded') + ) + end, + file_cache: file_cache + ) + end + + @all = QueryPackwerk::References.new(references, file_cache: file_cache) + end + + # Wrap the interface `where` with this type + sig do + params( + query_params: T.untyped, # Array, or anything responding to `===`, which can't be typed + query_fn: T.nilable(T.proc.params(arg0: T.untyped).returns(T::Boolean)) + ).returns(QueryPackwerk::References) + end + def where(**query_params, &query_fn) + # all's file cache, so a file is parsed once for every query until reload! + QueryPackwerk::References.new(super(**query_params, &query_fn), file_cache: all.file_cache) + end + + # Forgets the references, so the next query runs pks again + sig { returns(NilClass) } + def reload! + @all = nil + end + + private + + sig { params(name: String).returns(QueryPackwerk::Package) } + def package(name) + yml = Pathname.new(name).join(ParsePackwerk::PACKAGE_YML_NAME) + # pks follows a symlink to a pack, where ParsePackwerk's default `**/` glob doesn't + original_package = ParsePackwerk.find(name) || (ParsePackwerk::Package.from(yml) if yml.file?) + raise Pks::Error, "pks lists references from or to #{name}, which has no package.yml" unless original_package + + QueryPackwerk::Package.new(original_package: original_package) + end + end + end +end diff --git a/lib/query_packwerk/rule_rewriter/rule_set_rewriter.rb b/lib/query_packwerk/rule_rewriter/rule_set_rewriter.rb index 6fdcc03..c610b73 100644 --- a/lib/query_packwerk/rule_rewriter/rule_set_rewriter.rb +++ b/lib/query_packwerk/rule_rewriter/rule_set_rewriter.rb @@ -13,7 +13,7 @@ class RuleSetRewriter sig { returns(RuboCop::ProcessedSource) } attr_reader :source - sig { returns(RuboCop::AST::Node) } + sig { returns(T.nilable(RuboCop::AST::Node)) } attr_reader :ast sig { returns(Parser::Source::TreeRewriter) } @@ -35,7 +35,9 @@ def initialize(string, rules: RULES) def process @rules.each do |rule_class| rule = rule_class.new(@rewriter) - @ast.each_node { |node| rule.process(node) } + # nil for a call with a heredoc argument, which doesn't parse without the + # heredoc's body on the lines after it, so its arguments are left as they are + @ast&.each_node { |node| rule.process(node) } end @rewriter diff --git a/lib/query_packwerk/violation.rb b/lib/query_packwerk/violation.rb index 3daf4e1..9d2735d 100644 --- a/lib/query_packwerk/violation.rb +++ b/lib/query_packwerk/violation.rb @@ -6,15 +6,13 @@ module QueryPackwerk # Provides methods to analyze violation details including source location, contextual # information, and code patterns. Facilitates both detailed and anonymized views of # dependency violations between packages. - class Violation + class Violation < ConstantUse extend T::Sig - ALL_CAPS = /\A[A-Z_]+\z/ - - sig { returns(QueryPackwerk::Package) } + sig { override.returns(QueryPackwerk::Package) } attr_reader :producing_pack - sig { returns(QueryPackwerk::Package) } + sig { override.returns(QueryPackwerk::Package) } attr_reader :consuming_pack sig do @@ -25,6 +23,7 @@ class Violation ).void end def initialize(original_violation:, consuming_pack:, file_cache: QueryPackwerk::FileCache.new) + super(file_cache:) @original_violation = original_violation @producing_pack = T.let( @@ -36,28 +35,6 @@ def initialize(original_violation:, consuming_pack:, file_cache: QueryPackwerk:: QueryPackwerk::Package.new(original_package: consuming_pack), QueryPackwerk::Package ) - - @file_cache = file_cache - @cache_loaded = T.let(false, T::Boolean) - end - - sig { params(headers: T::Boolean).void } - def load_cache!(headers: false) - return true if @cache_loaded - - @file_cache.load!(*T.unsafe(files), headers: headers) - @cache_loaded = true - end - - sig { params(cache: QueryPackwerk::FileCache).void } - def set_cache!(cache) - @cache_loaded = false - @file_cache = cache - end - - sig { returns(Integer) } - def file_count - files.size end # Forwarding original properties explicitly. @@ -72,221 +49,58 @@ def to_package_name @original_violation.to_package_name end - sig { returns(String) } + sig { override.returns(String) } def class_name @original_violation.class_name end - sig { returns(T::Array[String]) } + sig { override.returns(T::Array[String]) } def files @original_violation.files end - # Addon methods - - # Whether or not the files containing violations match any provided globs - # - # 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| - files.any? do |file_name| - glob.is_a?(Regexp) ? glob.match?(file_name) : File.fnmatch?(glob, file_name) - end - end - end - - # All sources and their receiver chains across all files this violation covers - sig { returns(T::Array[RuboCop::AST::Node]) } - def sources - load_cache! - - files.flat_map do |file_name| - @file_cache.get_full_sources(file_name: file_name, class_name: class_name) - end - end - - # Adds additional file and line number information to each source - sig { returns(T::Array[T.any(String, T::Array[RuboCop::AST::Node])]) } - def sources_with_locations - load_cache! - - files.flat_map do |file_name| - @file_cache - .get_full_sources(file_name: file_name, class_name: class_name) - .map { |s| ["#{file_name}:#{s.loc.line}", s.source] } - end - end - - # Frequency of which each source occurs - sig { returns(T::Hash[String, Integer]) } - def source_counts - load_cache! - - sources = files.flat_map do |file_name| - @file_cache - .get_full_sources(file_name: file_name, class_name: class_name) - .map(&:source) - end - - sources.tally + # package_todo.yml records a violation's files but not where in them it is, + # so its sources are wherever a file names the constant + sig { override.params(file_name: String).returns(T::Array[RuboCop::AST::Node]) } + def full_sources_in(file_name) + @file_cache.get_full_sources(file_name: file_name, class_name: class_name) end - # Sources that have had their arguments anonymized - sig { returns(T::Array[String]) } - def anonymous_sources - load_cache! - - files.flat_map do |file_name| - @file_cache - .get_full_anonymous_sources(file_name: file_name, class_name: class_name) - end - end - - # sig { returns(T::Array[T.any(String, T::Array[String])]) } - sig { returns(T.untyped) } - def anonymous_sources_with_locations - load_cache! - - file_sources = files.flat_map do |file_name| - @file_cache.get_full_sources(file_name: file_name, class_name: class_name).map do |s| - ["#{file_name}:#{s.loc.line}", @file_cache.anonymize_arguments(s.source)] - end - end - - anonymous_source_groups = Hash.new { |h, source| h[source] = [] } - - file_sources.each_with_object(anonymous_source_groups) do |(location, source), groups| - groups[source] << location - end - end - - sig { params(start_offset: Integer, end_offset: Integer).returns(T.untyped) } - def sources_with_contexts(start_offset: 3, end_offset: 3) - load_cache! - - file_sources = files.flat_map do |file_name| - lines = @file_cache.get_file(file_name).lines - - @file_cache.get_full_sources(file_name: file_name, class_name: class_name).map do |s| - line_number = s.loc.line - start_pos = [line_number - start_offset, 1].max - end_pos = [line_number + end_offset, lines.size].min - - location = "#{file_name}:#{s.loc.line} (L#{start_pos}..#{end_pos})" - context = lines[(start_pos - 1)...end_pos] - full_context = unindent((context || ['']).join) - - [@file_cache.anonymize_arguments(s.source), "> #{location}\n\n#{full_context}"] - end - end - - anonymous_source_groups = Hash.new { |h, source| h[source] = [] } - - file_sources.each_with_object(anonymous_source_groups) do |(anonymous_source, full_source), groups| - groups[anonymous_source] << full_source - end - end - - # Like the above source frequency, except grouped by method "shape" rather than - # exact arguments - sig { returns(T::Hash[String, Integer]) } - def anonymous_source_counts - anonymous_sources.tally - end - - # True count of violations, as there can be multiple of the same violation - # in a file. - sig { returns(Integer) } - def count - files.sum do |file_name| - @file_cache.get_all_const_occurrences( - file_name: file_name, - class_name: class_name - ).size - end - end - - sig do - params(keys: T.nilable(T::Array[Symbol])).returns(T::Hash[Symbol, T.untyped]) - end - def runtime_keys(keys) - return {} unless defined?(Rails) - - runtime_values = {} - - return { is_active_record: false, is_constant: false } unless Kernel.const_defined?(class_name) - - if keys.nil? || keys.include?(:is_active_record) - constant = Kernel.const_get(class_name) # rubocop:disable Sorbet/ConstantsFromStrings - - value = @file_cache.set( - :is_active_record, - key: class_name, - value: constant.is_a?(Class) && constant < ApplicationRecord - ) - - runtime_values[:is_active_record] = value - end - - if keys.nil? || keys.include?(:is_constant) - value = @file_cache.set( - :is_constant, - key: class_name, - value: class_name.split('::').last&.match?(ALL_CAPS) - ) - - runtime_values[:is_constant] = value - end - - runtime_values - end - - sig do - params(keys: T.nilable(T::Array[Symbol])).returns(T::Hash[Symbol, T.untyped]) - end - def deconstruct_keys(keys) - all_values = { - constant_name: class_name, - pack_name: to_package_name, + QUERY_KEYS = T.let( + { + constant_name: lambda(&:class_name), + pack_name: lambda(&:to_package_name), # Type related properties, including convenience boolean handlers - type: type, - privacy: type == 'privacy', - dependency: type == 'dependency', + type: lambda(&:type), + privacy: ->(violation) { violation.type == 'privacy' }, + dependency: ->(violation) { violation.type == 'dependency' }, # Reaching into which pack produced the violated constant, and # which consumes the violated constant. - consuming_pack: consuming_pack.name, - producing_pack: producing_pack.name, + consuming_pack: ->(violation) { violation.consuming_pack.name }, + producing_pack: ->(violation) { violation.producing_pack.name }, # Same, except for owners - producing_owner: producing_pack.owner, - consuming_owner: consuming_pack.owner, + producing_owner: ->(violation) { violation.producing_pack.owner }, + consuming_owner: ->(violation) { violation.consuming_pack.owner }, # So why is this "owner" implying producer? Because the # owner field of the violation is producer-oriented. - owner: producing_pack.owner, - owned: producing_pack.owner.nil?, - - **runtime_keys(keys) - } + owner: ->(violation) { violation.producing_pack.owner }, + owned: ->(violation) { violation.producing_pack.owner != Packages::UNOWNED } + }.freeze, + T::Hash[Symbol, T.proc.params(arg0: QueryPackwerk::Violation).returns(T.untyped)] + ) - keys.nil? ? all_values : all_values.slice(*T.unsafe(keys)) + sig { override.returns(T::Hash[Symbol, T.proc.params(arg0: QueryPackwerk::Violation).returns(T.untyped)]) } + def query_keys + QUERY_KEYS end sig { returns(String) } def inspect "#<#{self.class.name} #{consuming_pack.name} -> #{class_name} (#{type})>" end - - private - - sig { params(string: String).returns(String) } - def unindent(string) - # Multi-line match, this is intentional - min_space = string.scan(/^\s*/).min_by(&:length) - string.gsub(/^#{min_space}/, '') - end end end diff --git a/lib/query_packwerk/violations.rb b/lib/query_packwerk/violations.rb index 0e3ca38..64b5c3b 100644 --- a/lib/query_packwerk/violations.rb +++ b/lib/query_packwerk/violations.rb @@ -1,25 +1,17 @@ # typed: strict # frozen_string_literal: true -require 'coderay' - module QueryPackwerk # A collection class for managing and querying sets of Packwerk violations. # Provides aggregation, filtering, and analysis methods for violation data, # including source extraction, contextual reporting, and consumer relationship mapping. # Implements Enumerable and QueryInterface for flexible data manipulation. - class Violations + class Violations < ConstantUses extend T::Sig extend T::Generic Elem = type_member { { fixed: QueryPackwerk::Violation } } - include Enumerable - include QueryInterface - - sig { override.returns(T::Array[QueryPackwerk::Violation]) } - attr_reader :original_collection - @all = T.let(nil, T.nilable(QueryPackwerk::Violations)) class << self @@ -52,232 +44,14 @@ def all ).returns(QueryPackwerk::Violations) end def where(**query_params, &query_fn) - QueryPackwerk::Violations.new(super(**query_params, &query_fn)) + # all's file cache, so a file is parsed once for every query until reload! + QueryPackwerk::Violations.new(super(**query_params, &query_fn), file_cache: all.file_cache) end - sig { void } + sig { returns(NilClass) } def reload! @all = nil end end - - sig do - params( - original_collection: T::Array[QueryPackwerk::Violation], - file_cache: QueryPackwerk::FileCache - ).void - end - def initialize(original_collection, file_cache: QueryPackwerk::FileCache.new) - @original_collection = original_collection - @file_cache = file_cache - - @original_collection.each do |violation| - violation.set_cache!(file_cache) - end - - @cache_loaded = T.let(false, T::Boolean) - @sources_loaded = T.let(false, T::Boolean) - end - - sig { void } - def load_cache! - return true if @cache_loaded - - warn "Prepopulating AST cache with #{file_count} files: " - start_time = Time.now - - @original_collection.each(&:load_cache!) - - finish_time = Time.now - start_time - warn '', "AST cache loaded in #{finish_time}" - @cache_loaded = true - end - - sig { void } - def load_sources! - return true if @sources_loaded - - unless @cache_loaded - load_cache! - warn - end - - warn "Prepopulating sources cache with #{count} violations: " - start_time = Time.now - - total_sources_loaded = @original_collection.sum do |violation| - $stderr.print '.' - violation.sources.size - end - - finish_time = Time.now - start_time - warn "Loaded #{total_sources_loaded} full sources in #{finish_time}" - - @sources_loaded = true - end - - sig { returns(Integer) } - def file_count - @original_collection.sum(&:file_count) - end - - sig { returns(T::Boolean) } - def empty? - @original_collection.empty? - end - - sig { returns(T::Boolean) } - def any? - !empty? - end - - # Gets all sources and their receiving chains grouped by the constant they've violated. - sig { returns(T.untyped) } - def raw_sources - load_sources! - - deep_merge_groups(@original_collection) do |v| - [v.class_name, v.sources] - end - end - - # Gets all sources and their receiving chains grouped by the constant they've violated. - sig { returns(T::Hash[String, T::Array[String]]) } - def sources - load_sources! - - deep_merge_groups(@original_collection) { |v| [v.class_name, v.sources.map(&:source)] }.transform_values(&:uniq) - end - - # In addition to the above also provide the file location and line number along with the - # source. - sig { returns(T::Hash[String, T::Array[String]]) } - def sources_with_locations - load_sources! - - deep_merge_groups(@original_collection) { |v| [v.class_name, v.sources_with_locations] } - end - - # Instead of returning all instances of the source, count how often each occurs, with the option to - # provide a threshold to remove lower-occurring items. - sig { params(threshold: Integer).returns(T::Hash[String, T::Hash[String, Integer]]) } - def source_counts(threshold: 0) - load_sources! - - deep_merge_counts(@original_collection, threshold:) { |v| [v.class_name, v.source_counts] } - end - - # "Anonymize" the arguments of sources by replacing all arguments with underscores to get a look - # at the "shape" of a function rather than its exact call (i.e. `test(1, 2, 3)` becomes `test(_, _, _)`). - # - # This also removes extra spacing, line-breaks, cbase constant sigils, and other extra information to - # give a clearer view of a call's "shape". - sig { returns(T::Hash[String, T::Array[String]]) } - def anonymous_sources - load_sources! - - deep_merge_groups(@original_collection) { |v| [v.class_name, v.anonymous_sources] }.transform_values(&:uniq) - end - - sig { returns(T::Hash[String, T::Hash[String, T::Array[String]]]) } - def anonymous_sources_with_locations - load_sources! - - deep_merge_hash_groups(@original_collection) { |v| [v.class_name, v.anonymous_sources_with_locations] } - end - - sig do - params(start_offset: Integer, end_offset: Integer).returns(T::Hash[String, T::Hash[String, T::Array[String]]]) - end - def sources_with_contexts(start_offset: 3, end_offset: 3) - load_sources! - - deep_merge_hash_groups(@original_collection) do |v| - [v.class_name, v.sources_with_contexts(start_offset:, end_offset:)] - end - end - - sig { params(start_offset: Integer, end_offset: Integer).returns(String) } - def sources_with_contexts_report(start_offset: 3, end_offset: 3) - contexts = sources_with_contexts(start_offset:, end_offset:) - output = +'' - - contexts.each do |violated_constant, anonymized_sources| - heavy_underline = '=' * violated_constant.size - output << "#{violated_constant}\n#{heavy_underline}\n\n" - - anonymized_sources.each do |anonymized_source, full_contexts| - light_underline = '-' * anonymized_source.size - output << "#{anonymized_source}\n#{light_underline}\n\n" - - full_contexts.each do |context| - output << highlight_ruby(context) - output << "\n\n" - end - end - end - - output - end - - # Like the above source counts, but uses anonymized sources to give a clearer look at how often each - # "shape" of a method is called across a set of violations. - sig { params(threshold: Integer).returns(T::Hash[String, T::Hash[String, Integer]]) } - def anonymous_source_counts(threshold: 0) - load_sources! - - deep_merge_counts(@original_collection, threshold:) { |v| [v.class_name, v.anonymous_source_counts] } - end - - # Find which packs consume these violations - sig { params(threshold: Integer).returns(T::Hash[String, Integer]) } - def consumers(threshold: 0) - tallies = @original_collection.map { |v| v.consuming_pack.name }.tally - threshold_filter_sort(tallies, threshold:) - end - - # Find which packs produce these violations - sig { params(threshold: Integer).returns(T::Hash[String, Integer]) } - def producers(threshold: 0) - tallies = @original_collection.map { |v| v.producing_pack.name }.tally - threshold_filter_sort(tallies, threshold:) - end - - # Filter for violations which include one of the provided file globs - sig { params(file_globs: T.any(String, Regexp)).returns(QueryPackwerk::Violations) } - def including_files(*file_globs) - filtered_violations = @original_collection.select do |violation| - T.unsafe(violation).includes_files?(*file_globs) # Sorbet hates splats - end - - QueryPackwerk::Violations.new(filtered_violations) - end - - # Filter for violations which do not include one of the provided file globs - sig { params(file_globs: T.any(String, Regexp)).returns(QueryPackwerk::Violations) } - def excluding_files(*file_globs) - filtered_violations = @original_collection.reject do |violation| - T.unsafe(violation).includes_files?(*file_globs) # Sorbet hates splats - end - - QueryPackwerk::Violations.new(filtered_violations) - end - - sig { returns(String) } - def inspect - arr = to_a.map(&:inspect) - if arr.empty? - "#<#{self.class.name} []>" - else - "#<#{self.class.name} [\n#{arr.join("\n")}\n]>" - end - end - - private - - sig { params(string: String).returns(String) } - def highlight_ruby(string) - CodeRay.encode(string, :ruby, :terminal) - end end end diff --git a/sorbet/rbi/todo.rbi b/sorbet/rbi/todo.rbi index 3cc0c49..851c88d 100644 --- a/sorbet/rbi/todo.rbi +++ b/sorbet/rbi/todo.rbi @@ -19,4 +19,4 @@ module ActiveSupport::Multibyte::Chars; end module ActiveSupport::SafeBuffer; end module ActiveSupport::StringInquirer; end module ActiveSupport::TimeZone; end -module QueryPackwerk::Violation::ApplicationRecord; end +module QueryPackwerk::ConstantUse::ApplicationRecord; end diff --git a/spec/query_packwerk/file_cache_spec.rb b/spec/query_packwerk/file_cache_spec.rb index 2aae00d..a20ea5c 100644 --- a/spec/query_packwerk/file_cache_spec.rb +++ b/spec/query_packwerk/file_cache_spec.rb @@ -10,6 +10,14 @@ .map { |node| [node.loc.line, node.source] } end + describe '#load!' do + it 'prints a dot for each file it parses, and none for a file it has' do + write_file('app/models/account.rb', "class Account < ApplicationRecord\nend\n") + + expect { file_cache.load!('app/models/account.rb', 'app/models/account.rb', headers: false) }.to output('.').to_stderr + end + end + describe '#get_full_sources' do it 'finds references in the Ruby code of an ERB template, on their template lines' do write_file('app/views/invoices/show.html.erb', <<~ERB) @@ -81,7 +89,7 @@ expect do sources_in('app/views/invoices/show.html.erb') sources_in('app/views/invoices/show.html.erb', class_name: '::Billing::Payment') - end.to output(%r{\ACould not parse app/views/invoices/show\.html\.erb:4 \(.+\), so its violations are not included\n\z}) + end.to output(%r{\ACould not parse app/views/invoices/show\.html\.erb:4 \(.+\), so its sources are not included\n\z}) .to_stderr expect(sources_in('app/views/invoices/show.html.erb')).to eq([]) end @@ -101,5 +109,98 @@ expect(sources_in('Gemfile.lock')).to eq([]) end + + it 'gets the calls on each use of the constant, rather than on an equal use around it' do + write_file('app/models/account.rb', <<~RUBY) + class Account < ApplicationRecord + def merge = Billing::Invoice.merge(Billing::Invoice) + + def latest = Billing::Invoice.latest.then { Billing::Invoice.latest } + end + RUBY + + expect(sources_in('app/models/account.rb')).to eq( + [ + [2, 'Billing::Invoice.merge(Billing::Invoice)'], + [2, 'Billing::Invoice'], + [4, 'Billing::Invoice.latest.then'], + [4, 'Billing::Invoice.latest'] + ] + ) + end + + it 'finds no use of a constant where it is only the namespace of another' do + write_file('app/models/account.rb', <<~RUBY) + class Account < ApplicationRecord + def billing = Billing.for(self) + + def invoice = Billing::Invoice.for(self) + end + RUBY + + expect(sources_in('app/models/account.rb', class_name: '::Billing')).to eq([[2, 'Billing.for(self)']]) + end + end + + describe '#get_full_source_at' do + define_method(:source_at) do |line, column| + file_cache.get_full_source_at(file_name: 'app/models/account.rb', line: line, column: column)&.source + end + + before do + write_file('app/models/account.rb', <<~RUBY) + class Account < ApplicationRecord + has_many :invoices, dependent: :destroy + + def overdue_lines = Billing::Invoice::Line.overdue_for(self) + + def summary = "Échéance ⏰ \#{Billing::Invoice.due_for(self).count}" + end + RUBY + end + + it 'finds the outermost constant that starts there, with the calls chained on it' do + expect(source_at(4, 22)).to eq('Billing::Invoice::Line.overdue_for(self)') + end + + it 'counts the column in bytes, as pks does' do + expect(source_at(6, 34)).to eq('Billing::Invoice.due_for(self).count') + end + + it 'finds a constant that no call is chained on, as an argument or a superclass' do + write_file('app/models/account.rb', <<~RUBY) + class Account < ApplicationRecord + def latest_number = Billing::Invoice.last&.number + + def invoice?(record) = record.is_a?(Billing::Invoice) + end + RUBY + + expect(source_at(1, 16)).to eq('ApplicationRecord') + expect(source_at(2, 22)).to eq('Billing::Invoice.last&.number') + expect(source_at(4, 38)).to eq('Billing::Invoice') + end + + it 'finds the call when no constant starts there, as for an association' do + expect(source_at(2, 2)).to eq('has_many :invoices, dependent: :destroy') + end + + it 'finds nothing when nothing starts there' do + expect(source_at(4, 23)).to be_nil + end + + it 'finds the calls on the constant there, rather than on an equal one around it' do + write_file('app/models/account.rb', "def merge = Billing::Invoice.merge(Billing::Invoice)\n") + + expect(source_at(1, 35)).to eq('Billing::Invoice') + end + + it 'finds nothing in a file that does not exist or does not parse' do + write_file('app/models/broken.rb', "Billing::Invoice.find(\n") + + expect(file_cache.get_full_source_at(file_name: 'app/models/missing.rb', line: 1, column: 0)).to be_nil + expect { expect(file_cache.get_full_source_at(file_name: 'app/models/broken.rb', line: 1, column: 0)).to be_nil } + .to output(%r{\ACould not parse app/models/broken\.rb}).to_stderr + end end end diff --git a/spec/query_packwerk/package_spec.rb b/spec/query_packwerk/package_spec.rb index 6129f2f..d6f41d8 100644 --- a/spec/query_packwerk/package_spec.rb +++ b/spec/query_packwerk/package_spec.rb @@ -129,3 +129,30 @@ end end end + +RSpec.describe QueryPackwerk::Package do + include_context 'listed references' + + describe '#incoming_references' do + it "returns references from other packages to this package's constants" do + references = QueryPackwerk.package('packs/honey_production').incoming_references + + expect(references).to be_a(QueryPackwerk::References) + expect(references.count).to eq(8) + end + end + + describe '#outgoing_references' do + it "returns references from this package to other packages' constants" do + references = QueryPackwerk.package('packs/apiary_reports').outgoing_references + + expect(references).to be_a(QueryPackwerk::References) + expect(references.map(&:location)).to eq( + [ + 'packs/apiary_reports/app/services/apiary_report.rb:2', + 'packs/apiary_reports/app/services/apiary_report.rb:4' + ] + ) + end + end +end diff --git a/spec/query_packwerk/pks_spec.rb b/spec/query_packwerk/pks_spec.rb new file mode 100644 index 0000000..5b068c1 --- /dev/null +++ b/spec/query_packwerk/pks_spec.rb @@ -0,0 +1,164 @@ +# typed: false +# frozen_string_literal: true + +require 'shellwords' + +RSpec.describe QueryPackwerk::Pks do + # A stand-in for pks that reports a version and prints references + define_method(:write_pks) do |path, version: '0.7.0', version_output: "pks #{version}", references: [], warning: '', exit_status: 0| + write_file(path, <<~SH) + #!/bin/sh + printf '%s' #{Shellwords.escape(warning)} >&2 + if [ "$1" = --version ]; then + printf '%s\\n' #{Shellwords.escape(version_output)} + else + printf '%s\\n' #{Shellwords.escape(JSON.generate('references' => references))} + exit #{exit_status} + fi + SH + File.chmod(0o755, path) + end + + define_method(:with_path) do |*directories, &block| + original = ENV.fetch('PATH', nil) + ENV['PATH'] = directories.map { |directory| File.expand_path(directory) }.join(File::PATH_SEPARATOR) + block.call + ensure + ENV['PATH'] = original + end + + let(:reference) do + { + 'file' => 'packs/foo/app/services/foo.rb', + 'line' => 3, + 'column' => 4, + 'constant_name' => '::Bar', + 'referencing_pack_name' => 'packs/foo', + 'defining_pack_name' => 'packs/bar', + 'violations' => [] + } + end + + describe '.list_references' do + context 'with pks installed' do + include_context 'referenced packs' + + it 'lists the references that the referenced packs context says it does' do + begin + described_class.executable + rescue QueryPackwerk::Pks::Error => e + # CI installs pks with bin/install-pks, so there the check runs rather than skips + raise if ENV['CI'] + + skip "#{e.message}\nTo install it here, run bin/install-pks and put tmp/pks on your PATH." + end + + expect(described_class.list_references).to eq(listed_references) + end + end + + it 'runs bin/pks, rather than pks on the PATH' do + write_pks('bin/pks', references: [reference]) + write_pks('path/pks', references: []) + + with_path('path') do + expect(described_class.list_references).to eq([reference]) + end + end + + it 'runs pks on the PATH when there is no bin/pks' do + write_pks('path/pks', references: [reference]) + + with_path('empty', 'path') do + expect(described_class.list_references).to eq([reference]) + end + end + + it 'says to make bin/pks executable, rather than run pks on the PATH' do + write_file('bin/pks', "#!/bin/sh\n") + write_pks('path/pks', references: [reference]) + + with_path('path') do + expect { described_class.list_references }.to raise_error( + QueryPackwerk::Pks::Error, "bin/pks isn't executable. To run it, `chmod +x bin/pks`" + ) + end + end + + it 'passes on what pks warns about' do + write_pks('bin/pks', warning: "Couldn't parse packs/foo/app/models/foo.rb\n") + + expect { described_class.list_references }.to output("Couldn't parse packs/foo/app/models/foo.rb\n").to_stderr + end + + it 'gets the version from what pks prints, rather than from what it warns about' do + write_pks('bin/pks', references: [reference], warning: "warning: running pks from ~/.cargo/bin\n") + + expect { expect(described_class.list_references).to eq([reference]) }.to output.to_stderr + end + + it 'reads what pks prints as UTF-8, whatever the locale' do + reference['file'] = 'packs/foo/app/services/récolte.rb' + write_pks('bin/pks', references: [reference]) + original = Encoding.default_external + Encoding.default_external = Encoding::US_ASCII + + expect(described_class.list_references).to eq([reference]) + ensure + Encoding.default_external = original + end + + it 'says how to install pks when there is none' do + with_path('empty') do + expect { described_class.list_references }.to raise_error( + QueryPackwerk::Pks::Error, + "query_packwerk finds references with pks, which isn't in bin/pks or on your PATH. " \ + 'To install it, see https://github.com/rubyatscale/pks/blob/main/INSTALLATION.md' + ) + end + end + + it 'says to upgrade a version of pks without list-references' do + write_pks('bin/pks', version: '0.6.0') + + expect { described_class.list_references }.to raise_error( + QueryPackwerk::Pks::Error, + 'query_packwerk finds references with `pks list-references`, which needs pks 0.7.0 or later, ' \ + "but #{File.expand_path('bin/pks')} is pks 0.6.0. " \ + 'To upgrade it, see https://github.com/rubyatscale/pks/blob/main/INSTALLATION.md' + ) + end + + it 'gets the version when pks prints more after it, as a build might' do + write_pks('bin/pks', version_output: 'pks 0.7.0 (abc1234)', references: [reference]) + + expect(described_class.list_references).to eq([reference]) + end + + it 'says when it cannot get the version of pks' do + write_pks('bin/pks', version_output: "error: unexpected argument '--version'") + + expect { described_class.list_references }.to raise_error( + QueryPackwerk::Pks::Error, + "Could not get the version of #{File.expand_path('bin/pks')}:\nerror: unexpected argument '--version'\n" + ) + end + + it 'checks the version each time it runs pks, as bin/pks can change with the branch' do + write_pks('bin/pks') + described_class.list_references + write_pks('bin/pks', version: '0.6.0') + + expect { described_class.list_references }.to raise_error(QueryPackwerk::Pks::Error, /is pks 0\.6\.0\./) + end + + it 'says what went wrong when pks fails' do + write_pks('bin/pks', warning: "Error: No pack found 'packs/foo'", exit_status: 2) + + expect { described_class.list_references }.to raise_error( + QueryPackwerk::Pks::Error, + "`#{File.expand_path('bin/pks')} list-references` failed:\nError: No pack found 'packs/foo'" + ) + end + end +end diff --git a/spec/query_packwerk/query_interface_spec.rb b/spec/query_packwerk/query_interface_spec.rb index 9c2213a..29f7004 100644 --- a/spec/query_packwerk/query_interface_spec.rb +++ b/spec/query_packwerk/query_interface_spec.rb @@ -105,12 +105,33 @@ class << self it 'can check for multiple potential Array matches' do expect(people.where(hobbies: %w[piano fencing]).count).to eq(2) end + + it 'can narrow what conditions find with a block' do + expect(people.where(adult: true) { |a_person| a_person.sex == 'male' }.map(&:name)).to eq(['Bob']) + end + + it 'can find people with only a block' do + expect(people.where { |a_person| a_person.sex == 'male' }.map(&:name)).to eq(%w[Bob Kai]) + end + + context 'when there are no people' do + let(:all_people) { [] } + + it 'finds nobody' do + expect(people.where(adult: true).count).to eq(0) + end + end end describe '#count' do it 'can get the count/size/length of a query' do expect(people.all.count).to eq(4) end + + it 'counts what an item or a block matches, as Enumerable does' do + expect(people.all.count { |a_person| a_person.age >= 18 }).to eq(3) + expect(people.all.count(all_people.first)).to eq(1) + end end describe '#each' do diff --git a/spec/query_packwerk/reference_spec.rb b/spec/query_packwerk/reference_spec.rb new file mode 100644 index 0000000..096529c --- /dev/null +++ b/spec/query_packwerk/reference_spec.rb @@ -0,0 +1,199 @@ +# typed: false +# frozen_string_literal: true + +RSpec.describe QueryPackwerk::Reference do + include_context 'listed references' + + define_method(:reference_at) do |location| + QueryPackwerk::References.all.find { |reference| reference.location == location } + end + + describe 'a reference that pks lists' do + subject(:reference) { reference_at('packs/hive_dashboard/app/services/beekeeping/apiary_status.rb:4') } + + it 'says where it is and which packs it is between' do + expect(reference).to have_attributes( + constant_name: '::HoneyProductionNectar', + class_name: '::HoneyProductionNectar', + file: 'packs/hive_dashboard/app/services/beekeeping/apiary_status.rb', + files: ['packs/hive_dashboard/app/services/beekeeping/apiary_status.rb'], + line: 4, + column: 6 + ) + expect(reference.consuming_pack.name).to eq('packs/hive_dashboard') + expect(reference.producing_pack.name).to eq('packs/honey_production') + end + + it 'says which rules it breaks' do + expect(reference.violations).to contain_exactly( + an_instance_of(described_class::Violation).and(have_attributes(type: 'privacy', strict: false, recorded: true)) + ) + expect(reference.violation_types).to eq(['privacy']) + expect(reference).not_to be_allowed + end + + it 'finds its source where pks says it is' do + expect(reference.sources_with_locations).to eq( + [['packs/hive_dashboard/app/services/beekeeping/apiary_status.rb:4', '::HoneyProductionNectar.as_of_now.find_by(apiary_id: 1)']] + ) + expect(reference.anonymous_sources).to eq(['HoneyProductionNectar.as_of_now.find_by(apiary_id: _)']) + expect(reference.count).to eq(1) + end + + it 'can be queried' do + expect(reference.deconstruct_keys(nil)).to include( + constant_name: '::HoneyProductionNectar', + pack_name: 'packs/honey_production', + file: 'packs/hive_dashboard/app/services/beekeeping/apiary_status.rb', + allowed: false, + violation_types: ['privacy'], + privacy: true, + dependency: false, + consuming_pack: 'packs/hive_dashboard', + producing_pack: 'packs/honey_production', + producing_owner: 'Honey', + consuming_owner: 'Unowned', + owner: 'Honey', + owned: true + ) + end + + it 'can be inspected' do + expect(reference.inspect).to eq( + '# ::HoneyProductionNectar (privacy) ' \ + 'at packs/hive_dashboard/app/services/beekeeping/apiary_status.rb:4>' + ) + end + end + + describe 'a reference in a file that names its constant elsewhere too' do + subject(:reference) { reference_at('packs/apiary_reports/app/services/apiary_report.rb:2') } + + it 'has only the source where pks says it is' do + expect(reference.count).to eq(1) + expect(reference.source_counts).to eq('HoneyProductionNectar.count' => 1) + expect(reference.anonymous_sources_with_locations).to eq( + 'HoneyProductionNectar.count' => ['packs/apiary_reports/app/services/apiary_report.rb:2'] + ) + expect(reference.sources_with_contexts(start_offset: 0, end_offset: 0)).to eq( + 'HoneyProductionNectar.count' => [ + "> packs/apiary_reports/app/services/apiary_report.rb:2 (L2..2)\n\ndef nectar_count = HoneyProductionNectar.count\n" + ] + ) + end + + it 'says which rules it breaks, and which of them a package_todo.yml records' do + expect(reference.violations.map { |violation| [violation.type, violation.recorded] }).to eq( + [['dependency', true], ['privacy', false]] + ) + expect(reference.unrecorded_violation_types).to eq(['privacy']) + expect(reference.deconstruct_keys(%i[violation_types dependency privacy])).to eq( + violation_types: %w[dependency privacy], dependency: true, privacy: true + ) + end + end + + it 'says whether the pack that enforces a rule is strict about it' do + listed_references.first['violations'] = [{ 'violation_type' => 'privacy', 'strict' => true, 'recorded' => false }] + + expect(reference_at('app/services/apiary_summary.rb:2').violations.map(&:strict)).to eq([true]) + end + + context 'when the app is loaded with Rails' do + before do + stub_const('Rails', Module.new) + application_record = stub_const('ApplicationRecord', Class.new) + stub_const('HoneyProductionNectar', Class.new(application_record)) + stub_const('ApiaryReportCard', Class.new) + end + + it 'says whether its constant is an Active Record model' do + expect( + [ + 'packs/apiary_reports/app/services/apiary_report.rb:2', + 'packs/hive_dashboard/app/services/report_link.rb:2', + 'app/services/apiary_summary.rb:2' + ].map { |location| reference_at(location).deconstruct_keys([:is_active_record]).fetch(:is_active_record) } + ).to eq([true, false, false]) + end + + it 'looks its constant up only for a query that asks about it' do + allow(Kernel).to receive(:const_defined?).and_call_original + + expect(QueryPackwerk::References.where(allowed: true).count).to eq(2) + # Once, as where checks the query's keys against the first reference's + expect(Kernel).to have_received(:const_defined?).once + end + end + + it 'is allowed when it breaks no rule' do + reference = reference_at('packs/hive_dashboard/app/services/beekeeping/apiary_status.rb:12') + + expect(reference).to be_allowed + expect(reference.inspect).to eq( + '# ::Beekeeping::Nectars (allowed) ' \ + 'at packs/hive_dashboard/app/services/beekeeping/apiary_status.rb:12>' + ) + end + + it 'finds a constant that the code names relatively' do + reference = reference_at('packs/hive_dashboard/app/services/beekeeping/apiary_status.rb:8') + + expect(reference.constant_name).to eq('::Beekeeping::NectarSource') + expect(reference.sources.map(&:source)).to eq(['NectarSource.where(apiary_id: 1).pluck(:name)']) + end + + it 'finds a constant after multibyte characters on its line, where pks counts bytes' do + reference = reference_at('packs/apiary_reports/app/services/apiary_report.rb:4') + + expect(reference.column).to eq(31) + expect(reference.sources.map(&:source)).to eq(['HoneyProductionNectar.model_name.human']) + end + + it 'finds the association that pks infers a reference from' do + reference = reference_at('packs/hive_dashboard/app/models/hive.rb:2') + + expect(reference.sources.map(&:source)).to eq(['has_many :honey_production_nectars, dependent: :destroy']) + expect(reference.anonymous_sources).to eq(['has_many _, dependent: _']) + end + + it 'finds nothing when the code at its position has changed since pks ran' do + write_file('packs/hive_dashboard/app/models/hive.rb', "class Hive < ApplicationRecord\nend\n") + + reference = reference_at('packs/hive_dashboard/app/models/hive.rb:2') + + expect(reference.sources).to eq([]) + expect(reference.count).to eq(0) + end + + context 'when the reference is in an ERB template' do + subject(:reference) { reference_at('packs/hive_dashboard/app/views/apiaries/show.html.erb') } + + it 'has no position' do + expect(reference).to have_attributes(line: nil, column: nil, location: 'packs/hive_dashboard/app/views/apiaries/show.html.erb') + end + + it 'finds every place the template names the constant, which pks lists once' do + expect(reference.sources_with_locations).to eq( + [ + ['packs/hive_dashboard/app/views/apiaries/show.html.erb:1', 'HoneyProductionNectar.as_of_now.exists?(apiary_id: @apiary.id)'], + ['packs/hive_dashboard/app/views/apiaries/show.html.erb:2', 'HoneyProductionNectar.as_of_now.find_by(apiary_id: @apiary.id).volume'] + ] + ) + expect(reference.count).to eq(2) + end + + it 'shows context from the template' do + expect(reference.sources_with_contexts(start_offset: 0, end_offset: 0)).to eq( + 'HoneyProductionNectar.as_of_now.exists?(apiary_id: _)' => [ + "> packs/hive_dashboard/app/views/apiaries/show.html.erb:1 (L1..1)\n\n" \ + "<% if HoneyProductionNectar.as_of_now.exists?(apiary_id: @apiary.id) %>\n" + ], + 'HoneyProductionNectar.as_of_now.find_by(apiary_id: _).volume' => [ + "> packs/hive_dashboard/app/views/apiaries/show.html.erb:2 (L2..2)\n\n" \ + "

<%= HoneyProductionNectar.as_of_now.find_by(apiary_id: @apiary.id).volume %>

\n" + ] + ) + end + end +end diff --git a/spec/query_packwerk/references_spec.rb b/spec/query_packwerk/references_spec.rb new file mode 100644 index 0000000..96db39c --- /dev/null +++ b/spec/query_packwerk/references_spec.rb @@ -0,0 +1,222 @@ +# typed: false +# frozen_string_literal: true + +RSpec.describe QueryPackwerk::References do + include_context 'listed references' + + describe '.all' do + it 'has each reference that pks lists' do + expect(described_class.all.map(&:location)).to eq( + [ + 'app/services/apiary_summary.rb:2', + 'packs/apiary_reports/app/services/apiary_report.rb:2', + 'packs/apiary_reports/app/services/apiary_report.rb:4', + 'packs/hive_dashboard/app/models/hive.rb:2', + 'packs/hive_dashboard/app/services/beekeeping/apiary_status.rb:4', + 'packs/hive_dashboard/app/services/beekeeping/apiary_status.rb:8', + 'packs/hive_dashboard/app/services/beekeeping/apiary_status.rb:12', + 'packs/hive_dashboard/app/services/report_link.rb:2', + 'packs/hive_dashboard/app/views/apiaries/show.html.erb' + ] + ) + end + + it 'runs pks once' do + 2.times { described_class.all } + + expect(QueryPackwerk::Pks).to have_received(:list_references).once + end + + it 'runs pks again after a reload' do + described_class.all + described_class.reload! + described_class.all + + expect(QueryPackwerk::Pks).to have_received(:list_references).twice + end + + it 'says when pks lists a pack that has no package.yml' do + listed_references.first['referencing_pack_name'] = 'packs/gone' + + expect { described_class.all }.to raise_error( + QueryPackwerk::Pks::Error, + 'pks lists references from or to packs/gone, which has no package.yml' + ) + end + + it 'finds a pack that a symlink points to, which ParsePackwerk does not' do + Dir.mktmpdir do |shared_pack| + File.write(File.join(shared_pack, 'package.yml'), "enforce_dependencies: true\n") + File.symlink(shared_pack, 'packs/wax_reports') + listed_references.first['referencing_pack_name'] = 'packs/wax_reports' + + expect(ParsePackwerk.find('packs/wax_reports')).to be_nil + expect(described_class.all.first.consuming_pack.name).to eq('packs/wax_reports') + end + end + end + + describe '.where' do + it 'finds the references that break no rule' do + expect(described_class.where(allowed: true).map(&:location)).to eq( + [ + 'app/services/apiary_summary.rb:2', + 'packs/hive_dashboard/app/services/beekeeping/apiary_status.rb:12' + ] + ) + end + + it 'finds the references that break a rule' do + expect(described_class.where(violation_types: 'dependency').map(&:location)).to eq( + [ + 'packs/apiary_reports/app/services/apiary_report.rb:2', + 'packs/apiary_reports/app/services/apiary_report.rb:4', + 'packs/hive_dashboard/app/services/report_link.rb:2' + ] + ) + end + + it 'finds the references that break a rule no package_todo.yml records' do + expect(described_class.where(unrecorded: true).map(&:location)).to eq( + [ + 'packs/apiary_reports/app/services/apiary_report.rb:2', + 'packs/apiary_reports/app/services/apiary_report.rb:4', + 'packs/hive_dashboard/app/models/hive.rb:2', + 'packs/hive_dashboard/app/services/beekeeping/apiary_status.rb:8', + 'packs/hive_dashboard/app/services/report_link.rb:2' + ] + ) + expect(described_class.where(unrecorded_violation_types: 'dependency').map(&:location)).to eq( + ['packs/hive_dashboard/app/services/report_link.rb:2'] + ) + end + + it 'finds the references to a pack without an owner' do + expect(described_class.where(owned: false).map(&:constant_name)).to eq(['::ApiaryReportCard']) + end + + it 'finds the references to constants named with or without their leading ::' do + expect(described_class.where(constant_name: %w[Beekeeping::Nectars ::ApiaryReportCard]).map(&:location)).to eq( + [ + 'app/services/apiary_summary.rb:2', + 'packs/hive_dashboard/app/services/beekeeping/apiary_status.rb:12', + 'packs/hive_dashboard/app/services/report_link.rb:2' + ] + ) + end + + it 'finds the references from a pack' do + expect(described_class.where(consuming_pack: '.').map(&:constant_name)).to eq(['::Beekeeping::Nectars']) + end + + it 'finds nothing when pks lists no references' do + listed_references.clear + + expect(described_class.where(producing_pack: 'packs/honey_production')).to be_empty + end + + it 'keeps the files that all parses, so each query parses a file only once' do + all = described_class.all + + expect(described_class.where(allowed: true).file_cache).to equal(all.file_cache) + expect(all.including_files('app/**').file_cache).to equal(all.file_cache) + expect(all.excluding_files('app/**').file_cache).to equal(all.file_cache) + end + end + + describe '#load_sources!' do + it 'prints a dot for each file it parses, rather than for each reference' do + references = described_class.where(producing_pack: 'packs/honey_production') + + expect { references.load_sources! }.to output( + /\APrepopulating AST cache with 5 files: \n\.{5}\nAST cache loaded in \S+\nPrepopulating sources cache with 8 references\nLoaded 9 full sources in \S+\n\z/ + ).to_stderr + end + end + + describe '#file_count' do + it 'counts each file once, however many references are in it' do + expect(described_class.all.file_count).to eq(6) + end + end + + describe '#sources_with_locations' do + it 'gets the sources of each constant, where pks finds them' do + expect(described_class.all.sources_with_locations).to eq( + { + '::Beekeeping::Nectars' => [ + ['app/services/apiary_summary.rb:2', 'Beekeeping::Nectars.status_for(apiary_id: 1)'], + ['packs/hive_dashboard/app/services/beekeeping/apiary_status.rb:12', 'Nectars.status_for(apiary_id: 1)'] + ], + '::HoneyProductionNectar' => [ + ['packs/apiary_reports/app/services/apiary_report.rb:2', 'HoneyProductionNectar.count'], + ['packs/apiary_reports/app/services/apiary_report.rb:4', 'HoneyProductionNectar.model_name.human'], + ['packs/hive_dashboard/app/models/hive.rb:2', 'has_many :honey_production_nectars, dependent: :destroy'], + ['packs/hive_dashboard/app/services/beekeeping/apiary_status.rb:4', '::HoneyProductionNectar.as_of_now.find_by(apiary_id: 1)'], + ['packs/hive_dashboard/app/views/apiaries/show.html.erb:1', 'HoneyProductionNectar.as_of_now.exists?(apiary_id: @apiary.id)'], + ['packs/hive_dashboard/app/views/apiaries/show.html.erb:2', 'HoneyProductionNectar.as_of_now.find_by(apiary_id: @apiary.id).volume'] + ], + '::Beekeeping::NectarSource' => [ + ['packs/hive_dashboard/app/services/beekeeping/apiary_status.rb:8', 'NectarSource.where(apiary_id: 1).pluck(:name)'] + ], + '::ApiaryReportCard' => [ + ['packs/hive_dashboard/app/services/report_link.rb:2', 'ApiaryReportCard.new'] + ] + } + ) + end + end + + describe '#anonymous_source_counts' do + it "shows how a pack's constants are used where the rules allow it" do + allowed = described_class.where(producing_pack: 'packs/honey_production', allowed: true) + + expect(allowed.anonymous_source_counts).to eq( + '::Beekeeping::Nectars' => { + 'Beekeeping::Nectars.status_for(apiary_id: _)' => 1, + 'Nectars.status_for(apiary_id: _)' => 1 + } + ) + end + end + + describe '#consumers' do + it 'counts the references from each pack' do + expect(described_class.all.consumers).to eq('packs/hive_dashboard' => 6, 'packs/apiary_reports' => 2, '.' => 1) + end + end + + describe '#producers' do + it 'counts the references to each pack' do + expect(described_class.all.producers).to eq('packs/honey_production' => 8, 'packs/apiary_reports' => 1) + end + end + + describe '#including_files' do + it 'finds the references in files that match' do + references = described_class.all.including_files('packs/apiary_reports/**') + + expect(references).to be_a(described_class) + expect(references.map(&:location)).to eq( + [ + 'packs/apiary_reports/app/services/apiary_report.rb:2', + 'packs/apiary_reports/app/services/apiary_report.rb:4' + ] + ) + end + end + + describe '#excluding_files' do + it 'finds the references in files that do not match' do + references = described_class.all.excluding_files('packs/hive_dashboard/**', 'app/**') + + expect(references).to be_a(described_class) + expect(references.map(&:location)).to eq( + [ + 'packs/apiary_reports/app/services/apiary_report.rb:2', + 'packs/apiary_reports/app/services/apiary_report.rb:4' + ] + ) + end + end +end diff --git a/spec/query_packwerk/rule_rewriter_spec.rb b/spec/query_packwerk/rule_rewriter_spec.rb index ca4c5c3..64aad99 100644 --- a/spec/query_packwerk/rule_rewriter_spec.rb +++ b/spec/query_packwerk/rule_rewriter_spec.rb @@ -66,4 +66,10 @@ RUBY end end + + context 'With a heredoc argument' do + it 'leaves the source as it is, since it does not parse without the heredoc body after it' do + expect(described_class.rewrite('Model.execute(1, <<~SQL)')).to eq('Model.execute(1, <<~SQL)') + end + end end diff --git a/spec/query_packwerk/violations_spec.rb b/spec/query_packwerk/violations_spec.rb index 3a812d3..28a01a1 100644 --- a/spec/query_packwerk/violations_spec.rb +++ b/spec/query_packwerk/violations_spec.rb @@ -16,6 +16,16 @@ expect(violations).to eq(['::Beekeeping::HoneyProduction::Nectars']) end + + it 'finds a constant named without its leading ::' do + violations = described_class.where(constant_name: 'Beekeeping::HoneyProduction::Nectars').map(&:class_name) + + expect(violations).to eq(['::Beekeeping::HoneyProduction::Nectars']) + end + + it 'keeps the files that all parses, so each query parses a file only once' do + expect(described_class.where(privacy: true).file_cache).to equal(described_class.all.file_cache) + end end describe '#empty? and #any?' do @@ -97,3 +107,16 @@ def up end end end + +RSpec.describe QueryPackwerk::Violations do + include_context 'referenced packs' + + describe '.where' do + it 'finds the violations of a pack with an owner' do + expect(described_class.where(owned: true).map { |violation| violation.consuming_pack.name }).to contain_exactly( + 'packs/hive_dashboard', 'packs/apiary_reports' + ) + expect(described_class.where(owned: false)).to be_empty + end + end +end diff --git a/spec/query_packwerk_spec.rb b/spec/query_packwerk_spec.rb index c96bf31..0f6d8f5 100644 --- a/spec/query_packwerk_spec.rb +++ b/spec/query_packwerk_spec.rb @@ -9,6 +9,10 @@ expect(described_class.package(package_name)).to be_a(QueryPackwerk::Package) end + it 'gets a package by its name in packs/, as violations_for does' do + expect(described_class.package('honey_production')&.name).to eq('packs/honey_production') + end + it 'returns nil if the package does not exist' do expect(described_class.package('not_here')).to be_nil end @@ -144,3 +148,78 @@ end end end + +RSpec.describe QueryPackwerk do + include_context 'listed references' + + describe '#references_for' do + it "gets all references from other packs to a pack's constants" do + references = described_class.references_for('honey_production') + + expect(references).to be_a(QueryPackwerk::References) + expect(references.count).to eq(8) + end + end + + describe '#references_from' do + it "gets all references from a pack to other packs' constants" do + expect(described_class.references_from('hive_dashboard').map(&:constant_name).tally).to eq( + '::HoneyProductionNectar' => 3, + '::Beekeeping::NectarSource' => 1, + '::Beekeeping::Nectars' => 1, + '::ApiaryReportCard' => 1 + ) + end + + it 'takes the name of a pack outside packs/, as the root pack' do + expect(described_class.references_from('.').map(&:location)).to eq(['app/services/apiary_summary.rb:2']) + end + end + + describe '#reload!' do + it 'runs pks again for the next query' do + described_class.references_for('honey_production') + described_class.reload! + described_class.references_for('honey_production') + + expect(QueryPackwerk::Pks).to have_received(:list_references).twice + end + + it 'returns nil, which the console shows plainly, as do the reload! of each collection' do + expect(described_class.reload!).to be_nil + expect([QueryPackwerk::Packages, QueryPackwerk::Violations, QueryPackwerk::References].map(&:reload!)).to eq([nil, nil, nil]) + end + + it 'reads the package_todo.yml files again' do + expect(described_class.todos_for('apiary_reports').count).to eq(1) + + write_package_todo(pack_name: 'packs/apiary_reports', references: {}) + described_class.reload! + + expect(described_class.todos_for('apiary_reports').count).to eq(0) + end + + it 'finds a pack added since, which pks lists references from' do + described_class.references_for('honey_production') + + write_package_yml('packs/wax_reports') + write_file('packs/wax_reports/app/services/wax_report.rb', <<~RUBY) + class WaxReport + def nectar_count = HoneyProductionNectar.count + end + RUBY + listed_references << { + 'file' => 'packs/wax_reports/app/services/wax_report.rb', 'line' => 2, 'column' => 21, + 'constant_name' => '::HoneyProductionNectar', + 'referencing_pack_name' => 'packs/wax_reports', 'defining_pack_name' => 'packs/honey_production', + 'violations' => [ + { 'violation_type' => 'dependency', 'strict' => false, 'recorded' => false }, + { 'violation_type' => 'privacy', 'strict' => false, 'recorded' => false } + ] + } + described_class.reload! + + expect(described_class.references_for('honey_production').consumers).to include('packs/wax_reports' => 1) + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b011a67..89de8c9 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,6 +5,7 @@ require 'query_packwerk' require_relative 'support/pack_helpers' require_relative 'support/pseudo_packs' +require_relative 'support/referenced_packs' require 'rubocop/rspec/support' require 'packs/rspec/support' require 'packs-specification' diff --git a/spec/support/referenced_packs.rb b/spec/support/referenced_packs.rb new file mode 100644 index 0000000..de2fdf7 --- /dev/null +++ b/spec/support/referenced_packs.rb @@ -0,0 +1,152 @@ +# typed: false +# frozen_string_literal: true + +# An app whose packs reference each other's constants in each way that pks +# lists: allowed or not, recorded in a package_todo.yml or not, by a relative +# name, through an association, in ERB, after multibyte characters, and to a +# pack without an owner. +RSpec.shared_context 'referenced packs' do + # QueryPackwerk keeps the packs it finds, so it'd keep these from, or for, + # specs that write other packs + around do |example| + QueryPackwerk.reload! + example.run + ensure + QueryPackwerk.reload! + end + + before do + write_file('packwerk.yml', "cache: false\n") + write_package_yml('.', enforce_privacy: false, enforce_dependencies: false) + write_package_yml('packs/honey_production', config: { 'owner' => 'Honey' }) + write_package_yml('packs/hive_dashboard', dependencies: ['packs/honey_production']) + write_package_yml('packs/apiary_reports') + + write_package_todo( + pack_name: 'packs/hive_dashboard', + references: { + 'packs/honey_production' => { + '::HoneyProductionNectar' => { + 'violations' => ['privacy'], + 'files' => [ + 'packs/hive_dashboard/app/services/beekeeping/apiary_status.rb', + 'packs/hive_dashboard/app/views/apiaries/show.html.erb' + ] + } + } + } + ) + write_package_todo( + pack_name: 'packs/apiary_reports', + references: { + 'packs/honey_production' => { + '::HoneyProductionNectar' => { + 'violations' => ['dependency'], + 'files' => ['packs/apiary_reports/app/services/apiary_report.rb'] + } + } + } + ) + + app_files.each { |file_name, source| write_file(file_name, source) } + end + + let(:app_files) do + { + 'packs/honey_production/app/models/honey_production_nectar.rb' => <<~RUBY, + class HoneyProductionNectar < ApplicationRecord + end + RUBY + 'packs/honey_production/app/models/beekeeping/nectar_source.rb' => <<~RUBY, + module Beekeeping + class NectarSource < ApplicationRecord + end + end + RUBY + 'packs/honey_production/app/public/beekeeping/nectars.rb' => <<~RUBY, + module Beekeeping + module Nectars + def self.status_for(apiary_id:); end + end + end + RUBY + 'packs/hive_dashboard/app/services/beekeeping/apiary_status.rb' => <<~RUBY, + module Beekeeping + class ApiaryStatus + def nectar + ::HoneyProductionNectar.as_of_now.find_by(apiary_id: 1) + end + + def sources + NectarSource.where(apiary_id: 1).pluck(:name) + end + + def status + Nectars.status_for(apiary_id: 1) + end + end + end + RUBY + 'packs/hive_dashboard/app/models/hive.rb' => <<~RUBY, + class Hive < ApplicationRecord + has_many :honey_production_nectars, dependent: :destroy + end + RUBY + 'packs/hive_dashboard/app/services/report_link.rb' => <<~RUBY, + class ReportLink + def card = ApiaryReportCard.new + end + RUBY + 'packs/hive_dashboard/app/views/apiaries/show.html.erb' => <<~ERB, + <% if HoneyProductionNectar.as_of_now.exists?(apiary_id: @apiary.id) %> +

<%= HoneyProductionNectar.as_of_now.find_by(apiary_id: @apiary.id).volume %>

+ <% end %> + ERB + 'packs/apiary_reports/app/services/apiary_report.rb' => <<~'RUBY', + class ApiaryReport + def nectar_count = HoneyProductionNectar.count + + def title = "Récolte 🍯 #{HoneyProductionNectar.model_name.human}" + end + RUBY + 'packs/apiary_reports/app/public/apiary_report_card.rb' => <<~RUBY, + class ApiaryReportCard + end + RUBY + 'app/services/apiary_summary.rb' => <<~RUBY + class ApiarySummary + def status = Beekeeping::Nectars.status_for(apiary_id: 1) + end + RUBY + } + end + + # What `pks list-references` prints for the app. spec/query_packwerk/pks_spec.rb + # checks this against pks itself, when it's installed. + let(:listed_references) do + JSON.parse(<<~JSON).fetch('references') + { + "references": [ + {"file": "app/services/apiary_summary.rb", "line": 2, "column": 15, "constant_name": "::Beekeeping::Nectars", "referencing_pack_name": ".", "defining_pack_name": "packs/honey_production", "violations": []}, + {"file": "packs/apiary_reports/app/services/apiary_report.rb", "line": 2, "column": 21, "constant_name": "::HoneyProductionNectar", "referencing_pack_name": "packs/apiary_reports", "defining_pack_name": "packs/honey_production", "violations": [{"violation_type": "dependency", "strict": false, "recorded": true}, {"violation_type": "privacy", "strict": false, "recorded": false}]}, + {"file": "packs/apiary_reports/app/services/apiary_report.rb", "line": 4, "column": 31, "constant_name": "::HoneyProductionNectar", "referencing_pack_name": "packs/apiary_reports", "defining_pack_name": "packs/honey_production", "violations": [{"violation_type": "dependency", "strict": false, "recorded": true}, {"violation_type": "privacy", "strict": false, "recorded": false}]}, + {"file": "packs/hive_dashboard/app/models/hive.rb", "line": 2, "column": 2, "constant_name": "::HoneyProductionNectar", "referencing_pack_name": "packs/hive_dashboard", "defining_pack_name": "packs/honey_production", "violations": [{"violation_type": "privacy", "strict": false, "recorded": false}]}, + {"file": "packs/hive_dashboard/app/services/beekeeping/apiary_status.rb", "line": 4, "column": 6, "constant_name": "::HoneyProductionNectar", "referencing_pack_name": "packs/hive_dashboard", "defining_pack_name": "packs/honey_production", "violations": [{"violation_type": "privacy", "strict": false, "recorded": true}]}, + {"file": "packs/hive_dashboard/app/services/beekeeping/apiary_status.rb", "line": 8, "column": 6, "constant_name": "::Beekeeping::NectarSource", "referencing_pack_name": "packs/hive_dashboard", "defining_pack_name": "packs/honey_production", "violations": [{"violation_type": "privacy", "strict": false, "recorded": false}]}, + {"file": "packs/hive_dashboard/app/services/beekeeping/apiary_status.rb", "line": 12, "column": 6, "constant_name": "::Beekeeping::Nectars", "referencing_pack_name": "packs/hive_dashboard", "defining_pack_name": "packs/honey_production", "violations": []}, + {"file": "packs/hive_dashboard/app/services/report_link.rb", "line": 2, "column": 13, "constant_name": "::ApiaryReportCard", "referencing_pack_name": "packs/hive_dashboard", "defining_pack_name": "packs/apiary_reports", "violations": [{"violation_type": "dependency", "strict": false, "recorded": false}]}, + {"file": "packs/hive_dashboard/app/views/apiaries/show.html.erb", "line": null, "column": null, "constant_name": "::HoneyProductionNectar", "referencing_pack_name": "packs/hive_dashboard", "defining_pack_name": "packs/honey_production", "violations": [{"violation_type": "privacy", "strict": false, "recorded": true}]} + ] + } + JSON + end +end + +# Stands in for pks, with what it lists for the referenced packs +RSpec.shared_context 'listed references' do + include_context 'referenced packs' + + before do + allow(QueryPackwerk::Pks).to receive(:list_references).and_return(listed_references) + end +end