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

Infinite loop when running with the --auto-correct and --stdin options #2502

Closed
olbat opened this issue Dec 15, 2015 · 15 comments
Closed

Infinite loop when running with the --auto-correct and --stdin options #2502

olbat opened this issue Dec 15, 2015 · 15 comments

Comments

@olbat
Copy link

olbat commented Dec 15, 2015

Rubocop detects an infinite loop when running the Style/SpecialGlobalVars code with the --auto-correct and --stdin option (rubocop version: 0.35.1).

To reproduce:

$ echo 'p $/' | rubocop --auto-correct --only="Style/SpecialGlobalVars" --format=simple --stdin -

The backtrace:

== - ==
C:  1:  3: [Corrected] Prefer $INPUT_RECORD_SEPARATOR or $RS from the stdlib 'English' module over $/.

0 files inspected, 1 offense detected, 1 offense corrected

~/.gem/ruby/2.1.0/gems/rubocop-0.35.1/lib/rubocop/runner.rb:181:in `check_for_infinite_loop': Infinite loop detected in ~/core/-. (RuboCop::Runner::InfiniteCorrectionLoop)
    from ~/.gem/ruby/2.1.0/gems/rubocop-0.35.1/lib/rubocop/runner.rb:152:in `block in do_inspection_loop'
    from ~/.gem/ruby/2.1.0/gems/rubocop-0.35.1/lib/rubocop/runner.rb:151:in `loop'
    from ~/.gem/ruby/2.1.0/gems/rubocop-0.35.1/lib/rubocop/runner.rb:151:in `do_inspection_loop'
    from ~/.gem/ruby/2.1.0/gems/rubocop-0.35.1/lib/rubocop/runner.rb:93:in `process_file'
    from ~/.gem/ruby/2.1.0/gems/rubocop-0.35.1/lib/rubocop/runner.rb:58:in `block in inspect_files'
    from ~/.gem/ruby/2.1.0/gems/rubocop-0.35.1/lib/rubocop/runner.rb:56:in `each'
    from ~/.gem/ruby/2.1.0/gems/rubocop-0.35.1/lib/rubocop/runner.rb:56:in `inspect_files'
    from ~/.gem/ruby/2.1.0/gems/rubocop-0.35.1/lib/rubocop/runner.rb:34:in `run'
    from ~/.gem/ruby/2.1.0/gems/rubocop-0.35.1/lib/rubocop/cli.rb:26:in `run'
    from ~/.gem/ruby/2.1.0/gems/rubocop-0.35.1/bin/rubocop:13:in `block in <top (required)>'
    from /usr/lib/ruby/2.1.0/benchmark.rb:294:in `realtime'
    from ~/.gem/ruby/2.1.0/gems/rubocop-0.35.1/bin/rubocop:12:in `<top (required)>'
    from ~/.gem/ruby/2.1.0/bin/rubocop:23:in `load'
    from ~/.gem/ruby/2.1.0/bin/rubocop:23:in `<main>'
@alexdowad
Copy link
Contributor

Please post the code which you are running RC on, or at least a small excerpt which can be used to reproduce the problem.

@mikegee
Copy link
Contributor

mikegee commented Dec 15, 2015

@alexdowad I ran the command given:

echo 'p $/' | rubocop --auto-correct --only="Style/SpecialGlobalVars" --format=simple --stdin -

and got the same error.

@alexdowad
Copy link
Contributor

@olbat @mikegee Sorry for the misunderstanding, and thank you. I'll have a look at this and see if I can fix it up.

@alexdowad
Copy link
Contributor

OK. The problem here is that "autocorrecting" code from stdin is a nonsensical thing to do.

After each round of autocorrections, RC writes the corrected code to the original source. So you know what it does in this case? ...It writes it out to a file called -. Pretty silly, huh?

RC should probably refuse to "autocorrect" code which comes from stdin. Other contributors, what do you think?

@rrosenblum
Copy link
Contributor

That sounds like a good idea to me.

It might be a cool to have RC be able to correct code coming from stdin. Is there an easy way to have RC write to stdout instead of a file called -?

@alexdowad
Copy link
Contributor

The trouble is that stdout is normally used for the diagnostic output. Or is it written to stderr? I'm not sure.

@rrosenblum
Copy link
Contributor

I miss understood the initial issue. When I run

echo 'p $/' | b rubocop --auto-correct --only="Style/SpecialGlobalVars" --format=simple --stdin -

The output is:

buffer: #<Parser::Source::Buffer:0x007fe1b50c3980>
== - ==
C:  1:  3: [Corrected] Prefer $INPUT_RECORD_SEPARATOR or $RS from the stdlib 'English' module over $/.

0 files inspected, 1 offense detected, 1 offense corrected
rubocop/lib/rubocop/runner.rb:181:in `check_for_infinite_loop': Infinite loop detected in rubocop/-. (RuboCop::Runner::InfiniteCorrectionLoop)
        from rubocop/runner.rb:152:in `block in do_inspection_loop'
        ...

I thought the issue was that RuboCop was running forever. In actuality, RuboCop is detecting that what it is doing will produce an infinite loop and aborting. I wonder how difficult it would be to isolate this situation from other infinite loops (not sure what causes other infinite loops), and finish running successfully instead of blowing up.

@alexdowad
Copy link
Contributor

The reason why it detects an "infinite loop" is because after it autocorrects the source and writes it to a file called -, it then reads it back from stdin again and compares that to the original source. Of course, it's the same. If it would read back what it had written out, it wouldn't throw an "infinite loop" error.

@jonas054
Copy link
Collaborator

We should exit with an error message if both --auto-correct and --stdin are given (easy), or support it by doing the auto-corrections in memory rather that writing to file (hard).

@alexdowad
Copy link
Contributor

I'm doing the latter right now.

@alexdowad
Copy link
Contributor

(And it's not really hard, by the way. You'll see the PR in 15 minutes or so.)

@alexdowad
Copy link
Contributor

OK, just pushed a fix here.

What do you think of ==================== as a delimiter between the warning summary and autocorrected code? Can we do better? (I was hoping that the warning summary was sent to stderr, but it goes to stdout.)

@olbat olbat changed the title Infinite loop when running the Style/SpecialGlobalVars cop Infinite loop when running with the --auto-correct and --stdin options Dec 29, 2015
@olbat
Copy link
Author

olbat commented Dec 29, 2015

Perfect, thank you for the quick fix ! :)

@eterps
Copy link

eterps commented Oct 7, 2016

@alexdowad the = based delimiter is fine, but it can't be disabled.
What if you want editor integration for autoformatting? In Vim for example rubocop would get the source on STDIN, and thanks to your fix it can now autocorrect to STDOUT, but the editor will then receive the delimiter and offense report.
For that to work we need to be able to disable the offense report (i.e. cat foo.rb |rubocop --auto-correct --no-offense-report --stdin - or something like that).
Your thoughts?

For now this seems the best workaround rubocop --auto-correct --stdin - 2>/dev/null |sed '1,/^====================$/d'

@alexdowad
Copy link
Contributor

@eterps, you are very right. I don't have any opinion on whether --no-offense-report is worth including in Rubocop or not. You can always create a custom Formatter instead. Or if you want to open a PR, please do so and see what @bbatsov will say about it.

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

No branches or pull requests

6 participants