Rubocop: Cops with "Include" param in default config break Ale integration

[promisedlandt]

In my opinion this is not a bug, but a request for advice on how to get the ALE plugin for vim running better with RuboCop.

Currently, the default configuration has an Include param for a few cops (all Rails related as far as I can see), e.g.:

Rails/InverseOf:
  Include:
    - app/models/**/*.rb

So if I copy a file from e.g. app/models/user.rb to tmp/models/user.rb, the InverseOf cop will not run on it, since Include matches relative to the .rubocop.yml, and not simply on the filename.
What's worse, if there is a rubocop:disable Rails/InverseOf directive, it will be automatically deleted. This happens because the cop is not running, so the Lint/UnneededCopDisableDirective / Lint/UnneededDisable cop thinks the disable is unneeded.

All of this is very understandable, but it breaks in combination with the popular ALE linter / fixer for vim. The way it works is that it write the current buffer to a file under /tmp/some_tmp_dir/some_tmp_dir/filename.rb and runs rubocop on that file, with the correct .rubocop.yml and using the --stdin option. (Note that it does not write to /tmp/some_tmp_dir/some_tmp_dir/app/models/filename.rb, which might have to be fixed in ALE).

A workaround would be to overwrite Include in .rubocop.yml for every affected cop, but then that will need to be done manually for new cops, and I think currently absolute paths are not supported.

A different idea would be try and match the Include pattern to the of the inspected file path (combined from the root .rubocop.yml), but that would probably be some implentation effort, because the current File.fnmatch? logic does not support it.

Third idea: remove the Includes from the default config. I have no idea what side effects this will have, apart from a very small perfomance impact.

Do you have any ideas on how to improve on this?

For reference, an example issue on ALE.

---

Expected behavior

RuboCop runs with the same configuration will lint a file the same regardless of it's location.

Actual behavior

Some cops are only enabled only for certain file patterns.

Steps to reproduce the problem

In some directory:

mkdir -p app/models
mkdir -p /tmp/app/models
cat > app/models/my_model.rb <<MODEL
class MyModel < ApplicationRecord
  # rubocop:disable Rails/InverseOf
  has_one :something
  # rubocop:enable Rails/InverseOf
end
MODEL

cat > /tmp/app/models/my_model.rb <<MODEL
class MyModel < ApplicationRecord
  # rubocop:disable Rails/InverseOf
  has_one :something
  # rubocop:enable Rails/InverseOf
end
MODEL

cat > .rubocop.yml <<RUBOCOP
AllCops:
  DisabledByDefault: true

Rails/InverseOf:
  Enabled: true
RUBOCOP

rubocop app/models/my_model.rb
rubocop /tmp/app/models/my_model.rb

RuboCop version

Include the output of rubocop -V or bundle exec rubocop -V if using Bundler. Here's an example:

0.58.2, but I suspect it affects many / all versions.

[promisedlandt]

I had a thought over the weekend - would it make sense to match Include / Exclude on the stdin parameter instead of the path on disk?

[w0rp]

I think the best way to fix this would be to add support for fixing a file via stdin and also letting rubocop know where the file will be on disk. That will make it possible to fix a file without having to write to the file. ESLint does this with --stdin-filename FILENAME --stdin --fix-dry-run.

[mikegee]

I believe this issue is resolved since --stdin takes a path argument.

[w0rp]

Can you use something like rubocop --auto-correct --stdin filename < temporary_file and read the updated file from stdout?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[w0rp]

Here is some activity. Don't use bots to close issues automatically. It doesn't make sense, and there's nothing wrong with having a lot of issues open. ALE has 293 open issues. The oldest issue is from February 2017, and it's still valid. Software issues don't go "stale." The concept is ridiculous. They remain until someone feels like resolving them.

[mikegee]

@w0rp why not comment on whether the issue is still relevant? I think it is resolved.

[w0rp]

I already asked a question about it on January 30th. Look above.

[mikegee]

Yeah, the issue I rightly stale because nobody wants to the work. Closing seems appropriate to me.

[bbatsov]

@w0rp I understand your perspective, but I don't think it makes sense to keep issues open indefinitely if no progress is happening on them. More about this - https://metaredux.com/posts/2019/05/09/always-be-closing.html

[w0rp]

The philosophy that issues should be closed just because they are old makes no sense at all. Having old issues open doesn't harm anyone. I'd had issues open for years that are later closed when someone finally decides to fix them. You don't have to fix the issues, just leave them open and someone else might submit a patch for them some day.

[promisedlandt]

My apologies, as this did not affect me in my job any more, and I have no free time, I did not give it the attention it deserves.
This weekend, I will check whether the problem still exists. If it is fixed, as @mikegee believes, we can close it without philosophical discussion :)

That being said, ALE already used --stdin with a parameter when I posted the issue, as is visible in the issue I linked in the original post.

[w0rp]

ALE doesn't use --stdin for fixing files at the moment. I'm waiting for an answer to my previous question, as quoted here.

Can you use something like rubocop --auto-correct --stdin filename < temporary_file and read the updated file from stdout?

[promisedlandt]

I just double checked, issue still persists, due to the issue w0rp mentioned.

ALE doesn't use --stdin for fixing files at the moment.

You are correct of course. It was only used for linting.

I'm waiting for an answer to my previous question, as quoted here.

Can you use something like rubocop --auto-correct --stdin filename < temporary_file and read the updated file from stdout?

Would be good to get a definitive answer from a maintainer, but from what I can see, there is no formatter for that.

I have written the dumbest possible custom formatter I could think of to solve this:

class FixedOutputFormatter < RuboCop::Formatter::BaseFormatter                                                                                                                 
  def file_finished(file, _offenses)                                                                                                                                           
    puts File.read(file)                                                                                                                                                       
  end                                                                                                                                                                          
end

Is this something we could get into RuboCop core? Otherwise, I think it should be possible to ship it with ALE.

[Mange]

I'm pretty sure this is resolved now. I had a similar error so I found this bug, but my problem was caused by some weird interaction with --force-exclusion.

In order to isolate this problem I did some experiments with the --stdin option, and the filename passed to it is definitely considered correctly.

› rubocop --force-exclusion --stdin test.rb < config/initializers/rgeo_support_guard.rb 
(→ bin/rubocop --force-exclusion --stdin test.rb)
Inspecting 1 file
.

1 file inspected, no offenses detected
› rubocop --force-exclusion --stdin config/initializers/rgeo_support_guard.rb < config/initializers/rgeo_support_guard.rb 
(→ bin/rubocop --force-exclusion --stdin config/initializers/rgeo_support_guard.rb)
Inspecting 1 file
C

Offenses:

config/initializers/rgeo_support_guard.rb:34:3: C: Rails/Exit: Do not use exit in Rails applications.
  exit 1
  ^^^^

1 file inspected, 1 offense detected

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[w0rp]

Here is some activity.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[stale[bot]]

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.