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

Rules breaking the code in .erb files #222

Open
RichardsonWTR opened this issue Jul 8, 2021 · 6 comments
Open

Rules breaking the code in .erb files #222

RichardsonWTR opened this issue Jul 8, 2021 · 6 comments

Comments

@RichardsonWTR
Copy link

I think this gem assumes that every block of Ruby in a .erb file is a different Ruby file.
In my case, I've explicitly disabled some rules, to make this gem not break my code.

       Layout/InitialIndentation:
        Enabled: false # This rule removes the "<%" e "<%="  from the code, if enabled.
      Layout/TrailingEmptyLines:
        Enabled: false
      Layout/TrailingWhitespace:
        Enabled: false  # This rule removes the "<%" e "<%="  from the code, if enabled.
      Layout/LeadingEmptyLines:
        Enabled: false

I suggest 3 options for this:

  • These rules should be placed in the readme for ease of acess, telling the users to disable them
  • These rules should be disabled by the erb lint gem
  • The erb lint gem souldn't treat every block of Ruby code as a different Ruby file.
@RichardsonWTR
Copy link
Author

Related to #189

@boardfish
Copy link

boardfish commented Oct 15, 2021

In addition, it's also a good idea to disable these cops:

      # Enabling this adds a frozen string literal magic comment to every block.
      Style/FrozenStringLiteralComment:
        Enabled: false
      # erb_lint reports errors with this cop.
      Style/MultilineTernaryOperator:
        Enabled: false
      # False flags for view files. ViewComponents should never see this, though, as they should never need to assign in a view.
      Lint/UselessAssignment:
        Exclude:
          - "app/views/**/*"
      # Empty blocks have utility in views, e.g. for resetting content_for and
      # rendering slots without args.
      Lint/EmptyBlock:
        Enabled: false

@boardfish
Copy link

I think it makes sense to have these cops (besides maybe the Lint cops) disabled by default from within the gem.

seanpdoyle added a commit to thoughtbot/suspenders that referenced this issue Nov 22, 2022
Configure the [erb_lint][] utility to apply our `standard`-backed
RuboCop linting tool to our application's `.erb` files.

The initial `.erb-lint.yml` file is configured according to the gem's
[README.md][]. Additionally, there are some key RuboCop rules that we're
disabling [according to some feedback from the community of `erb_lint`
users][issue].

The Rails tasks defined in `lib/tasks/erblint.rake` are inspired by
those defined in [standard/rake.rb][]. They define the
`erblint:autocorrect` and `erblint` tasks to execute the `erblint`
command with and without the `--autocorrect` flag. Additional arguments
can be passed in with the `ERBLINTOPTS` environment variable.

By default, executing `rails standard` will execute `rails erblint`, and
`rails standard:fix` will execute `rails erblint:autocorrect`.

Also depend on the [erblint-github][] gem to provide additional,
accessibility-focused rules for how we statically analyze ERB templates.

Finally, the [ERBLint][] tool depends on [BetterHtml][], and claims to
execute BetterHtml's [ERBSafety][] test suite. In our experience, this
isn't the case, and [requires manual configuration][better_html_tests].

This commit also introduces the `config/better_html.yml` configuration
file, and makes sure that `BetterHtml` and `ERBLint` consistently read
its contents.

[erb_lint]: https://github.com/Shopify/erb-lint
[README.md]: https://github.com/Shopify/erb-lint#configuration
[standard/rake.rb]: https://github.com/testdouble/standard/blob/main/lib/standard/rake.rb
[exe/erblint]: https://github.com/Shopify/erb-lint/blob/main/exe/erblint
[issue]: Shopify/erb_lint#222 (comment)
[erblint-github]: https://github.com/github/erblint-github
[ERBLint]: https://github.com/Shopify/erb-lint
[ERBSafety]: https://github.com/Shopify/erb-lint#erbsafety
[BetterHtml]: https://github.com/Shopify/better-html
[better_html_tests]: https://github.com/Shopify/better-html#testing-for-valid-html-and-erb
seanpdoyle added a commit to thoughtbot/suspenders that referenced this issue Nov 22, 2022
Configure the [erb_lint][] utility to apply our `standard`-backed
RuboCop linting tool to our application's `.erb` files.

The initial `.erb-lint.yml` file is configured according to the gem's
[README.md][]. Additionally, there are some key RuboCop rules that we're
disabling [according to some feedback from the community of `erb_lint`
users][issue].

The Rails tasks defined in `lib/tasks/erblint.rake` are inspired by
those defined in [standard/rake.rb][]. They define the
`erblint:autocorrect` and `erblint` tasks to execute the `erblint`
command with and without the `--autocorrect` flag. Additional arguments
can be passed in with the `ERBLINTOPTS` environment variable.

By default, executing `rails standard` will execute `rails erblint`, and
`rails standard:fix` will execute `rails erblint:autocorrect`.

Also depend on the [erblint-github][] gem to provide additional,
accessibility-focused rules for how we statically analyze ERB templates.

Finally, the [ERBLint][] tool depends on [BetterHtml][], and claims to
execute BetterHtml's [ERBSafety][] test suite. In our experience, this
isn't the case, and [requires manual configuration][better_html_tests].

This commit also introduces the `config/better_html.yml` configuration
file, and makes sure that `BetterHtml` and `ERBLint` consistently read
its contents.

[erb_lint]: https://github.com/Shopify/erb-lint
[README.md]: https://github.com/Shopify/erb-lint#configuration
[standard/rake.rb]: https://github.com/testdouble/standard/blob/main/lib/standard/rake.rb
[exe/erblint]: https://github.com/Shopify/erb-lint/blob/main/exe/erblint
[issue]: Shopify/erb_lint#222 (comment)
[erblint-github]: https://github.com/github/erblint-github
[ERBLint]: https://github.com/Shopify/erb-lint
[ERBSafety]: https://github.com/Shopify/erb-lint#erbsafety
[BetterHtml]: https://github.com/Shopify/better-html
[better_html_tests]: https://github.com/Shopify/better-html#testing-for-valid-html-and-erb
@dtgay
Copy link

dtgay commented Jan 9, 2023

Seems like there might be some breaking behavior related to this if someone writes a comment with <% # The comment %> instead of the more-appropriate <%# The comment %>.

ShalI I file this here or open a new issue?:

image

@boardfish
Copy link

I'd hazard a guess that ties in with the need for this rule to be disabled:

Layout/InitialIndentation:
  Enabled: false # This rule removes the "<%" e "<%="  from the code, if enabled.

@dtgay
Copy link

dtgay commented Jan 13, 2023

Hm, looks like I had that one disabled. Interestingly it was only on this line and one other, both comment lines where the author put a space betwixt the % and #. Something to keep an eye out for, I suppose.

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

No branches or pull requests

3 participants