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

Autocorrect reacts to EmptyLineAfterMagicComment config #7261

Closed
jcraigk opened this issue Aug 6, 2019 · 3 comments · Fixed by #7310
Closed

Autocorrect reacts to EmptyLineAfterMagicComment config #7261

jcraigk opened this issue Aug 6, 2019 · 3 comments · Fixed by #7310

Comments

@jcraigk
Copy link

jcraigk commented Aug 6, 2019

Basic Problem

When running rubocop -a to fix auto-correctable offenses, # frozen_string_literal: true and a blank line are added to the top of every file. It would be desirable for this newline not to be present in the case of Layout/EmptyLineAfterMagicComment being disabled.

Solution

When running rubocop -a, do not place an empty line after # frozen_string_literal: true if Layout/EmptyLineAfterMagicComment is disabled for the given file or altogether.

@jcraigk jcraigk changed the title Autocorrect should be aware of EmptyLineAfterMagicComment Autocorrect reacts to EmptyLineAfterMagicComment config Aug 6, 2019
@buehmann
Copy link
Contributor

buehmann commented Aug 7, 2019

I believe a cop should never change its behaviour because another cop is disabled.

I think there is a simpler solution: FrozenStringLiteralComment should only be concerned about inserting/removing the magic comment as necessary but not about its spacing. So it should never insert empty lines.

The spacing would then be the responsibility of EmptyLineAfterMagicComment.

@Drenmi
Copy link
Collaborator

Drenmi commented Aug 25, 2019

I believe a cop should never change its behaviour because another cop is disabled.

This is the ideal scenario. We do have this kind of logic in parts of the codebase at the moment, though I'm not entirely sure it's needed.

I think there is a simpler solution: FrozenStringLiteralComment should only be concerned about inserting/removing the magic comment as necessary but not about its spacing. So it should never insert empty lines.

The spacing would then be the responsibility of EmptyLineAfterMagicComment

I think this sounds reasonable, and is in line with how many other cops work. It is the reason we have the concept of the "correction loop", where after an auto-correct, the file will be re-inspected by all cops, in case the correction caused another offence that can be corrected.

@buehmann
Copy link
Contributor

@Drenmi Thanks, I added a PR for this: #7310

buehmann added a commit to buehmann/rubocop that referenced this issue Aug 28, 2019
…mment

`FrozenStringLiteralComment` should only be concerned about
inserting/removing the magic comment as necessary but not about its
spacing. So it should never insert empty lines.

The spacing is the responsibility of `EmptyLineAfterMagicComment`.
bbatsov pushed a commit that referenced this issue Aug 28, 2019
…7310)

`FrozenStringLiteralComment` should only be concerned about
inserting/removing the magic comment as necessary but not about its
spacing. So it should never insert empty lines.

The spacing is the responsibility of `EmptyLineAfterMagicComment`.
tas50 added a commit to chef/cookstyle that referenced this issue Oct 30, 2019
This resolves a large number of warnings and gives us a new cop for
automatically migrating the namespace of the cop disables in comments

* [#7407](rubocop/rubocop#7407): Make `Style/FormatStringToken` work inside hashes. ([@buehmann][])
* [#7389](rubocop/rubocop#7389): Fix an issue where passing a formatter might result in an error depending on what character it started with. ([@jfhinchcliffe][])
* [#7397](rubocop/rubocop#7397): Fix extra comments being added to the correction of `Style/SafeNavigation`. ([@rrosenblum][])
* [#7378](rubocop/rubocop#7378): Fix heredoc edge cases in `Layout/EmptyLineAfterGuardClause`. ([@gsamokovarov][])
* [#7404](rubocop/rubocop#7404): Fix a false negative for `Layout/IndentAssignment` when multiple assignment with line breaks on each line. ([@koic][])
* [#7256](rubocop/rubocop#7256): Fix an error of `Style/RedundantParentheses` on method calls where the first argument begins with a hash literal. ([@halfwhole][])
* [#7263](rubocop/rubocop#7263): Make `Layout/SpaceInsideArrayLiteralBrackets` properly handle tab-indented arrays. ([@buehmann][])
* [#7252](rubocop/rubocop#7252): Prevent infinite loops by making `Layout/SpaceInsideStringInterpolation` skip over interpolations that start or end with a line break. ([@buehmann][])
* [#7262](rubocop/rubocop#7262): `Lint/FormatParameterMismatch` did not recognize named format sequences like `%.2<name>f` where the name appears after some modifiers. ([@buehmann][])
* [#7253](rubocop/rubocop#7253): Fix an error for `Lint/NumberConversion` when `#to_i` called without a receiver. ([@koic][])
* [#7271](rubocop/rubocop#7271), [#6498](rubocop/rubocop#6498): Fix an interference between `Style/TrailingCommaIn*Literal` and `Layout/Multiline*BraceLayout` for arrays and hashes. ([@buehmann][])
* [#7241](rubocop/rubocop#7241): Make `Style/FrozenStringLiteralComment` match only true & false. ([@tejasbubane][])
* [#7290](rubocop/rubocop#7290): Handle inner conditional inside `else` in `Style/ConditionalAssignment`. ([@jonas054][])
* [#5788](rubocop/rubocop#5788): Allow block arguments on separate lines if line would be too long in `Layout/MultilineBlockLayout`. ([@jonas054][])
* [#7305](rubocop/rubocop#7305): Register `Style/BlockDelimiters` offense when block result is assigned to an attribute. ([@mvz][])
* [#4802](rubocop/rubocop#4802): Don't leave any `Lint/UnneededCopEnableDirective` offenses undetected/uncorrected. ([@jonas054][])
* [#7326](rubocop/rubocop#7326): Fix a false positive for `Style/AccessModifierDeclarations` when access modifier name is used for hash literal value. ([@koic][])
* [#3591](rubocop/rubocop#3591): Handle modifier `if`/`unless` correctly in `Lint/UselessAssignment`. ([@jonas054][])
* [#7161](rubocop/rubocop#7161): Fix `Style/SafeNavigation` cop for preserve comments inside if expression. ([@tejasbubane][])
* [#5212](rubocop/rubocop#5212): Avoid false positive for braces that are needed to preserve semantics in `Style/BracesAroundHashParameters`. ([@jonas054][])
* [#7353](rubocop/rubocop#7353): Fix a false positive for `Style/RedundantSelf` when receiver and multiple assigned lvalue have the same name. ([@koic][])
* [#7353](rubocop/rubocop#7353): Fix a false positive for `Style/RedundantSelf` when a self receiver is used as a method argument. ([@koic][])
* [#7358](rubocop/rubocop#7358): Fix an incorrect autocorrect for `Style/NestedModifier` when parentheses are required in method arguments. ([@koic][])
* [#7361](rubocop/rubocop#7361): Fix a false positive for `Style/TernaryParentheses` when only the closing parenthesis is used in the last line of condition. ([@koic][])
* [#7369](rubocop/rubocop#7369): Fix an infinite loop error for `Layout/IndentAssignment` with `Layout/IndentFirstArgument` when using multiple assignment. ([@koic][])
* [#7177](rubocop/rubocop#7177), [#7370](rubocop/rubocop#7370): When correcting alignment, do not insert spaces into string literals. ([@buehmann][])
* [#7367](rubocop/rubocop#7367): Fix an error for `Style/OrAssignment` cop when `then` branch body is empty. ([@koic][])
* [#7363](rubocop/rubocop#7363): Fix an incorrect autocorrect for `Layout/SpaceInsideBlockBraces` and `Style/BlockDelimiters` when using multiline empty braces. ([@koic][])
* [#7212](rubocop/rubocop#7212): Fix a false positive for `Layout/EmptyLinesAroundAccessModifier` and `UselessAccessModifier` when using method with the same name as access modifier around a method definition. ([@koic][])
* [#7217](rubocop/rubocop#7217): Make `Style/TrailingMethodEndStatement` work on more than the first `def`. ([@buehmann][])
* [#7190](rubocop/rubocop#7190): Support lower case drive letters on Windows. ([@jonas054][])
* Fix the auto-correction of `Lint/UnneededSplatExpansion` when the splat expansion of `Array.new` with a block is assigned to a variable. ([@rrosenblum][])
* [#5628](rubocop/rubocop#5628): Fix an error of `Layout/SpaceInsideStringInterpolation` on interpolations with multiple statements. ([@buehmann][])
* [#7128](rubocop/rubocop#7128): Make `Metrics/LineLength` aware of shebang. ([@koic][])
* [#6861](rubocop/rubocop#6861): Fix a false positive for `Layout/IndentationWidth` when using `EnforcedStyle: outdent` of `Layout/AccessModifierIndentation`. ([@koic][])
* [#7235](rubocop/rubocop#7235): Fix an error where `Style/ConditionalAssignment` would swallow a nested `if` condition. ([@buehmann][])
* [#7242](rubocop/rubocop#7242): Make `Style/ConstantVisibility` work on non-trivial class and module bodies. ([@buehmann][])
* [#5265](rubocop/rubocop#5265): Improved `Layout/ExtraSpacing` cop to handle nested consecutive assignments. ([@jfelchner][])
* [#7215](rubocop/rubocop#7215): Make it clear what's wrong in the message from `Style/GuardClause`. ([@jonas054][])
* [#7245](rubocop/rubocop#7245): Make cops detect string interpolations in more contexts: inside of backticks, regular expressions, and symbols. ([@buehmann][])
 [#7275](rubocop/rubocop#7275): Make `Style/VariableName` aware argument names when invoking a method. ([@koic][])
* [#3534](rubocop/rubocop#3534): Make `Style/IfUnlessModifier` report and auto-correct modifier lines that are too long. ([@jonas054][])
* [#7261](rubocop/rubocop#7261): `Style/FrozenStringLiteralComment` no longer inserts an empty line after the comment. This is left to `Layout/EmptyLineAfterMagicComment`. ([@buehmann][])
* [#7091](rubocop/rubocop#7091): `Style/FormatStringToken` now detects format sequences with flags and modifiers. ([@buehmann][])
* [#7319](rubocop/rubocop#7319): Rename `IgnoredMethodPatterns` option to `IgnoredPatterns` option for `Style/MethodCallWithArgsParentheses`. ([@koic][])
* [#7345](rubocop/rubocop#7345): Mark unsafe for `Style/YodaCondition`. ([@koic][])
* [#7170](rubocop/rubocop#7170): Fix a false positive for `Layout/RescueEnsureAlignment` when def line is preceded with `private_class_method`. ([@tatsuyafw][])
* [#7186](rubocop/rubocop#7186): Fix a false positive for `Style/MixinUsage` when using inside multiline block and `if` condition is after `include`. ([@koic][])
* [#7099](rubocop/rubocop#7099): Fix an error of `Layout/RescueEnsureAlignment` on assigned blocks. ([@tatsuyafw][])
* [#5088](rubocop/rubocop#5088): Fix an error of `Layout/MultilineMethodCallIndentation` on method chains inside an argument. ([@buehmann][])
* [#4719](rubocop/rubocop#4719): Make `Layout/Tab` detect tabs between string literals. ([@buehmann][])
* [#7203](rubocop/rubocop#7203): Fix an infinite loop error for `Layout/SpaceInsideBlockBraces` when `EnforcedStyle: no_space` with `SpaceBeforeBlockParameters: false` are set in multiline block. ([@koic][])
* [#6653](rubocop/rubocop#6653): Fix a bug where `Layout/IndentHeredoc` would remove empty lines when autocorrecting heredocs. ([@buehmann][])
* [#7188](rubocop/rubocop#7188): Include inspected file location in auto-correction error. ([@pocke][])

Signed-off-by: Tim Smith <tsmith@chef.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants