Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add class-level comments #144

Merged
merged 1 commit into from
Oct 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/packwerk/application_load_paths.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require "bundler"

module Packwerk
# Extracts the load paths from the analyzed application so that we can map constant names to paths.
module ApplicationLoadPaths
class << self
extend T::Sig
Expand Down
2 changes: 2 additions & 0 deletions lib/packwerk/application_validator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
require "yaml"

module Packwerk
# Checks the structure of the application and its packwerk configuration to make sure we can run a check and deliver
# correct results.
class ApplicationValidator
def initialize(config_file_path:, configuration:, environment:)
@config_file_path = config_file_path
Expand Down
1 change: 1 addition & 0 deletions lib/packwerk/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# frozen_string_literal: true

module Packwerk
# A command-line interface to Packwerk.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one feels a little too much like describe the class name and I don't think it adds any value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I was on the fence about this one, but then thought - does everyone know CLI means command-line interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have the same doubts as me I should probably remove this one.

class Cli
extend T::Sig

Expand Down
5 changes: 3 additions & 2 deletions lib/packwerk/const_node_inspector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ class ConstNodeInspector
def constant_name_from_node(node, ancestors:)
return nil unless Node.constant?(node)
parent = ancestors.first

# Only process the root `const` node for namespaced constant references. For example, in the
# reference `Spam::Eggs::Thing`, we only process the const node associated with `Spam`.
return nil unless root_constant?(parent)

if parent && constant_in_module_or_class_definition?(node, parent: parent)
Expand All @@ -30,8 +33,6 @@ def constant_name_from_node(node, ancestors:)

private

# Only process the root `const` node for namespaced constant references. For example, in the
# reference `Spam::Eggs::Thing`, we only process the const node associated with `Spam`.
sig { params(parent: T.nilable(AST::Node)).returns(T::Boolean) }
def root_constant?(parent)
!(parent && Node.constant?(parent))
Expand Down
2 changes: 1 addition & 1 deletion lib/packwerk/constant_name_inspector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
require "ast"

module Packwerk
# An interface describing some object that can extract a constant name from an AST node
# An interface describing an object that can extract a constant name from an AST node.
module ConstantNameInspector
extend T::Sig
extend T::Helpers
Expand Down
2 changes: 2 additions & 0 deletions lib/packwerk/graph.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
# frozen_string_literal: true

module Packwerk
# A general implementation of a graph data structure with the ability to check for - and list - cycles.
class Graph
# @param [Array<Array>] edges The edges of the graph; An edge being represented as an Array of two nodes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be we using sorbet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we could use sorbet, that's true - I wanted to keep this PR to comments only so we could more easily merge it. 🤔

Sorbet would make it way more explicit and also be guaranteed to match the code, so definitely better. Will think about doing it later, want to play around with restructuring the code first in a separate PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if we were using sorbet we should be documenting our methods and parameters.

def initialize(*edges)
@edges = edges.uniq
@cycles = Set.new
Expand Down
1 change: 1 addition & 0 deletions lib/packwerk/inflector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require "active_support/inflector"

module Packwerk
# A custom inflector used, among other things, to map between constant names and file names.
class Inflector
class << self
extend T::Sig
Expand Down
1 change: 1 addition & 0 deletions lib/packwerk/node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require "parser/ast/node"

module Packwerk
# Convenience methods for working with AST nodes.
module Node
class TypeError < ArgumentError; end
Location = Struct.new(:line, :column)
Expand Down
1 change: 1 addition & 0 deletions lib/packwerk/node_processor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# frozen_string_literal: true

module Packwerk
# Processes a single node in an abstract syntax tree (AST) using the provided checkers.
class NodeProcessor
extend T::Sig

Expand Down
1 change: 1 addition & 0 deletions lib/packwerk/node_visitor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# frozen_string_literal: true

module Packwerk
# Visits all nodes of an AST, processing them using a given node processor.
class NodeVisitor
def initialize(node_processor:)
@node_processor = node_processor
Expand Down
3 changes: 3 additions & 0 deletions lib/packwerk/package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
# frozen_string_literal: true

module Packwerk
# The basic unit of modularity for packwerk; a folder that has been declared to define a package.
# The package contains all constants defined in files in this folder and all subfolders that are not packages
# themselves.
class Package
include Comparable

Expand Down
1 change: 1 addition & 0 deletions lib/packwerk/package_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require "pathname"

module Packwerk
# A set of {Packwerk::Package}s as well as methods to parse packages from the filesystem.
class PackageSet
include Enumerable

Expand Down
1 change: 1 addition & 0 deletions lib/packwerk/parsed_constant_definitions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require "ast/node"

module Packwerk
# A collection of constant definitions parsed from an Abstract Syntax Tree (AST).
class ParsedConstantDefinitions
def initialize(root_node:)
@local_definitions = {}
Expand Down
1 change: 1 addition & 0 deletions lib/packwerk/reference.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@
# frozen_string_literal: true

module Packwerk
# A reference from a file in one package to a constant that may be defined in a different package.
Reference = Struct.new(:source_package, :relative_path, :constant, :source_location)
end
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
module Packwerk
module ReferenceChecking
module Checkers
# Checks whether a given reference conforms to the configured graph of dependencies.
class DependencyChecker
extend T::Sig
include Checker
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
module Packwerk
module ReferenceChecking
module Checkers
# Checks whether a given reference references a private constant of another package.
class PrivacyChecker
extend T::Sig
include Checker
Expand Down
2 changes: 1 addition & 1 deletion lib/packwerk/reference_extractor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# frozen_string_literal: true

module Packwerk
# extracts a possible constant reference from a given AST node
# Extracts a possible constant reference from a given AST node.
class ReferenceExtractor
extend T::Sig

Expand Down
1 change: 1 addition & 0 deletions lib/packwerk/reference_offense.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# frozen_string_literal: true

module Packwerk
# An offense related to a {Packwerk::Reference}.
class ReferenceOffense < Offense
extend T::Sig
extend T::Helpers
Expand Down
1 change: 1 addition & 0 deletions lib/packwerk/run_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require "constant_resolver"

module Packwerk
# Holds the context of a Packwerk run across multiple files.
class RunContext
extend T::Sig

Expand Down