Repository navigation
Ruby: Add RuboCop support for Ruby in ERB templates - #2260
joelhawksley wants to merge 22 commits into
Conversation
There was a problem hiding this comment.
Thanks for getting this started!
I reserved the rubocop-herb gem name to run the Herb Linter as part of a regular RuboCop run, but I could also see that gem to have tighter RuboCop integration in the future for what you are proposing here.
I think if we'd want to land something like this feature, it probably belongs into rubocop-herb.
I am suggesting the opposite (I think?): we should run RuboCop as part of Herb, not the other way around. I don't know if the semantics matter in this case though.
@marcoroth my mental model of the Herb linter has been that it is meant to replace ERB Lint out of the box, and ERB Lint supports RuboCop as one of its linters: https://github.com/Shopify/erb_lint#linters. I think dropping ERB Lint (which doesn't get much attention these days) is a good value proposition for adopting Herb. I'd like to drop our use of ERB Lint at GitHub, but we use the ERB Lint RuboCop linter, so this is a blocker. Which means I'm motivated to get |
|
Yeah, I think that
This PR is providing the work for 2). So, if you'd want to migrate the mechanics of this pull request into |
|
Alright @marcoroth, I've gotten this working to the point of catching RuboCop errors in |
As part of testing marcoroth/herb#2260, I ran the PR against this repository. `rubocop-herb` found and autofixed RuboCop rule violations in six .html.erb files, using the existing RuboCop rule configuration for this project.
d1d11f6 to
5705225
Compare
Replace the duplicate Ruby source builder with Herb.extract_ruby while retaining range collection for safe autocorrection. Normalize masked multibyte characters to RuboCop's character offsets. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Inline the minimal RuboCop extraction hook around Herb.extract_ruby and remove the standalone extractor, range collector, and autocorrection restriction patches. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Refresh the path gem lock entry for Herb 0.11.0 and align extraction tests with the current Herb binding behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
4472bc2 to
75e2ebe
Compare
Signed-off-by: Joel Hawksley <joel@hawksley.org>
Register every .erb and .herb file for RuboCop extraction, apply template-specific exclusions consistently, and cover non-HTML ERB and Herb files in integration tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Register the Herb extractor directly through RuboCop::Runner.ruby_extractors, matching RuboCop's established template plugins. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Keep invalid extracted sources in RuboCop's pipeline so Lint/Syntax can report malformed Ruby, while continuing to skip valid template fragments with no AST. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Moving back to draft while I look at byte vs. character position logic gaps between RuboCop and Herb ❤️ |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add research comparing marcoroth/herb#2260 with tk0miya/rubocop-herb
…aracter-positions' into joelhawksley-add-erb-rubocop
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62861432-cba5-4e6b-b0fe-fad60ea33328
Herb.extract_ruby now preserves Unicode character positions, so the plugin no longer needs its own position-preserving workaround. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62861432-cba5-4e6b-b0fe-fad60ea33328
- Revert extract_ruby Unicode position change; align positions in the plugin - Drop offenses anchored in markup and corrections that would edit markup - Register the extractor once; single source of truth for template extensions - Remove nonexistent Layout/LeadingEmptyLine, exclude cops with ERB false positives - Rewrite tests (default config, markup protection, cop existence) and run them with rake test Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e554b8ed-a44a-4452-9ccf-06e6e96101dc
319ea1a to
7a8b82f
Compare
Bumps RuboCop from 1.84.0 to 1.92.0. 1.84.0 has an autocorrect bug: with the `parser_whitequark` engine, `Style/RedundantParentheses` rewrites `(x rescue false) || y` to `x rescue false || y`, which changes behavior. It was fixed in 1.89.0 by rubocop/rubocop#15472. This was found while testing #2260. New offenses from the upgrade: - `Style/SelectByKind`: autocorrected `select { |x| x.is_a?(Foo) }` to `grep(Foo)`. - `Lint/InterpolationCheck`: excluded for `test/`, where single-quoted ERB strings containing `#{}` are intentional. - `Style/OneClassPerFile`: excluded for the Rust analysis Ruby fixtures. `bundle exec rubocop` reports no offenses, and `rake test:all` passes.
This adds
rubocop-herb, a RuboCop plugin that runs a project’s configured RuboCop cops against Ruby embedded in.erband.herbtemplates.The plugin uses Herb's
extract_rubybinding to produce position-preserving Ruby for the entire template and passes it through RuboCop’s nativeruby_extractorsAPI. This preserves control flow and variable usage across ERB tags while mapping offenses and autocorrections back to their original template locations. Existing configuration continues to work, including custom cops, inherited configuration, exclusions, target Ruby versions,--only, and safe or unsafe autocorrection.Cops that depend on physical Ruby file contents or obscured template layout are excluded for template files by default. The integration lives under
gems/rubocop-herband is enabled through the standard RuboCop plugin configuration:This PR implements responsibility (2) from the review discussion. Running Herb’s own linter rules as part of RuboCop remains separate future work for the
herb-linterintegration.