seattlerb / ruby_parser

ruby_parser is a ruby parser written in pure ruby. It outputs s-expressions which can be manipulated and converted back to ruby via the ruby2ruby gem.
http://www.zenspider.com/projects/ruby_parser.html
476 stars 100 forks source link

End line number not working correctly #330

Closed lucianoruizmo closed 2 years ago

lucianoruizmo commented 2 years ago

Hey! In CodeClimate, we are trying to update the version of the sexp_processor gem, but we are hitting an error that seems to be related to ruby_parser.

We are currently making those updates in this PR. However, when we run our tests, there are 2 that are failing on the expected end line.

Failures:

  1) CC::Engine::Analyzers::SexpLines violation location gets appropriate locations for rescue blocks
     Failure/Error: expect(locations[0].end_line).to eq(7)

       expected: 7
            got: 4

       (compared using ==)
     # ./spec/cc/engine/analyzers/sexp_lines_spec.rb:21:in `block (3 levels) in <module:Analyzers>'

  2) CC::Engine::Analyzers::Ruby::Main#run calculates locations correctly for conditional statements
     Failure/Error:
       expect(json["location"]).to eq({
         "path" => "foo.rb",
         "lines" => { "begin" => 2, "end" => 12 },
       })

       expected: {"lines"=>{"begin"=>2, "end"=>12}, "path"=>"foo.rb"}
            got: {"lines"=>{"begin"=>2, "end"=>11}, "path"=>"foo.rb"}

       (compared using ==)

       Diff:
       @@ -1 +1 @@
       -"lines" => {"begin"=>2, "end"=>12},
       +"lines" => {"begin"=>2, "end"=>11},

     # ./spec/cc/engine/analyzers/ruby/main_spec.rb:62:in `block (3 levels) in <module:Analyzers>'
     # ./spec/spec_helper.rb:27:in `block (4 levels) in <top (required)>'
     # ./spec/spec_helper.rb:26:in `chdir'
     # ./spec/spec_helper.rb:26:in `block (3 levels) in <top (required)>'
     # ./spec/spec_helper.rb:23:in `block (2 levels) in <top (required)>'

Finished in 21.29 seconds (files took 0.73824 seconds to load)
157 examples, 2 failures, 1 pending

Failed examples:

rspec ./spec/cc/engine/analyzers/sexp_lines_spec.rb:6 # CC::Engine::Analyzers::SexpLines violation location gets appropriate locations for rescue blocks
rspec ./spec/cc/engine/analyzers/ruby/main_spec.rb:25 # CC::Engine::Analyzers::Ruby::Main#run calculates locations correctly for conditional statements

It looks related to #292. We are not sure if we are missing something in the upgrade to avoid this conflicts. This repo is public if you need to check the code, as well as you can ask us if you need more insights on this.

Thanks!

efueger commented 2 years ago

@zenspider 👋🏼 - lmk if you need more info for the above ^^

zenspider commented 2 years ago

Have you looked at the code being parsed? Are the line numbers correct?

The last big release of RP did a lot to address line numbers:

https://github.com/seattlerb/ruby_parser/blob/master/History.rdoc#3180--2021-10-27-

On Mar 7, 2022, at 12:35, Emily Fueger @.***> wrote:

 @zenspider 👋🏼 - lmk if you need more info for the above ^^

— Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you were mentioned.

zenspider commented 2 years ago

Anything?

efueger commented 2 years ago

@zenspider - asking the team to take a look. thanks for the nudge!

zenspider commented 2 years ago

Closing. Feel free to reopen with actual source code that shows a problem.