search

lsegal / yard

YARD is a Ruby Documentation tool. The Y stands for "Yay!"
http://yardoc.org
MIT License
1.95k stars 398 forks source link

yield_handler_spec.rb test fails with recent ruby3.3.0dev #1514

Closed mtasaka closed 8 months ago

mtasaka commented 1 year ago

Testing yard head ( https://github.com/lsegal/yard/commit/2d197a381c5d4cc5c55b2c60fff992b31c986361 ), with ruby 3.3.0dev spec test fails like below.

So exactly https://github.com/ruby/ruby/commit/b5e23d3e3b5ff2f5328aa43a2392ebe7c951a222 is making yard testsuite fail. I don't know in detail what yard or the above ruby commit does, however it looks like the above ruby commit rejects some of yield usage.

$ rspec -rspec_helper spec

Failures:

  1) YARD::Handlers::Ruby::YieldHandler only parses yield blocks in methods
     Failure/Error: raise ProxyMethodError, "Proxy cannot call method ##{meth} on object '#{path}'"

     YARD::CodeObjects::ProxyMethodError:
       Proxy cannot call method #tag on object 'Testing'
     # ./lib/yard/code_objects/proxy.rb:194:in `rescue in method_missing'
     # ./lib/yard/code_objects/proxy.rb:191:in `method_missing'
     # ./spec/handlers/yield_handler_spec.rb:8:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:129:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # NoMethodError:
     #   undefined method `tag' for an instance of YARD::CodeObjects::Proxy
     #   ./lib/yard/code_objects/proxy.rb:192:in `method_missing'

  2) YARD::Handlers::Ruby::YieldHandler handles an empty yield statement
     Failure/Error: raise ProxyMethodError, "Proxy cannot call method ##{meth} on object '#{path}'"

     YARD::CodeObjects::ProxyMethodError:
       Proxy cannot call method #tag on object 'Testing#mymethod'
     # ./lib/yard/code_objects/proxy.rb:194:in `rescue in method_missing'
     # ./lib/yard/code_objects/proxy.rb:191:in `method_missing'
     # ./spec/handlers/yield_handler_spec.rb:13:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:129:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # NoMethodError:
     #   undefined method `tag' for an instance of YARD::CodeObjects::Proxy
     #   ./lib/yard/code_objects/proxy.rb:192:in `method_missing'

  3) YARD::Handlers::Ruby::YieldHandler does not document a yield statement in a method with either @yield or @yieldparam
     Failure/Error: raise ProxyMethodError, "Proxy cannot call method ##{meth} on object '#{path}'"

     YARD::CodeObjects::ProxyMethodError:
       Proxy cannot call method #tag on object 'Testing#mymethod2'
     # ./lib/yard/code_objects/proxy.rb:194:in `rescue in method_missing'
     # ./lib/yard/code_objects/proxy.rb:191:in `method_missing'
     # ./spec/handlers/yield_handler_spec.rb:18:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:129:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # NoMethodError:
     #   undefined method `tag' for an instance of YARD::CodeObjects::Proxy
     #   ./lib/yard/code_objects/proxy.rb:192:in `method_missing'

  4) YARD::Handlers::Ruby::YieldHandler handles any arbitrary yield statement
     Failure/Error: raise ProxyMethodError, "Proxy cannot call method ##{meth} on object '#{path}'"

     YARD::CodeObjects::ProxyMethodError:
       Proxy cannot call method #tag on object 'Testing#mymethod5'
     # ./lib/yard/code_objects/proxy.rb:194:in `rescue in method_missing'
     # ./lib/yard/code_objects/proxy.rb:191:in `method_missing'
     # ./spec/handlers/yield_handler_spec.rb:30:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:129:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # NoMethodError:
     #   undefined method `tag' for an instance of YARD::CodeObjects::Proxy
     #   ./lib/yard/code_objects/proxy.rb:192:in `method_missing'

  5) YARD::Handlers::Ruby::YieldHandler handles parentheses
     Failure/Error: raise ProxyMethodError, "Proxy cannot call method ##{meth} on object '#{path}'"

     YARD::CodeObjects::ProxyMethodError:
       Proxy cannot call method #tag on object 'Testing#mymethod6'
     # ./lib/yard/code_objects/proxy.rb:194:in `rescue in method_missing'
     # ./lib/yard/code_objects/proxy.rb:191:in `method_missing'
     # ./spec/handlers/yield_handler_spec.rb:34:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:129:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # NoMethodError:
     #   undefined method `tag' for an instance of YARD::CodeObjects::Proxy
     #   ./lib/yard/code_objects/proxy.rb:192:in `method_missing'

  6) YARD::Handlers::Ruby::YieldHandler only documents the first yield statement in a method (limitation of yield handler)
     Failure/Error: raise ProxyMethodError, "Proxy cannot call method ##{meth} on object '#{path}'"

     YARD::CodeObjects::ProxyMethodError:
       Proxy cannot call method #tag on object 'Testing#mymethod7'
     # ./lib/yard/code_objects/proxy.rb:194:in `rescue in method_missing'
     # ./lib/yard/code_objects/proxy.rb:191:in `method_missing'
     # ./spec/handlers/yield_handler_spec.rb:38:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:129:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # NoMethodError:
     #   undefined method `tag' for an instance of YARD::CodeObjects::Proxy
     #   ./lib/yard/code_objects/proxy.rb:192:in `method_missing'

  7) YARD::Handlers::Ruby::YieldHandler handles `self` keyword and list object type as yieldparam for _self
     Failure/Error: raise ProxyMethodError, "Proxy cannot call method ##{meth} on object '#{path}'"

     YARD::CodeObjects::ProxyMethodError:
       Proxy cannot call method #tag on object 'Testing#mymethod8'
     # ./lib/yard/code_objects/proxy.rb:194:in `rescue in method_missing'
     # ./lib/yard/code_objects/proxy.rb:191:in `method_missing'
     # ./spec/handlers/yield_handler_spec.rb:42:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:129:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # NoMethodError:
     #   undefined method `tag' for an instance of YARD::CodeObjects::Proxy
     #   ./lib/yard/code_objects/proxy.rb:192:in `method_missing'

  8) YARD::Handlers::Ruby::YieldHandler handles `super` keyword and document it under _super
     Failure/Error: raise ProxyMethodError, "Proxy cannot call method ##{meth} on object '#{path}'"

     YARD::CodeObjects::ProxyMethodError:
       Proxy cannot call method #tag on object 'Testing#mymethod9'
     # ./lib/yard/code_objects/proxy.rb:194:in `rescue in method_missing'
     # ./lib/yard/code_objects/proxy.rb:191:in `method_missing'
     # ./spec/handlers/yield_handler_spec.rb:48:in `block (2 levels) in <top (required)>'
     # ./spec/spec_helper.rb:129:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # NoMethodError:
     #   undefined method `tag' for an instance of YARD::CodeObjects::Proxy
     #   ./lib/yard/code_objects/proxy.rb:192:in `method_missing'
mateusdeap commented 1 year ago

I did some preliminary testing here and, although I wasn't able to confirm how this is connected to the commit linked, one thing is for certain: When calling P(:Testing) in ruby 3.2.2, the returned class is an instance of YARD::CodeObjects::ClassObject and it has 126 methods. tag, which is defined in code_objects/base.rb:548 is among them.

Where as running this against 3.3.0 dev's latest commit, the object returned by P(:Testing) has only 72 methods and it's an instance of YARD::CodeObjects::Proxy.

I'm unsure how this is supposed to work, but given the docs for the P method, seems to me that ruby 3.3.0 is returning the correct object, but it's weird

mateusdeap commented 1 year ago

So, after more digging, and maybe dealing specifically with the issue at hand, the file parsed in the yield handler spec has invalid ruby syntax, as of the latest commit.

The tests in https://github.com/ruby/ruby/commit/b5e23d3e3b5ff2f5328aa43a2392ebe7c951a222 assert that simply writing yield inside a class is invalid. Going by the assertions, we'd have to write:

class Testing
  defined?(yield)
end

I did as much, but another error surfaced related to the locale spec. I'll open a PR with the change which could be modified to fix all failing specs as they appear.

mateusdeap commented 1 year ago

So, I made a mistake. I think I might've edited the test file by accident, but indeed we didn't have yield but yield x, y, x. It's still invalid code according to the tests, if I understood it right.

~That being said, I've tested the specs using the latest 3.3.0 preview1 and they pass. I'm getting a failure still, however, but I think it's something on my end, because it fails on all ruby versions and it wasn't failing before. So maybe this issue won't be an issue?~

EDIT: Just confirmed that all specs pass with ruby 3.3.0 preview 1

mtasaka commented 1 year ago

So trying to shorten test spec file yield_handler_spec.rb : https://github.com/lsegal/yard/blob/698b22cc2340230856ab17a25cfefa889f1c8289/spec/handlers/yield_handler_spec.rb as:

# frozen_string_literal: true
require File.dirname(__FILE__) + '/spec_helper'

RSpec.describe "YARD::Handlers::Ruby::#{LEGACY_PARSER ? "Legacy::" : ""}YieldHandler" do
  before(:all) { parse_file :yield_handler_001, __FILE__ }

  it "only parses yield blocks in methods" do
    expect(P(:Testing).tag(:yield)).to be nil
  end
end

Then I see the following message:

YARD::Handlers::Ruby::YieldHandler
[warn]: Syntax error in `/builddir/build/build/yard-0.9.34/spec/handlers/examples/yield_handler_001.rb.txt`:(3,2): invalid yield
[warn]: ParserSyntaxError: syntax error in `/builddir/build/BUILD/yard-0.9.34/spec/handlers/examples/yield_handler_001.rb.txt`:(3,2): Invalid yield
[warn]: Stack trace:
    /builddir/build/BUILD/yard-0.9.34/lib/yard/parser/ruby/ruby_parser.rb:607:in `on_parse_error'
    /builddir/build/BUILD/yard-0.9.34/lib/yard/parser/ruby/ruby_parser.rb:56:in `parse'
    /builddir/build/BUILD/yard-0.9.34/lib/yard/parser/ruby/ruby_parser.rb:56:in `parse'
    /builddir/build/BUILD/yard-0.9.34/lib/yard/parser/ruby/ruby_parser.rb:17:in `parse'
    /builddir/build/BUILD/yard-0.9.34/lib/yard/parser/source_parser.rb:442:in `parse'
    /builddir/build/BUILD/yard-0.9.34/lib/yard/parser/source_parser.rb:46:in `block in parse'

So to make whole spec test suite successful, the following change seems enough:

diff --git a/spec/handlers/examples/yield_handler_001.rb.txt b/spec/handlers/examples/yield_handler_001.rb.txt
index 668f8183..ecc43276 100644
--- a/spec/handlers/examples/yield_handler_001.rb.txt
+++ b/spec/handlers/examples/yield_handler_001.rb.txt
@@ -1,7 +1,4 @@
 class Testing
-  # Ignore yields outside methods
-  yield x, y, z
-
   # Should document this
   def mymethod
     yield
mtasaka commented 1 year ago

For now, I am going to create PR (i.e. remove invalid yield usage from spec example). This seems to make spec testsuite pass even on ruby3.3.