diff --git a/USAGE.md b/USAGE.md index 2b11677e8..810fa660d 100644 --- a/USAGE.md +++ b/USAGE.md @@ -8,7 +8,7 @@ * [Getting started](#getting-started) * [Setting up Spring](#setting-up-spring) * [Configuring Packwerk](#configuring-packwerk) - * [Using a custom ERB parser](#using-a-custom-erb-parser) + * [How to override a parser](#how-to-override-a-parser) * [Validating the package system](#validating-the-package-system) * [Defining packages](#defining-packages) * [Package metadata](#package-metadata) @@ -83,12 +83,37 @@ Packwerk reads from the `packwerk.yml` configuration file in the root directory. | cache | false | when true, caches the results of parsing files | | cache_directory | tmp/cache/packwerk | the directory that will hold the packwerk cache | -### Using a custom ERB parser +### Using custom parsers -You can specify a custom ERB parser if needed. For example, if you're using `<%graphql>` tags from https://github.com/github/graphql-client in your ERBs, you can use a custom parser subclass to comment them out so that Packwerk can parse the rest of the file: +You can specify a custom parser to parse different file formats (e.g. slim or haml) ```ruby +class SlimParser + include Packwerk::FileParser + + REGEX = /\.slim\Z/ + + def call + # Your parsing logic here + end + + def match?(path:) + REGEX.match?(path) + end +end +``` + +### How to override a parser + +You can override an existing parser if needed. For example, if you're using `<%graphql>` tags from https://github.com/github/graphql-client in your ERBs, you can use a custom parser subclass to comment them out so that Packwerk can parse the rest of the file: + +```ruby +# Remove `Packwerk::Parsers::Erb` from the registered parsers. +Packwerk::FileParser.remove(Packwerk::Parsers::Erb) + class CustomParser < Packwerk::Parsers::Erb + include Packwerk::FileParser + def parse_buffer(buffer, file_path:) preprocessed_source = buffer.source @@ -101,8 +126,6 @@ class CustomParser < Packwerk::Parsers::Erb super(preprocessed_buffer, file_path: file_path) end end - -Packwerk::Parsers::Factory.instance.erb_parser_class = CustomParser ``` ## Using the cache diff --git a/lib/packwerk.rb b/lib/packwerk.rb index 39023d1ca..deb3c29eb 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -19,6 +19,7 @@ module Packwerk autoload :Configuration autoload :ConstantContext autoload :Commands + autoload :FileParser autoload :Node autoload :Offense autoload :OffenseCollection diff --git a/lib/packwerk/file_parser.rb b/lib/packwerk/file_parser.rb new file mode 100644 index 000000000..aed52f1dd --- /dev/null +++ b/lib/packwerk/file_parser.rb @@ -0,0 +1,42 @@ +# typed: strict +# frozen_string_literal: true + +module Packwerk + module FileParser + extend T::Helpers + extend T::Sig + + requires_ancestor { Kernel } + + interface! + + @parsers = T.let([], T::Array[Class]) + + class << self + extend T::Sig + + sig { params(base: Class).void } + def included(base) + @parsers << base + end + + sig { returns(T::Array[FileParser]) } + def all + T.unsafe(@parsers).map(&:new) + end + + sig { params(base: Class).void } + def remove(base) + @parsers.delete(base) + end + end + + sig { abstract.params(io: T.any(IO, StringIO), file_path: String).returns(T.untyped) } + def call(io:, file_path:) + end + + sig { abstract.params(path: String).returns(T::Boolean) } + def match?(path:) + end + end +end diff --git a/lib/packwerk/file_processor.rb b/lib/packwerk/file_processor.rb index 6c3a4c1ac..a9becce78 100644 --- a/lib/packwerk/file_processor.rb +++ b/lib/packwerk/file_processor.rb @@ -38,16 +38,18 @@ class ProcessedFile < T::Struct params(relative_file: String).returns(ProcessedFile) end def call(relative_file) - parser = parser_for(relative_file) - if parser.nil? + parsers = parsers_for(relative_file) + if parsers.empty? return ProcessedFile.new(offenses: [UnknownFileTypeResult.new(file: relative_file)]) end unresolved_references = @cache.with_cache(relative_file) do - node = parse_into_ast(relative_file, parser) - return ProcessedFile.new unless node + parsers.flat_map do |parser| + node = parse_into_ast(relative_file, parser) + return ProcessedFile.new unless node - references_from_ast(node, relative_file) + references_from_ast(node, relative_file) + end end ProcessedFile.new(unresolved_references: unresolved_references) @@ -81,15 +83,15 @@ def references_from_ast(node, relative_file) references end - sig { params(relative_file: String, parser: Parsers::ParserInterface).returns(T.untyped) } + sig { params(relative_file: String, parser: FileParser).returns(T.untyped) } def parse_into_ast(relative_file, parser) File.open(relative_file, "r", nil, external_encoding: Encoding::UTF_8) do |file| parser.call(io: file, file_path: relative_file) end end - sig { params(file_path: String).returns(T.nilable(Parsers::ParserInterface)) } - def parser_for(file_path) + sig { params(file_path: String).returns(T::Array[FileParser]) } + def parsers_for(file_path) @parser_factory.for_path(file_path) end end diff --git a/lib/packwerk/parsers.rb b/lib/packwerk/parsers.rb index a9f74dbb5..f4ea493e1 100644 --- a/lib/packwerk/parsers.rb +++ b/lib/packwerk/parsers.rb @@ -5,9 +5,11 @@ module Packwerk module Parsers autoload :Erb, "packwerk/parsers/erb" autoload :Factory, "packwerk/parsers/factory" - autoload :ParserInterface, "packwerk/parsers/parser_interface" autoload :Ruby, "packwerk/parsers/ruby" + # Require parsers so that they are registered with FileParser + Dir[File.join(__dir__, "parsers", "*.rb")].each { |file| require file } + class ParseResult < Offense; end class ParseError < StandardError diff --git a/lib/packwerk/parsers/erb.rb b/lib/packwerk/parsers/erb.rb index 64e0502ad..77d1cd936 100644 --- a/lib/packwerk/parsers/erb.rb +++ b/lib/packwerk/parsers/erb.rb @@ -11,7 +11,10 @@ module Parsers class Erb extend T::Sig - include ParserInterface + include Packwerk::FileParser + + ERB_REGEX = /\.erb\Z/ + private_constant :ERB_REGEX sig { params(parser_class: T.untyped, ruby_parser: Ruby).void } def initialize(parser_class: BetterHtml::Parser, ruby_parser: Ruby.new) @@ -38,6 +41,11 @@ def parse_buffer(buffer, file_path:) raise Parsers::ParseError, result end + sig { override.params(path: String).returns(T::Boolean) } + def match?(path:) + ERB_REGEX.match?(path) + end + private sig do diff --git a/lib/packwerk/parsers/factory.rb b/lib/packwerk/parsers/factory.rb index d33680bf1..65e6fc228 100644 --- a/lib/packwerk/parsers/factory.rb +++ b/lib/packwerk/parsers/factory.rb @@ -9,43 +9,9 @@ class Factory extend T::Sig include Singleton - RUBY_REGEX = %r{ - # Although not important for regex, these are ordered from most likely to match to least likely. - \.(rb|rake|builder|gemspec|ru)\Z - | - (Gemfile|Rakefile)\Z - }x - private_constant :RUBY_REGEX - - ERB_REGEX = /\.erb\Z/ - private_constant :ERB_REGEX - - sig { void } - def initialize - @ruby_parser = T.let(nil, T.nilable(ParserInterface)) - @erb_parser = T.let(nil, T.nilable(ParserInterface)) - @erb_parser_class = T.let(nil, T.nilable(Class)) - end - - sig { params(path: String).returns(T.nilable(ParserInterface)) } + sig { params(path: String).returns(T::Array[Packwerk::FileParser]) } def for_path(path) - case path - when RUBY_REGEX - @ruby_parser ||= Ruby.new - when ERB_REGEX - @erb_parser ||= T.unsafe(erb_parser_class).new - end - end - - sig { returns(Class) } - def erb_parser_class - @erb_parser_class ||= Erb - end - - sig { params(klass: T.nilable(Class)).void } - def erb_parser_class=(klass) - @erb_parser_class = klass - @erb_parser = nil + Packwerk::FileParser.all.select { |parser| parser.match?(path: path) } end end end diff --git a/lib/packwerk/parsers/parser_interface.rb b/lib/packwerk/parsers/parser_interface.rb deleted file mode 100644 index eba079505..000000000 --- a/lib/packwerk/parsers/parser_interface.rb +++ /dev/null @@ -1,19 +0,0 @@ -# typed: strict -# frozen_string_literal: true - -module Packwerk - module Parsers - module ParserInterface - extend T::Helpers - extend T::Sig - - requires_ancestor { Kernel } - - interface! - - sig { abstract.params(io: T.any(IO, StringIO), file_path: String).returns(T.untyped) } - def call(io:, file_path:) - end - end - end -end diff --git a/lib/packwerk/parsers/ruby.rb b/lib/packwerk/parsers/ruby.rb index b218e20c2..58a10b2bc 100644 --- a/lib/packwerk/parsers/ruby.rb +++ b/lib/packwerk/parsers/ruby.rb @@ -9,7 +9,15 @@ module Parsers class Ruby extend T::Sig - include ParserInterface + include Packwerk::FileParser + + RUBY_REGEX = %r{ + # Although not important for regex, these are ordered from most likely to match to least likely. + \.(rb|rake|builder|gemspec|ru)\Z + | + (Gemfile|Rakefile)\Z + }x + private_constant :RUBY_REGEX class RaiseExceptionsParser < Parser::CurrentRuby extend T::Sig @@ -49,6 +57,11 @@ def call(io:, file_path: "") result = ParseResult.new(file: file_path, message: "Syntax error: #{e}") raise Parsers::ParseError, result end + + sig { override.params(path: String).returns(T::Boolean) } + def match?(path:) + RUBY_REGEX.match?(path) + end end end end diff --git a/test/unit/packwerk/parsers/factory_test.rb b/test/unit/packwerk/parsers/factory_test.rb index b0b3af765..4704e2e2d 100644 --- a/test/unit/packwerk/parsers/factory_test.rb +++ b/test/unit/packwerk/parsers/factory_test.rb @@ -10,44 +10,58 @@ module Packwerk module Parsers class FactoryTest < Minitest::Test test "#for_path gives ruby parser for common Ruby paths" do - assert_instance_of(Parsers::Ruby, factory.for_path("foo.rb")) - assert_instance_of(Parsers::Ruby, factory.for_path("relative/path/to/foo.ru")) - assert_instance_of(Parsers::Ruby, factory.for_path("foo.rake")) - assert_instance_of(Parsers::Ruby, factory.for_path("foo.builder")) - assert_instance_of(Parsers::Ruby, factory.for_path("in/repo/gem/foo.gemspec")) - assert_instance_of(Parsers::Ruby, factory.for_path("Gemfile")) - assert_instance_of(Parsers::Ruby, factory.for_path("some/path/Rakefile")) + assert_instance_of(Parsers::Ruby, factory.for_path("foo.rb").first) + assert_instance_of(Parsers::Ruby, factory.for_path("relative/path/to/foo.ru").first) + assert_instance_of(Parsers::Ruby, factory.for_path("foo.rake").first) + assert_instance_of(Parsers::Ruby, factory.for_path("foo.builder").first) + assert_instance_of(Parsers::Ruby, factory.for_path("in/repo/gem/foo.gemspec").first) + assert_instance_of(Parsers::Ruby, factory.for_path("Gemfile").first) + assert_instance_of(Parsers::Ruby, factory.for_path("some/path/Rakefile").first) end test "#for_path gives ERB parser for common ERB paths" do - assert_instance_of(Parsers::Erb, factory.for_path("foo.html.erb")) - assert_instance_of(Parsers::Erb, factory.for_path("foo.md.erb")) - assert_instance_of(Parsers::Erb, factory.for_path("/sub/directory/foo.erb")) + assert_instance_of(Parsers::Erb, factory.for_path("foo.html.erb").first) + assert_instance_of(Parsers::Erb, factory.for_path("foo.md.erb").first) + assert_instance_of(Parsers::Erb, factory.for_path("/sub/directory/foo.erb").first) + end + + test "#for_path gives multiple parsers for matching paths" do + fake_class_1 = Class.new do + T.unsafe(self).include(Packwerk::FileParser) - fake_class = Class.new do - T.unsafe(self).include(ParserInterface) + def match?(path:) + /\.haml\Z/.match?(path) + end end - with_erb_parser_class(fake_class) do - assert_instance_of(fake_class, factory.for_path("foo.html.erb")) + fake_class_2 = Class.new do + T.unsafe(self).include(Packwerk::FileParser) + + def match?(path:) + /\.haml\Z/.match?(path) + end end + + factories = factory.for_path("foo.haml") + assert_equal(2, factories.size) + assert_instance_of(fake_class_1, factories[0]) + assert_instance_of(fake_class_2, factories[1]) + + Packwerk::FileParser.remove(fake_class_1) + Packwerk::FileParser.remove(fake_class_2) + + factories = factory.for_path("foo.haml") + assert_equal(0, factories.size) end - test "#for_path gives nil for unknown path" do - assert_nil(factory.for_path("not_a_ruby.rb.txt")) - assert_nil(factory.for_path("some/path/rb")) - assert_nil(factory.for_path("compoennts/foo/body.erb.html")) + test "#for_path gives empty array for unknown path" do + assert_empty(factory.for_path("not_a_ruby.rb.txt")) + assert_empty(factory.for_path("some/path/rb")) + assert_empty(factory.for_path("compoennts/foo/body.erb.html")) end private - def with_erb_parser_class(klass) - factory.erb_parser_class = klass - yield - ensure - factory.erb_parser_class = nil - end - def factory Parsers::Factory.instance end