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

Remove console from Lint/Debugger #7636

Merged
merged 1 commit into from Jan 9, 2020

Conversation

gsamokovarov
Copy link
Contributor

It's ambiguous and we can play games and try not to recognize:

console(with, arguments_or) do
  with_a_block
end

But, other use cases will come up. Web Console supports
binding.console, which is safe to recognize, so let's drop
the ambiguous console one.

It's ambiguous and we can play games and try not to recognize:

```ruby
console do
  with_a_block
end
```

But, other use cases will come up. Web Console supports
`binding.console`, which is safe to recognize by a linter, so let's drop
the ambiguous `console` one.
@koic koic merged commit 1a6ef08 into rubocop:master Jan 9, 2020
@koic
Copy link
Member

koic commented Jan 9, 2020

Thanks!

@gsamokovarov gsamokovarov deleted the console-ambiguity branch January 9, 2020 08:57
@bquorning
Copy link
Contributor

console do is marked as an offense again in v1.3.0, I think because after #8929.

@bbatsov
Copy link
Collaborator

bbatsov commented Nov 16, 2020

Yep, it seems that's a regression.

dvandersluis added a commit to dvandersluis/rubocop that referenced this pull request Feb 10, 2021
…vers for debug methods lead to offenses.

Prior to rubocop#8929, `Lint/Debugger` looked for a specific receiver/method pair, but that PR changed it so that any of the `DebuggerReceivers` would apply, *or also* no receiver. This caused regressions (rubocop#7636 and rubocop#9500, for instance) because method names specified as debugger by this cop would now register offenses when called with no receiver.

In order to fix it, I changed the configuration for `Lint/Debugger` a bit, but in a way that should not cause any major incompatibility. The allow list is now specified including its receiver so that the receiver can be checked. Existing configurations that just pass in a method name will be treated as expecting no receiver (there's a slight incompatibility there because previously `foo` would be matched by `Kernel.foo` but I think this is ok (and is resolvable by adding `Kernel.foo` to the configuration).

I've also grouped the debugger methods by source (it's often confusing to me where each method comes from), which also allows an end-user to disable a pre-defined group of methods by specifying `Byebug: ~` for example.

Finally I've deprecated the `DebuggerReceivers` configuration as it is no longer useful.
bbatsov pushed a commit that referenced this pull request Feb 12, 2021
…r debug methods lead to offenses.

Prior to #8929, `Lint/Debugger` looked for a specific receiver/method pair, but that PR changed it so that any of the `DebuggerReceivers` would apply, *or also* no receiver. This caused regressions (#7636 and #9500, for instance) because method names specified as debugger by this cop would now register offenses when called with no receiver.

In order to fix it, I changed the configuration for `Lint/Debugger` a bit, but in a way that should not cause any major incompatibility. The allow list is now specified including its receiver so that the receiver can be checked. Existing configurations that just pass in a method name will be treated as expecting no receiver (there's a slight incompatibility there because previously `foo` would be matched by `Kernel.foo` but I think this is ok (and is resolvable by adding `Kernel.foo` to the configuration).

I've also grouped the debugger methods by source (it's often confusing to me where each method comes from), which also allows an end-user to disable a pre-defined group of methods by specifying `Byebug: ~` for example.

Finally I've deprecated the `DebuggerReceivers` configuration as it is no longer useful.
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

4 participants