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

Recognize rails/web-console debug calls in Lint/Debugger #7296

Merged
merged 1 commit into from Dec 31, 2019

Conversation

gsamokovarov
Copy link
Contributor

@gsamokovarov gsamokovarov commented Aug 19, 2019

Rails' Web Console has been around for a while now and I like debugging my applications with it. I recently shipped a console call in production (this is what spawns it). While the console call is not a security vulnerability, it results in a NoMethodError as Web Console is not available in production, I would have loved to have this caught by a linter.

Do you think it has a place in Lint/Debugger?

@gsamokovarov gsamokovarov force-pushed the console-in-lint-debuggers branch 2 times, most recently from f10c085 to 33362fa Compare August 19, 2019 12:59
@gsamokovarov gsamokovarov changed the title Recognize rails/web-console debugg calls in Lint/Debugger Recognize rails/web-console debug calls in Lint/Debugger Aug 19, 2019
@bbatsov
Copy link
Collaborator

bbatsov commented Aug 19, 2019

My only concern is that the name of the method is a bit generic and might generate false positives in some TTY libraries. :-)

Copy link
Contributor

@rrosenblum rrosenblum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll have to rebase the changes. It looks like Lint/Debugger has been updated since this PR was created.

@bbatsov
Copy link
Collaborator

bbatsov commented Dec 30, 2019

@gsamokovarov ping :-)

@gsamokovarov gsamokovarov force-pushed the console-in-lint-debuggers branch 2 times, most recently from fb66a2b to 74598c6 Compare December 31, 2019 11:10
@gsamokovarov
Copy link
Contributor Author

Bozhidar, sorry for the delays. I have recognized both console and binding.console debugging calls here and tweaked the changelog as the time passed by. 😅

@bbatsov bbatsov merged commit 23d5e4b into rubocop:master Dec 31, 2019
@bbatsov
Copy link
Collaborator

bbatsov commented Dec 31, 2019

Thanks!

@gsamokovarov gsamokovarov deleted the console-in-lint-debuggers branch December 31, 2019 12:52
@bquorning
Copy link
Contributor

My only concern is that the name of the method is a bit generic and might generate false positives in some TTY libraries. :-)

Rails itself has a #console method: https://api.rubyonrails.org/classes/Rails/Application.html#method-i-console

@serggl
Copy link

serggl commented Jan 7, 2020

This change introduce a false-positive in Rails railties which has a console hook (executed once rails c starts).
So this code is now raises an offence:

require 'rails'

class RavenConsole < ::Rails::Railtie
  console do
    if Rails.env.production? || Rails.env.staging?
      Raven.configuration.environments << Rails.env.to_sym
      puts 'Sentry capture was disabled for this console session'
    end
  end
end

IMO console keyword capture should be removed from this cop

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 7, 2020

@gsamokovarov ^^

@gsamokovarov
Copy link
Contributor Author

I'll look into this, I may leave only binding.console as a debugging call.

@gsamokovarov
Copy link
Contributor Author

Dropped console in #7636.

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

Successfully merging this pull request may close these issues.

None yet

5 participants