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

Style/Filename gives wrong diagnostic for some known ruby file names #4250

Closed
MarkVillacampa opened this issue Apr 7, 2017 · 4 comments
Closed
Labels

Comments

@MarkVillacampa
Copy link

MarkVillacampa commented Apr 7, 2017

Rubocop will interpret a file named Dangerfile as having ruby source, but will throw an invalid name error.

Diving in Rubocop's source, I've identified the list of file names to be considered Ruby source:

https://github.com/bbatsov/rubocop/blob/66405e118491afd8e8ec574cf2dc18549ef9a2df/lib/rubocop/target_finder.rb#L32-L52

In default.yml there is an interesting comment in the Style/Filename cop:

Style/FileName:
  # File names listed in `AllCops:Include` are excluded by default. Add extra
  # excludes here.

https://github.com/bbatsov/rubocop/blob/ef729dfcc5d15e038db24579b761bc0b56991de0/config/default.yml#L503-L505

If we go to AllCops:Include in the same file, we find an interesting list of files:

https://github.com/bbatsov/rubocop/blob/ef729dfcc5d15e038db24579b761bc0b56991de0/config/default.yml#L9-L29

These two lists have a big overlap but are not identical.

As I see it, to solve my particular issue I would need to add Dangerfile to AllCops:Include, but I'm wondering if these two lists of potential Ruby files shouldn't be merged instead of being maintained separately?

I want to hear what folks think. I'll be happy to send a PR when/if someone more knowledgeable in the codebase gives me some pointers :)

Expected behavior

The Style/Filename cop show show be triggered by known Ruby file names.

Actual behavior

$ rubocop Dangerfile
Inspecting 1 file
C

Offenses:

Dangerfile:1:1: C: The name of this source file (Dangerfile) should use snake_case.
# Make it more obvious that a PR is a work in progress and shouldn't be merged yet
^

1 file inspected, 1 offense detected

Steps to reproduce the problem

1- Create a file named Dangerfile
2- Run rubocop Dangerfile

RuboCop version

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

bbatsov commented Apr 8, 2017

Seems like an oversight on our part. Guess we should merge those lists and ideally rely on the configuration so users can change the list themselves.

@bbatsov bbatsov added the bug label Apr 8, 2017
bbatsov added a commit that referenced this issue Apr 11, 2017
This commit mostly syncs up AllCops/Include with the code we use to
detect if something is a Ruby file or not.

That's just a quick temporary fix.
As mentioned in the ticket - a better solution would to unify to the
different lists that we currently employ.
@bbatsov
Copy link
Collaborator

bbatsov commented Apr 11, 2017

I've synced up the two lists for now, but down the road this duplication should really be taken care of.

@ryanhageman
Copy link
Contributor

Is this still an issue?

@koic
Copy link
Member

koic commented Aug 27, 2018

@ryanhageman Thank you for mentioning. This issue has been solved by edge RuboCop with #5882.

@koic koic closed this as completed Aug 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants