Skip to content

Commit

Permalink
Support AutoCorrect: contextual option for LSP
Browse files Browse the repository at this point in the history
## Summary

This PR introduces `AutoCorrect: contextual` option that prevents autocorrection
during typing in LSP.

This option extends the existing `AutoCorrect` parameter.

- Before: `AutoCorrect: true` or `AutoCorrect: false`
- After: `AutoCorrect: always`, `AutoCorrect: disabled`, or `AutoCorrect: contextual`

Note, `AutoCorrect: always` maintains compatibility with `AutoCorrect: true`,
and `AutoCorrect: disabled` maintains compatibility with `AutoCorrect: false`.

## Details

For example, `Style/EmptyMethod` should not autocorrect to one-liner while in the process of
writing the body of the method.
This isn't an issue with autocorrection itself, but rather due to differing requirements
in contexts such as editing (LSP) and command line (e.g., CI).

So, `AutoCorrect: contextual` is set for some cops like this. There may be other cases,
but for the obvious following cops. The classification is as follows:

### Cops that might remove code being edited

- `Layout/EmptyComment` cop
- `Lint/EmptyConditionalBody` cop
- `Lint/EmptyEnsure` cop
- `Lint/EmptyInterpolation` cop
- `Lint/TrailingCommaInAttributeDeclaration` cop
- `Lint/UselessAccessModifier` cop
- `Lint/UselessAssignment` cop
- `Lint/UselessMethodDefinition` cop
- `Lint/UselessTimes` cop
- `Lint/Void` cop
- `Style/EmptyElse` cop
- `Style/RedundantInitialize` cop

### Cops that might adjust code being edited

- `Lint/UnusedBlockArgument` cop
- `Lint/UnusedMethodArgument` cop
- `Style/EmptyHeredoc` cop
- `Style/EmptyMethod` cop

As a result, it allows for a distinction in the use of autocorrection between
the LSP and command-line contexts.

## Other Information

Here is why the parameter name was chosen as `contextual`.

In this PR, autocorrection from LSP is always not applied in the case of `contextual`.
However, even when `contextual` is set, I was considering a possibility for
autocorrection to be forcibly applied from LSP under certain conditions.

Therefore, the initial thought was to name it `contextual`, but it might have been
too abstract in context, so it was changed to `contextual`.
  • Loading branch information
koic authored and bbatsov committed Feb 11, 2024
1 parent 8b3656d commit 772054e
Show file tree
Hide file tree
Showing 13 changed files with 200 additions and 31 deletions.
1 change: 1 addition & 0 deletions changelog/new_extend_autocorrect_option_for_lsp.md
@@ -0,0 +1 @@
* [#12657](https://github.com/rubocop/rubocop/pull/12657): Support `AutoCorrect: contextual` option for LSP. ([@koic][])
44 changes: 33 additions & 11 deletions config/default.yml
Expand Up @@ -556,7 +556,9 @@ Layout/ElseAlignment:
Layout/EmptyComment:
Description: 'Checks empty comment.'
Enabled: true
AutoCorrect: contextual
VersionAdded: '0.53'
VersionChanged: '<<next>>'
AllowBorderComment: true
AllowMarginComment: true

Expand Down Expand Up @@ -1825,16 +1827,18 @@ Lint/EmptyClass:
Lint/EmptyConditionalBody:
Description: 'Checks for the presence of `if`, `elsif` and `unless` branches without a body.'
Enabled: true
AutoCorrect: contextual
SafeAutoCorrect: false
AllowComments: true
VersionAdded: '0.89'
VersionChanged: '1.34'
VersionChanged: '<<next>>'

Lint/EmptyEnsure:
Description: 'Checks for empty ensure block.'
Enabled: true
AutoCorrect: contextual
VersionAdded: '0.10'
VersionChanged: '0.48'
VersionChanged: '<<next>>'

Lint/EmptyExpression:
Description: 'Checks for empty expressions.'
Expand All @@ -1856,8 +1860,9 @@ Lint/EmptyInPattern:
Lint/EmptyInterpolation:
Description: 'Checks for empty string interpolation.'
Enabled: true
AutoCorrect: contextual
VersionAdded: '0.20'
VersionChanged: '0.45'
VersionChanged: '<<next>>'

Lint/EmptyWhen:
Description: 'Checks for `when` branches with empty bodies.'
Expand Down Expand Up @@ -2395,7 +2400,9 @@ Lint/TopLevelReturnWithArgument:
Lint/TrailingCommaInAttributeDeclaration:
Description: 'Checks for trailing commas in attribute declarations.'
Enabled: true
AutoCorrect: contextual
VersionAdded: '0.90'
VersionChanged: '<<next>>'

Lint/TripleQuotes:
Description: 'Checks for useless triple quote constructs.'
Expand Down Expand Up @@ -2455,17 +2462,19 @@ Lint/UnusedBlockArgument:
Description: 'Checks for unused block arguments.'
StyleGuide: '#underscore-unused-vars'
Enabled: true
AutoCorrect: contextual
VersionAdded: '0.21'
VersionChanged: '0.22'
VersionChanged: '<<next>>'
IgnoreEmptyBlocks: true
AllowUnusedKeywordArguments: false

Lint/UnusedMethodArgument:
Description: 'Checks for unused method arguments.'
StyleGuide: '#underscore-unused-vars'
Enabled: true
AutoCorrect: contextual
VersionAdded: '0.21'
VersionChanged: '0.81'
VersionChanged: '<<next>>'
AllowUnusedKeywordArguments: false
IgnoreEmptyMethods: true
IgnoreNotImplementedMethods: true
Expand All @@ -2489,17 +2498,19 @@ Lint/UriRegexp:
Lint/UselessAccessModifier:
Description: 'Checks for useless access modifiers.'
Enabled: true
AutoCorrect: contextual
VersionAdded: '0.20'
VersionChanged: '0.83'
VersionChanged: '<<next>>'
ContextCreatingMethods: []
MethodCreatingMethods: []

Lint/UselessAssignment:
Description: 'Checks for useless assignment to a local variable.'
StyleGuide: '#underscore-unused-vars'
Enabled: true
AutoCorrect: contextual
VersionAdded: '0.11'
VersionChanged: '1.51'
VersionChanged: '<<next>>'
SafeAutoCorrect: false

Lint/UselessElseWithoutRescue:
Expand All @@ -2511,8 +2522,9 @@ Lint/UselessElseWithoutRescue:
Lint/UselessMethodDefinition:
Description: 'Checks for useless method definitions.'
Enabled: true
AutoCorrect: contextual
VersionAdded: '0.90'
VersionChanged: '0.91'
VersionChanged: '<<next>>'
Safe: false

Lint/UselessRescue:
Expand All @@ -2535,13 +2547,17 @@ Lint/UselessSetterCall:
Lint/UselessTimes:
Description: 'Checks for useless `Integer#times` calls.'
Enabled: true
VersionAdded: '0.91'
Safe: false
AutoCorrect: contextual
VersionAdded: '0.91'
VersionChanged: '<<next>>'

Lint/Void:
Description: 'Possible use of operator/literal/variable in void context.'
Enabled: true
AutoCorrect: contextual
VersionAdded: '0.9'
VersionChanged: '<<next>>'
CheckForMethodsWithNoSideEffects: false

#################### Metrics ###############################
Expand Down Expand Up @@ -3701,8 +3717,9 @@ Style/EmptyCaseCondition:
Style/EmptyElse:
Description: 'Avoid empty else-clauses.'
Enabled: true
AutoCorrect: contextual
VersionAdded: '0.28'
VersionChanged: '0.32'
VersionChanged: '<<next>>'
EnforcedStyle: both
# empty - warn only on empty `else`
# nil - warn on `else` with nil in it
Expand All @@ -3716,7 +3733,9 @@ Style/EmptyElse:
Style/EmptyHeredoc:
Description: 'Checks for using empty heredoc to reduce redundancy.'
Enabled: pending
AutoCorrect: contextual
VersionAdded: '1.32'
VersionChanged: '<<next>>'

Style/EmptyLambdaParameter:
Description: 'Omit parens for empty lambda parameters.'
Expand All @@ -3734,7 +3753,9 @@ Style/EmptyMethod:
Description: 'Checks the formatting of empty method definitions.'
StyleGuide: '#no-single-line-methods'
Enabled: true
AutoCorrect: contextual
VersionAdded: '0.46'
VersionChanged: '<<next>>'
EnforcedStyle: compact
SupportedStyles:
- compact
Expand Down Expand Up @@ -4978,10 +4999,11 @@ Style/RedundantHeredocDelimiterQuotes:
Style/RedundantInitialize:
Description: 'Checks for redundant `initialize` methods.'
Enabled: pending
AutoCorrect: contextual
Safe: false
AllowComments: true
VersionAdded: '1.27'
VersionChanged: '1.28'
VersionChanged: '<<next>>'

Style/RedundantInterpolation:
Description: 'Checks for strings that are just an interpolated expression.'
Expand Down
39 changes: 36 additions & 3 deletions docs/modules/ROOT/pages/configuration.adoc
Expand Up @@ -567,13 +567,46 @@ These details will only be seen when RuboCop is run with the `--extra-details` f

=== AutoCorrect

Cops that support the `--autocorrect` option can have that support
disabled. For example:
Cops that support the `--autocorrect` option offer flexible settings for autocorrection.
These settings can be specified in the configuration file as follows:

- `always`
- `contextual`
- `disabled`

==== `always (Default)`

This setting enables autocorrection always by default. For backward compatibility, `true` is treated the same as `always`.

[source,yaml]
----
Style/PerlBackrefs:
AutoCorrect: always # or true
----

==== `contextual`

This setting enables autocorrection when launched from the `rubocop` command, but it is not available through LSP.
e.g., `rubocop --lsp` or a program where `RuboCop::LSP.enable` has been applied.

Inspections via the command line are treated as code that has been finalized.

[source,yaml]
----
Style/PerlBackrefs:
AutoCorrect: contextual
----

This setting prevents autocorrection during editing in the editor.

==== `disabled`

This setting disables autocorrection. For backward compatibility, `false` is treated the same as `disabled`.

[source,yaml]
----
Style/PerlBackrefs:
AutoCorrect: false
AutoCorrect: disabled # or false
----

== Common configuration parameters
Expand Down
17 changes: 13 additions & 4 deletions lib/rubocop/config_validator.rb
Expand Up @@ -18,6 +18,7 @@ class ConfigValidator
# @api private
CONFIG_CHECK_KEYS = %w[Enabled Safe SafeAutoCorrect AutoCorrect].to_set.freeze
CONFIG_CHECK_DEPARTMENTS = %w[pending override_department].freeze
CONFIG_CHECK_AUTOCORRECTS = %w[always contextual disabled].freeze
private_constant :CONFIG_CHECK_KEYS, :CONFIG_CHECK_DEPARTMENTS

def_delegators :@config, :smart_loaded_path, :for_all_cops
Expand Down Expand Up @@ -248,23 +249,31 @@ def reject_conflicting_safe_settings
end
end

# rubocop:disable Metrics/PerceivedComplexity, Metrics/CyclomaticComplexity
def check_cop_config_value(hash, parent = nil)
hash.each do |key, value|
check_cop_config_value(value, key) if value.is_a?(Hash)

next unless CONFIG_CHECK_KEYS.include?(key) && value.is_a?(String)

next if key == 'Enabled' && CONFIG_CHECK_DEPARTMENTS.include?(value)
if key == 'Enabled' && !CONFIG_CHECK_DEPARTMENTS.include?(value)
supposed_values = 'a boolean'
elsif key == 'AutoCorrect' && !CONFIG_CHECK_AUTOCORRECTS.include?(value)
supposed_values = '`always`, `contextual`, `disabled`, or a boolean'
else
next
end

raise ValidationError, msg_not_boolean(parent, key, value)
raise ValidationError, param_error_message(parent, key, value, supposed_values)
end
end
# rubocop:enable Metrics/PerceivedComplexity, Metrics/CyclomaticComplexity

# FIXME: Handling colors in exception messages like this is ugly.
def msg_not_boolean(parent, key, value)
def param_error_message(parent, key, value, supposed_values)
"#{Rainbow('').reset}" \
"Property #{Rainbow(key).yellow} of #{Rainbow(parent).yellow} cop " \
"is supposed to be a boolean and #{Rainbow(value).yellow} is not."
"is supposed to be #{supposed_values} and #{Rainbow(value).yellow} is not."
end
end
end
7 changes: 6 additions & 1 deletion lib/rubocop/cop/autocorrect_logic.rb
Expand Up @@ -32,7 +32,12 @@ def autocorrect_enabled?
# allow turning off autocorrect on a cop by cop basis
return true unless cop_config

return false if cop_config['AutoCorrect'] == false
# `false` is the same as `disabled` for backward compatibility.
return false if ['disabled', false].include?(cop_config['AutoCorrect'])

# When LSP is enabled, it is considered as editing source code,
# and autocorrection with `AutoCorrect: contextual` will not be performed.
return false if contextual_autocorrect? && LSP.enabled?

# :safe_autocorrect is a derived option based on several command-line
# arguments - see RuboCop::Options#add_autocorrection_options
Expand Down
13 changes: 12 additions & 1 deletion lib/rubocop/cop/base.rb
Expand Up @@ -305,6 +305,17 @@ def begin_investigation(processed_source, offset: 0, original: processed_source)
@current_original = original
end

# @api private
def always_autocorrect?
# `true` is the same as `'always'` for backward compatibility.
['always', true].include?(cop_config.fetch('AutoCorrect', 'always'))
end

# @api private
def contextual_autocorrect?
cop_config.fetch('AutoCorrect', 'always') == 'contextual'
end

def inspect # :nodoc:
"#<#{self.class.name}:#{object_id} @config=#{@config} @options=#{@options}>"
end
Expand Down Expand Up @@ -389,7 +400,7 @@ def correct(range)
def use_corrector(range, corrector)
if autocorrect?
attempt_correction(range, corrector)
elsif corrector && cop_config.fetch('AutoCorrect', true)
elsif corrector && (always_autocorrect? || (contextual_autocorrect? && !LSP.enabled?))
:uncorrected
else
:unsupported
Expand Down
4 changes: 3 additions & 1 deletion lib/rubocop/cops_documentation_generator.rb
Expand Up @@ -97,7 +97,9 @@ def properties(cop)
'Version Changed'
]
autocorrect = if cop.support_autocorrect?
"Yes#{' (Unsafe)' unless cop.new(config).safe_autocorrect?}"
context = cop.new.always_autocorrect? ? 'Always' : 'Command-line only'

"#{context}#{' (Unsafe)' unless cop.new(config).safe_autocorrect?}"
else
'No'
end
Expand Down
8 changes: 4 additions & 4 deletions lib/rubocop/rspec/shared_contexts.rb
Expand Up @@ -100,10 +100,10 @@ def source_range(range, buffer: source_buffer)
let(:cur_cop_config) do
RuboCop::ConfigLoader
.default_configuration.for_cop(cop_class)
.merge({
'Enabled' => true, # in case it is 'pending'
'AutoCorrect' => true # in case defaults set it to false
})
.merge(
'Enabled' => true, # in case it is 'pending'
'AutoCorrect' => 'always' # in case defaults set it to 'disabled' or false
)
.merge(cop_config)
end

Expand Down
8 changes: 4 additions & 4 deletions spec/rubocop/cli/autocorrect_spec.rb
Expand Up @@ -1077,7 +1077,7 @@ def func
create_file('example.rb', source)
create_file('.rubocop.yml', <<~YAML)
Layout/DefEndAlignment:
AutoCorrect: true
AutoCorrect: always
YAML
expect(cli.run(['--autocorrect-all'])).to eq(0)
corrected = <<~RUBY
Expand Down Expand Up @@ -1869,7 +1869,7 @@ def self.some_method(foo, bar: 1)
AllCops:
TargetRubyVersion: 2.7
Style/Semicolon:
AutoCorrect: false
AutoCorrect: disabled
YAML
create_file('example.rb', src)
exit_status = cli.run(
Expand Down Expand Up @@ -1942,7 +1942,7 @@ def self.some_method(foo, bar: 1)
create_file('example.rb', 'puts "Hello", 123456')
create_file('.rubocop.yml', <<~YAML)
Style/StringLiterals:
AutoCorrect: false
AutoCorrect: disabled
YAML
expect(cli.run(%w[--autocorrect-all])).to eq(1)
expect($stderr.string).to eq('')
Expand Down Expand Up @@ -2852,7 +2852,7 @@ def foo(a)
create_file('.rubocop.yml', <<~YAML)
Layout/BeginEndAlignment:
EnforcedStyleAlignWith: start_of_line
AutoCorrect: true
AutoCorrect: always
Layout/CaseIndentation:
EnforcedStyle: end
Expand Down
4 changes: 2 additions & 2 deletions spec/rubocop/cli/options_spec.rb
Expand Up @@ -2006,11 +2006,11 @@ def f
end
end

context 'when setting `AutoCorrect: false` for `Style/StringLiterals`' do
context 'when setting `AutoCorrect: disabled` for `Style/StringLiterals`' do
before do
create_file('.rubocop.yml', <<~YAML)
Style/StringLiterals:
AutoCorrect: false
AutoCorrect: disabled
YAML
end

Expand Down

0 comments on commit 772054e

Please sign in to comment.