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

Implement Rubocop DSL RBI compiler #1046

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
67 changes: 67 additions & 0 deletions lib/tapioca/dsl/compilers/rubocop.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# typed: strict
# frozen_string_literal: true

return unless defined?(RuboCop::AST::NodePattern::Macros)

module Tapioca
module Dsl
module Compilers
# `Tapioca::Dsl::Compilers::RuboCop` generates types for RuboCop cops.
# RuboCop uses macros to define methods leveraging "AST node patterns".
# For example, in this cop
#
# class MyCop < Base
# def_node_matcher :matches_some_pattern?, "..."
#
# def on_send(node)
# return unless matches_some_pattern?(node)
# # ...
# end
# end
#
# the use of `def_node_matcher` will generate the method
# `matches_some_pattern?`, for which this compiler will generate a `sig`.
#
# More complex uses are also supported, including:
#
# - Usage of `def_node_search`
# - Parameter specification
# - Default parameter specification, including generating sigs for
# `without_defaults_*` methods
class RuboCop < Compiler
ConstantType = type_member do
{ fixed: T.all(Module, Extensions::RuboCop) }
end

class << self
extend T::Sig
sig { override.returns(T::Array[T.all(Module, Extensions::RuboCop)]) }
def gather_constants
T.cast(
extenders_of(::RuboCop::AST::NodePattern::Macros).select { |constant| name_of(constant) },
T::Array[T.all(Module, Extensions::RuboCop)],
)
end
end

sig { override.void }
def decorate
return if node_methods.empty?

root.create_path(constant) do |cop_klass|
node_methods.each do |name|
create_method_from_def(cop_klass, constant.instance_method(name))
end
end
end

private

sig { returns(T::Array[Extensions::RuboCop::MethodName]) }
def node_methods
constant.__tapioca_node_methods
end
end
end
end
end
45 changes: 45 additions & 0 deletions lib/tapioca/dsl/extensions/rubocop.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# typed: strict
# frozen_string_literal: true

begin
require "rubocop-ast"
rescue LoadError
return
end

module Tapioca
module Dsl
module Compilers
module Extensions
module RuboCop
extend T::Sig

MethodName = T.type_alias { T.any(String, Symbol) }

sig { params(name: MethodName, _args: T.untyped, defaults: T.untyped).returns(MethodName) }
def def_node_matcher(name, *_args, **defaults)
__tapioca_node_methods << name
__tapioca_node_methods << :"without_defaults_#{name}" unless defaults.empty?

super
end

sig { params(name: MethodName, _args: T.untyped, defaults: T.untyped).returns(MethodName) }
def def_node_search(name, *_args, **defaults)
__tapioca_node_methods << name
__tapioca_node_methods << :"without_defaults_#{name}" unless defaults.empty?

super
end

sig { returns(T::Array[MethodName]) }
def __tapioca_node_methods
@__tapioca_node_methods ||= T.let([], T.nilable(T::Array[MethodName]))
end

::RuboCop::AST::NodePattern::Macros.prepend(self)
end
end
end
end
end
27 changes: 27 additions & 0 deletions lib/tapioca/runtime/reflection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,33 @@ def descendants_of(klass)
T.unsafe(result)
end

# Returns an array with all modules which extend the supplied module
# (i.e. all modules whose singleton class, or ancestor thereof, includes the supplied module).
#
# module M; end
# extenders_of(M) # => []
#
# module E
# extend M
# end
# extenders_of(M) # => [E]
#
# class P
# extend M
# end
# extenders_of(M) # => [E, P]
#
# class C < P; end
# extenders_of(M) # => [E, P, C]
sig { params(mod: Module).returns(T::Array[Module]) }
def extenders_of(mod)
result = ObjectSpace.each_object(Module).select do |m|
T.cast(m, Module).singleton_class.included_modules.include?(mod)
end

T.cast(result, T::Array[Module])
end

# Examines the call stack to identify the closest location where a "require" is performed
# by searching for the label "<top (required)>". If none is found, it returns the location
# labeled "<main>", which is the original call site.
Expand Down
24 changes: 24 additions & 0 deletions manual/compiler_rubocop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
## RuboCop

`Tapioca::Dsl::Compilers::RuboCop` generates types for RuboCop cops.
RuboCop uses macros to define methods leveraging "AST node patterns".
For example, in this cop

class MyCop < Base
def_node_matcher :matches_some_pattern?, "..."

def on_send(node)
return unless matches_some_pattern?(node)
# ...
end
end

the use of `def_node_matcher` will generate the method
`matches_some_pattern?`, for which this compiler will generate a `sig`.

More complex uses are also supported, including:

- Usage of `def_node_search`
- Parameter specification
- Default parameter specification, including generating sigs for
`without_defaults_*` methods
1 change: 1 addition & 0 deletions manual/compilers.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ In the following section you will find all available DSL compilers:
* [MixedInClassAttributes](compiler_mixedinclassattributes.md)
* [Protobuf](compiler_protobuf.md)
* [RailsGenerators](compiler_railsgenerators.md)
* [RuboCop](compiler_rubocop.md)
* [SidekiqWorker](compiler_sidekiqworker.md)
* [SmartProperties](compiler_smartproperties.md)
* [StateMachines](compiler_statemachines.md)
Expand Down
174 changes: 174 additions & 0 deletions spec/tapioca/dsl/compilers/rubocop_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
# typed: strict
# frozen_string_literal: true

require "spec_helper"

module Tapioca
module Dsl
module Compilers
class RuboCopSpec < ::DslSpec
class << self
extend T::Sig

sig { override.returns(String) }
def target_class_file
# Against convention, RuboCop uses "rubocop" in its file names, so we do too.
super.gsub("rubo_cop", "rubocop")
end
Comment on lines +14 to +17
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Originally, I'd renamed the files to rubo_cop.rb too, due to the loading not working, but I like this better, as it allows us to keep both the constant names and file names matching RuboCop's convention (which was the whole point of renaming the constants).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we still need this since we don't rename the file in the DSL command?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This allows us to have

  • lib/tapioca/dsl/compilers/rubocop.rb
  • lib/tapioca/dsl/extensions/rubocop.rb
  • spec/tapioca/dsl/compilers/rubocop_spec.rb

instead of

  • lib/tapioca/dsl/compilers/rubo_cop.rb
  • lib/tapioca/dsl/extensions/rubo_cop.rb
  • spec/tapioca/dsl/compilers/rubo_cop_spec.rb

which I think is preferable, especially given it's really just limited to telling the spec file how to require the correct compiler.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would rather rename the constant Rubocop or let the file be named rubo_cop rather than play with this.

It seems all the files under dsl/compilers follow the same convention. class CompilerName -> compiler_name.rb. I'm not sure why we should deviate from this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤷 I was trying to match Rubocop's own naming convention (e.g. RuboCop::Version is defined in lib/rubocop/version.rb, not lib/rubo_cop/version.rb), but given the can of worms it would be to do this for the RBI files, maybe it's not worth doing for the compiler files either. It just feels weird seeing rubo_cop TBH, but I'm glad to deal with it if you think it's not worth the extra method in the test file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, so circling back to this, a80c4ec makes the changes necessary to avoid this workaround, but my gut feeling is that this leads to the code being more confusing, because now we're dealing with a mix of constants that include RuboCop and constants that include Rubocop, and it's not obvious why they differ.

Likewise, we could go for the rubo_cop file naming, but then we'd have a mid of those files and ones like .rubocop.yml, again without an obvious reason why.

Therefore, I think it's overall more straightforward to keep the RuboCop naming to match upstream, and be consistent across our constants and theirs, and have this one workaround method with the comment documenting why it's there. I think this would be in line with the "principle of least surprise", in that this one method makes the rest of the app less surprising because things just work.

end

describe "Tapioca::Dsl::Compilers::RuboCop" do
sig { void }
def before_setup
require "rubocop"
require "rubocop-sorbet"
require "tapioca/dsl/extensions/rubocop"
super
end

describe "initialize" do
it "gathered constants exclude irrelevant classes" do
gathered_constants = gather_constants do
add_ruby_file("content.rb", <<~RUBY)
class Unrelated
end
RUBY
end
assert_empty(gathered_constants)
end

it "gathers constants extending RuboCop::AST::NodePattern::Macros in gems" do
# Sample of miscellaneous constants that should be found from Rubocop and plugins
missing_constants = [
"RuboCop::Cop::Bundler::GemVersion",
"RuboCop::Cop::Cop",
"RuboCop::Cop::Gemspec::DependencyVersion",
"RuboCop::Cop::Lint::Void",
"RuboCop::Cop::Metrics::ClassLength",
"RuboCop::Cop::Migration::DepartmentName",
"RuboCop::Cop::Naming::MethodName",
"RuboCop::Cop::Security::CompoundHash",
"RuboCop::Cop::Sorbet::ValidSigil",
"RuboCop::Cop::Style::YodaCondition",
] - gathered_constants

assert_empty(missing_constants, "expected constants to be gathered")
end

it "gathers constants extending RuboCop::AST::NodePattern::Macros in the host app" do
gathered_constants = gather_constants do
add_ruby_file("content.rb", <<~RUBY)
class MyCop < ::RuboCop::Cop::Base
end

class MyLegacyCop < ::RuboCop::Cop::Cop
end

module MyMacroModule
extend ::RuboCop::AST::NodePattern::Macros
end

module ::RuboCop
module Cop
module MyApp
class MyNamespacedCop < Base
end
end
end
end
RUBY
end

assert_equal(
["MyCop", "MyLegacyCop", "MyMacroModule", "RuboCop::Cop::MyApp::MyNamespacedCop"],
gathered_constants,
)
end
end

describe "decorate" do
it "generates empty RBI when no DSL used" do
add_ruby_file("content.rb", <<~RUBY)
class MyCop < ::RuboCop::Cop::Base
sambostock marked this conversation as resolved.
Show resolved Hide resolved
def on_send(node);end
end
RUBY

expected = <<~RBI
# typed: strong
RBI

assert_equal(expected, rbi_for(:MyCop))
end

it "generates correct RBI file" do
add_ruby_file("content.rb", <<~RUBY)
class MyCop < ::RuboCop::Cop::Base
def_node_matcher :some_matcher, "(...)"
def_node_matcher :some_matcher_with_params, "(%1 %two ...)"
def_node_matcher :some_matcher_with_params_and_defaults, "(%1 %two ...)", two: :default
Comment on lines +107 to +109
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that these result in different method signatures, and in the last case, in a second method being generated (without_defaults_...).

The implementation of def_node_matcher can be found in ::RuboCop::AST::NodePattern::Macros, for reference.

def_node_matcher :some_predicate_matcher?, "(...)"
def_node_search :some_search, "(...)"
def_node_search :some_search_with_params, "(%1 %two ...)"
def_node_search :some_search_with_params_and_defaults, "(%1 %two ...)", two: :default

def on_send(node);end
end
RUBY

expected = <<~RBI
# typed: strong

class MyCop
sig { params(param0: T.untyped).returns(T.untyped) }
def some_matcher(param0 = T.unsafe(nil)); end

sig { params(param0: T.untyped, param1: T.untyped, two: T.untyped).returns(T.untyped) }
def some_matcher_with_params(param0 = T.unsafe(nil), param1, two:); end

sig { params(args: T.untyped, values: T.untyped).returns(T.untyped) }
def some_matcher_with_params_and_defaults(*args, **values); end

sig { params(param0: T.untyped).returns(T.untyped) }
def some_predicate_matcher?(param0 = T.unsafe(nil)); end

sig { params(param0: T.untyped).returns(T.untyped) }
def some_search(param0); end

sig { params(param0: T.untyped, param1: T.untyped, two: T.untyped).returns(T.untyped) }
def some_search_with_params(param0, param1, two:); end

sig { params(args: T.untyped, values: T.untyped).returns(T.untyped) }
def some_search_with_params_and_defaults(*args, **values); end

sig { params(param0: T.untyped, param1: T.untyped, two: T.untyped).returns(T.untyped) }
def without_defaults_some_matcher_with_params_and_defaults(param0 = T.unsafe(nil), param1, two:); end

sig { params(param0: T.untyped, param1: T.untyped, two: T.untyped).returns(T.untyped) }
def without_defaults_some_search_with_params_and_defaults(param0, param1, two:); end
end
RBI

assert_equal(expected, rbi_for(:MyCop))
end
end

private

# Gathers constants introduced in the given block excluding constants that already existed prior to the block.
sig { params(block: T.proc.void).returns(T::Array[String]) }
def gather_constants(&block)
existing_constants = T.let(
Runtime::Reflection
.extenders_of(::RuboCop::AST::NodePattern::Macros)
.filter_map { |constant| Runtime::Reflection.name_of(constant) },
T::Array[String],
)
yield
gathered_constants - existing_constants
end
end
end
end
end
end
Loading