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

Cannot lint arbitrary files #4245

Closed
doits opened this issue Apr 5, 2017 · 7 comments
Closed

Cannot lint arbitrary files #4245

doits opened this issue Apr 5, 2017 · 7 comments
Labels
enhancement high priority A ticket considered important by RuboCop's Core Team

Comments

@doits
Copy link

doits commented Apr 5, 2017

This is a follow up to #4242

Currently I cannot lint a custom file. For example, if I have a file named bla with the following content:

"some string"

and I run rubocop

rubocop bla

it does not lint it:

→   rubocop bla
Inspecting 0 files

0 files inspected, no offenses detected

When I rename it to bla.rb, it lints it:

Inspecting 1 file
C

Offenses:

bla.rb:1:1: C: Prefer single-quoted strings when you don't need string interpolation or special symbols.
"some string"
^^^^^^^^^^^^^

1 file inspected, 1 offense detected

When I pass file at command line, I am saying "lint this please". Rubocop should not ignore it.

This has nothing to do when I pass a directory to rubocop. Then it is OK it lints only ruby files (or better to say files it thinks are ruby files), because that is reasonable. But when I explicitly pass a file on the command line, then it is 100% clear that I want it to be linted by rubocop - or else I would not pass it.


Expected behavior

It should lint every file I pass in at command line explicitly.

Actual behavior

It ignores files.

RuboCop version

$ rubocop -V
0.48.1 (using Parser 2.4.0.0, running on ruby 2.4.1 x86_64-darwin16)
@bbatsov
Copy link
Collaborator

bbatsov commented Apr 5, 2017

This has nothing to do when I pass a directory to rubocop. Then it is OK it lints only ruby files (or better to say files it thinks are ruby files), because that is reasonable. But when I explicitly pass a file on the command line, then it is 100% clear that I want it to be linted by rubocop - or else I would not pass it.

I think you didn't understand me last time around. The problem has nothing to do with directories and everything to do with what happens when you do rubocop bla* or something like this. There's absolutely no way for RuboCop to know if some file was explicitly passed to it or came as a result of the shell expanding some globbing pattern into multiple files. If someone has an idea how to handle this gracefully - I'm all ears. :-)

@doits
Copy link
Author

doits commented Apr 5, 2017

To quote myself from the other issue:

Regarding to globbing: Yes, globbing on command line is invisible to rubocop. But when I glob on the command line, then I have to make sure that I pass correct files. That is not the responsibility of rubocop to filter my bad glob, or at least it should be optional behind a flag.

So given a directory ...

some_dir/ruby_file.rb
some_dir/markdown_file.md

... if I run ...

rubocop some_dir/*

... which expands to ...

rubocop some_dir/ruby_file.rb some_dir/markdown_file.md

... then of course rubocop should lint both files. My glob was wrong.

But if I run ...

rubocop some_dir

... then rubocop should only lint ruby files.

From my perspective this behaviour is a good default and can be tweaked by command line flags of course (like --force-exclude).

Or am I still missing something?

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 6, 2017

Well, I'm ok with an approach like this one, although I don't really know how everyone's using RuboCop so it might cause problems for some people. If someone wants to work in the direction of implementing your idea, that's fine by me.

@andreaswachowski
Copy link
Contributor

To give another example and perhaps provide a fuller picture for Rubocop's usage scenarios, consider slim-lint. In order to lint the Ruby parts of a slim template, slim-lint creates a temporary file which consists of just those Ruby parts, with all slim-templating removed. The resulting file is in no way a proper Ruby file, nevertheless one could argue that it should be processed by Rubocop, since it is passed explicitly. The workaround, of course, is to simply provide the Tempfiles with an *.rb extension (sds/slim-lint#55).

I don't know how other linters (e.g. haml-lint) integrate Rubocop, but this might be worth a look.

Finally, I found it rather surprising that Rubocop silently ignores such files. In slim-lint's case (with version <= 0.11), this meant that all of a sudden no Rubocop warnings whatsoever were displayed. Someone upgrading Rubocop from 0.47 to 0.48 without upgrading slim-lint to >= 0.12 will not receive Rubocop warnings in slim templates anymore (!). It might be helpful if Rubocop displayed a warning when ignoring files.

@herwinw
Copy link

herwinw commented Apr 20, 2017

It is possible to lint arbitrary files when they are included in a rubocop config. As a proof of concept, I have the following .rubocop.yml:

AllCops:
  Include:
    - 'dsl/**/*'

I created a file dsl/ruby.dsl with the same contents as lib/ruby.rb:

module Foo
  BAR  = 'baz'
end

This warns with Style/MutableConstant for both files (no specific reason for this warning, it was just one that's very easy to trigger):

$ rubocop -D -c .rubocop.yml 
Inspecting 2 files
CC

Offenses:

dsl/ruby.dsl:2:9: C: Style/MutableConstant: Freeze mutable objects assigned to constants.
  BAR = 'baz'
        ^^^^^
lib/ruby.rb:2:9: C: Style/MutableConstant: Freeze mutable objects assigned to constants.
  BAR = 'baz'

If I pass both files on the command line, the .dsl file is ignored

$ rubocop -D -c .rubocop.yml lib/ruby.rb dsl/ruby.dsl
Inspecting 1 file
C

Offenses:

lib/ruby.rb:2:9: C: Style/MutableConstant: Freeze mutable objects assigned to constants.
  BAR = 'baz'
        ^^^^^

1 file inspected, 1 offense detected

I think adding the explicit configuration in target_finder.rb would be the easiest solution and provides a workable alternative for most use cases.

@swrobel
Copy link
Contributor

swrobel commented Mar 24, 2018

I ended up here after banging my head against the wall based on the docs which led me to believe that you could pass arbitrary files at the command line. If this behavior isn't expected to work, I'll submit a PR to update the docs.

I've noticed it won't even lint these files if I explicitly add them like so

AllCops:
  Include:
    - '**/Envfile'
$ rubocop -d Envfile
For /Users/swrobel/Code/rubocoptest: configuration from /Users/swrobel/Code/rubocoptest/.rubocop.yml
Default configuration from /Users/swrobel/Code/rubocoptest/vendor/bundle/gems/rubocop-0.54.0/config/default.yml
Inheriting configuration from /Users/swrobel/Code/rubocoptest/vendor/bundle/gems/rubocop-0.54.0/config/enabled.yml
Inheriting configuration from /Users/swrobel/Code/rubocoptest/vendor/bundle/gems/rubocop-0.54.0/config/disabled.yml
Inspecting 0 files


0 files inspected, no offenses detected
Finished in 0.1115319998934865 seconds

$ rubocop -d            
For /Users/swrobel/Code/rubocoptest: configuration from /Users/swrobel/Code/rubocoptest/.rubocop.yml
Default configuration from /Users/swrobel/Code/rubocoptest/vendor/bundle/gems/rubocop-0.54.0/config/default.yml
Inheriting configuration from /Users/swrobel/Code/rubocoptest/vendor/bundle/gems/rubocop-0.54.0/config/enabled.yml
Inheriting configuration from /Users/swrobel/Code/rubocoptest/vendor/bundle/gems/rubocop-0.54.0/config/disabled.yml
Inspecting 3 files
Scanning /Users/swrobel/Code/rubocoptest/Envfile

It will lint them if I add the magic comment #!.*ruby to the file

@bbatsov
Copy link
Collaborator

bbatsov commented Sep 19, 2018

@swrobel In recent versions the explicit Include will work.

As I said before the real problem is what to do with shell globbings and stuff. I agree that if some files are explicitly passed they should probably be linted, but if the files list was expanded or generated by some other command that expects RuboCop to do some filtering that'd be bad.

Probably we need some command line flag to turn-on the built-in filtering on demand. A PR would be welcome!

@bbatsov bbatsov added the high priority A ticket considered important by RuboCop's Core Team label Sep 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement high priority A ticket considered important by RuboCop's Core Team
Projects
None yet
Development

No branches or pull requests

5 participants