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 auto-correcting sigil and frozen_string_literal #46

Closed
lastgabs opened this issue Sep 23, 2020 · 2 comments
Closed

Rubocop auto-correcting sigil and frozen_string_literal #46

lastgabs opened this issue Sep 23, 2020 · 2 comments

Comments

@lastgabs
Copy link

lastgabs commented Sep 23, 2020

Hi folks!

When creating new files in Rails applications and running rubocop -a to let it auto-correct the files, rubocop autocorrects by adding the sigil correctly, but then it can't autocorrect the missing frozen string literal comment.

I've tried this in two different repos, one on v0.4.0 and one on v0.4.1 and both behave the same way.

Here's some output:
I created the following class app/models/this_test.rb:

class ThisTest
  THIS_TEST = true
end

Ran rubocop -a:

 9:02PM $ rubocop -a
Inspecting 50 files
......................C...........................

Offenses:

app/models/this_test.rb:1:1: C: [Corrected] Sorbet/FalseSigil: No Sorbet sigil found in file. Try a typed: false to start (you can also use rubocop -a to automatically add this).
class ThisTest
^^^^^
app/models/this_test.rb:1:1: C: Style/FrozenStringLiteralComment: Missing frozen string literal comment.
# typed: false
^

50 files inspected, 2 offenses detected, 1 offense corrected

Checked the file, which now had a sigil but no # frozen_string_literal: true:

# typed: false
class ThisTest
  THIS_TEST = true
end

Running rubocop, it outputs the expected:

 9:03PM $ rubocop
Inspecting 50 files
......................C...........................

Offenses:

app/models/this_test.rb:1:1: C: Style/FrozenStringLiteralComment: Missing frozen string literal comment.
# typed: false
^

50 files inspected, 1 offense detected

Running rubocop -a a second time to see if it gets less confused now that the sigil has already been fixed, but it still can't fix it:

 9:03PM $ rubocop -a
Inspecting 50 files
......................C...........................

Offenses:

app/models/this_test.rb:1:1: C: Style/FrozenStringLiteralComment: Missing frozen string literal comment.
# typed: false
^

50 files inspected, 1 offense detected

cc @Shopify/sorbet

@KaanOzkan
Copy link
Contributor

KaanOzkan commented Sep 23, 2020

Edit: I initially tested this in shopify core and it was working fine. I don't think it's specific to a version now. I realized now any version works properly on core. Still WIP.

I tested this on v0.3.7 and functionality is working as expected. I'll try to bisect the specific commit that introduced this. Nothing stood out to me on first glance.

Output on 0.3.7:

λ rubocop -a test.rb
Inspecting 1 file
C

Offenses:

test.rb:1:1: C: [Corrected] Sorbet/EnforceSigilOrder: Magic comments should be in the following order: typed, encoding, warn_indent, frozen_string_literal.
# frozen_string_literal: true
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
test.rb:1:1: C: [Corrected] Sorbet/FalseSigil: No Sorbet sigil found in file. Try a typed: false to start (you can also use rubocop -a to automatically add this).
class ThisTest
^^^^^
test.rb:1:1: C: [Corrected] Style/FrozenStringLiteralComment: Missing frozen string literal comment.
class ThisTest
^
test.rb:2:1: C: [Corrected] Sorbet/EnforceSigilOrder: Magic comments should be in the following order: typed, encoding, warn_indent, frozen_string_literal.
# typed: false
^^^^^^^^^^^^^^

1 file inspected, 4 offenses detected, 4 offenses corrected

# typed: false
# frozen_string_literal: true
class ThisTest
  THIS_TEST = true
end

@KaanOzkan
Copy link
Contributor

KaanOzkan commented Sep 24, 2020

Team found out that this was due to the new version of Rubocop being used. There was a significant change in Rubocop at version 0.87 for Lint/FrozenStringLiterals. As a result bundle exec rubocop -a no longer fixes these errors as the autocorrect is deemed "unsafe". You should use bundle exec rubocop -A. (Consider inserting it within the dev style alias since this can be easily missed)

Ref: rubocop/rubocop#8360 (comment) and rubocop/rubocop#8529 is the PR that made the change.

Closing this issue as it's not rubocop-sorbet specific.

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