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

Fix for syntax error discarding #35

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

europ
Copy link

@europ europ commented Feb 13, 2018

The result of pronto-rubocop does not include syntactic errors.

Missing syntactic error in the result of pronto-rubocop was solved by mapping this error on the last element of the patch.

File containing syntax error:

while a < 15
	(
print a, " "
    if a == 10 then
        prin"made it to ten!!"

    a = a + 1
end

Variable offences's value in method inspect(patch) in file lib/pronto/rubocop.rb:
https://github.com/prontolabs/pronto-rubocop/blob/master/lib/pronto/rubocop.rb#L38

[42] pry(#<Pronto::Rubocop>)> offences
=> [#<RuboCop::Cop::Offense:0x0000564daa700d20
  @cop_name="Lint/Syntax",
  @location=#<Parser::Source::Range /root/miq_bot/testrepo/error.rb 99...99>,
  @message=
   "unexpected token $end\n(Using Ruby 2.3 parser; configure using `TargetRubyVersion` parameter, under `AllCops`)",
  @severity=#<RuboCop::Cop::Severity:0x0000564daa7014f0 @name=:error>,
  @status=:uncorrected>]

before:

The syntactic error is missing in the result of the rubocop.

[168] pry(#<Pronto::Rubocop>)> offences.sort.reject(&:disabled?).map do |offence|
[168] pry(#<Pronto::Rubocop>)*   patch.added_lines        
[168] pry(#<Pronto::Rubocop>)*   .select { |line| line.new_lineno == offence.line }          
[168] pry(#<Pronto::Rubocop>)*   .map { |line| new_message(offence, line) }          
[168] pry(#<Pronto::Rubocop>)* end        
=> [[]]

after:

The syntactic error message is assigned to the last element of patch variable.

[174] pry(#<Pronto::Rubocop>)> offences.sort.reject(&:disabled?).map do |offence|
[174] pry(#<Pronto::Rubocop>)*   patch.added_lines        
[174] pry(#<Pronto::Rubocop>)*   .select { |line| line.new_lineno == offence.line }          
[174] pry(#<Pronto::Rubocop>)*   .map { |line| new_message(offence, line) }          
[174] pry(#<Pronto::Rubocop>)* end.concat(        
[174] pry(#<Pronto::Rubocop>)*   offences.sort.reject(&:disabled?).select do |offence|        
[174] pry(#<Pronto::Rubocop>)*     offence.cop_name == "Lint/Syntax"          
[174] pry(#<Pronto::Rubocop>)*   end.map do |offence|          
[174] pry(#<Pronto::Rubocop>)*     new_message(offence, patch.added_lines.last)          
[174] pry(#<Pronto::Rubocop>)*   end          
[174] pry(#<Pronto::Rubocop>)* )        
=> [[],
 #<Pronto::Message:0x0000564da7503070
  @commit_sha="dfa09b8920c8932aa0454dcffb93966fc1c9b2f5",
  @level=:error,
  @line=
   #<struct Pronto::Git::Line
    line=
     #<Rugged::Diff::Line:47445775201620 {line_origin: :addition, content: "end\n">,
    patch=
     #<struct Pronto::Git::Patch
      patch=#<Rugged::Patch:47445789709400>,
      repo=
       #<Pronto::Git::Repository:0x0000564daae22db0
        @repo=
         #<Rugged::Repository:47445789710020 {path: "/root/miq_bot/testrepo/.git/"}>>>,
    hunk=
     #<Rugged::Diff::Hunk:47445775202180 {header: "@@ -0,0 +1,8 @@\n", count: 8}>>,
  @msg=
   "unexpected token $end\n(Using Ruby 2.3 parser; configure using `TargetRubyVersion` parameter, under `AllCops`)",
  @path="error.rb",
  @runner=Pronto::Rubocop>]

Informations (version, platform, engine)

[175] pry(#<Pronto::Rubocop>)> RUBY_VERSION
=> "2.3.1"
[176] pry(#<Pronto::Rubocop>)> RUBY_PLATFORM
=> "x86_64-linux-gnu"
[177] pry(#<Pronto::Rubocop>)> RUBY_ENGINE
=> "ruby"
[178] pry(#<Pronto::Rubocop>)> Pronto::RubocopVersion::VERSION
=> "0.9.0"
[179] pry(#<Pronto::Rubocop>)> ::RuboCop::Version.version
=> "0.52.1"

/cc
@skateman
@romanblanco

@europ europ changed the title [WIP] syntax error (discarding => [WIP] fix of syntax error discarding Feb 13, 2018
@europ europ changed the title [WIP] fix of syntax error discarding Fix for syntax error discarding Feb 13, 2018
@skateman
Copy link

@mmozuras is this something you would want? Maybe this is not The Right Way™ but the syntax errors are not always mappable to patch changes 😞

@europ europ changed the title Fix for syntax error discarding [WIP] Fix for syntax error discarding Feb 20, 2018
@europ europ force-pushed the method_inspect_fix branch 2 times, most recently from 27d777b to 42f2d41 Compare February 21, 2018 13:52
@europ europ changed the title [WIP] Fix for syntax error discarding Fix for syntax error discarding Feb 21, 2018
@europ europ force-pushed the method_inspect_fix branch 5 times, most recently from d2f2d09 to 43c6a67 Compare February 23, 2018 09:32
@europ europ force-pushed the method_inspect_fix branch from 43c6a67 to c72a920 Compare April 9, 2018 08:08
europ added 4 commits April 11, 2018 11:07
Offences with syntax error were dropped, now it is fixed by handling them. The return value of the method is the same as before but syntax error offences are included too if any exists.
Due to "undefined method `allow' for #<RSpec::XX>" this line had to be removed.
The functionality of one method was divided into two logically functioning parts. One method obtain offences and the other one inspects these offences.
New tests were added which are testing if any of syntactic error offences is dropped.
@europ europ force-pushed the method_inspect_fix branch from c72a920 to c0e39a9 Compare April 11, 2018 09:08
@doomspork
Copy link
Member

@europ sorry it's taken someone so long to get back to you. I'm open to accepting this change if you'd be willing to rebase and resolve conflicts 👌

@prontolabs/core any thoughts or objections?

@hlascelles
Copy link
Contributor

@skateman, looks good. If you would rebase, that would be excellent. (I am not a maintainer, just would like to see this in).

@ashkulz
Copy link
Member

ashkulz commented Jan 31, 2021

Agreed, there should be a 0.11.1 release soon -- I'd like to include this PR in it 🙂

@europ
Copy link
Author

europ commented Feb 4, 2021

@skateman can you fix this? I am currently unavailable.

@skateman
Copy link

skateman commented Feb 5, 2021

I have no push rights to your repo...

@europ
Copy link
Author

europ commented Feb 5, 2021

@skateman you have them, from now

@ashkulz
Copy link
Member

ashkulz commented Feb 13, 2021

Any luck on rebasing this PR? I'm hoping to make a release soon with RuboCop >= 1.0 support.

@hlascelles
Copy link
Contributor

Just checking, is this mandatory / the fix for RuboCop 1.x? Or can there be another PR to get us to that version?

@ashkulz
Copy link
Member

ashkulz commented Feb 23, 2021

@hlascelles the 0.11.1 release with 1.0 support has already happened; I guess this will be present in a followup release (0.11.2?).

@hlascelles
Copy link
Contributor

Ah, fantastic thank you!

@ashkulz
Copy link
Member

ashkulz commented Jan 17, 2023

@europ would you be willing to rebase this PR?

@hlascelles
Copy link
Contributor

Hi @europ... Could you rebase this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants