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

Lint/Debugger does not catch require "debug/start" #12721

Closed
odinhb opened this issue Feb 28, 2024 · 3 comments
Closed

Lint/Debugger does not catch require "debug/start" #12721

odinhb opened this issue Feb 28, 2024 · 3 comments
Labels
feature request good first issue Easy task, suitable for newcomers to the project

Comments

@odinhb
Copy link

odinhb commented Feb 28, 2024

There is a feature in the debug gem where you can require the gem and enter the debugger in a single statement. I don't think other debuggers have this so it is being missed by the Lint/Debugger cop.

The feature allows the following statements to start a debugger:

require 'debug/start' # Same as rdbg.
require 'debug/open' # Same as rdbg --open.
require 'debug/open_nonstop' # Same as rdbg --open --nonstop.

I don't think we want any of these to accidentally make it to production.

Expected behavior

Lint/Debugger should complain about this code, since it is a debugger breakpoint the same way as putting debugger in your code is:

def a_method
  x = 2 + 2

  require "debug/start"

  x * 2
end

Actual behavior

1 file inspected, no offenses detected

Steps to reproduce the problem

Put require "debug/start" in your code. $ rubocop

Attempted workaround

This has no effect:

Lint/Debugger:
  DebuggerMethods:
    requires:
      "require \"debug/start\""

I would assume this cop configuration does not support checking for the specific argument to require. Maybe I'm missing something?

RuboCop version

$ bundle exec rubocop -V
1.56.3 (using Parser 3.2.2.3, rubocop-ast 1.29.0, running on ruby 3.2.2) [x86_64-linux]
  - rubocop-performance 1.19.0
  - rubocop-rspec 2.24.0

and

$ bundle exec rubocop -V
1.56.3 (using Parser 3.2.2.3, rubocop-ast 1.29.0, running on ruby 3.2.1) [x86_64-linux]
  - rubocop-capybara 2.18.0
  - rubocop-factory_bot 2.23.1
  - rubocop-performance 1.19.0
  - rubocop-rails 2.21.0
  - rubocop-rspec 2.24.0
@bbatsov bbatsov added feature request good first issue Easy task, suitable for newcomers to the project labels Mar 6, 2024
Earlopain added a commit to Earlopain/rubocop that referenced this issue Mar 9, 2024
I debated adding requires like `debug` or `pry` but believe that would
do more harm than good.
RuboCops own spec_helper requires pry. This seems to come from some sort of template.
Earlopain added a commit to Earlopain/rubocop that referenced this issue Mar 10, 2024
I debated adding requires like `debug` or `pry` but believe that would
do more harm than good.
RuboCops own spec_helper requires pry. This seems to come from some sort of template.
@st0012
Copy link

st0012 commented Apr 15, 2024

debug/open_nonstop should be excluded from this list as it's NOT a breakpoint require. It simply activates the debugger in remote mode but doesn't stop the program. And its main usage is to be put behind a flag to enable convenient debugger-editor integration. Here's an example from the Ruby LSP. And according to this issue: rails/rails#51225, some Rails projects also use it the same way. So please exclude it from the current list. cc @Earlopain

koic added a commit to koic/rubocop that referenced this issue Apr 15, 2024
Follow up rubocop#12721 (comment).

This PR excludes `debug/open_nonstop` from `Lint/Debugger` by default.
@koic
Copy link
Member

koic commented Apr 15, 2024

@st0012 Thank you for the feedback. I've opened #12845 to solve it.

@st0012
Copy link

st0012 commented Apr 15, 2024

@koic Thank you very much 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request good first issue Easy task, suitable for newcomers to the project
Projects
None yet
Development

No branches or pull requests

4 participants