From 3a8b828d368108a977846964097d741d4e5b680e Mon Sep 17 00:00:00 2001 From: Yuta Miyama Date: Wed, 13 Oct 2021 12:53:58 -0400 Subject: [PATCH 1/6] Put files into buckets of responsibilities/seams --- lib/packwerk/{ => checking_references/checkers}/checker.rb | 0 .../{ => checking_references/checkers}/dependency_checker.rb | 0 .../{ => checking_references/checkers}/privacy_checker.rb | 0 lib/packwerk/{ => checking_references}/deprecated_references.rb | 0 lib/packwerk/{ => checking_references}/offense.rb | 0 lib/packwerk/{ => checking_references}/offense_collection.rb | 0 lib/packwerk/{ => checking_references}/reference_offense.rb | 0 lib/packwerk/{ => checking_references}/violation_type.rb | 0 lib/packwerk/{ => configurations}/application_load_paths.rb | 0 lib/packwerk/{ => configurations}/application_validator.rb | 0 lib/packwerk/{ => configurations}/configuration.rb | 0 lib/packwerk/{ => configurations}/graph.rb | 0 lib/packwerk/{ => configurations}/sanity_checker.rb | 0 .../{ => extracting_references}/association_inspector.rb | 0 .../{ => extracting_references}/const_node_inspector.rb | 0 lib/packwerk/{ => extracting_references}/constant_discovery.rb | 0 .../{ => extracting_references}/constant_name_inspector.rb | 0 .../{ => extracting_references}/parsed_constant_definitions.rb | 0 lib/packwerk/{ => extracting_references}/reference.rb | 0 lib/packwerk/{ => extracting_references}/reference_extractor.rb | 0 lib/packwerk/file_processor.rb | 1 + lib/packwerk/{ => finding_files}/files_for_processing.rb | 2 +- .../offenses_formatter_type.rb} | 0 23 files changed, 2 insertions(+), 1 deletion(-) rename lib/packwerk/{ => checking_references/checkers}/checker.rb (100%) rename lib/packwerk/{ => checking_references/checkers}/dependency_checker.rb (100%) rename lib/packwerk/{ => checking_references/checkers}/privacy_checker.rb (100%) rename lib/packwerk/{ => checking_references}/deprecated_references.rb (100%) rename lib/packwerk/{ => checking_references}/offense.rb (100%) rename lib/packwerk/{ => checking_references}/offense_collection.rb (100%) rename lib/packwerk/{ => checking_references}/reference_offense.rb (100%) rename lib/packwerk/{ => checking_references}/violation_type.rb (100%) rename lib/packwerk/{ => configurations}/application_load_paths.rb (100%) rename lib/packwerk/{ => configurations}/application_validator.rb (100%) rename lib/packwerk/{ => configurations}/configuration.rb (100%) rename lib/packwerk/{ => configurations}/graph.rb (100%) rename lib/packwerk/{ => configurations}/sanity_checker.rb (100%) rename lib/packwerk/{ => extracting_references}/association_inspector.rb (100%) rename lib/packwerk/{ => extracting_references}/const_node_inspector.rb (100%) rename lib/packwerk/{ => extracting_references}/constant_discovery.rb (100%) rename lib/packwerk/{ => extracting_references}/constant_name_inspector.rb (100%) rename lib/packwerk/{ => extracting_references}/parsed_constant_definitions.rb (100%) rename lib/packwerk/{ => extracting_references}/reference.rb (100%) rename lib/packwerk/{ => extracting_references}/reference_extractor.rb (100%) rename lib/packwerk/{ => finding_files}/files_for_processing.rb (98%) rename lib/packwerk/{offenses_formatter.rb => formatters/offenses_formatter_type.rb} (100%) diff --git a/lib/packwerk/checker.rb b/lib/packwerk/checking_references/checkers/checker.rb similarity index 100% rename from lib/packwerk/checker.rb rename to lib/packwerk/checking_references/checkers/checker.rb diff --git a/lib/packwerk/dependency_checker.rb b/lib/packwerk/checking_references/checkers/dependency_checker.rb similarity index 100% rename from lib/packwerk/dependency_checker.rb rename to lib/packwerk/checking_references/checkers/dependency_checker.rb diff --git a/lib/packwerk/privacy_checker.rb b/lib/packwerk/checking_references/checkers/privacy_checker.rb similarity index 100% rename from lib/packwerk/privacy_checker.rb rename to lib/packwerk/checking_references/checkers/privacy_checker.rb diff --git a/lib/packwerk/deprecated_references.rb b/lib/packwerk/checking_references/deprecated_references.rb similarity index 100% rename from lib/packwerk/deprecated_references.rb rename to lib/packwerk/checking_references/deprecated_references.rb diff --git a/lib/packwerk/offense.rb b/lib/packwerk/checking_references/offense.rb similarity index 100% rename from lib/packwerk/offense.rb rename to lib/packwerk/checking_references/offense.rb diff --git a/lib/packwerk/offense_collection.rb b/lib/packwerk/checking_references/offense_collection.rb similarity index 100% rename from lib/packwerk/offense_collection.rb rename to lib/packwerk/checking_references/offense_collection.rb diff --git a/lib/packwerk/reference_offense.rb b/lib/packwerk/checking_references/reference_offense.rb similarity index 100% rename from lib/packwerk/reference_offense.rb rename to lib/packwerk/checking_references/reference_offense.rb diff --git a/lib/packwerk/violation_type.rb b/lib/packwerk/checking_references/violation_type.rb similarity index 100% rename from lib/packwerk/violation_type.rb rename to lib/packwerk/checking_references/violation_type.rb diff --git a/lib/packwerk/application_load_paths.rb b/lib/packwerk/configurations/application_load_paths.rb similarity index 100% rename from lib/packwerk/application_load_paths.rb rename to lib/packwerk/configurations/application_load_paths.rb diff --git a/lib/packwerk/application_validator.rb b/lib/packwerk/configurations/application_validator.rb similarity index 100% rename from lib/packwerk/application_validator.rb rename to lib/packwerk/configurations/application_validator.rb diff --git a/lib/packwerk/configuration.rb b/lib/packwerk/configurations/configuration.rb similarity index 100% rename from lib/packwerk/configuration.rb rename to lib/packwerk/configurations/configuration.rb diff --git a/lib/packwerk/graph.rb b/lib/packwerk/configurations/graph.rb similarity index 100% rename from lib/packwerk/graph.rb rename to lib/packwerk/configurations/graph.rb diff --git a/lib/packwerk/sanity_checker.rb b/lib/packwerk/configurations/sanity_checker.rb similarity index 100% rename from lib/packwerk/sanity_checker.rb rename to lib/packwerk/configurations/sanity_checker.rb diff --git a/lib/packwerk/association_inspector.rb b/lib/packwerk/extracting_references/association_inspector.rb similarity index 100% rename from lib/packwerk/association_inspector.rb rename to lib/packwerk/extracting_references/association_inspector.rb diff --git a/lib/packwerk/const_node_inspector.rb b/lib/packwerk/extracting_references/const_node_inspector.rb similarity index 100% rename from lib/packwerk/const_node_inspector.rb rename to lib/packwerk/extracting_references/const_node_inspector.rb diff --git a/lib/packwerk/constant_discovery.rb b/lib/packwerk/extracting_references/constant_discovery.rb similarity index 100% rename from lib/packwerk/constant_discovery.rb rename to lib/packwerk/extracting_references/constant_discovery.rb diff --git a/lib/packwerk/constant_name_inspector.rb b/lib/packwerk/extracting_references/constant_name_inspector.rb similarity index 100% rename from lib/packwerk/constant_name_inspector.rb rename to lib/packwerk/extracting_references/constant_name_inspector.rb diff --git a/lib/packwerk/parsed_constant_definitions.rb b/lib/packwerk/extracting_references/parsed_constant_definitions.rb similarity index 100% rename from lib/packwerk/parsed_constant_definitions.rb rename to lib/packwerk/extracting_references/parsed_constant_definitions.rb diff --git a/lib/packwerk/reference.rb b/lib/packwerk/extracting_references/reference.rb similarity index 100% rename from lib/packwerk/reference.rb rename to lib/packwerk/extracting_references/reference.rb diff --git a/lib/packwerk/reference_extractor.rb b/lib/packwerk/extracting_references/reference_extractor.rb similarity index 100% rename from lib/packwerk/reference_extractor.rb rename to lib/packwerk/extracting_references/reference_extractor.rb diff --git a/lib/packwerk/file_processor.rb b/lib/packwerk/file_processor.rb index 4b9f2023c..c1d9d783c 100644 --- a/lib/packwerk/file_processor.rb +++ b/lib/packwerk/file_processor.rb @@ -4,6 +4,7 @@ require "ast/node" module Packwerk + # from file path to node class FileProcessor class UnknownFileTypeResult < Offense def initialize(file:) diff --git a/lib/packwerk/files_for_processing.rb b/lib/packwerk/finding_files/files_for_processing.rb similarity index 98% rename from lib/packwerk/files_for_processing.rb rename to lib/packwerk/finding_files/files_for_processing.rb index 684c05115..c3e28b22d 100644 --- a/lib/packwerk/files_for_processing.rb +++ b/lib/packwerk/finding_files/files_for_processing.rb @@ -1,7 +1,7 @@ # typed: true # frozen_string_literal: true -module Packwerk +module Packwerk::FindingFiles class FilesForProcessing class << self def fetch(paths:, configuration:) diff --git a/lib/packwerk/offenses_formatter.rb b/lib/packwerk/formatters/offenses_formatter_type.rb similarity index 100% rename from lib/packwerk/offenses_formatter.rb rename to lib/packwerk/formatters/offenses_formatter_type.rb From 6fcd8a59dc8002642993ce072c7f903ed7154e4f Mon Sep 17 00:00:00 2001 From: Yuta Miyama Date: Wed, 13 Oct 2021 14:44:12 -0400 Subject: [PATCH 2/6] Add temp README for each folder --- lib/packwerk/configurations/README.md | 3 +++ lib/packwerk/extracting_references/README.md | 4 ++++ lib/packwerk/{ => extracting_references}/inflector.rb | 0 lib/packwerk/node_processor.rb | 1 + lib/packwerk/{ => output_styles}/output_style.rb | 0 lib/packwerk/{ => parsers}/parsers.rb | 0 lib/packwerk/run_context.rb | 1 + 7 files changed, 9 insertions(+) create mode 100644 lib/packwerk/configurations/README.md create mode 100644 lib/packwerk/extracting_references/README.md rename lib/packwerk/{ => extracting_references}/inflector.rb (100%) rename lib/packwerk/{ => output_styles}/output_style.rb (100%) rename lib/packwerk/{ => parsers}/parsers.rb (100%) diff --git a/lib/packwerk/configurations/README.md b/lib/packwerk/configurations/README.md new file mode 100644 index 000000000..0c27b5255 --- /dev/null +++ b/lib/packwerk/configurations/README.md @@ -0,0 +1,3 @@ +### Configurations + +Includes validators which are outside of primary `check` runs. diff --git a/lib/packwerk/extracting_references/README.md b/lib/packwerk/extracting_references/README.md new file mode 100644 index 000000000..14a0bb037 --- /dev/null +++ b/lib/packwerk/extracting_references/README.md @@ -0,0 +1,4 @@ +# Extracting references + +- Given a `AST::Node`, this module extracts `Packwerk::Reference`. +- `Packwerk::Reference` contains `constant` information, that are worked by `Inflector`. diff --git a/lib/packwerk/inflector.rb b/lib/packwerk/extracting_references/inflector.rb similarity index 100% rename from lib/packwerk/inflector.rb rename to lib/packwerk/extracting_references/inflector.rb diff --git a/lib/packwerk/node_processor.rb b/lib/packwerk/node_processor.rb index 26d4fb212..342871b7c 100644 --- a/lib/packwerk/node_processor.rb +++ b/lib/packwerk/node_processor.rb @@ -2,6 +2,7 @@ # frozen_string_literal: true module Packwerk + # from node to reference to offence class NodeProcessor extend T::Sig diff --git a/lib/packwerk/output_style.rb b/lib/packwerk/output_styles/output_style.rb similarity index 100% rename from lib/packwerk/output_style.rb rename to lib/packwerk/output_styles/output_style.rb diff --git a/lib/packwerk/parsers.rb b/lib/packwerk/parsers/parsers.rb similarity index 100% rename from lib/packwerk/parsers.rb rename to lib/packwerk/parsers/parsers.rb diff --git a/lib/packwerk/run_context.rb b/lib/packwerk/run_context.rb index 6d5179d7d..bbdba92a7 100644 --- a/lib/packwerk/run_context.rb +++ b/lib/packwerk/run_context.rb @@ -4,6 +4,7 @@ require "constant_resolver" module Packwerk + # used to depend as a part of Rubocop class RunContext extend T::Sig From e46b25321497ed5edc153f43c9fc03a83dfe9465 Mon Sep 17 00:00:00 2001 From: Yuta Miyama Date: Wed, 13 Oct 2021 14:46:13 -0400 Subject: [PATCH 3/6] Add a bit more comments --- lib/packwerk/finding_files/files_for_processing.rb | 2 +- lib/packwerk/run_context.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/packwerk/finding_files/files_for_processing.rb b/lib/packwerk/finding_files/files_for_processing.rb index c3e28b22d..684c05115 100644 --- a/lib/packwerk/finding_files/files_for_processing.rb +++ b/lib/packwerk/finding_files/files_for_processing.rb @@ -1,7 +1,7 @@ # typed: true # frozen_string_literal: true -module Packwerk::FindingFiles +module Packwerk class FilesForProcessing class << self def fetch(paths:, configuration:) diff --git a/lib/packwerk/run_context.rb b/lib/packwerk/run_context.rb index bbdba92a7..0f30b2670 100644 --- a/lib/packwerk/run_context.rb +++ b/lib/packwerk/run_context.rb @@ -4,7 +4,8 @@ require "constant_resolver" module Packwerk - # used to depend as a part of Rubocop + # Packwerk used to run as a part of Rubocop. + # Now that Packwerk is a standalone script, this structure shouldn't be necessary. class RunContext extend T::Sig From 896797e5e340f69b0c42ae015e5c14a69c30cb1f Mon Sep 17 00:00:00 2001 From: Yuta Miyama Date: Wed, 13 Oct 2021 15:13:02 -0400 Subject: [PATCH 4/6] Write down the data transformation step at to clarify its steps --- lib/packwerk/file_processor.rb | 2 ++ lib/packwerk/run_context.rb | 27 ++++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/packwerk/file_processor.rb b/lib/packwerk/file_processor.rb index c1d9d783c..95579e82c 100644 --- a/lib/packwerk/file_processor.rb +++ b/lib/packwerk/file_processor.rb @@ -21,6 +21,7 @@ def call(file_path) parser = @parser_factory.for_path(file_path) return [UnknownFileTypeResult.new(file: file_path)] if parser.nil? + # path to node node = File.open(file_path, "r", external_encoding: Encoding::UTF_8) do |file| parser.call(io: file, file_path: file_path) rescue Parsers::ParseError => e @@ -32,6 +33,7 @@ def call(file_path) node_processor = @node_processor_factory.for(filename: file_path, node: node) node_visitor = Packwerk::NodeVisitor.new(node_processor: node_processor) + # node to offence node_visitor.visit(node, ancestors: [], result: result) end result diff --git a/lib/packwerk/run_context.rb b/lib/packwerk/run_context.rb index 0f30b2670..2511dc9c6 100644 --- a/lib/packwerk/run_context.rb +++ b/lib/packwerk/run_context.rb @@ -54,7 +54,32 @@ def initialize( sig { params(file: String).returns(T::Array[T.nilable(::Packwerk::Offense)]) } def process_file(file:) - file_processor.call(file) + # 1. file path to node + # It needs to return ancestors relative to node + node, ancestors = file_processor.call(file) + + # Inside NodeProcessor - @reference_extractor.reference_from_node(node, ancestors: ancestors, file_path: @filename) + # 2. node to constant + # 3. constant to reference + @constant_name_inspectors.each do |inspector| + constant_name = inspector.constant_name_from_node(node, ancestors: ancestors) + break if constant_name + end + + reference_from_constant(constant_name, node: node, ancestors: ancestors, file_path: file_path) if constant_name + + # Inside NodeProcessor + # 4. reference to an offence + @checkers.each_with_object([]) do |checker, violations| + next unless checker.invalid_reference?(reference) + offense = Packwerk::ReferenceOffense.new( + location: Node.location(node), + reference: reference, + violation_type: checker.violation_type + ) + violations << offense + end + end private From ac57f566b488a896883f81747e7ca3c651eb3414 Mon Sep 17 00:00:00 2001 From: Yuta Miyama Date: Wed, 13 Oct 2021 15:43:55 -0400 Subject: [PATCH 5/6] Run a draft documentation of Checker interface --- .../checking_references/checkers/README.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 lib/packwerk/checking_references/checkers/README.md diff --git a/lib/packwerk/checking_references/checkers/README.md b/lib/packwerk/checking_references/checkers/README.md new file mode 100644 index 000000000..94db5f327 --- /dev/null +++ b/lib/packwerk/checking_references/checkers/README.md @@ -0,0 +1,19 @@ +# Packwerk::Checkers + +- Checker implements `invalid_reference?(reference)`, which takes a `Packwerk::Reference` and returns Boolean. + +### Packwerk::Reference + +- It contains the information about source package and destination package. + + +## Example: DependencyChecker + +- Dependency means any outgoing dependency from the source package. +- If source package is enforcing dependency, it will check if declared dependency includes the destination package. + + +## Example: PrivacyChecker + +- Privacy means any incoming dependency towards the destination package. +- If destination package is enforcing privacy, it till check if source package references non-public constants. From 009df05d046e9137fafab07b7962ba99cfa0d1f8 Mon Sep 17 00:00:00 2001 From: Yuta Miyama Date: Thu, 14 Oct 2021 11:50:42 -0400 Subject: [PATCH 6/6] More flattened extraction --- lib/packwerk/run_context.rb | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/packwerk/run_context.rb b/lib/packwerk/run_context.rb index 2511dc9c6..45a892eee 100644 --- a/lib/packwerk/run_context.rb +++ b/lib/packwerk/run_context.rb @@ -60,13 +60,29 @@ def process_file(file:) # Inside NodeProcessor - @reference_extractor.reference_from_node(node, ancestors: ancestors, file_path: @filename) # 2. node to constant - # 3. constant to reference @constant_name_inspectors.each do |inspector| constant_name = inspector.constant_name_from_node(node, ancestors: ancestors) break if constant_name end - reference_from_constant(constant_name, node: node, ancestors: ancestors, file_path: file_path) if constant_name + # Inside ReferenceExtractor - reference_from_constant(constant_name, node: node, ancestors: ancestors, file_path: file_path) if constant_name + # 3. constant to reference + namespace_path = Node.enclosing_namespace_path(node, ancestors: ancestors) + return if local_reference?(constant_name, Node.name_location(node), namespace_path) + + constant = + @context_provider.context_for( + constant_name, + current_namespace_path: namespace_path + ) + return if constant&.package.nil? + + relative_path = Pathname.new(file_path).relative_path_from(@root_path).to_s + + source_package = @context_provider.package_from_path(relative_path) + return if source_package == constant.package + + Reference.new(source_package, relative_path, constant) # Inside NodeProcessor # 4. reference to an offence