diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a3c96a7..801b495b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## main +### New Features + +* [#141](https://github.com/dduugg/yard-sorbet/issues/141) Merge RBI sigs into existing documentation + ### Bug Fixes * Handle multiple invocations of `mixes_in_class_methods` within a class diff --git a/README.md b/README.md index b26323b2..25005e79 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,7 @@ A YARD [plugin](https://rubydoc.info/gems/yard/file/docs/GettingStarted.md#Plugi - Generates constant definitions from `T::Enum` enums - Modules marked `abstract!` or `interface!` are tagged `@abstract` - Modules using `mixes_in_class_methods` will attach class methods +- Merges `sig`s in rbi files with source code documentation (rbi files must come after source code in yard configuration) ## Usage diff --git a/lib/yard-sorbet/handlers/sig_handler.rb b/lib/yard-sorbet/handlers/sig_handler.rb index b977154c..359489a2 100644 --- a/lib/yard-sorbet/handlers/sig_handler.rb +++ b/lib/yard-sorbet/handlers/sig_handler.rb @@ -10,43 +10,93 @@ class SigHandler < YARD::Handlers::Ruby::Base handles method_call(:sig) namespace_only - # These node types attached to sigs represent attr_* declarations - ATTR_NODE_TYPES = T.let(%i[command fcall].freeze, T::Array[Symbol]) - private_constant :ATTR_NODE_TYPES + # YARD types that can have docstrings attached to them + Documentable = T.type_alias do + T.any( + YARD::CodeObjects::MethodObject, YARD::Parser::Ruby::MethodCallNode, YARD::Parser::Ruby::MethodDefinitionNode + ) + end + private_constant :Documentable # Swap the method definition docstring and the sig docstring. # Parse relevant parts of the `sig` and include them as well. sig { void } def process method_node = NodeUtils.get_method_node(NodeUtils.sibling_node(statement)) - docstring, directives = Directives.extract_directives(statement.docstring) - parse_sig(method_node, docstring) - method_node.docstring = docstring.to_raw - Directives.add_directives(method_node.docstring, directives) + case method_node + when YARD::Parser::Ruby::MethodDefinitionNode then process_def(method_node) + when YARD::Parser::Ruby::MethodCallNode then process_attr(method_node) + end statement.docstring = nil end private - sig { params(method_node: YARD::Parser::Ruby::AstNode, docstring: YARD::Docstring).void } - def parse_sig(method_node, docstring) + sig { params(def_node: YARD::Parser::Ruby::MethodDefinitionNode).void } + def process_def(def_node) + separator = scope == :instance && def_node.type == :def ? '#' : '.' + registered = YARD::Registry.at("#{namespace}#{separator}#{def_node.method_name(true)}") + if registered + parse_node(registered, registered.docstring) + # Since we're probably in an RBI file, delete the def node, which could otherwise erroneously override the + # visibility setting + NodeUtils.delete_node(def_node) + else + parse_node(def_node, statement.docstring) + end + end + + sig { params(attr_node: YARD::Parser::Ruby::MethodCallNode).void } + def process_attr(attr_node) + return if merged_into_attr?(attr_node) + + parse_node(attr_node, statement.docstring, include_params: false) + end + + # An attr* sig can be merged into a previous attr* docstring if it is the only parameter passed to the attr* + # declaration. This is to avoid needing to rewrite the source code to separate merged and unmerged attr* + # declarations. + sig { params(attr_node: YARD::Parser::Ruby::MethodCallNode).returns(T::Boolean) } + def merged_into_attr?(attr_node) + names = NodeUtils.validated_attribute_names(attr_node) + return false if names.size != 1 + + attrs = namespace.attributes[scope][names[0]] + return false if attrs.nil? || attrs.empty? + + document_attr_methods(attrs.values.compact) + attr_node.docstring = nil + true + end + + sig { params(method_objects: T::Array[YARD::CodeObjects::MethodObject]).void } + def document_attr_methods(method_objects) + method_objects.each { parse_node(_1, _1.docstring, include_params: false) } + end + + sig { params(attach_to: Documentable, docstring: T.nilable(String), include_params: T::Boolean).void } + def parse_node(attach_to, docstring, include_params: true) + existing_docstring = docstring.is_a?(YARD::Docstring) + docstring, directives = Directives.extract_directives(docstring) unless existing_docstring + parse_sig(docstring, include_params: include_params) + attach_to.docstring = docstring.to_raw + Directives.add_directives(attach_to.docstring, directives) unless existing_docstring + end + + sig { params(docstring: YARD::Docstring, include_params: T::Boolean).void } + def parse_sig(docstring, include_params: true) NodeUtils.bfs_traverse(statement) do |node| case node.source when 'returns' then parse_return(node, docstring) - when 'params' then parse_params(method_node, node, docstring) + when 'params' then parse_params(node, docstring) if include_params when 'void' then TagUtils.upsert_tag(docstring, 'return', TagUtils::VOID_RETURN_TYPE) when 'abstract' then TagUtils.upsert_tag(docstring, 'abstract') end end end - sig do - params(method_node: YARD::Parser::Ruby::AstNode, node: YARD::Parser::Ruby::AstNode, docstring: YARD::Docstring) - .void - end - def parse_params(method_node, node, docstring) - return if ATTR_NODE_TYPES.include?(method_node.type) - + sig { params(node: YARD::Parser::Ruby::AstNode, docstring: YARD::Docstring).void } + def parse_params(node, docstring) sibling = NodeUtils.sibling_node(node) sibling.dig(0, 0).each do |param| param_name = param.dig(0, 0) diff --git a/lib/yard-sorbet/node_utils.rb b/lib/yard-sorbet/node_utils.rb index 98fba39e..4873f92f 100644 --- a/lib/yard-sorbet/node_utils.rb +++ b/lib/yard-sorbet/node_utils.rb @@ -28,6 +28,11 @@ def self.bfs_traverse(node, &_blk) end end + sig { params(node: YARD::Parser::Ruby::AstNode).void } + def self.delete_node(node) + node.parent.children.delete(node) + end + # Gets the node that a sorbet `sig` can be attached do, bypassing visisbility modifiers and the like sig { params(node: YARD::Parser::Ruby::AstNode).returns(SigableNode) } def self.get_method_node(node) @@ -51,5 +56,17 @@ def self.sigable_node?(node) else false end end + + # @see https://github.com/lsegal/yard/blob/main/lib/yard/handlers/ruby/attribute_handler.rb + # YARD::Handlers::Ruby::AttributeHandler.validated_attribute_names + sig { params(attr_node: YARD::Parser::Ruby::MethodCallNode).returns(T::Array[String]) } + def self.validated_attribute_names(attr_node) + attr_node.parameters(false).map do |obj| + case obj + when YARD::Parser::Ruby::LiteralNode then obj[0][0].source + else raise YARD::Parser::UndocumentableError, obj.source + end + end + end end end diff --git a/spec/data/sig_handler.rbi.txt b/spec/data/sig_handler.rbi.txt new file mode 100644 index 00000000..19ed8bb2 --- /dev/null +++ b/spec/data/sig_handler.rbi.txt @@ -0,0 +1,24 @@ +module Merge + class A + sig { returns(Numeric) } + attr_accessor :a_foo + + sig { returns(T.nilable(String)) } + attr_reader :a_bar + + sig { params(writer: Integer).returns(Integer) } + attr_writer :a_baz + + sig { returns(String) } + def foo; end + + sig { params(a: Integer).returns(Float) } + def self.bar(a); end + + sig { returns(String) } + def baz; end + + sig { returns(Integer) } + def bat; end + end +end diff --git a/spec/data/sig_handler.txt b/spec/data/sig_handler.txt index 17aefd17..cbaa9e75 100644 --- a/spec/data/sig_handler.txt +++ b/spec/data/sig_handler.txt @@ -294,7 +294,7 @@ end class AttrSigs sig {returns(String)} - attr_accessor :my_accessor + attr_accessor 'my_accessor' sig {returns(Integer)} attr_reader :my_reader @@ -337,3 +337,25 @@ class Nodes sig { returns(INT) } def returns_const; 1; end end + +module Merge + class A + # annotated attr_accessor + attr_accessor :a_foo + + attr_reader :a_bar + + attr_writer :a_baz + + # The foo instance method for A + def foo; end + + # The bar singleton method for A + def self.bar(a); end + + private def baz; end + + # @return the result + def bat; end + end +end diff --git a/spec/yard_sorbet/handlers/sig_handler_spec.rb b/spec/yard_sorbet/handlers/sig_handler_spec.rb index 9fca407e..34662ad7 100644 --- a/spec/yard_sorbet/handlers/sig_handler_spec.rb +++ b/spec/yard_sorbet/handlers/sig_handler_spec.rb @@ -2,11 +2,67 @@ # frozen_string_literal: true RSpec.describe YARDSorbet::Handlers::SigHandler do - path = File.join(File.expand_path('../../data', __dir__), 'sig_handler.txt') - - before do + # The :all (and corresponding rubocop disable) isn't strictly necessary, but it speeds up tests considerably + before(:all) do # rubocop:disable RSpec/BeforeAfterAll YARD::Registry.clear + path = File.join(File.expand_path('../../data', __dir__), 'sig_handler.txt') YARD::Parser::SourceParser.parse(path) + rbi_path = File.join(File.expand_path('../../data', __dir__), 'sig_handler.rbi.txt') + YARD::Parser::SourceParser.parse(rbi_path) + end + + describe 'Merging an RBI file' do + it 'includes docstring from original attr_accessor' do + expect(YARD::Registry.at('Merge::A#a_foo').docstring).to eq('annotated attr_accessor') + end + + it 'merges attr_accessor sig' do + expect(YARD::Registry.at('Merge::A#a_foo').tag(:return).types).to eq(['Numeric']) + end + + it 'includes docstring from original attr_reader' do + expect(YARD::Registry.at('Merge::A#a_bar').docstring).to eq('Returns the value of attribute a_bar.') + end + + it 'merges attr_reader sig' do + expect(YARD::Registry.at('Merge::A#a_bar').tag(:return).types).to eq(%w[String nil]) + end + + it 'includes docstring from original attr_writer' do + expect(YARD::Registry.at('Merge::A#a_baz=').docstring).to eq('Sets the attribute a_baz') + end + + it 'merges attr_writer sig' do + expect(YARD::Registry.at('Merge::A#a_baz=').tag(:return).types).to eq(['Integer']) + end + + it 'includes docstring from original instance def' do + expect(YARD::Registry.at('Merge::A#foo').docstring).to eq('The foo instance method for A') + end + + it 'merges instance def sig' do + expect(YARD::Registry.at('Merge::A#foo').tag(:return).types).to eq(['String']) + end + + it 'includes docstring from original singleton def' do + expect(YARD::Registry.at('Merge::A.bar').docstring).to eq('The bar singleton method for A') + end + + it 'merges singleton def sig' do + expect(YARD::Registry.at('Merge::A.bar').tag(:return).types).to eq(['Float']) + end + + it 'preserves the visibility of the original method' do + expect(YARD::Registry.at('Merge::A#baz').visibility).to be(:private) + end + + it 'merges sig return type with return tag' do + expect(YARD::Registry.at('Merge::A#bat').tag(:return).types).to eq(['Integer']) + end + + it 'merges return tag comment with sig return type' do + expect(YARD::Registry.at('Merge::A#bat').tag(:return).text).to eq('the result') + end end describe 'attaching to method' do @@ -240,7 +296,7 @@ end it 'preserves visibility modifier' do - expect(YARD::Registry.at('CollectionSigs#fixed_hash').visibility).to eq(:protected) + expect(YARD::Registry.at('CollectionSigs#fixed_hash').visibility).to be(:protected) end end @@ -440,4 +496,21 @@ end end end + + describe 'Unparsable sigs' do + before do + allow(log).to receive(:warn) + YARD::Parser::SourceParser.parse_string(<<~RUBY) + class Test + CONST = :foo + sig { returns(Integer) } + attr_reader CONST + end + RUBY + end + + it 'warn when parsing an attr* with a constant param' do + expect(log).to have_received(:warn).with(/Undocumentable CONST/).twice + end + end end