From 748d989b185dbdd7abe6487ce4e4583c61ffdc40 Mon Sep 17 00:00:00 2001 From: Jeff Carbonella Date: Thu, 7 May 2020 08:48:33 -0600 Subject: [PATCH] [Fix #7929] Accept frozen_string_literal anywhere in leading comment lines The frozen_string_literal comment can exist anywhere in the "leading comment lines" (i.e. before the first line of ruby code). If it appears after any ruby code it is ignored which does not cause an errors but could cause the absence of an expected error which is bad. The previous implementation was checking if this comment was one of the first three comments which had two issues: - The comments it was checking could be anywhere, even after ruby code - Since it was only checking three comments, it's possible that you can have an appropriate frozen_string_literal but this cop would show an error. This commit updates the logic to look at all leading comment lines, which may be more or less than three depending on where the first non-comment token is. --- CHANGELOG.md | 2 + .../cop/mixin/frozen_string_literal.rb | 11 +- .../frozen_string_literal_comment_spec.rb | 118 ++++++++++++++++++ 3 files changed, 130 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51d1fc33d5a..7365217d65a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ ### Bug fixes +* [#7929](https://github.com/rubocop-hq/rubocop/issues/7929): Fix `Style/FrozenStringLiteralComment` to accept frozen_string_literal anywhere in leading comment lines. ([@jeffcarbs][]) * [#7882](https://github.com/rubocop-hq/rubocop/pull/7882): Fix `Style/CaseEquality` when `AllowOnConstant` is `true` and the method receiver is implicit. ([@rafaelfranca][]) * [#7790](https://github.com/rubocop-hq/rubocop/issues/7790): Fix `--parallel` and `--ignore-parent-exclusion` combination. ([@jonas054][]) * [#7881](https://github.com/rubocop-hq/rubocop/issues/7881): Fix `--parallel` and `--force-default-config` combination. ([@jonas054][]) @@ -4485,3 +4486,4 @@ [@saurabhmaurya15]: https://github.com/saurabhmaurya15 [@DracoAter]: https://github.com/DracoAter [@diogoosorio]: https://github.com/diogoosorio +[@jeffcarbs]: https://github.com/jeffcarbs diff --git a/lib/rubocop/cop/mixin/frozen_string_literal.rb b/lib/rubocop/cop/mixin/frozen_string_literal.rb index 19ac1c28d5f..8df8782cfdf 100644 --- a/lib/rubocop/cop/mixin/frozen_string_literal.rb +++ b/lib/rubocop/cop/mixin/frozen_string_literal.rb @@ -46,7 +46,16 @@ def frozen_string_literal_specified? end def leading_comment_lines - processed_source.comments.first(3).map(&:text) + first_non_comment_token = processed_source.tokens.find do |token| + !token.comment? + end + + if first_non_comment_token + # `line` is 1-indexed so we need to subtract 1 to get the array index + processed_source.lines[0...first_non_comment_token.line - 1] + else + processed_source.lines + end end end end diff --git a/spec/rubocop/cop/style/frozen_string_literal_comment_spec.rb b/spec/rubocop/cop/style/frozen_string_literal_comment_spec.rb index 2266188a070..5323a414027 100644 --- a/spec/rubocop/cop/style/frozen_string_literal_comment_spec.rb +++ b/spec/rubocop/cop/style/frozen_string_literal_comment_spec.rb @@ -249,6 +249,39 @@ RUBY end + it 'accepts a frozen string literal comment after other comments' do + expect_no_offenses(<<~RUBY) + #!/usr/bin/env ruby + # encoding: utf-8 + # + # These are more comments + # frozen_string_literal: false + puts 1 + RUBY + end + + it 'registers an offense for having a frozen string literal comment ' \ + 'under ruby code' do + expect_offense(<<~RUBY) + # encoding: utf-8 + ^ Missing frozen string literal comment. + + puts 1 + # frozen_string_literal: true + RUBY + + # Since we're only looking at the leading comments, a + # `frozen_string_literal` comment elsewhere in the code is invisible + # to this cop so the autocorrect won't remove it. + expect_correction(<<~RUBY) + # encoding: utf-8 + # frozen_string_literal: true + + puts 1 + # frozen_string_literal: true + RUBY + end + it 'registers an offense for an extra first empty line' do pending 'There is a flaw that skips adding caret symbol in this case, ' \ 'making it impossible to use `expect_offense` matcher' @@ -463,6 +496,36 @@ puts 1 RUBY end + + it 'registers an offense for having a frozen string literal comment ' \ + 'after other comments' do + expect_offense(<<~RUBY) + #!/usr/bin/env ruby + # encoding: utf-8 + # + # These are more comments + # frozen_string_literal: false + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unnecessary frozen string literal comment. + puts 1 + RUBY + + expect_correction(<<~RUBY) + #!/usr/bin/env ruby + # encoding: utf-8 + # + # These are more comments + puts 1 + RUBY + end + + it 'accepts a frozen string literal comment under ruby code' do + expect_no_offenses(<<~RUBY) + # encoding: utf-8 + + puts 1 + # frozen_string_literal: true + RUBY + end end context 'always_true' do @@ -959,6 +1022,61 @@ RUBY end + it 'accepts a frozen string literal comment after other comments' do + expect_no_offenses(<<~RUBY) + #!/usr/bin/env ruby + # encoding: utf-8 + # + # These are more comments + # frozen_string_literal: true + puts 1 + RUBY + end + + it 'registers an offense for a disabled frozen string literal comment ' \ + 'after other comments' do + expect_offense(<<~RUBY) + #!/usr/bin/env ruby + # encoding: utf-8 + # + # These are more comments + # frozen_string_literal: false + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Frozen string literal comment must be set to `true`. + puts 1 + RUBY + + expect_correction(<<~RUBY) + #!/usr/bin/env ruby + # encoding: utf-8 + # + # These are more comments + # frozen_string_literal: true + puts 1 + RUBY + end + + it 'registers an offense for having a frozen string literal comment ' \ + 'under ruby code' do + expect_offense(<<~RUBY) + # encoding: utf-8 + ^ Missing magic comment `# frozen_string_literal: true`. + + puts 1 + # frozen_string_literal: true + RUBY + + # Since we're only looking at the leading comments, a + # `frozen_string_literal` comment elsewhere in the code is invisible + # to this cop so the autocorrect won't remove it. + expect_correction(<<~RUBY) + # encoding: utf-8 + # frozen_string_literal: true + + puts 1 + # frozen_string_literal: true + RUBY + end + it 'registers an offense for a disabled frozen string literal comment ' \ 'under shebang with no other code' do expect_offense(<<~RUBY)