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

RuboCop gives false positive Lint/Syntax errors for valid Ruby 3.1 syntax in slim files #154

Open
kreintjes opened this issue Feb 20, 2023 · 2 comments

Comments

@kreintjes
Copy link

Ruby 3.1 includes new syntax to leave the hash value if it is a variable called the same as the hash key: https://bugs.ruby-lang.org/issues/14579. However there sometimes are cases this new syntax can be applied (as also suggested by RuboCop) in Slim files while doing so results in false positive RuboCop Lint/Syntax errors afterwards. For example the following code:

= simple_form_for(@object) do |f|
  = render 'fields', f:

  - content_for :something do
    | Some content

Results in the following error:

file.html.slim:4 [W] RuboCop: Lint/Syntax: unexpected token tSYMBOL
(Using Ruby 3.1 parser; configure using `TargetRubyVersion` parameter, under `AllCops`)

While this code is actually valid, parses and runs. I therefore would expect slim-lint/RuboCop not to produce an error for it though. Interestingly, similar code in Ruby also gives a RuboCop Lint/Syntax error for it, but in that case the code is actually invalid (Ruby itself gives a syntax error for it as well) and doesn't run. While in slim-lint the code is valid (and does run) and the error is a false positive.

It can be fixed by using parentheses around the render call:

= simple_form_for(@chart_configurator) do |f|
  = render('fields', f:)

  - content_for :something do
    | Some content

Or by including the omitted hash value f:

= simple_form_for(@chart_configurator) do |f|
  = render 'fields', f: f

  - content_for :something do
    | Some content

Interestingly the (false) syntax error is only produced when the omitted hash value was last and is followed immediately by some other Ruby code with a block on the same indenting level. The following examples all work correctly (no error is produced):

= simple_form_for(@chart_configurator) do |f|
  = render 'fields', f:, foo: 'bar'

  - content_for :something do
    | Some content
= simple_form_for(@chart_configurator) do |f|
  = render 'fields', f:

  div = 'Hi'

  - content_for :something do
    | Some content
= simple_form_for(@chart_configurator) do |f|
  = render 'fields', f:

- content_for :something do
  | Some content
= simple_form_for(@chart_configurator) do |f|
  = render 'fields', f:

  = f.submit
@23tux
Copy link

23tux commented Mar 14, 2023

We have the same issue, is there any news on this?

@kreintjes
Copy link
Author

kreintjes commented Mar 14, 2023

I have heard no further news about this and didn't investigate further. In the end we ended up with disabling the Lint/Syntax RuboCop for Slim-Lint completely, so we can start using the new syntax (in all cases) without having to add extra parentheses or come up with other hacks. This does lead us to potentially missing Ruby syntax errors in Slim files, but we think (hope) our test suite and/or manual testing will detect that anyway.

To disable the Cop completely, add the following to your .slim-lint.yml config:

linters:
  RuboCop:
    ignored_cops:
      # The Lint/Syntax cop sometimes reports syntax errors on valid code when ommiting the hash value in Ruby 3.1 (following the Style/HashSyntax cop), see https://github.com/sds/slim-lint/issues/154.
      - Lint/Syntax
      # List below is from the slim-lint default config, but unfortunately you can't add stuff without overriding the whole list
      - Layout/ArgumentAlignment
      - Layout/ArrayAlignment
      - Layout/BlockAlignment
      - Layout/ClosingParenthesisIndentation
      - Layout/EmptyLineAfterGuardClause
      - Layout/EndAlignment
      - Layout/FirstArgumentIndentation
      - Layout/FirstArrayElementIndentation
      - Layout/FirstHashElementIndentation
      - Layout/FirstParameterIndentation
      - Layout/HashAlignment
      - Layout/IndentationConsistency
      - Layout/IndentationWidth
      - Layout/InitialIndentation
      - Layout/LineEndStringConcatenationIndentation
      - Layout/LineLength
      - Layout/MultilineArrayBraceLayout
      - Layout/MultilineAssignmentLayout
      - Layout/MultilineHashBraceLayout
      - Layout/MultilineMethodCallBraceLayout
      - Layout/MultilineMethodCallIndentation
      - Layout/MultilineMethodDefinitionBraceLayout
      - Layout/MultilineOperationIndentation
      - Layout/ParameterAlignment
      - Layout/TrailingEmptyLines
      - Layout/TrailingWhitespace
      - Lint/Void
      - Metrics/BlockLength
      - Metrics/BlockNesting
      - Naming/FileName
      - Style/FrozenStringLiteralComment
      - Style/IdenticalConditionalBranches
      - Style/IfUnlessModifier
      - Style/Next
      - Style/WhileUntilDo
      - Style/WhileUntilModifier

Maybe we should just add the parentheses though, since without it in Ruby itself it could lead to problems or syntax errors as well. There are several issues about it for RuboCop too (although not exactly the same problem I think):

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

2 participants