flyerhzm / rails_best_practices

a code metric tool for rails projects
http://rails-bestpractices.com
MIT License
4.17k stars 276 forks source link

False positives when running on Linux #329

Closed lucascaton closed 6 years ago

lucascaton commented 6 years ago

A project of mine has rails_best_practices set up as part of its build.

It works fine locally and it used to work in CircleCI 1.0. When I migrated it to CircleCi 2.0, it started behaving strangely.

I'm sorry if this issue is related to the CI and not to rails_best_practices; I've even opened a ticket to find out, but didn't have luck so far.

Problem

$ bundle exec rails_best_practices

Source Code: |=================================================================|

/home/circleci/project/app/controllers/concerns/transactions_reports.rb:6
  - remove unused methods (ConfirmationsController#set_balance_report)

/home/circleci/project/app/controllers/concerns/filterable.rb:6
  - remove unused methods (ConfirmationsController#set_period)

Found 2 warnings. Exited with code 1

The log displays app/controllers/concerns/transactions_reports, but says the problem is in ConfirmationsController.

It doesn't make any sense, the concern is not even included in that controller.

Any ideas what's going on? Thank you!

flyerhzm commented 6 years ago

@lucascaton it's hard for me to debug if it works fine locally

lucascaton commented 6 years ago

@flyerhzm yep, I totally understand. I filed an issue here just in case anyone has any idea. I have no idea what's going on :)

flyerhzm commented 6 years ago

@lucascaton you can ssh to circleci build host and do some debug

lucascaton commented 6 years ago

Thanks @flyerhzm, I've just tried that, but I've got the same error that's displayed on CircleCI web interface:

circleci@c2d15b9f1b88:~/project$ bundle exec rails_best_practices
Source Code: |==========================================================================================================================================================================|
/home/circleci/project/app/controllers/concerns/calendar_events.rb:6 - remove unused methods (ScheduledTransactionsController#set_date_and_transactions)
/home/circleci/project/app/controllers/concerns/calendar_events.rb:12 - remove unused methods (ScheduledTransactionsController#set_events)

Please go to https://rails-bestpractices.com to see more useful Rails Best Practices.

Found 2 warnings.

I've compared the Ruby, Rails, rails_best_practices and also code_analyzer versions to make sure I've been using the same versions on both CircleCI and also locally.

I've also reviewed my code: both set_date_and_transactions and set_events methods are defined in a concern (module) and aren't included on ScheduledTransactionsController at all 😕

lucascaton commented 6 years ago

Hey @flyerhzm, good news - I was able to replicate it here: https://circleci.com/gh/lucascaton/rails_best_practices-issue-329/3

/home/circleci/project/app/controllers/concerns/filterable.rb:6
- remove unused methods (PagesController#set_period)

⬆️ It suggests to remove the "unused" method PagesController#set_period but PagesController doesn't even have that method, as you can see in the repo: https://github.com/lucascaton/rails_best_practices-issue-329/blob/master/app/controllers/pages_controller.rb


Note: the results are intermittent: build #2 ran for the exact same commit, but didn't show that line.

ghost commented 6 years ago

@flyerhzm any news on this? thanks.

lucascaton commented 6 years ago

@flyerhzm I was just told it happens locally on Ubuntu. I was also able to replicate on Slackware.

The issue doesn't happen on macOS though.

Cheers.

flyerhzm commented 6 years ago

@lucascaton it should be fixed in version 1.19.3, please try.

lucascaton commented 6 years ago

The issue has been fixed, thank you very much @flyerhzm!