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

Inspection failure with new Style/ExplicitBlockArgument in RuboCop 0.89 #117

Closed
swrobel opened this issue Aug 29, 2020 · 1 comment
Closed
Labels

Comments

@swrobel
Copy link

swrobel commented Aug 29, 2020

The Style/ExplicitBlockArgument cop was just added in RuboCop 0.89, and it seems to take issue with the use of yield as the child of a partial, ex:

body
  = render 'partial'
    = yield

It errors out with An error occurred while Style/ExplicitBlockArgument cop was inspecting app/views/layouts/mailer.slim.rb:16:0, even if I set the following:

  RuboCop:
    ignored_cops:
      - Style/ExplicitBlockArgument

Which makes me think that ignored_cops aren't actually prevented from running, but instead are run by rubocop and then ignored by slim-lint. I'm not exactly sure what the best course of action is for fixing this:

  1. Reporting upstream to RuboCop? Not sure how they triage since the line number in my layout file that I'm given is beyond the last line of the actual layout (line 16, vs only 12 in the actual layout)
  2. Lock RuboCop to a lower version
  3. Figure out a way to prevent RuboCop from even running ignored_cops
  4. Something else I'm not thinking of...

FYI, this isn't urgent because I can just exclude this layout from my slim-lint runs.

@sds sds added the question label Sep 20, 2020
@sds
Copy link
Owner

sds commented Sep 20, 2020

Hey @swrobel, sorry for the delayed response—missed this notification.

I wasn't able to reproduce your issue locally, even with RuboCop 0.89. Perhaps the underlying bug is related to a dependent gem (parser perhaps) and has since been fixed?

Zooming out, to answer your broader questions (which are quite valid).

  1. When there is a bug with a RuboCop cop itself, preference is to report upstream, since regardless of how the bug occurs it's still a bug and I'm sure the RuboCop team would want to fix it.
  2. Locking RuboCop works as a temporary mitigation, but I understand this is far from ideal for teams who want to use the latest and greatest features. I would too.
  3. We could implement this today, but it imposes an even tighter coupling between Slim-Lint and RuboCop. We could trivially specify the --except flag to the RuboCop CLI with a list of all ignored_cops, but then you will get hard failures the moment RuboCop changes the names/namespaces of cops, and you'll now need to submit a pull request to Slim-Lint to update its list, and bump the minimum RuboCop version in the process. The reason we implemented the current solution (to just have RuboCop run whatever cops are configured) and then filter them out on the Slim-Lint side avoids this problem, though one could argue that perhaps you want to always be made aware when Slim-Lint is filtering a cop that was renamed. I personally don't want to continue maintaining PRs just to update a list of cop names every RuboCop release.
  4. You can define the SLIM_LINT_RUBOCOP_CONF environment variable which uses inherit_from to inherit from your actual .rubocop.yml, and then disables the specific cops you want to disable. I haven't tried this, and don't know if it works, but posting it in case it's worth trying.

I'm going to close this as the original issue seems to be resolved, and some answers to your questions have been provided. I hope this was helpful.

@sds sds closed this as completed Sep 20, 2020
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

2 participants