diff --git a/CHANGELOG.md b/CHANGELOG.md index de689a8c993..f3c5770ec36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## master (unreleased) +### New features + +* [#7868](https://github.com/rubocop-hq/rubocop/pull/7868): `Cop::Base` is the new recommended base class for cops. Extensive refactoring of `Team`, `Commissioner`. ([@marcandre][]) + ## 0.86.0 (2020-06-22) ### New features diff --git a/docs/modules/ROOT/nav.adoc b/docs/modules/ROOT/nav.adoc index c7f2d73015b..eed21f7fb60 100644 --- a/docs/modules/ROOT/nav.adoc +++ b/docs/modules/ROOT/nav.adoc @@ -18,6 +18,7 @@ ** xref:cops_naming.adoc[Naming] ** xref:cops_security.adoc[Security] ** xref:cops_style.adoc[Style] +** xref:v1_upgrade_notes.adoc[Style] * xref:extensions.adoc[Extensions] * xref:integration_with_other_tools.adoc[Integration with Other Tools] * xref:automated_code_review.adoc[Automated Code Review] diff --git a/docs/modules/ROOT/pages/cops_lint.adoc b/docs/modules/ROOT/pages/cops_lint.adoc index f41dcc12721..f87a4bae008 100644 --- a/docs/modules/ROOT/pages/cops_lint.adoc +++ b/docs/modules/ROOT/pages/cops_lint.adoc @@ -3274,8 +3274,7 @@ end | - |=== -This is not actually a cop. It does not inspect anything. It just -provides methods to repack Parser's diagnostics/errors +This cop repacks Parser's diagnostics/errors into RuboCop's offenses. == Lint/ToJSON diff --git a/docs/modules/ROOT/pages/development.adoc b/docs/modules/ROOT/pages/development.adoc index 74f520f2bad..41a576da415 100644 --- a/docs/modules/ROOT/pages/development.adoc +++ b/docs/modules/ROOT/pages/development.adoc @@ -186,10 +186,13 @@ the expression achieved previously: [source,ruby] ---- def_node_matcher :not_empty_call?, <<~PATTERN - (send (send (...) :empty?) :!) + (send (send $(...) :empty?) :!) PATTERN ---- +Note that we added a `$` sign to capture the "expression" in `!.empty?`, +it will become useful later. + Get yourself familiar with the AST node hooks that https://www.rubydoc.info/gems/parser/Parser/AST/Processor[`parser`] and https://www.rubydoc.info/gems/rubocop-ast/RuboCop/AST/Traversal[`rubocop-ast`] @@ -222,11 +225,11 @@ module RuboCop # # # good # array.any? - class SimplifyNotEmptyWithAny < Cop + class SimplifyNotEmptyWithAny < Base MSG = 'Use `.any?` and remove the negation part.'.freeze def_node_matcher :not_empty_call?, <<~PATTERN - (send (send (...) :empty?) :!) + (send (send $(...) :empty?) :!) PATTERN def on_send(node) @@ -244,10 +247,7 @@ Update the spec to cover the expected syntax: [source,ruby] ---- -describe RuboCop::Cop::Style::SimplifyNotEmptyWithAny do - let(:config) { RuboCop::Config.new } - subject(:cop) { described_class.new(config) } - +describe RuboCop::Cop::Style::SimplifyNotEmptyWithAny, :config do it 'registers an offense when using `!a.empty?`' do expect_offense(<<~RUBY) !array.empty? @@ -267,33 +267,45 @@ end === Auto-correct The auto-correct can help humans automatically fix offenses that have been detected. -It's necessary to define an `autocorrect` method that returns a lambda -https://github.com/whitequark/parser/blob/master/lib/parser/rewriter.rb[rewriter] -with the corrector where you can give instructions about what to do with the +It's necessary to `extend AutoCorrector`. +The method `add_offense` yields a corrector object that is a thin wrapper on +https://www.rubydoc.info/gems/parser/Parser/Source/TreeRewriter[parser's TreeRewriter] +to which you can give instructions about what to do with the offensive node. Let's start with a simple spec to cover it: [source,ruby] ---- -it 'autocorrect `!a.empty?` to `a.any?` ' do - expect(autocorrect_source('!a.empty?')).to eq('a.any?') +it 'corrects `!a.empty?`' do + expect_offense(<<~RUBY) + !array.empty? + ^^^^^^^^^^^^^ Use `.any?` and remove the negation part. + RUBY + + expect_correction(<<~RUBY) + array.any? + RUBY end ---- -And then define the `autocorrect` method on the cop side: +And then add the autocorrecting block on the cop side: [source,ruby] ---- -def autocorrect(node) - lambda do |corrector| - internal_expression = node.children[0].children[0].source - corrector.replace(node, "#{internal_expression}.any?") +extend AutoCorrector + +def on_send(node) + expression = not_empty_call?(node) + return unless expression + + add_offense(node) do |corrector| + corrector.replace(node, "#{expression.source}.any?") end end ---- -The corrector allows you to `insert_after` and `insert_before` or +The corrector allows you to `insert_after`, `insert_before`, `wrap` or `replace` a specific node or in any specific range of the code. Range can be determined on `node.location` where it brings specific @@ -319,12 +331,13 @@ And then on the autocorrect method, you just need to use the `cop_config` it: [source,ruby] ---- -def autocorrect(node) - lambda do |corrector| - internal_expression = node.children[0].children[0].source - replacement = cop_config['ReplaceAnyWith'] || "any?" - new_expression = "#{internal_expression}.#{replacement}" - corrector.replace(node, new_expression) +def on_send(node) + expression = not_empty_call?(node) + return unless expression + + add_offense(node) do |corrector| + replacement = cop_config['ReplaceAnyWith'] || 'any?' + corrector.replace(node, "#{expression.source}.#{replacement}") end end ---- diff --git a/docs/modules/ROOT/pages/v1_upgrade_notes.adoc b/docs/modules/ROOT/pages/v1_upgrade_notes.adoc new file mode 100644 index 00000000000..93cc1a4665a --- /dev/null +++ b/docs/modules/ROOT/pages/v1_upgrade_notes.adoc @@ -0,0 +1,258 @@ += v1 Upgrade Notes +:doctype: book + +== Cop Upgrade guide + +Your custom cops should continue to work in v1. + +Nevertheless it is suggested that you tweak them to use the v1 API by following the following steps: + +1) Your class should inherit from `RuboCop::Cop::Base` instead of `RuboCop::Cop::Cop`. + +2) Locate your calls to `add_offense` and make sure that you pass as the first argument either an `AST::Node`, a `::Parser::Source::Comment` or a `::Parser::Source::Range`, and no `location:` named parameter. + +[discrete] +==== Example: + +[source,ruby] +---- +# Before +class MySillyCop < Cop + def on_send(node) + if node.method_name == :- + add_offense(node, location: :selector, message: "Be positive") + end + end +end + +# After +class MySillyCop < Base + def on_send(node) + if node.method_name == :- + add_offense(node.loc.selector, message: "Be positive") + end + end +end +---- + +=== If your class supports autocorrection + +Your class must `extend AutoCorrector`. + +The `corrector` is now yielded from `add_offense`. Move the code of your method `autocorrect` in that block and do not wrap your correction in a lambda. `Corrector` are more powerful and can now be `merge`d. + +==== Example: + +[source,ruby] +---- +# Before +class MySillyCorrectingCop < Cop + def on_send(node) + if node.method_name == :- + add_offense(node, location: :selector, message: 'Be positive') + end + end + + def autocorrect(node) + lambda do |corrector| + corrector.replace(node.loc.selector, '+') + end + end +end + +# After +class MySillyCorrectingCop < Base + extend AutoCorrector + + def on_send(node) + if node.method_name == :- + add_offense(node.loc.selector, message: 'Be positive') do |corrector| + corrector.replace(node.loc.selector, '+') + end + end + end +end +---- + +=== If your class uses instance variables + +Do not use RuboCop's internal instance variables. If you used `@processed_source`, use `processed_source`. If you have a need to access an instance variable, open an issue with your use case. + +Cop instances will be called on different `processed_source` if the class supports it. In all cases it is recommended to to invalidate your cache data during the `on_new_investigation` callback: + +[source,ruby] +---- +# Before +class MyCachingCop < Cop + def on_send(node) + if my_cached_data[node] + #... + end + end + + def my_cached_data + @data ||= processed_source.comments.map { # ... } + end +end + +# After +class MyCachingCop < Base + # ... same as above, plus... + def on_new_investigation + @data = nil + super # Be nice and call super for callback + end +end +---- + +=== Other API changes + +If your cop uses `investigate`, `investigate_post_walk`, `join_force?`, or internal classes like `Corrector`, `Commissioner`, `Team`, these have changed. See the <>. + +=== Upgrading specs + +It is highly recommended you use `expect_offense` / `expect_correction` / `expect_no_offense` in your specs, e.g.: + +[source,ruby] +---- +require 'rubocop/rspec/support' + +RSpec.describe RuboCop::Cop::Custom::MySillyCorrectingCop, :config do + # No need for `let(:cop)` + it 'is positive' do + expect_offense(<<~RUBY) + 42 + 2 - 2 + ^ Be positive + RUBY + + expect_correction(<<~RUBY) + 42 + 2 + 2 + RUBY + end + + it 'does not register an offense for calls to `despair`' do + expect_no_offenses(<<~RUBY) + "don't".despair + RUBY + end +end +---- + +In the unlikely case where you use the class `RuboCop::Cop::Corrector` directly, it has changed a bit but you can ease your transition with `RuboCop::Cop::Legacy::Corrector` that is meant to be somewhat backwards compatible. You will need to `require 'rubocop/cop/legacy/corrector'`. + +== Detailed API Changes + +This section lists all changes (big or small) to the API. It is meant for maintainers of the nuts & bolts of RuboCop; most cop writers will not be impacted by these and are thus not the target audience. + +=== Base class + +_Legacy_: Cops inherit from `Cop::Cop`. + +_Current_: Cops inherit from `Cop::Base`. Having a different base class makes the implementation much cleaner and makes it easy to signal which API is being used. `Cop::Cop` inherits from `Cop::Base` and refines some methods for backward compatiblity. + +=== `add_offense` API + +==== arguments + +_Legacy:_ interface allowed for a `node`, with an optional `location` (symbol or range) or a range with a mandatory range as the location. Some cops were abusing the `node` argument and passing very different things. + +_Current:_ pass a range (or node as a shortcut for `node.loc.expression`), no `location:`. No abuse tolerated. + +==== de-dupping changes + +Both de-dup on `range` and won't process the duplicated offenses at all. + +_Legacy:_ if offenses on same `node` but different `range`: considered as multiple offenses but a single auto-correct call. + +_Current:_ not applicable and not needed with autocorrection's API. + +==== yield + +Both yield under the same conditions (unless cop is disabled for that line), but: + +_Legacy:_ yields after offense added to `#offenses` + +_Current:_ yields before offense is added to `#offenses`. + +Even the legacy mode yields a corrector, but if a developer uses it an error will be raised asking her to inherit from `Cop::Base` instead. + +=== Auto Correction + +==== `#autocorrect` + +_Legacy:_ calls `autocorrect` unless it is disabled / autocorrect is off. + +_Current:_ yields a corrector unless it is disabled. The corrector will be ignored if autocorrecting is off, etc. No support for `autocorrect` method, but a warning is issued if that method is still defined. + +==== Empty corrections + +_Legacy:_ `autocorrect` could return `nil` / `false` in cases where it couldn't actually make a correction. + +_Current:_ No special API. Cases where no corrections are made are automatically detected. + +==== Correction timing + +_Legacy:_ the lambda was called only later in the process, and only under specific conditions (if the auto-correct setting is turned on, etc.) + +_Current:_ correction is built immediately (assuming the cop isn't disabled for the line) and applied later in the process. + +==== Exception handling + +Both: `Commissioner` will rescue all ``StandardError``s during analysis (unless `option[:raise_error]`) and store a corresponding `ErrorWithAnalyzedFileLocation` in its error list. This is done when calling the cop's `on_send` & al., or when calling `investigate` / `investigate_post_walk` callback. + +_Legacy:_ autocorrecting cops were treating errors differently depending on when they occurred. Some errors were silently ignored. Others were rescued as above. Others crashed. Some code in `Team` would rescue errors and add them to the list of errors but I don't think the code worked. + +_Current:_ `Team` no longer has any special error handling to do as potential exceptions happen when `Commissioner` is running. + +==== Other error handling + +_Legacy:_ Clobbering errors are silently ignored. Calling `insert_before` with ranges that extend beyond the source code was silently fixed. + +_Current:_ Such errors are not ignored. It is still ok that a given Cop's corrections clobber another Cop's, but any given Cop should not issue corrections that clobber each other, or with invalid ranges, otherwise these will be listed in the processing errors. + +==== `#corrections` + +_Legacy:_ Corrections were held in `#corrections` as an array of lambdas. A proxy was written to maintain compatibility with `+cop.corrections << ...+`, `+cop.corrections.concat ...+`, etc. + +_Current:_ Corrections are held in `current_corrector`, a `Corrector` which inherits from `Source::TreeRewriter`. + +==== `#support_autocorrect?` + +_Legacy:_ was an instance method. + +_Current:_ now a class method. + +==== Joining forces + +_Legacy:_ `join_force?(force_class)` was called with every force class + +_Current:_ `self.joining_forces` is now used to return the force (or an array of forces) to join. + +=== Cop persistence + +Cops can now be persisted between files. By default new cop instances are created for each source. See `support_multiple_source?` documentation. + +=== Internal classes + +==== `Corrector` + +_Legacy:_ `initialize` accepted a second argument (an array of lambdas). Available through `Legacy::Corrector` if needed. + +_Current:_ derives from `parser`'s `TreeRewriter`. No second argument to `initialize`; not needed as correctors can be merged. + +==== `Commissioner` & `Team` + +Refactored for better separation of concern, being reusable, better result reporting and better error handling. + +=== Misc API changes + +* internal API clarified for Commissioner. It calls `begin_investigation` and receives the results in `complete_investigation`. +* New method `add_global_offense` for offenses that are not attached to a location in particular; it's used for Syntax errors only right now. +* `#offenses`: No longer accessible. +* Callbacks `investigate(processed_source)` and `investigate_post_walk(processed_source)` are renamed `on_new_investigation` and `on_investigation_end` and don't accept an argument; all `on_` callbacks should rely on `processed_source`. +* `#find_location` is deprecated. +* `Correction` is deprecated. +* A few registry access methods were moved from `Cop` to `Registry` both for correctness (e.g. `MyCop.qualified_cop_name` did not work nor made sense) and so that `Cop::Cop` no longer holds any necessary code anymore. Backwards compatibility is maintained. + ** `Cop.registry` \=> `Registry.global` + ** `Cop.all` \=> `Registry.all` + ** `Cop.qualified_cop_name` \=> `Registry.qualified_cop_name` diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 37ab45d5900..c8095dde8e8 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -30,6 +30,7 @@ require_relative 'rubocop/cop/autocorrect_logic' require_relative 'rubocop/cop/badge' require_relative 'rubocop/cop/registry' +require_relative 'rubocop/cop/base' require_relative 'rubocop/cop/cop' require_relative 'rubocop/cop/commissioner' require_relative 'rubocop/cop/corrector' @@ -52,6 +53,7 @@ require_relative 'rubocop/cop/mixin/array_min_size' require_relative 'rubocop/cop/mixin/array_syntax' require_relative 'rubocop/cop/mixin/alignment' +require_relative 'rubocop/cop/mixin/auto_corrector' require_relative 'rubocop/cop/mixin/check_assignment' require_relative 'rubocop/cop/mixin/check_line_breakable' require_relative 'rubocop/cop/mixin/configurable_max' diff --git a/lib/rubocop/cli/command/show_cops.rb b/lib/rubocop/cli/command/show_cops.rb index 0b983667e1f..1e0a57a5700 100644 --- a/lib/rubocop/cli/command/show_cops.rb +++ b/lib/rubocop/cli/command/show_cops.rb @@ -49,7 +49,7 @@ def print_cops_of_department(registry, department, show_all) def print_cop_details(cops) cops.each do |cop| - puts '# Supports --auto-correct' if cop.new(@config).support_autocorrect? + puts '# Supports --auto-correct' if cop.support_autocorrect? puts "#{cop.cop_name}:" puts config_lines(cop) puts diff --git a/lib/rubocop/cop/autocorrect_logic.rb b/lib/rubocop/cop/autocorrect_logic.rb index b9271b9a67e..206e338b0b9 100644 --- a/lib/rubocop/cop/autocorrect_logic.rb +++ b/lib/rubocop/cop/autocorrect_logic.rb @@ -13,11 +13,7 @@ def autocorrect_requested? end def correctable? - support_autocorrect? || disable_uncorrectable? - end - - def support_autocorrect? - respond_to?(:autocorrect) + self.class.support_autocorrect? || disable_uncorrectable? end def disable_uncorrectable? @@ -40,8 +36,9 @@ def autocorrect_enabled? true end - def disable_offense(node) - range = node.location.expression + private + + def disable_offense(range) eol_comment = " # rubocop:todo #{cop_name}" needed_line_length = (range.source_line + eol_comment).length if needed_line_length <= max_line_length @@ -52,8 +49,6 @@ def disable_offense(node) end end - private - def range_of_first_line(range) begin_of_first_line = range.begin_pos - range.column end_of_first_line = begin_of_first_line + range.source_line.length @@ -82,23 +77,18 @@ def max_line_length end def disable_offense_at_end_of_line(range, eol_comment) - ->(corrector) { corrector.insert_after(range, eol_comment) } + Corrector.new(range).insert_after(range, eol_comment) end def disable_offense_before_and_after(range_by_lines) - lambda do |corrector| - range_with_newline = range_by_lines.resize(range_by_lines.size + 1) - leading_whitespace = range_by_lines.source_line[/^\s*/] - - corrector.insert_before( - range_with_newline, - "#{leading_whitespace}# rubocop:todo #{cop_name}\n" - ) - corrector.insert_after( - range_with_newline, - "#{leading_whitespace}# rubocop:enable #{cop_name}\n" - ) - end + range_with_newline = range_by_lines.resize(range_by_lines.size + 1) + leading_whitespace = range_by_lines.source_line[/^\s*/] + + Corrector.new(range_by_lines).wrap( + range_with_newline, + "#{leading_whitespace}# rubocop:todo #{cop_name}\n", + "#{leading_whitespace}# rubocop:enable #{cop_name}\n" + ) end end end diff --git a/lib/rubocop/cop/base.rb b/lib/rubocop/cop/base.rb new file mode 100644 index 00000000000..869297b09f0 --- /dev/null +++ b/lib/rubocop/cop/base.rb @@ -0,0 +1,399 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # A scaffold for concrete cops. + # + # The Cop::Base class is meant to be extended. + # + # Cops track offenses and can autocorrect them on the fly. + # + # A commissioner object is responsible for traversing the AST and invoking + # the specific callbacks on each cop. + # + # First the callback `on_new_investigation` is called; + # if a cop needs to do its own processing of the AST or depends on + # something else. + # + # Then callbacks like `on_def`, `on_send` (see AST::Traversal) are called + # with their respective nodes. + # + # Finally the callback `on_investigation_end` is called. + # + # Within these callbacks, cops are meant to call `add_offense` or + # `add_global_offense`. Use the `processed_source` method to + # get the currently processed source being investigated. + # + # In case of invalid syntax / unparseable content, + # the callback `on_other_file` is called instead of all the other + # `on_...` callbacks. + # + # Private methods are not meant for custom cops consumption, + # nor are any instance variables. + # + class Base # rubocop:disable Metrics/ClassLength + extend RuboCop::AST::Sexp + extend NodePattern::Macros + include RuboCop::AST::Sexp + include Util + include IgnoredNode + include AutocorrectLogic + + attr_reader :config, :processed_source + + # Reports of an investigation. + # Immutable + # Consider creation API private + InvestigationReport = Struct.new(:cop, :processed_source, :offenses, :corrector) + + # List of cops that should not try to autocorrect at the same + # time as this cop + # + # @return [Array] + # + # @api public + def self.autocorrect_incompatible_with + [] + end + + def initialize(config = nil, options = nil) + @config = config || Config.new + @options = options || { debug: false } + reset_investigation + end + + # Called before all on_... have been called + # When refining this method, always call `super` + def on_new_investigation + # Typically do nothing here + end + + # Called after all on_... have been called + # When refining this method, always call `super` + def on_investigation_end + # Typically do nothing here + end + + # Called instead of all on_... callbacks for unrecognized files / syntax errors + # When refining this method, always call `super` + def on_other_file + # Typically do nothing here + end + + # Override and return the Force class(es) you need to join + def self.joining_forces; end + + # Gets called if no message is specified when calling `add_offense` or + # `add_global_offense` + # Cops are discouraged to override this; instead pass your message directly + def message(_range = nil) + self.class::MSG + end + + # Adds an offense that has no particular location. + # No correction can be applied to global offenses + def add_global_offense(message = nil, severity: nil) + severity = find_severity(nil, severity) + message = find_message(nil, message) + @current_offenses << + Offense.new(severity, Offense::NO_LOCATION, message, name, :unsupported) + end + + # Adds an offense on the specified range (or node with an expression) + # Unless that offense is disabled for this range, a corrector will be yielded + # to provide the cop the opportunity to autocorrect the offense. + # If message is not specified, the method `message` will be called. + def add_offense(node_or_range, message: nil, severity: nil, &block) + range = range_from_node_or_range(node_or_range) + return unless @current_offense_locations.add?(range) + + range_to_pass = callback_argument(range) + + severity = find_severity(range_to_pass, severity) + message = find_message(range_to_pass, message) + + status, corrector = enabled_line?(range.line) ? correct(range, &block) : :disabled + + @current_offenses << Offense.new(severity, range, message, name, status, corrector) + end + + # This method should be overridden when a cop's behavior depends + # on state that lives outside of these locations: + # + # (1) the file under inspection + # (2) the cop's source code + # (3) the config (eg a .rubocop.yml file) + # + # For example, some cops may want to look at other parts of + # the codebase being inspected to find violations. A cop may + # use the presence or absence of file `foo.rb` to determine + # whether a certain violation exists in `bar.rb`. + # + # Overriding this method allows the cop to indicate to RuboCop's + # ResultCache system when those external dependencies change, + # ie when the ResultCache should be invalidated. + def external_dependency_checksum + nil + end + + def self.inherited(subclass) + Registry.global.enlist(subclass) + end + + # Call for abstract Cop classes + def self.exclude_from_registry + Registry.global.dismiss(self) + end + + # Returns if class supports auto_correct. + # It is recommended to extend AutoCorrector instead of overriding + def self.support_autocorrect? + false + end + + ### Naming + + def self.badge + @badge ||= Badge.for(name) + end + + def self.cop_name + badge.to_s + end + + def self.department + badge.department + end + + def self.lint? + department == :Lint + end + + # Returns true if the cop name or the cop namespace matches any of the + # given names. + def self.match?(given_names) + return false unless given_names + + given_names.include?(cop_name) || + given_names.include?(department.to_s) + end + + def cop_name + @cop_name ||= self.class.cop_name + end + + alias name cop_name + + ### Configuration Helpers + + def cop_config + # Use department configuration as basis, but let individual cop + # configuration override. + @cop_config ||= @config.for_cop(self.class.department.to_s) + .merge(@config.for_cop(self)) + end + + def config_to_allow_offenses + Formatter::DisabledConfigFormatter + .config_to_allow_offenses[cop_name] ||= {} + end + + def config_to_allow_offenses=(hash) + Formatter::DisabledConfigFormatter.config_to_allow_offenses[cop_name] = + hash + end + + def target_ruby_version + @config.target_ruby_version + end + + def target_rails_version + @config.target_rails_version + end + + def relevant_file?(file) + file == RuboCop::AST::ProcessedSource::STRING_SOURCE_NAME || + file_name_matches_any?(file, 'Include', true) && + !file_name_matches_any?(file, 'Exclude', false) + end + + def excluded_file?(file) + !relevant_file?(file) + end + + ### Persistence + + # Override if your cop should be called repeatedly for mutliple investigations + # Between calls to `on_new_investigation` and `on_investigation_end`, + # the result of `processed_source` will remain constant. + # You should invalidate any caches that depend on the current `processed_source` + # in the `on_new_investigation` callback. + # If your cop does autocorrections, be aware that your instance may be called + # multiple times with the same `processed_source.path` but different content. + def self.support_multiple_source? + false + end + + # @api private + # Called between investigations + def ready + return self if self.class.support_multiple_source? + + self.class.new(@config, @options) + end + + ### Reserved for Cop::Cop + + # @deprecated Make potential errors with previous API more obvious + def offenses + raise 'The offenses are not directly available; ' \ + 'they are returned as the result of the investigation' + end + + private + + ### Reserved for Cop::Cop + + def callback_argument(range) + range + end + + def apply_correction(corrector) + @current_corrector&.merge!(corrector) if corrector + end + + def correction_strategy + return :unsupported unless correctable? + return :uncorrected unless autocorrect? + + :attempt_correction + end + + ### Reserved for Commissioner: + + # Called before any investigation + def begin_investigation(processed_source) + @current_offenses = [] + @current_offense_locations = Set[] + @currently_disabled_lines = Set[] + @processed_source = processed_source + @current_corrector = Corrector.new(@processed_source) if @processed_source.valid_syntax? + end + + # Called to complete an investigation + def complete_investigation + InvestigationReport.new(self, processed_source, @current_offenses, @current_corrector) + ensure + reset_investigation + end + + ### Actually private methods + + def reset_investigation + @currently_disabled_lines = @current_offenses = @processed_source = @current_corrector = nil + end + + # @return [Symbol, Corrector] offense status + def correct(range) + status = correction_strategy + + if block_given? + corrector = Corrector.new(self) + yield corrector + if !corrector.empty? && !self.class.support_autocorrect? + raise "The Cop #{name} must `extend AutoCorrector` to be able to autocorrect" + end + end + + status = attempt_correction(range, corrector) if status == :attempt_correction + + [status, corrector] + end + + # @return [Symbol] offense status + def attempt_correction(range, corrector) + if corrector && !corrector.empty? + status = :corrected + elsif disable_uncorrectable? + corrector = disable_uncorrectable(range) + status = :corrected_with_todo + else + return :uncorrected + end + + apply_correction(corrector) if corrector + status + end + + def disable_uncorrectable(range) + line = range.line + return unless @currently_disabled_lines.add?(line) + + disable_offense(range) + end + + def range_from_node_or_range(node_or_range) + if node_or_range.respond_to?(:loc) + node_or_range.loc.expression + elsif node_or_range.is_a?(::Parser::Source::Range) + node_or_range + else + extra = ' (call `add_global_offense`)' if node_or_range.nil? + raise "Expected a Source::Range, got #{node_or_range.inspect}#{extra}" + end + end + + def find_message(range, message) + annotate(message || message(range)) + end + + def annotate(message) + RuboCop::Cop::MessageAnnotator.new( + config, cop_name, cop_config, @options + ).annotate(message) + end + + def file_name_matches_any?(file, parameter, default_result) + patterns = cop_config[parameter] + return default_result unless patterns + + path = nil + patterns.any? do |pattern| + # Try to match the absolute path, as Exclude properties are absolute. + next true if match_path?(pattern, file) + + # Try with relative path. + path ||= config.path_relative_to_config(file) + match_path?(pattern, path) + end + end + + def enabled_line?(line_number) + return true if @options[:ignore_disable_comments] || !@processed_source + + @processed_source.comment_config.cop_enabled_at_line?(self, line_number) + end + + def find_severity(_range, severity) + custom_severity || severity || default_severity + end + + def default_severity + self.class.lint? ? :warning : :convention + end + + def custom_severity + severity = cop_config['Severity'] + return unless severity + + if Severity::NAMES.include?(severity.to_sym) + severity.to_sym + else + message = "Warning: Invalid severity '#{severity}'. " \ + "Valid severities are #{Severity::NAMES.join(', ')}." + warn(Rainbow(message).red) + end + end + end + end +end diff --git a/lib/rubocop/cop/bundler/insecure_protocol_source.rb b/lib/rubocop/cop/bundler/insecure_protocol_source.rb index 26006e2b567..4eed7ef3a86 100644 --- a/lib/rubocop/cop/bundler/insecure_protocol_source.rb +++ b/lib/rubocop/cop/bundler/insecure_protocol_source.rb @@ -25,8 +25,9 @@ module Bundler # # good # source 'https://rubygems.org' # strongly recommended # source 'http://rubygems.org' - class InsecureProtocolSource < Cop + class InsecureProtocolSource < Base include RangeHelp + extend AutoCorrector MSG = 'The source `:%s` is deprecated because HTTP requests ' \ 'are insecure. ' \ @@ -35,34 +36,23 @@ class InsecureProtocolSource < Cop def_node_matcher :insecure_protocol_source?, <<~PATTERN (send nil? :source - (sym ${:gemcutter :rubygems :rubyforge})) + $(sym ${:gemcutter :rubygems :rubyforge})) PATTERN def on_send(node) - insecure_protocol_source?(node) do |source| + insecure_protocol_source?(node) do |source_node, source| message = format(MSG, source: source) add_offense( - node, - location: range(node.first_argument.loc.expression), + source_node, message: message - ) + ) do |corrector| + corrector.replace( + source_node, "'https://rubygems.org'" + ) + end end end - - def autocorrect(node) - lambda do |corrector| - corrector.replace( - node.first_argument, "'https://rubygems.org'" - ) - end - end - - private - - def range(node) - range_between(node.begin_pos, node.end_pos) - end end end end diff --git a/lib/rubocop/cop/commissioner.rb b/lib/rubocop/cop/commissioner.rb index 4f19e011ec8..080431ba28a 100644 --- a/lib/rubocop/cop/commissioner.rb +++ b/lib/rubocop/cop/commissioner.rb @@ -7,6 +7,35 @@ module Cop class Commissioner include RuboCop::AST::Traversal + # How a Commissioner returns the results of the investigation + # as a list of Cop::InvestigationReport and any errors caught + # during the investigation. + # Immutable + # Consider creation API private + InvestigationReport = Struct.new(:processed_source, :cop_reports, :errors) do + def cops + @cops ||= cop_reports.map(&:cop) + end + + def offenses_per_cop + @offenses_per_cop ||= cop_reports.map(&:offenses) + end + + def correctors + @correctors ||= cop_reports.map(&:corrector) + end + + def offenses + @offenses ||= offenses_per_cop.flatten(1) + end + + def merge(investigation) + InvestigationReport.new(processed_source, + cop_reports + investigation.cop_reports, + errors + investigation.errors) + end + end + attr_reader :errors def initialize(cops, forces = [], options = {}) @@ -15,7 +44,7 @@ def initialize(cops, forces = [], options = {}) @options = options @callbacks = {} - reset_errors + reset end # Create methods like :on_send, :on_super, etc. They will be called @@ -34,15 +63,21 @@ def initialize(cops, forces = [], options = {}) end end + # @return [InvestigationReport] def investigate(processed_source) - reset_errors - reset_callbacks - prepare(processed_source) - invoke_custom_processing(@cops, processed_source) - invoke_custom_processing(@forces, processed_source) - walk(processed_source.ast) unless processed_source.blank? - invoke_custom_post_walk_processing(@cops, processed_source) - @cops.flat_map(&:offenses) + reset + + @cops.each { |cop| cop.send :begin_investigation, processed_source } + if processed_source.valid_syntax? + invoke(:on_new_investigation, @cops) + invoke(:investigate, @forces, processed_source) + walk(processed_source.ast) unless @cops.empty? + invoke(:on_investigation_end, @cops) + else + invoke(:on_other_file, @cops) + end + reports = @cops.map { |cop| cop.send(:complete_investigation) } + InvestigationReport.new(processed_source, reports, @errors) end private @@ -58,52 +93,15 @@ def trigger_responding_cops(callback, node) end end - def reset_errors + def reset @errors = [] + @callbacks = {} end - def reset_callbacks - @callbacks.clear - end - - # TODO: Bad design. - def prepare(processed_source) - @cops.each { |cop| cop.processed_source = processed_source } - end - - # There are cops/forces that require their own custom processing. - # If they define the #investigate method, all input parameters passed - # to the commissioner will be passed to the cop too in order to do - # its own processing. - # - # These custom processors are invoked before the AST traversal, - # so they can build initial state that is later used by callbacks - # during the AST traversal. - def invoke_custom_processing(cops_or_forces, processed_source) - cops_or_forces.each do |cop| - next unless cop.respond_to?(:investigate) - - with_cop_error_handling(cop) do - cop.investigate(processed_source) - end - end - end - - # There are cops that require their own custom processing **after** - # the AST traversal. By performing the walk before invoking these - # custom processors, we allow these cops to build their own - # state during the primary AST traversal instead of performing their - # own AST traversals. Minimizing the number of walks is more efficient. - # - # If they define the #investigate_post_walk method, all input parameters - # passed to the commissioner will be passed to the cop too in order to do - # its own processing. - def invoke_custom_post_walk_processing(cops, processed_source) + def invoke(callback, cops, *args) cops.each do |cop| - next unless cop.respond_to?(:investigate_post_walk) - with_cop_error_handling(cop) do - cop.investigate_post_walk(processed_source) + cop.send(callback, *args) end end end diff --git a/lib/rubocop/cop/cop.rb b/lib/rubocop/cop/cop.rb index 96d5fb2759f..1db7052910c 100644 --- a/lib/rubocop/cop/cop.rb +++ b/lib/rubocop/cop/cop.rb @@ -1,36 +1,19 @@ # frozen_string_literal: true require 'uri' +require_relative 'legacy/corrections_proxy.rb' module RuboCop module Cop - # A scaffold for concrete cops. - # - # The Cop class is meant to be extended. - # - # Cops track offenses and can autocorrect them on the fly. - # - # A commissioner object is responsible for traversing the AST and invoking - # the specific callbacks on each cop. - # If a cop needs to do its own processing of the AST or depends on - # something else, it should define the `#investigate` method and do - # the processing there. - # - # @example - # - # class CustomCop < Cop - # def investigate(processed_source) - # # Do custom processing - # end - # end - class Cop # rubocop:disable Metrics/ClassLength - extend RuboCop::AST::Sexp - extend NodePattern::Macros - include RuboCop::AST::Sexp - include Util - include IgnoredNode - include AutocorrectLogic + # @deprecated Use Cop::Base instead + # Legacy scaffold for Cops. + # See https://docs.rubocop.org/rubocop/cop_api_v1_changelog.html + class Cop < Base + attr_reader :offenses + exclude_from_registry + + # @deprecated Correction = Struct.new(:lambda, :node, :cop) do def call(corrector) lambda.call(corrector) @@ -41,90 +24,16 @@ def call(corrector) end end - attr_reader :config, :offenses, :corrections - attr_accessor :processed_source # TODO: Bad design. - - def self.inherited(subclass) - Registry.global.enlist(subclass) - end - - def self.exclude_from_registry - Registry.global.dismiss(self) - end - - def self.badge - @badge ||= Badge.for(name) - end - - def self.cop_name - badge.to_s - end - - def self.department - badge.department - end - - def self.lint? - department == :Lint - end - - # Returns true if the cop name or the cop namespace matches any of the - # given names. - def self.match?(given_names) - return false unless given_names - - given_names.include?(cop_name) || - given_names.include?(department.to_s) - end - - # List of cops that should not try to autocorrect at the same - # time as this cop - # - # @return [Array] - # - # @api public - def self.autocorrect_incompatible_with - [] - end - - def initialize(config = nil, options = nil) - @config = config || Config.new - @options = options || { debug: false } - - @offenses = [] - @corrections = [] - @corrected_nodes = {} - @corrected_nodes.compare_by_identity - @processed_source = nil - end - - def join_force?(_force_class) - false - end - - def cop_config - # Use department configuration as basis, but let individual cop - # configuration override. - @cop_config ||= @config.for_cop(self.class.department.to_s) - .merge(@config.for_cop(self)) - end - - def message(_node = nil) - self.class::MSG - end - - def add_offense(node, location: :expression, message: nil, severity: nil) - loc = find_location(node, location) - - return if duplicate_location?(loc) - - severity = find_severity(node, severity) - message = find_message(node, message) - - status = enabled_line?(loc.line) ? correct(node) : :disabled - - @offenses << Offense.new(severity, loc, message, name, status) - yield if block_given? && status != :disabled + def add_offense(node_or_range, location: :expression, message: nil, severity: nil, &block) + @v0_argument = node_or_range + range = find_location(node_or_range, location) + if block.nil? && !autocorrect? + super(range, message: message, severity: severity) + else + super(range, message: message, severity: severity) do |corrector| + emulate_v0_callsequence(corrector, &block) + end + end end def find_location(node, loc) @@ -132,108 +41,43 @@ def find_location(node, loc) loc.is_a?(Symbol) ? node.loc.public_send(loc) : loc end - def duplicate_location?(location) - @offenses.any? { |o| o.location == location } + # @deprecated Use class method + def support_autocorrect? + # warn 'deprecated, use cop.class.support_autocorrect?' TODO + self.class.support_autocorrect? end - def correct(node) # rubocop:disable Metrics/PerceivedComplexity, Metrics/MethodLength - reason = reason_to_not_correct(node) - return reason if reason - - @corrected_nodes[node] = true - - if support_autocorrect? - correction = autocorrect(node) - - if correction - @corrections << Correction.new(correction, node, self) - :corrected - elsif disable_uncorrectable? - disable_uncorrectable(node) - :corrected_with_todo - else - :uncorrected - end - elsif disable_uncorrectable? - disable_uncorrectable(node) - :corrected_with_todo - end - end - - def reason_to_not_correct(node) - return :unsupported unless correctable? - return :uncorrected unless autocorrect? - return :already_corrected if @corrected_nodes.key?(node) - - nil + def self.support_autocorrect? + method_defined?(:autocorrect) end - def disable_uncorrectable(node) - return unless node - - @disabled_lines ||= {} - line = node.location.line - return if @disabled_lines.key?(line) + def self.joining_forces + return unless method_defined?(:join_force?) - @disabled_lines[line] = true - @corrections << Correction.new(disable_offense(node), node, self) - end - - def config_to_allow_offenses - Formatter::DisabledConfigFormatter - .config_to_allow_offenses[cop_name] ||= {} - end - - def config_to_allow_offenses=(hash) - Formatter::DisabledConfigFormatter.config_to_allow_offenses[cop_name] = - hash - end - - def target_ruby_version - @config.target_ruby_version - end - - def target_rails_version - @config.target_rails_version - end - - def parse(source, path = nil) - ProcessedSource.new(source, target_ruby_version, path) - end - - def cop_name - @cop_name ||= self.class.cop_name + cop = new + Force.all.select do |force_class| + cop.join_force?(force_class) + end end - alias name cop_name + # @deprecated + def corrections + # warn 'Cop#corrections is deprecated' TODO + return [] unless @last_corrector - def relevant_file?(file) - file == RuboCop::AST::ProcessedSource::STRING_SOURCE_NAME || - file_name_matches_any?(file, 'Include', true) && - !file_name_matches_any?(file, 'Exclude', false) + Legacy::CorrectionsProxy.new(@last_corrector) end - def excluded_file?(file) - !relevant_file?(file) + # Called before all on_... have been called + def on_new_investigation + investigate(processed_source) if respond_to?(:investigate) + super end - # This method should be overridden when a cop's behavior depends - # on state that lives outside of these locations: - # - # (1) the file under inspection - # (2) the cop's source code - # (3) the config (eg a .rubocop.yml file) - # - # For example, some cops may want to look at other parts of - # the codebase being inspected to find violations. A cop may - # use the presence or absence of file `foo.rb` to determine - # whether a certain violation exists in `bar.rb`. - # - # Overriding this method allows the cop to indicate to RuboCop's - # ResultCache system when those external dependencies change, - # ie when the ResultCache should be invalidated. - def external_dependency_checksum - nil + # Called after all on_... have been called + def on_investigation_end + investigate_post_walk(processed_source) if respond_to?(:investigate_post_walk) + super end ### Deprecated registry access @@ -253,58 +97,63 @@ def self.qualified_cop_name(name, origin) Registry.qualified_cop_name(name, origin) end + # @deprecated + # Open issue if there's a valid use case to include this in Base + def parse(source, path = nil) + ProcessedSource.new(source, target_ruby_version, path) + end + private - def find_message(node, message) - annotate(message || message(node)) + def begin_investigation(processed_source) + super + @offenses = @current_offenses + @last_corrector = @current_corrector end - def annotate(message) - RuboCop::Cop::MessageAnnotator.new( - config, cop_name, cop_config, @options - ).annotate(message) + # Override Base + def callback_argument(_range) + @v0_argument end - def file_name_matches_any?(file, parameter, default_result) - patterns = cop_config[parameter] - return default_result unless patterns - - path = nil - patterns.any? do |pattern| - # Try to match the absolute path, as Exclude properties are absolute. - next true if match_path?(pattern, file) + def apply_correction(corrector) + suppress_clobbering { super } + end - # Try with relative path. - path ||= config.path_relative_to_config(file) - match_path?(pattern, path) + # Just for legacy + def emulate_v0_callsequence(corrector) + lambda = correction_lambda + yield corrector if block_given? + unless corrector.empty? + raise 'Your cop must inherit from Cop::Base and extend AutoCorrector' end - end - def enabled_line?(line_number) - return true if @options[:ignore_disable_comments] || !@processed_source + return unless lambda - @processed_source.comment_config.cop_enabled_at_line?(self, line_number) + suppress_clobbering do + lambda.call(corrector) + end end - def find_severity(_node, severity) - custom_severity || severity || default_severity - end + def correction_lambda + return unless correction_strategy == :attempt_correction && support_autocorrect? - def default_severity - self.class.lint? ? :warning : :convention + dedup_on_node(@v0_argument) do + autocorrect(@v0_argument) + end end - def custom_severity - severity = cop_config['Severity'] - return unless severity + def dedup_on_node(node) + @corrected_nodes ||= {}.compare_by_identity + yield unless @corrected_nodes.key?(node) + ensure + @corrected_nodes[node] = true + end - if Severity::NAMES.include?(severity.to_sym) - severity.to_sym - else - message = "Warning: Invalid severity '#{severity}'. " \ - "Valid severities are #{Severity::NAMES.join(', ')}." - warn(Rainbow(message).red) - end + def suppress_clobbering + yield + rescue ::Parser::ClobberingError + # ignore Clobbering errors end end end diff --git a/lib/rubocop/cop/corrector.rb b/lib/rubocop/cop/corrector.rb index 84243ec21d1..a8ca6a9b2e0 100644 --- a/lib/rubocop/cop/corrector.rb +++ b/lib/rubocop/cop/corrector.rb @@ -8,117 +8,25 @@ module Cop # Important! # The nodes modified by the corrections should be part of the # AST of the source_buffer. - class Corrector + class Corrector < ::Parser::Source::TreeRewriter + # @param source [Parser::Source::Buffer, or anything + # leading to one via `(processed_source.)buffer`] # - # @param source_buffer [Parser::Source::Buffer] - # @param corrections [Array(#call)] - # Array of Objects that respond to #call. They will receive the - # corrector itself and should use its method to modify the source. - # - # @example - # - # class AndOrCorrector - # def initialize(node) - # @node = node - # end - # - # def call(corrector) - # replacement = (@node.type == :and ? '&&' : '||') - # corrector.replace(@node.loc.operator, replacement) - # end - # end - # - # corrections = [AndOrCorrector.new(node)] - # corrector = Corrector.new(source_buffer, corrections) - def initialize(source_buffer, corrections = []) - @source_buffer = source_buffer - raise 'source_buffer should be a Parser::Source::Buffer' unless \ - source_buffer.is_a? Parser::Source::Buffer - - @corrections = corrections - @source_rewriter = Parser::Source::TreeRewriter.new( - source_buffer, + # corrector = Corrector.new(cop) + def initialize(source) + source = self.class.source_buffer(source) + super( + source, different_replacements: :raise, swallowed_insertions: :raise, crossing_deletions: :accept ) - @diagnostics = [] # Don't print warnings to stderr if corrections conflict with each other - @source_rewriter.diagnostics.consumer = lambda do |diagnostic| - @diagnostics << diagnostic - end - end - - attr_reader :corrections, :diagnostics - - # Does the actual rewrite and returns string corresponding to - # the rewritten source. - # - # @return [String] - def rewrite - @corrections.each do |correction| - begin - @source_rewriter.transaction do - correction.call(self) - end - rescue ErrorWithAnalyzedFileLocation => e - raise e unless e.cause.is_a?(::Parser::ClobberingError) - end - end - - @source_rewriter.process - end - - # Removes the source range. - # - # @param [Parser::Source::Range, Rubocop::AST::Node] range or node - def remove(node_or_range) - range = to_range(node_or_range) - @source_rewriter.remove(range) - end - - # Inserts new code before the given source range. - # - # @param [Parser::Source::Range, Rubocop::AST::Node] range or node - # @param [String] content - def insert_before(node_or_range, content) - range = to_range(node_or_range) - # TODO: Fix Cops using bad ranges instead - if range.end_pos > @source_buffer.source.size - range = range.with(end_pos: @source_buffer.source.size) - end - - @source_rewriter.insert_before(range, content) - end - - # Inserts new code after the given source range. - # - # @param [Parser::Source::Range, Rubocop::AST::Node] range or node - # @param [String] content - def insert_after(node_or_range, content) - range = to_range(node_or_range) - @source_rewriter.insert_after(range, content) + diagnostics.consumer = ->(diagnostic) {} end - # Wraps the given source range with the given before and after texts - # - # @param [Parser::Source::Range, Rubocop::AST::Node] range or node - # @param [String] before - # @param [String] after - def wrap(node_or_range, before, after) - range = to_range(node_or_range) - @source_rewriter.wrap(range, before, after) - end - - # Replaces the code of the source range `range` with `content`. - # - # @param [Parser::Source::Range, Rubocop::AST::Node] range or node - # @param [String] content - def replace(node_or_range, content) - range = to_range(node_or_range) - @source_rewriter.replace(range, content) - end + alias rewrite process # Legacy # Removes `size` characters prior to the source range. # @@ -126,10 +34,11 @@ def replace(node_or_range, content) # @param [Integer] size def remove_preceding(node_or_range, size) range = to_range(node_or_range) - to_remove = Parser::Source::Range.new(range.source_buffer, - range.begin_pos - size, - range.begin_pos) - @source_rewriter.remove(to_remove) + to_remove = range.with( + begin_pos: range.begin_pos - size, + end_pos: range.begin_pos + ) + remove(to_remove) end # Removes `size` characters from the beginning of the given range. @@ -140,10 +49,8 @@ def remove_preceding(node_or_range, size) # @param [Integer] size def remove_leading(node_or_range, size) range = to_range(node_or_range) - to_remove = Parser::Source::Range.new(range.source_buffer, - range.begin_pos, - range.begin_pos + size) - @source_rewriter.remove(to_remove) + to_remove = range.with(end_pos: range.begin_pos + size) + remove(to_remove) end # Removes `size` characters from the end of the given range. @@ -154,10 +61,22 @@ def remove_leading(node_or_range, size) # @param [Integer] size def remove_trailing(node_or_range, size) range = to_range(node_or_range) - to_remove = Parser::Source::Range.new(range.source_buffer, - range.end_pos - size, - range.end_pos) - @source_rewriter.remove(to_remove) + to_remove = range.with(begin_pos: range.end_pos - size) + remove(to_remove) + end + + # Duck typing for get to a ::Parser::Source::Buffer + def self.source_buffer(source) + source = source.processed_source if source.respond_to?(:processed_source) + source = source.buffer if source.respond_to?(:buffer) + source = source.source_buffer if source.respond_to?(:source_buffer) + + unless source.is_a? ::Parser::Source::Buffer + raise TypeError, 'Expected argument to lead to a Parser::Source::Buffer ' \ + "but got #{source.inspect}" + end + + source end private @@ -178,8 +97,12 @@ def to_range(node_or_range) range end + def check_range_validity(node_or_range) + super(to_range(node_or_range)) + end + def validate_buffer(buffer) - return if buffer == @source_buffer + return if buffer == source_buffer unless buffer.is_a?(::Parser::Source::Buffer) # actually this should be enforced by parser gem diff --git a/lib/rubocop/cop/generator.rb b/lib/rubocop/cop/generator.rb index dc2562bcdf1..9eaf2ef5a8c 100644 --- a/lib/rubocop/cop/generator.rb +++ b/lib/rubocop/cop/generator.rb @@ -54,7 +54,7 @@ module %s # # good # good_foo_method(args) # - class %s < Cop + class %s < Base # TODO: Implement the cop in here. # # In many cases, you can use a node matcher for matching node pattern. diff --git a/lib/rubocop/cop/internal_affairs/node_type_predicate.rb b/lib/rubocop/cop/internal_affairs/node_type_predicate.rb index 7981eeeb19b..c73777af031 100644 --- a/lib/rubocop/cop/internal_affairs/node_type_predicate.rb +++ b/lib/rubocop/cop/internal_affairs/node_type_predicate.rb @@ -13,7 +13,9 @@ module InternalAffairs # # good # node.send_type? # - class NodeTypePredicate < Cop + class NodeTypePredicate < Base + extend AutoCorrector + MSG = 'Use `#%s_type?` to check node type.' def_node_matcher :node_type_check, <<~PATTERN @@ -21,21 +23,16 @@ class NodeTypePredicate < Cop PATTERN def on_send(node) - node_type_check(node) do |_receiver, node_type| + node_type_check(node) do |receiver, node_type| return unless Parser::Meta::NODE_TYPES.include?(node_type) - add_offense(node, message: format(MSG, type: node_type)) - end - end - - def autocorrect(node) - receiver, node_type = node_type_check(node) - range = Parser::Source::Range.new(node.source_range.source_buffer, - receiver.loc.expression.end_pos + 1, - node.loc.expression.end_pos) - - lambda do |corrector| - corrector.replace(range, "#{node_type}_type?") + message = format(MSG, type: node_type) + add_offense(node, message: message) do |corrector| + range = node.loc.expression.with( + begin_pos: receiver.loc.expression.end_pos + 1 + ) + corrector.replace(range, "#{node_type}_type?") + end end end end diff --git a/lib/rubocop/cop/layout/case_indentation.rb b/lib/rubocop/cop/layout/case_indentation.rb index b9eebe88ca7..2f604d568d2 100644 --- a/lib/rubocop/cop/layout/case_indentation.rb +++ b/lib/rubocop/cop/layout/case_indentation.rb @@ -67,10 +67,11 @@ module Layout # else # y / 3 # end - class CaseIndentation < Cop + class CaseIndentation < Base include Alignment include ConfigurableEnforcedStyle include RangeHelp + extend AutoCorrector MSG = 'Indent `when` %s `%s`.' @@ -82,16 +83,6 @@ def on_case(case_node) end end - def autocorrect(node) - whitespace = whitespace_range(node) - - return false unless whitespace.source.strip.empty? - - lambda do |corrector| - corrector.replace(whitespace, replacement(node)) - end - end - private def check_when(when_node) @@ -114,22 +105,30 @@ def indentation_width end def incorrect_style(when_node) + add_offense(when_node.loc.keyword) do |corrector| + detect_incorrect_style(when_node) + + whitespace = whitespace_range(when_node) + + corrector.replace(whitespace, replacement(when_node)) if whitespace.source.strip.empty? + end + end + + def detect_incorrect_style(when_node) when_column = when_node.loc.keyword.column base_column = base_column(when_node.parent, alternative_style) - add_offense(when_node, location: :keyword, message: message(style)) do - if when_column == base_column - opposite_style_detected - else - unrecognized_style_detected - end + if when_column == base_column + opposite_style_detected + else + unrecognized_style_detected end end - def message(base) + def find_message(*) depth = indent_one_step? ? 'one step more than' : 'as deep as' - format(MSG, depth: depth, base: base) + format(MSG, depth: depth, base: style) end def base_column(case_node, base) diff --git a/lib/rubocop/cop/layout/first_argument_indentation.rb b/lib/rubocop/cop/layout/first_argument_indentation.rb index 435a6ff11ab..918025eac11 100644 --- a/lib/rubocop/cop/layout/first_argument_indentation.rb +++ b/lib/rubocop/cop/layout/first_argument_indentation.rb @@ -241,6 +241,10 @@ def comment_lines .select { |c| begins_its_line?(c.loc.expression) } .map { |c| c.loc.line } end + + def on_new_investigation + @comment_lines = nil + end end end end diff --git a/lib/rubocop/cop/layout/multiline_block_layout.rb b/lib/rubocop/cop/layout/multiline_block_layout.rb index c9f8c8be112..e1bd2b2203f 100644 --- a/lib/rubocop/cop/layout/multiline_block_layout.rb +++ b/lib/rubocop/cop/layout/multiline_block_layout.rb @@ -105,7 +105,6 @@ def line_break_necessary_in_args?(node) def add_offense_for_expression(node, expr, msg) expression = expr.source_range range = range_between(expression.begin_pos, expression.end_pos) - add_offense(node, location: range, message: msg) end diff --git a/lib/rubocop/cop/layout/space_around_block_parameters.rb b/lib/rubocop/cop/layout/space_around_block_parameters.rb index b35185c4164..efdf9b2fad5 100644 --- a/lib/rubocop/cop/layout/space_around_block_parameters.rb +++ b/lib/rubocop/cop/layout/space_around_block_parameters.rb @@ -24,9 +24,10 @@ module Layout # # good # {}.each { | x, y | puts x } # ->( x, y ) { puts x } - class SpaceAroundBlockParameters < Cop + class SpaceAroundBlockParameters < Base include ConfigurableEnforcedStyle include RangeHelp + extend AutoCorrector def on_block(node) arguments = node.arguments @@ -38,23 +39,6 @@ def on_block(node) check_each_arg(arguments) end - # @param target [RuboCop::AST::Node,Parser::Source::Range] - def autocorrect(target) - lambda do |corrector| - if target.is_a?(RuboCop::AST::Node) - if target.parent.children.first == target - corrector.insert_before(target, ' ') - else - corrector.insert_after(target, ' ') - end - elsif /^\s+$/.match?(target.source) - corrector.remove(target) - else - corrector.insert_after(target, ' ') - end - end - end - private def pipes(arguments) @@ -97,7 +81,7 @@ def check_no_space_style_inside_pipes(args, opening_pipe, closing_pipe) check_no_space(opening_pipe.end_pos, first.begin_pos, 'Space before first') - check_no_space(last_end_pos_inside_pipes(last.end_pos), + check_no_space(last_end_pos_inside_pipes(last), closing_pipe.begin_pos, 'Space after last') end @@ -118,7 +102,7 @@ def check_opening_pipe_space(args, opening_pipe) def check_closing_pipe_space(args, closing_pipe) last = args.last.source_range - last_end_pos = last_end_pos_inside_pipes(last.end_pos) + last_end_pos = last_end_pos_inside_pipes(last) check_space(last_end_pos, closing_pipe.begin_pos, last, 'after last block parameter') @@ -126,8 +110,9 @@ def check_closing_pipe_space(args, closing_pipe) 'Extra space after last') end - def last_end_pos_inside_pipes(pos) - processed_source.buffer.source[pos] == ',' ? pos + 1 : pos + def last_end_pos_inside_pipes(range) + pos = range.end_pos + range.source_buffer.source[pos] == ',' ? pos + 1 : pos end def check_each_arg(args) @@ -151,7 +136,14 @@ def check_space(space_begin_pos, space_end_pos, range, msg, node = nil) return if space_begin_pos != space_end_pos target = node || range - add_offense(target, location: range, message: "Space #{msg} missing.") + message = "Space #{msg} missing." + add_offense(target, message: message) do |corrector| + if node + corrector.insert_before(node, ' ') + else + corrector.insert_after(target, ' ') + end + end end def check_no_space(space_begin_pos, space_end_pos, msg) @@ -160,8 +152,10 @@ def check_no_space(space_begin_pos, space_end_pos, msg) range = range_between(space_begin_pos, space_end_pos) return if range.source.include?("\n") - add_offense(range, location: range, - message: "#{msg} block parameter detected.") + message = "#{msg} block parameter detected." + add_offense(range, message: message) do |corrector| + corrector.remove(range) + end end end end diff --git a/lib/rubocop/cop/legacy/corrections_proxy.rb b/lib/rubocop/cop/legacy/corrections_proxy.rb new file mode 100644 index 00000000000..afd9c8e9ef0 --- /dev/null +++ b/lib/rubocop/cop/legacy/corrections_proxy.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Legacy + # Legacy support for Corrector#corrections + # See https://docs.rubocop.org/rubocop/cop_api_v1_changelog.html + class CorrectionsProxy + def initialize(corrector) + @corrector = corrector + end + + def <<(callable) + suppress_clobbering do + @corrector.transaction do + callable.call(@corrector) + end + end + end + + def empty? + @corrector.empty? + end + + def concat(corrections) + if corrections.is_a?(CorrectionsProxy) + suppress_clobbering do + corrector.merge!(corrections.corrector) + end + else + corrections.each { |correction| self << correction } + end + end + + protected + + attr_reader :corrector + + private + + def suppress_clobbering + yield + rescue ::Parser::ClobberingError + # ignore Clobbering errors + end + end + end + end +end diff --git a/lib/rubocop/cop/legacy/corrector.rb b/lib/rubocop/cop/legacy/corrector.rb new file mode 100644 index 00000000000..6b625f7c726 --- /dev/null +++ b/lib/rubocop/cop/legacy/corrector.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Legacy + # Legacy Corrector for v0 API support. + # See https://docs.rubocop.org/rubocop/cop_api_v1_changelog.html + class Corrector < RuboCop::Cop::Corrector + # Support legacy second argument + def initialize(source, corr = []) + super(source) + if corr.is_a?(CorrectionsProxy) + merge!(corr.send(:corrector)) + else + # warn "Corrector.new with corrections is deprecated." unless corr.empty? TODO + corr.each do |c| + corrections << c + end + end + end + + def corrections + # warn "#corrections is deprecated. Open an issue if you have a valid usecase." TODO + CorrectionsProxy.new(self) + end + end + end + end +end diff --git a/lib/rubocop/cop/lint/redundant_cop_disable_directive.rb b/lib/rubocop/cop/lint/redundant_cop_disable_directive.rb index 70f60a60494..872da67c3c5 100644 --- a/lib/rubocop/cop/lint/redundant_cop_disable_directive.rb +++ b/lib/rubocop/cop/lint/redundant_cop_disable_directive.rb @@ -25,34 +25,34 @@ module Lint # # # good # x += 1 - class RedundantCopDisableDirective < Cop + class RedundantCopDisableDirective < Base include RangeHelp + extend AutoCorrector COP_NAME = 'Lint/RedundantCopDisableDirective' - def check(offenses, cop_disabled_line_ranges, comments) + attr_accessor :offenses_to_check + + def initialize(config = nil, options = nil, offenses = nil) + @offenses_to_check = offenses + super(config, options) + end + + def on_new_investigation + return unless offenses_to_check + + comments = processed_source.comments + cop_disabled_line_ranges = processed_source.disabled_line_ranges + redundant_cops = Hash.new { |h, k| h[k] = Set.new } each_redundant_disable(cop_disabled_line_ranges, - offenses, comments) do |comment, redundant_cop| + offenses_to_check, comments) do |comment, redundant_cop| redundant_cops[comment].add(redundant_cop) end add_offenses(redundant_cops) - end - - def autocorrect(args) - lambda do |corrector| - ranges, range = *args # Ranges are sorted by position. - - range = if range.source.start_with?('#') - comment_range_with_surrounding_space(range) - else - directive_range_in_list(range, ranges) - end - - corrector.remove(range) - end + super end private @@ -187,10 +187,12 @@ def add_offense_for_entire_comment(comment, cops) cop_list = cops.sort.map { |c| describe(c) } add_offense( - [[location], location], - location: location, + location, message: "Unnecessary disabling of #{cop_list.join(', ')}." - ) + ) do |corrector| + range = comment_range_with_surrounding_space(location) + corrector.remove(range) + end end def add_offense_for_some_cops(comment, cops) @@ -200,10 +202,12 @@ def add_offense_for_some_cops(comment, cops) cop_ranges.each do |cop, range| add_offense( - [ranges, range], - location: range, + range, message: "Unnecessary disabling of #{describe(cop)}." - ) + ) do |corrector| + range = directive_range_in_list(range, ranges) + corrector.remove(range) + end end end diff --git a/lib/rubocop/cop/lint/syntax.rb b/lib/rubocop/cop/lint/syntax.rb index efa2436c684..77393bc8ce9 100644 --- a/lib/rubocop/cop/lint/syntax.rb +++ b/lib/rubocop/cop/lint/syntax.rb @@ -3,49 +3,34 @@ module RuboCop module Cop module Lint - # This is not actually a cop. It does not inspect anything. It just - # provides methods to repack Parser's diagnostics/errors + # This cop repacks Parser's diagnostics/errors # into RuboCop's offenses. - class Syntax < Cop - PseudoSourceRange = Struct.new(:line, :column, :source_line, :begin_pos, - :end_pos) - - ERROR_SOURCE_RANGE = PseudoSourceRange.new(1, 0, '', 0, 1).freeze - - def self.offenses_from_processed_source(processed_source, - config, options) - cop = new(config, options) - - cop.add_offense_from_error(processed_source.parser_error) if processed_source.parser_error - + class Syntax < Base + def on_other_file + add_offense_from_error(processed_source.parser_error) if processed_source.parser_error processed_source.diagnostics.each do |diagnostic| - cop.add_offense_from_diagnostic(diagnostic, - processed_source.ruby_version) + add_offense_from_diagnostic(diagnostic, + processed_source.ruby_version) end - - cop.offenses + super end + private + def add_offense_from_diagnostic(diagnostic, ruby_version) message = "#{diagnostic.message}\n(Using Ruby #{ruby_version} parser; " \ 'configure using `TargetRubyVersion` parameter, under `AllCops`)' - add_offense(nil, - location: diagnostic.location, + add_offense(diagnostic.location, message: message, severity: diagnostic.level) end def add_offense_from_error(error) message = beautify_message(error.message) - add_offense(nil, - location: ERROR_SOURCE_RANGE, - message: message, - severity: :fatal) + add_global_offense(message, severity: :fatal) end - private - def beautify_message(message) message = message.capitalize message << '.' unless message.end_with?('.') diff --git a/lib/rubocop/cop/mixin/auto_corrector.rb b/lib/rubocop/cop/mixin/auto_corrector.rb new file mode 100644 index 00000000000..e7c58754073 --- /dev/null +++ b/lib/rubocop/cop/mixin/auto_corrector.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # extend this module to signal autocorrection support + module AutoCorrector + def support_autocorrect? + true + end + end + end +end diff --git a/lib/rubocop/cop/mixin/surrounding_space.rb b/lib/rubocop/cop/mixin/surrounding_space.rb index 2b79134c5e1..25297e8005a 100644 --- a/lib/rubocop/cop/mixin/surrounding_space.rb +++ b/lib/rubocop/cop/mixin/surrounding_space.rb @@ -14,7 +14,7 @@ module SurroundingSpace private def side_space_range(range:, side:) - buffer = @processed_source.buffer + buffer = processed_source.buffer src = buffer.source begin_pos = range.begin_pos @@ -47,7 +47,7 @@ def index_of_last_token(node) def token_table @token_table ||= begin table = {} - @processed_source.tokens.each_with_index do |t, ix| + processed_source.tokens.each_with_index do |t, ix| table[t.line] ||= {} table[t.line][t.column] = ix end @@ -55,6 +55,11 @@ def token_table end end + def on_new_investigation + @token_table = nil + super + end + def no_space_offenses(node, # rubocop:disable Metrics/ParameterLists left_token, right_token, diff --git a/lib/rubocop/cop/offense.rb b/lib/rubocop/cop/offense.rb index 74d597732f8..dee95e60ca1 100644 --- a/lib/rubocop/cop/offense.rb +++ b/lib/rubocop/cop/offense.rb @@ -54,14 +54,28 @@ class Offense # @api private attr_reader :status + # @api public + # + # @!attribute [r] corrector + # + # @return [Corrector | nil] + # the autocorrection for this offense, or `nil` when not available + attr_reader :corrector + + PseudoSourceRange = Struct.new(:line, :column, :source_line, :begin_pos, + :end_pos) + + NO_LOCATION = PseudoSourceRange.new(1, 0, '', 0, 1).freeze + # @api private - def initialize(severity, location, message, cop_name, - status = :uncorrected) + def initialize(severity, location, message, cop_name, # rubocop:disable Metrics/ParameterLists + status = :uncorrected, corrector = nil) @severity = RuboCop::Cop::Severity.new(severity) @location = location @message = message.freeze @cop_name = cop_name.freeze @status = status + @corrector = corrector freeze end diff --git a/lib/rubocop/cop/style/redundant_exception.rb b/lib/rubocop/cop/style/redundant_exception.rb index 805a8e4eff5..b2b7b380612 100644 --- a/lib/rubocop/cop/style/redundant_exception.rb +++ b/lib/rubocop/cop/style/redundant_exception.rb @@ -16,21 +16,22 @@ module Style # # # Good # raise 'message' - class RedundantException < Cop + class RedundantException < Base + extend AutoCorrector + MSG_1 = 'Redundant `RuntimeError` argument can be removed.' MSG_2 = 'Redundant `RuntimeError.new` call can be replaced with ' \ 'just the message.' + # Switch `raise RuntimeError, 'message'` to `raise 'message'`, and + # `raise RuntimeError.new('message')` to `raise 'message'`. def on_send(node) - exploded?(node) { return add_offense(node, message: MSG_1) } - compact?(node) { add_offense(node, message: MSG_2) } + fix_exploded(node) || fix_compact(node) end - # Switch `raise RuntimeError, 'message'` to `raise 'message'`, and - # `raise RuntimeError.new('message')` to `raise 'message'`. - def autocorrect(node) # rubocop:disable Metrics/MethodLength + def fix_exploded(node) exploded?(node) do |command, message| - return lambda do |corrector| + add_offense(node, message: MSG_1) do |corrector| if node.parenthesized? corrector.replace(node, "#{command}(#{message.source})") @@ -40,8 +41,11 @@ def autocorrect(node) # rubocop:disable Metrics/MethodLength end end end + end + + def fix_compact(node) compact?(node) do |new_call, message| - lambda do |corrector| + add_offense(node, message: MSG_2) do |corrector| corrector.replace(new_call, message.source) end end diff --git a/lib/rubocop/cop/team.rb b/lib/rubocop/cop/team.rb index 59e1f2e1c12..254d48b8546 100644 --- a/lib/rubocop/cop/team.rb +++ b/lib/rubocop/cop/team.rb @@ -10,23 +10,16 @@ module Cop # first the ones needed for autocorrection (if any), then the rest # (unless autocorrections happened). class Team - DEFAULT_OPTIONS = { - auto_correct: false, - debug: false - }.freeze - - Investigation = Struct.new(:offenses, :errors) - attr_reader :errors, :warnings, :updated_source_file, :cops alias updated_source_file? updated_source_file - def initialize(cops, config = nil, options = nil) + def initialize(cops, config = nil, options = {}) @cops = cops @config = config - @options = options || DEFAULT_OPTIONS - @errors = [] - @warnings = [] + @options = options + reset + @ready = true validate_config end @@ -40,15 +33,14 @@ def self.new(cop_or_classes, config, options = {}) end # @return [Team] with cops assembled from the given `cop_classes` - def self.mobilize(cop_classes, config, options = nil) - options ||= DEFAULT_OPTIONS + def self.mobilize(cop_classes, config, options = {}) cops = mobilize_cops(cop_classes, config, options) new(cops, config, options) end # @return [Array] - def self.mobilize_cops(cop_classes, config, options = nil) - options ||= DEFAULT_OPTIONS + def self.mobilize_cops(cop_classes, config, options = {}) + cop_classes = Registry.new(cop_classes.to_a) unless cop_classes.is_a?(Registry) only = options.fetch(:only, []) safe = options.fetch(:safe, false) cop_classes.enabled(config, only, safe).map do |cop_class| @@ -64,93 +56,102 @@ def debug? @options[:debug] end + # @deprecated. Use investigate + # @return Array def inspect_file(processed_source) - # If we got any syntax errors, return only the syntax offenses. - unless processed_source.valid_syntax? - return Lint::Syntax.offenses_from_processed_source( - processed_source, @config, @options - ) + investigate(processed_source).offenses + end + + # @return [Commissioner::InvestigationReport] + def investigate(processed_source) + be_ready + + # The autocorrection process may have to be repeated multiple times + # until there are no corrections left to perform + # To speed things up, run auto-correcting cops by themselves, and only + # run the other cops when no corrections are left + on_duty = roundup_relevant_cops(processed_source.file_path) + + autocorrect_cops, other_cops = on_duty.partition(&:autocorrect?) + + report = investigate_partial(autocorrect_cops, processed_source) + + unless autocorrect(processed_source, report) + # If we corrected some errors, another round of inspection will be + # done, and any other offenses will be caught then, so only need + # to check other_cops if no correction was done + report = report.merge(investigate_partial(other_cops, processed_source)) end - offenses(processed_source) + process_errors(processed_source.path, report.errors) + + report + ensure + @ready = false end + # @deprecated def forces - @forces ||= forces_for(cops) + @forces ||= self.class.forces_for(cops) end - def forces_for(cops) - Force.all.each_with_object([]) do |force_class, forces| - joining_cops = cops.select { |cop| cop.join_force?(force_class) } - next if joining_cops.empty? + # @return [Array] needed for the given cops + def self.forces_for(cops) + needed = Hash.new { |h, k| h[k] = [] } + cops.each do |cop| + Array(cop.class.joining_forces).each { |force| needed[force] << cop } + end - forces << force_class.new(joining_cops) + needed.map do |force_class, joining_cops| + force_class.new(joining_cops) end end - def autocorrect(buffer, cops) + def external_dependency_checksum + keys = cops.map(&:external_dependency_checksum).compact + Digest::SHA1.hexdigest(keys.join) + end + + private + + def autocorrect(processed_source, report) @updated_source_file = false return unless autocorrect? - new_source = autocorrect_all_cops(buffer, cops) + new_source = autocorrect_report(report) - return if new_source == buffer.source + return unless new_source if @options[:stdin] # holds source read in from stdin, when --stdin option is used @options[:stdin] = new_source else - filename = buffer.name + filename = processed_source.buffer.name File.open(filename, 'w') { |f| f.write(new_source) } end @updated_source_file = true - rescue RuboCop::ErrorWithAnalyzedFileLocation => e - process_errors(buffer.name, [e]) - raise e.cause - end - - def external_dependency_checksum - keys = cops.map(&:external_dependency_checksum).compact - Digest::SHA1.hexdigest(keys.join) end - private - - def offenses(processed_source) # rubocop:disable Metrics/AbcSize - # The autocorrection process may have to be repeated multiple times - # until there are no corrections left to perform - # To speed things up, run auto-correcting cops by themselves, and only - # run the other cops when no corrections are left - on_duty = roundup_relevant_cops(processed_source.file_path) - - autocorrect_cops, other_cops = on_duty.partition(&:autocorrect?) - - autocorrect = investigate(autocorrect_cops, processed_source) - - if autocorrect(processed_source.buffer, autocorrect_cops) - # We corrected some errors. Another round of inspection will be - # done, and any other offenses will be caught then, so we don't - # need to continue. - return autocorrect.offenses - end - - other = investigate(other_cops, processed_source) + def be_ready + return if @ready - errors = [*autocorrect.errors, *other.errors] - process_errors(processed_source.path, errors) - - autocorrect.offenses.concat(other.offenses) + reset + @cops.map!(&:ready) + @ready = true end - def investigate(cops, processed_source) - return Investigation.new([], {}) if cops.empty? - - commissioner = Commissioner.new(cops, forces_for(cops), @options) - offenses = commissioner.investigate(processed_source) + def reset + @errors = [] + @warnings = [] + end - Investigation.new(offenses, commissioner.errors) + # @return [Commissioner::InvestigationReport] + def investigate_partial(cops, processed_source) + commissioner = Commissioner.new(cops, self.class.forces_for(cops), @options) + commissioner.investigate(processed_source) end + # @return [Array] def roundup_relevant_cops(filename) cops.reject do |cop| cop.excluded_file?(filename) || @@ -171,30 +172,45 @@ def support_target_rails_version?(cop) cop.class.support_target_rails_version?(cop.target_rails_version) end - def autocorrect_all_cops(buffer, cops) - corrector = Corrector.new(buffer) + def autocorrect_report(report) + corrector = collate_corrections(report) - collate_corrections(corrector, cops) + corrector.rewrite unless corrector.empty? + end - if !corrector.corrections.empty? - corrector.rewrite - else - buffer.source + def collate_corrections(report) + corrector = Corrector.new(report.processed_source) + + each_corrector(report) do |to_merge| + suppress_clobbering do + corrector.merge!(to_merge) + end end + + corrector end - def collate_corrections(corrector, cops) + def each_corrector(report) skips = Set.new + report.cop_reports.each do |cop_report| + cop = cop_report.cop + corrector = cop_report.corrector - cops.each do |cop| - next if cop.corrections.empty? + next if corrector.nil? || corrector.empty? next if skips.include?(cop.class) - corrector.corrections.concat(cop.corrections) + yield corrector + skips.merge(cop.class.autocorrect_incompatible_with) end end + def suppress_clobbering + yield + rescue ::Parser::ClobberingError + # ignore Clobbering errors + end + def validate_config cops.each do |cop| cop.validate_config if cop.respond_to?(:validate_config) diff --git a/lib/rubocop/rspec/cop_helper.rb b/lib/rubocop/rspec/cop_helper.rb index 3e898b5d289..fd357554cff 100644 --- a/lib/rubocop/rspec/cop_helper.rb +++ b/lib/rubocop/rspec/cop_helper.rb @@ -43,14 +43,14 @@ def autocorrect_source(source, file = nil) processed_source = parse_source(source, file) _investigate(cop, processed_source) - corrector = - RuboCop::Cop::Corrector.new(processed_source.buffer, cop.corrections) - corrector.rewrite + @last_corrector.rewrite end def _investigate(cop, processed_source) team = RuboCop::Cop::Team.new([cop], nil, raise_error: true) - team.inspect_file(processed_source) + report = team.investigate(processed_source) + @last_corrector = report.correctors.first || RuboCop::Cop::Corrector.new(processed_source) + report.offenses end end diff --git a/lib/rubocop/rspec/expect_offense.rb b/lib/rubocop/rspec/expect_offense.rb index 30fd29a7a72..ea9d3710966 100644 --- a/lib/rubocop/rspec/expect_offense.rb +++ b/lib/rubocop/rspec/expect_offense.rb @@ -100,7 +100,7 @@ def format_offense(source, **replacements) end # rubocop:disable Metrics/AbcSize, Metrics/MethodLength - def expect_offense(source, file = nil, **replacements) + def expect_offense(source, file = nil, severity: nil, **replacements) source = format_offense(source, **replacements) RuboCop::Formatter::DisabledConfigFormatter .config_to_allow_offenses = {} @@ -118,11 +118,12 @@ def expect_offense(source, file = nil, **replacements) raise 'Error parsing example code' unless @processed_source.valid_syntax? - _investigate(cop, @processed_source) + offenses = _investigate(cop, @processed_source) actual_annotations = - expected_annotations.with_offense_annotations(cop.offenses) + expected_annotations.with_offense_annotations(offenses) expect(actual_annotations.to_s).to eq(expected_annotations.to_s) + expect(offenses.map(&:severity).uniq).to eq([severity]) if severity end def expect_correction(correction, loop: true) @@ -132,12 +133,10 @@ def expect_correction(correction, loop: true) new_source = loop do iteration += 1 - corrector = - RuboCop::Cop::Corrector.new(@processed_source.buffer, cop.corrections) - corrected_source = corrector.rewrite + corrected_source = @last_corrector.rewrite break corrected_source unless loop - break corrected_source if cop.corrections.empty? + break corrected_source if @last_corrector.empty? break corrected_source if corrected_source == @processed_source.buffer.source if iteration > RuboCop::Runner::MAX_ITERATIONS @@ -145,9 +144,6 @@ def expect_correction(correction, loop: true) end # Prepare for next loop - cop.instance_variable_set(:@corrections, []) - # Cache invalidation. This is bad! - cop.instance_variable_set(:@token_table, nil) @processed_source = parse_source(corrected_source, @processed_source.path) _investigate(cop, @processed_source) @@ -160,24 +156,22 @@ def expect_correction(correction, loop: true) def expect_no_corrections raise '`expect_no_corrections` must follow `expect_offense`' unless @processed_source - return if cop.corrections.empty? + return if @last_corrector.empty? # In order to print a nice diff, e.g. what source got corrected to, # we need to run the actual corrections - corrector = - RuboCop::Cop::Corrector.new(@processed_source.buffer, cop.corrections) - new_source = corrector.rewrite + new_source = @last_corrector.rewrite expect(new_source).to eq(@processed_source.buffer.source) end def expect_no_offenses(source, file = nil) - inspect_source(source, file) + offenses = inspect_source(source, file) expected_annotations = AnnotatedSource.parse(source) actual_annotations = - expected_annotations.with_offense_annotations(cop.offenses) + expected_annotations.with_offense_annotations(offenses) expect(actual_annotations.to_s).to eq(source) end diff --git a/lib/rubocop/rspec/shared_contexts.rb b/lib/rubocop/rspec/shared_contexts.rb index d9f108606db..b92b054e730 100644 --- a/lib/rubocop/rspec/shared_contexts.rb +++ b/lib/rubocop/rspec/shared_contexts.rb @@ -45,11 +45,11 @@ let(:source) { 'code = {some: :ruby}' } let(:cop_class) do - if described_class.is_a?(Class) && described_class < RuboCop::Cop::Cop - described_class - else - RuboCop::Cop::Cop + unless described_class.is_a?(Class) && described_class < RuboCop::Cop::Base + raise 'Specify which cop class to use (e.g `let(:cop_class) { RuboCop::Cop::Base }`, ' \ + 'or RuboCop::Cop::Cop for legacy)' end + described_class end let(:cop_config) { {} } @@ -95,8 +95,9 @@ def source_range(range, buffer: source_buffer) end let(:cop) do - cop_class.new(config, cop_options) - .tap { |cop| cop.processed_source = processed_source } + cop_class.new(config, cop_options).tap do |cop| + cop.send :begin_investigation, processed_source + end end end diff --git a/lib/rubocop/runner.rb b/lib/rubocop/runner.rb index b2469163b18..ea5970014b3 100644 --- a/lib/rubocop/runner.rb +++ b/lib/rubocop/runner.rb @@ -120,7 +120,8 @@ def file_offenses(file) file_offense_cache(file) do source = get_processed_source(file) source, offenses = do_inspection_loop(file, source) - add_redundant_disables(file, offenses.compact.sort, source) + offenses = add_redundant_disables(file, offenses.compact.sort, source) + offenses.sort.reject(&:disabled?).freeze end end @@ -151,16 +152,30 @@ def file_offense_cache(file) end def add_redundant_disables(file, offenses, source) - if check_for_redundant_disables?(source) - redundant_cop_disable_directive(file) do |cop| - cop.check(offenses, source.disabled_line_ranges, source.comments) - offenses += cop.offenses - offenses += autocorrect_redundant_disables(file, source, cop, - offenses) + team_for_redundant_disables(file, offenses, source) do |team| + new_offenses, redundant_updated = inspect_file(source, team) + offenses += new_offenses + if redundant_updated + # Do one extra inspection loop if any redundant disables were + # removed. This is done in order to find rubocop:enable directives that + # have now become useless. + _source, new_offenses = do_inspection_loop(file, + get_processed_source(file)) + offenses |= new_offenses end end + offenses + end - offenses.sort.reject(&:disabled?).freeze + def team_for_redundant_disables(file, offenses, source) + return unless check_for_redundant_disables?(source) + + config = @config_store.for_file(file) + team = Cop::Team.mobilize([Cop::Lint::RedundantCopDisableDirective], config, @options) + return if team.cops.empty? + + team.cops.first.offenses_to_check = offenses + yield team end def check_for_redundant_disables?(source) @@ -180,22 +195,6 @@ def filtered_run? @options[:except] || @options[:only] end - def autocorrect_redundant_disables(file, source, cop, offenses) - cop.processed_source = source - - team = Cop::Team.mobilize(RuboCop::Cop::Registry.new, nil, @options) - team.autocorrect(source.buffer, [cop]) - - return [] unless team.updated_source_file? - - # Do one extra inspection loop if any redundant disables were - # removed. This is done in order to find rubocop:enable directives that - # have now become useless. - _source, new_offenses = do_inspection_loop(file, - get_processed_source(file)) - new_offenses - offenses - end - def file_started(file) puts "Scanning #{file}" if @options[:debug] formatter_set.file_started(file, @@ -293,13 +292,16 @@ def check_for_infinite_loop(processed_source, offenses) @processed_sources << checksum end - def inspect_file(processed_source) - config = @config_store.for_file(processed_source.path) - team = Cop::Team.mobilize(mobilized_cop_classes(config), config, @options) - offenses = team.inspect_file(processed_source) + def inspect_file(processed_source, team = mobilize_team(processed_source)) + report = team.investigate(processed_source) @errors.concat(team.errors) @warnings.concat(team.warnings) - [offenses, team.updated_source_file?] + [report.offenses, team.updated_source_file?] + end + + def mobilize_team(processed_source) + config = @config_store.for_file(processed_source.path) + Cop::Team.mobilize(mobilized_cop_classes(config), config, @options) end def mobilized_cop_classes(config) diff --git a/rubocop.gemspec b/rubocop.gemspec index f95fbe1c517..3fbd0074eab 100644 --- a/rubocop.gemspec +++ b/rubocop.gemspec @@ -34,7 +34,7 @@ Gem::Specification.new do |s| } s.add_runtime_dependency('parallel', '~> 1.10') - s.add_runtime_dependency('parser', '>= 2.7.0.1') + s.add_runtime_dependency('parser', '>= 2.7.1.1') s.add_runtime_dependency('rainbow', '>= 2.2.2', '< 4.0') s.add_runtime_dependency('regexp_parser', '>= 1.7') s.add_runtime_dependency('rexml') diff --git a/spec/rubocop/cli_spec.rb b/spec/rubocop/cli_spec.rb index 61c76aa5097..921b999b957 100644 --- a/spec/rubocop/cli_spec.rb +++ b/spec/rubocop/cli_spec.rb @@ -152,6 +152,7 @@ def and_with_args it 'registers an offense for a syntax error' do create_file('example.rb', ['class Test', 'en']) expect(cli.run(['--format', 'emacs', 'example.rb'])).to eq(1) + expect($stderr.string).to eq '' expect($stdout.string) .to eq(["#{abs('example.rb')}:3:1: E: Lint/Syntax: unexpected " \ 'token $end (Using Ruby 2.4 parser; configure using ' \ @@ -183,6 +184,7 @@ def and_with_args it 'can process a file with an invalid UTF-8 byte sequence' do create_file('example.rb', ["# #{'f9'.hex.chr}#{'29'.hex.chr}"]) expect(cli.run(['--format', 'emacs', 'example.rb'])).to eq(1) + expect($stderr.string).to eq '' expect($stdout.string) .to eq(<<~RESULT) #{abs('example.rb')}:1:1: F: Lint/Syntax: Invalid byte sequence in utf-8. diff --git a/spec/rubocop/cop/commissioner_spec.rb b/spec/rubocop/cop/commissioner_spec.rb index 2c89933b08e..3434c457bc2 100644 --- a/spec/rubocop/cop/commissioner_spec.rb +++ b/spec/rubocop/cop/commissioner_spec.rb @@ -2,11 +2,19 @@ RSpec.describe RuboCop::Cop::Commissioner do describe '#investigate' do - subject(:offenses) { commissioner.investigate(processed_source) } + subject(:offenses) do + report.offenses + end + let(:report) { commissioner.investigate(processed_source) } + let(:cop_offenses) { [] } + let(:cop_report) do + RuboCop::Cop::Base::InvestigationReport + .new(nil, processed_source, cop_offenses, nil) + end let(:cop) do # rubocop:disable RSpec/VerifiedDoubles - double(RuboCop::Cop::Cop, offenses: []).as_null_object + double(RuboCop::Cop::Base, complete_investigation: cop_report).as_null_object # rubocop:enable RSpec/VerifiedDoubles end let(:cops) { [cop] } @@ -21,10 +29,12 @@ def method RUBY let(:processed_source) { parse_source(source, 'file.rb') } - it 'returns all offenses found by the cops' do - allow(cop).to receive(:offenses).and_return([1]) + context 'when a cop reports offenses' do + let(:cop_offenses) { [Object.new] } - expect(offenses).to eq [1] + it 'returns all offenses found by the cops' do + expect(offenses).to eq cop_offenses + end end it 'traverses the AST and invoke cops specific callbacks' do @@ -62,11 +72,21 @@ def method it 'passes the input params to all cops/forces that implement their own' \ ' #investigate method' do - expect(cop).to receive(:investigate).with(processed_source) + expect(cop).to receive(:on_new_investigation).with(no_args) expect(force).to receive(:investigate).with(processed_source) offenses end end + + context 'when given a source with parsing errors' do + let(:source) { '(' } + + it 'only calls on_other_file' do + expect(cop).not_to receive(:on_new_investigation) + expect(cop).to receive(:on_other_file) + offenses + end + end end end diff --git a/spec/rubocop/cop/cop_spec.rb b/spec/rubocop/cop/cop_spec.rb index 58337e8b92e..e4be992865a 100644 --- a/spec/rubocop/cop/cop_spec.rb +++ b/spec/rubocop/cop/cop_spec.rb @@ -263,15 +263,13 @@ end describe '#autocorrect?' do - # dummy config for a generic cop instance - subject { cop.autocorrect? } let(:support_autocorrect) { true } let(:disable_uncorrectable) { false } before do - allow(cop).to receive(:support_autocorrect?) { support_autocorrect } + allow(cop.class).to receive(:support_autocorrect?) { support_autocorrect } allow(cop).to receive(:disable_uncorrectable?) { disable_uncorrectable } end diff --git a/spec/rubocop/cop/generator_spec.rb b/spec/rubocop/cop/generator_spec.rb index ebbb6887403..afa453373ff 100644 --- a/spec/rubocop/cop/generator_spec.rb +++ b/spec/rubocop/cop/generator_spec.rb @@ -58,7 +58,7 @@ module Style # # good # good_foo_method(args) # - class FakeCop < Cop + class FakeCop < Base # TODO: Implement the cop in here. # # In many cases, you can use a node matcher for matching node pattern. diff --git a/spec/rubocop/cop/layout/array_alignment_spec.rb b/spec/rubocop/cop/layout/array_alignment_spec.rb index a9b108d3d8d..e44d68f89a6 100644 --- a/spec/rubocop/cop/layout/array_alignment_spec.rb +++ b/spec/rubocop/cop/layout/array_alignment_spec.rb @@ -74,7 +74,7 @@ [:l4]]]] RUBY - expect_correction(<<~RUBY) + expect_correction(<<~RUBY, loop: false) [:l1, [:l2, [:l3, @@ -233,7 +233,7 @@ [:l4]]]] RUBY - expect_correction(<<~RUBY) + expect_correction(<<~RUBY, loop: false) [:l1, [:l2, [:l3, diff --git a/spec/rubocop/cop/layout/first_argument_indentation_spec.rb b/spec/rubocop/cop/layout/first_argument_indentation_spec.rb index b04347c4fb6..fc74777ecf3 100644 --- a/spec/rubocop/cop/layout/first_argument_indentation_spec.rb +++ b/spec/rubocop/cop/layout/first_argument_indentation_spec.rb @@ -64,7 +64,7 @@ RUBY # The first `)` Will be corrected by IndentationConsistency. - expect_correction(<<~RUBY) + expect_correction(<<~RUBY, loop: false) foo( bar( 7 @@ -481,7 +481,7 @@ RUBY # The first `)` Will be corrected by IndentationConsistency. - expect_correction(<<~RUBY) + expect_correction(<<~RUBY, loop: false) foo( bar( 7 diff --git a/spec/rubocop/cop/layout/indentation_consistency_spec.rb b/spec/rubocop/cop/layout/indentation_consistency_spec.rb index c4320c2445f..e0c3a0b0ec6 100644 --- a/spec/rubocop/cop/layout/indentation_consistency_spec.rb +++ b/spec/rubocop/cop/layout/indentation_consistency_spec.rb @@ -768,7 +768,7 @@ def priv RUBY # The offense on line 4 is corrected, affecting lines 4 to 11. - expect_correction(<<~RUBY) + expect_correction(<<~RUBY, loop: false) require 'spec_helper' describe ArticlesController do render_views diff --git a/spec/rubocop/cop/lint/redundant_cop_disable_directive_spec.rb b/spec/rubocop/cop/lint/redundant_cop_disable_directive_spec.rb index a8c4f4b28bc..53652a7f016 100644 --- a/spec/rubocop/cop/lint/redundant_cop_disable_directive_spec.rb +++ b/spec/rubocop/cop/lint/redundant_cop_disable_directive_spec.rb @@ -1,114 +1,62 @@ # frozen_string_literal: true +require 'rubocop/cop/legacy/corrector' + RSpec.describe RuboCop::Cop::Lint::RedundantCopDisableDirective, :config do describe '.check' do - let(:cop_options) { { auto_correct: true } } - let(:comments) { processed_source.comments } - let(:corrected_source) do - RuboCop::Cop::Corrector - .new(processed_source.buffer, cop.corrections) - .rewrite - end + subject(:resulting_offenses) { cop.send(:complete_investigation).offenses } - before do - $stderr = StringIO.new # rubocop:disable RSpec/ExpectOutput - cop.check(offenses, cop_disabled_line_ranges, comments) - end + let(:offenses) { [] } + let(:cop) { cop_class.new(config, cop_options, offenses) } + + before { $stderr = StringIO.new } # rubocop:disable RSpec/ExpectOutput context 'when there are no disabled lines' do - let(:offenses) { [] } - let(:cop_disabled_line_ranges) { {} } let(:source) { '' } - it 'returns an empty array' do - expect(cop.offenses).to eq([]) + it 'returns no offense' do + expect_no_offenses(source) end end context 'when there are disabled lines' do context 'and there are no offenses' do - let(:offenses) { [] } - context 'and a comment disables' do context 'one cop' do - let(:source) { "# rubocop:disable Metrics/MethodLength\n" } - let(:cop_disabled_line_ranges) do - { 'Metrics/MethodLength' => [1..Float::INFINITY] } - end - it 'returns an offense' do - expect(cop.messages) - .to eq(['Unnecessary disabling of `Metrics/MethodLength`.']) - expect(cop.highlights) - .to eq(['# rubocop:disable Metrics/MethodLength']) - end - - it 'gives the right cop name' do - expect(cop.name).to eq('Lint/RedundantCopDisableDirective') - end - - it 'autocorrects' do - expect(corrected_source).to eq('') + expect_offense(<<~RUBY) + # rubocop:disable Metrics/MethodLength + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unnecessary disabling of `Metrics/MethodLength`. + RUBY + expect_correction('') end end context 'an unknown cop' do - let(:source) { '# rubocop:disable UnknownCop' } - let(:cop_disabled_line_ranges) do - { 'UnknownCop' => [1..Float::INFINITY] } - end - it 'returns an offense' do - expect(cop.messages) - .to eq(['Unnecessary disabling of `UnknownCop` (unknown cop).']) - expect(cop.highlights) - .to eq(['# rubocop:disable UnknownCop']) - end - end - - context 'itself' do - let(:source) do - '# rubocop:disable Lint/RedundantCopDisableDirective' - end - let(:cop_disabled_line_ranges) do - { 'Lint/RedundantCopDisableDirective' => [1..Float::INFINITY] } - end - - it 'does not return an offense' do - expect(cop.offenses.empty?).to be(true) + expect_offense(<<~RUBY) + # rubocop:disable UnknownCop + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unnecessary disabling of `UnknownCop` (unknown cop). + RUBY + expect_correction('') end end context 'itself and another cop' do context 'disabled on the same range' do - let(:source) do - '# rubocop:disable Lint/RedundantCopDisableDirective, ' \ - 'Metrics/ClassLength' - end - - let(:cop_disabled_line_ranges) do - { 'Lint/RedundantCopDisableDirective' => [1..Float::INFINITY], - 'Metrics/ClassLength' => [1..Float::INFINITY] } - end - - it 'does not return an offense' do - expect(cop.offenses.empty?).to be(true) + it 'returns no offense' do + expect_no_offenses(<<~RUBY) + # rubocop:disable Lint/RedundantCopDisableDirective, Metrics/ClassLength + RUBY end end context 'disabled on different ranges' do - let(:source) do - ['# rubocop:disable Lint/RedundantCopDisableDirective', - '# rubocop:disable Metrics/ClassLength'].join("\n") - end - - let(:cop_disabled_line_ranges) do - { 'Lint/RedundantCopDisableDirective' => [1..Float::INFINITY], - 'Metrics/ClassLength' => [2..Float::INFINITY] } - end - - it 'does not return an offense' do - expect(cop.offenses.empty?).to be(true) + it 'returns no offense' do + expect_no_offenses(<<~RUBY) + # rubocop:disable Lint/RedundantCopDisableDirective + # rubocop:disable Metrics/ClassLength + RUBY end end @@ -119,44 +67,23 @@ '# rubocop:disable Metrics/ClassLength'].join("\n") end - let(:cop_disabled_line_ranges) do - { 'Lint/RedundantCopDisableDirective' => [1..Float::INFINITY], - 'Metrics/ClassLength' => [(2..3), (3..Float::INFINITY)] } - end - - it 'does not return an offense' do - expect(cop.offenses.empty?).to be(true) + it 'returns no offense' do + expect_no_offenses(source) end end end context 'multiple cops' do - let(:source) do - '# rubocop:disable Metrics/MethodLength, Metrics/ClassLength' - end - let(:cop_disabled_line_ranges) do - { 'Metrics/ClassLength' => [1..Float::INFINITY], - 'Metrics/MethodLength' => [1..Float::INFINITY] } - end - it 'returns an offense' do - expect(cop.messages) - .to eq(['Unnecessary disabling of `Metrics/ClassLength`, ' \ - '`Metrics/MethodLength`.']) + expect_offense(<<~RUBY.gsub("<\n", '')) # Wrap lines & avoid issue with JRuby + # rubocop:disable Metrics/MethodLength, Metrics/ClassLength + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ < + Unnecessary disabling of `Metrics/ClassLength`, `Metrics/MethodLength`. + RUBY end end context 'multiple cops, and one of them has offenses' do - let(:source) do - '# rubocop:disable Metrics/MethodLength, Metrics/ClassLength, ' \ - 'Lint/Debugger, Lint/AmbiguousOperator' - end - let(:cop_disabled_line_ranges) do - { 'Metrics/ClassLength' => [1..Float::INFINITY], - 'Metrics/MethodLength' => [1..Float::INFINITY], - 'Lint/Debugger' => [1..Float::INFINITY], - 'Lint/AmbiguousOperator' => [1..Float::INFINITY] } - end let(:offenses) do [ RuboCop::Cop::Offense.new(:convention, @@ -167,30 +94,21 @@ end it 'returns an offense' do - expect(cop.messages) - .to eq(['Unnecessary disabling of `Metrics/MethodLength`.', - 'Unnecessary disabling of `Lint/Debugger`.', - 'Unnecessary disabling of `Lint/AmbiguousOperator`.']) - expect(cop.highlights).to eq(['Metrics/MethodLength', - 'Lint/Debugger', - 'Lint/AmbiguousOperator']) - end - - it 'autocorrects' do - expect(corrected_source).to eq( - '# rubocop:disable Metrics/ClassLength' - ) + expect_offense(<<~RUBY.gsub("<\n", '')) # Wrap lines & avoid issue with JRuby + # rubocop:disable Metrics/MethodLength, Metrics/ClassLength, Lint/Debugger, < + Lint/AmbiguousOperator + ^^^^^^^^^^^^^^^^^^^^ Unnecessary disabling of `Metrics/MethodLength`. + ^^^^^^^^^^^^^ Unnecessary disabling of `Lint/Debugger`. + < + ^^^^^^^^^^^^^^^^^^^^^^ Unnecessary disabling of `Lint/AmbiguousOperator`. + RUBY + expect_correction(<<~RUBY) + # rubocop:disable Metrics/ClassLength + RUBY end end context 'multiple cops, and the leftmost one has no offenses' do - let(:source) do - '# rubocop:disable Metrics/ClassLength, Metrics/MethodLength' - end - let(:cop_disabled_line_ranges) do - { 'Metrics/ClassLength' => [1..Float::INFINITY], - 'Metrics/MethodLength' => [1..Float::INFINITY] } - end let(:offenses) do [ RuboCop::Cop::Offense.new(:convention, @@ -201,46 +119,41 @@ end it 'returns an offense' do - expect(cop.messages) - .to eq(['Unnecessary disabling of `Metrics/ClassLength`.']) - expect(cop.highlights).to eq(['Metrics/ClassLength']) - end - - it 'autocorrects' do - expect(corrected_source).to eq( - '# rubocop:disable Metrics/MethodLength' - ) + expect_offense(<<~RUBY) + # rubocop:disable Metrics/ClassLength, Metrics/MethodLength + ^^^^^^^^^^^^^^^^^^^ Unnecessary disabling of `Metrics/ClassLength`. + RUBY + expect_correction(<<~RUBY) + # rubocop:disable Metrics/MethodLength + RUBY end end context 'multiple cops, with abbreviated names' do context 'one of them has offenses' do - let(:source) do - '# rubocop:disable MethodLength, ClassLength, Debugger' - end - let(:cop_disabled_line_ranges) do - { 'Metrics/ClassLength' => [1..Float::INFINITY], - 'Metrics/MethodLength' => [1..Float::INFINITY], - 'Lint/Debugger' => [1..Float::INFINITY] } - end let(:offenses) do [ RuboCop::Cop::Offense.new(:convention, - OpenStruct.new(line: 7, column: 0), + OpenStruct.new(line: 4, column: 0), 'Method has too many lines.', 'Metrics/MethodLength') ] end - it 'returns an offense and warns about missing departments' do - expect(cop.messages) - .to eq(['Unnecessary disabling of `Metrics/ClassLength`.', - 'Unnecessary disabling of `Lint/Debugger`.']) - expect(cop.highlights).to eq(%w[ClassLength Debugger]) + it 'returns an offense' do + expect_offense(<<~RUBY) + puts 1 + # rubocop:disable MethodLength, ClassLength, Debugger + ^^^^^^^^^^^ Unnecessary disabling of `Metrics/ClassLength`. + ^^^^^^^^ Unnecessary disabling of `Lint/Debugger`. + # + # offense here + RUBY + expect($stderr.string).to eq(<<~OUTPUT) - test: Warning: no department given for MethodLength. - test: Warning: no department given for ClassLength. - test: Warning: no department given for Debugger. + (string): Warning: no department given for MethodLength. + (string): Warning: no department given for ClassLength. + (string): Warning: no department given for Debugger. OUTPUT end end @@ -248,64 +161,46 @@ context 'comment is not at the beginning of the file' do context 'and not all cops have offenses' do - let(:source) do - <<~RUBY - puts 1 - # rubocop:disable Metrics/MethodLength, Metrics/ClassLength - RUBY - end - let(:cop_disabled_line_ranges) do - { 'Metrics/ClassLength' => [2..Float::INFINITY], - 'Metrics/MethodLength' => [2..Float::INFINITY] } - end let(:offenses) do [ RuboCop::Cop::Offense.new(:convention, - OpenStruct.new(line: 7, column: 0), + OpenStruct.new(line: 4, column: 0), 'Method has too many lines.', 'Metrics/MethodLength') ] end - it 'registers an offense' do - expect(cop.messages).to eq( - ['Unnecessary disabling of `Metrics/ClassLength`.'] - ) - expect(cop.highlights).to eq(['Metrics/ClassLength']) + it 'returns an offense' do + expect_offense(<<~RUBY) + puts 1 + # rubocop:disable Metrics/MethodLength, Metrics/ClassLength + ^^^^^^^^^^^^^^^^^^^ Unnecessary disabling of `Metrics/ClassLength`. + # + # offense here + RUBY end end end context 'misspelled cops' do - let(:source) do - '# rubocop:disable Metrics/MethodLenght, KlassLength' - end - let(:cop_disabled_line_ranges) do - { 'KlassLength' => [1..Float::INFINITY], - 'Metrics/MethodLenght' => [1..Float::INFINITY] } - end - it 'returns an offense' do - expect(cop.messages) - .to eq(['Unnecessary disabling of `KlassLength` (unknown ' \ + message = 'Unnecessary disabling of `KlassLength` (unknown ' \ 'cop), `Metrics/MethodLenght` (did you mean ' \ - '`Metrics/MethodLength`?).']) + '`Metrics/MethodLength`?).' + + expect_offense(<<~RUBY) + # rubocop:disable Metrics/MethodLenght, KlassLength + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{message} + RUBY end end context 'all cops' do - let(:source) { '# rubocop : disable all' } - let(:cop_disabled_line_ranges) do - { - 'Metrics/MethodLength' => [1..Float::INFINITY], - 'Metrics/ClassLength' => [1..Float::INFINITY] - # etc... (no need to include all cops here) - } - end - it 'returns an offense' do - expect(cop.messages).to eq(['Unnecessary disabling of all cops.']) - expect(cop.highlights).to eq([source]) + expect_offense(<<~RUBY) + # rubocop : disable all + ^^^^^^^^^^^^^^^^^^^^^^^ Unnecessary disabling of all cops. + RUBY end end @@ -316,13 +211,8 @@ '# rubocop:disable all'].join("\n") end - let(:cop_disabled_line_ranges) do - { 'Lint/RedundantCopDisableDirective' => [1..Float::INFINITY], - 'all' => [2..Float::INFINITY] } - end - - it 'does not return an offense' do - expect(cop.offenses.empty?).to be(true) + it 'returns no offense' do + expect_no_offenses(source) end end end @@ -345,53 +235,39 @@ context 'and a comment disables' do context 'one cop twice' do - let(:source) do - <<~RUBY + let(:offense_lines) { [3, 8] } + + it 'returns an offense' do + expect_offense(<<~RUBY) class One # rubocop:disable Style/ClassVars - @@class_var = 1 + @@class_var = 1 # offense here end class Two # rubocop:disable Style/ClassVars - @@class_var = 2 + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unnecessary disabling of `Style/ClassVars`. + @@class_var = 2 # offense and here + # rubocop:enable Style/ClassVars end RUBY end - let(:offense_lines) { [3, 8] } - let(:cop_disabled_line_ranges) do - { 'Style/ClassVars' => [2..7, 7..9] } - end - - it 'returns an offense' do - expect(cop.messages) - .to eq(['Unnecessary disabling of `Style/ClassVars`.']) - expect(cop.highlights) - .to eq(['# rubocop:disable Style/ClassVars']) - end end context 'one cop and then all cops' do - let(:source) do - <<~RUBY + let(:offense_lines) { [4] } + + it 'returns an offense' do + expect_offense(<<~RUBY) class One # rubocop:disable Style/ClassVars + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unnecessary disabling of `Style/ClassVars`. # rubocop:disable all @@class_var = 1 + # offense here end RUBY end - let(:offense_lines) { [4] } - let(:cop_disabled_line_ranges) do - { 'Style/ClassVars' => [2..3, 3..Float::INFINITY] } - end - - it 'returns an offense' do - expect(cop.messages) - .to eq(['Unnecessary disabling of `Style/ClassVars`.']) - expect(cop.highlights) - .to eq(['# rubocop:disable Style/ClassVars']) - end end end end @@ -400,7 +276,7 @@ class One let(:offenses) do [ RuboCop::Cop::Offense.new(:convention, - OpenStruct.new(line: 7, column: 0), + OpenStruct.new(line: 3, column: 0), 'Tab detected.', 'Layout/IndentationStyle') ] @@ -409,44 +285,32 @@ class One context 'and a comment disables' do context 'that cop' do let(:source) { '# rubocop:disable Layout/IndentationStyle' } - let(:cop_disabled_line_ranges) do - { 'Layout/IndentationStyle' => [1..100] } - end - it 'returns an empty array' do - expect(cop.offenses.empty?).to be(true) + it 'returns no offense' do + expect_no_offenses(source) end end context 'that cop but on other lines' do - let(:source) do - ("\n" * 9) << '# rubocop:disable Layout/IndentationStyle' - end - let(:cop_disabled_line_ranges) do - { 'Layout/IndentationStyle' => [10..12] } - end - it 'returns an offense' do - expect(cop.messages) - .to eq(['Unnecessary disabling of `Layout/IndentationStyle`.']) - expect(cop.highlights).to eq( - ['# rubocop:disable Layout/IndentationStyle'] - ) + expect_offense(<<~RUBY) + # 1 + # 2 + # 3, offense here + # 4 + # rubocop:disable Layout/IndentationStyle + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Unnecessary disabling of `Layout/IndentationStyle`. + # + # rubocop:enable Layout/IndentationStyle + RUBY end end context 'all cops' do let(:source) { '# rubocop : disable all' } - let(:cop_disabled_line_ranges) do - { - 'Metrics/MethodLength' => [1..Float::INFINITY], - 'Metrics/ClassLength' => [1..Float::INFINITY] - # etc... (no need to include all cops here) - } - end - it 'returns an empty array' do - expect(cop.offenses.empty?).to be(true) + it 'returns no offense' do + expect_no_offenses(source) end end end diff --git a/spec/rubocop/cop/lint/syntax_spec.rb b/spec/rubocop/cop/lint/syntax_spec.rb index 785fa5ebc82..23dd515c9e2 100644 --- a/spec/rubocop/cop/lint/syntax_spec.rb +++ b/spec/rubocop/cop/lint/syntax_spec.rb @@ -1,18 +1,9 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::Lint::Syntax do - let(:options) { nil } - let(:ruby_version) { 2.4 } - let(:path) { 'test.rb' } - let(:processed_source) do - RuboCop::ProcessedSource.new(source, ruby_version, path) - end - +RSpec.describe RuboCop::Cop::Lint::Syntax, :config do describe '.offenses_from_processed_source' do - let(:offenses) do - described_class.offenses_from_processed_source(processed_source, - nil, options) - end + let(:commissioner) { RuboCop::Cop::Commissioner.new([cop]) } + let(:offenses) { commissioner.investigate(processed_source).offenses } context 'with a diagnostic error' do let(:source) { '(' } @@ -29,7 +20,7 @@ end context 'with --display-cop-names option' do - let(:options) { { display_cop_names: true } } + let(:cop_options) { { display_cop_names: true } } it 'returns an offense with cop name' do expect(offenses.size).to eq(1) @@ -44,7 +35,7 @@ end context 'with --auto-correct --disable-uncorrectable options' do - let(:options) { { auto_correct: true, disable_uncorrectable: true } } + let(:cop_options) { { auto_correct: true, disable_uncorrectable: true } } it 'returns an offense' do expect(offenses.size).to eq(1) @@ -69,11 +60,11 @@ offense = offenses.first expect(offense.message).to eq('Invalid byte sequence in utf-8.') expect(offense.severity).to eq(:fatal) - expect(offense.location).to eq(described_class::ERROR_SOURCE_RANGE) + expect(offense.location).to eq(RuboCop::Cop::Offense::NO_LOCATION) end context 'with --display-cop-names option' do - let(:options) { { display_cop_names: true } } + let(:cop_options) { { display_cop_names: true } } it 'returns an offense with cop name' do expect(offenses.size).to eq(1) diff --git a/spec/rubocop/cop/team_spec.rb b/spec/rubocop/cop/team_spec.rb index f6bd13c7aaf..1fcb841a3a6 100644 --- a/spec/rubocop/cop/team_spec.rb +++ b/spec/rubocop/cop/team_spec.rb @@ -5,7 +5,7 @@ let(:cop_classes) { RuboCop::Cop::Cop.registry } let(:config) { RuboCop::ConfigLoader.default_configuration } - let(:options) { nil } + let(:options) { {} } let(:ruby_version) { RuboCop::TargetRuby.supported_versions.last } before do @@ -91,8 +91,8 @@ def a include FileHelper let(:file_path) { '/tmp/example.rb' } + let(:source) { RuboCop::ProcessedSource.from_file(file_path, ruby_version) } let(:offenses) do - source = RuboCop::ProcessedSource.from_file(file_path, ruby_version) team.inspect_file(source) end @@ -206,10 +206,29 @@ def a it 'records Team#errors' do source = RuboCop::ProcessedSource.from_file(file_path, ruby_version) - expect { team.inspect_file(source) }.to raise_error(cause) + team.inspect_file(source) expect($stderr.string).to include(error_message) end end + + context 'when done twice' do + let(:persisting_cop_class) do + klass = Class.new(RuboCop::Cop::Base) + klass.exclude_from_registry + klass.define_singleton_method(:support_multiple_source?) { true } + stub_const('Test::Persisting', klass) + klass + end + let(:cop_classes) { [persisting_cop_class, RuboCop::Cop::Base] } + + it 'allows cops to get ready' do + before = team.cops.dup + team.inspect_file(source) + team.inspect_file(source) + expect(team.cops).to match_array([be(before.first), be_a(RuboCop::Cop::Base)]) + expect(team.cops.last).not_to be(before.last) + end + end end describe '#cops' do @@ -217,7 +236,7 @@ def a it 'returns cop instances' do expect(cops.empty?).to be(false) - expect(cops.all? { |c| c.is_a?(RuboCop::Cop::Cop) }).to be_truthy + expect(cops.all? { |c| c.is_a?(RuboCop::Cop::Base) }).to be_truthy end context 'when only some cop classes are passed to .new' do diff --git a/spec/rubocop/formatter/clang_style_formatter_spec.rb b/spec/rubocop/formatter/clang_style_formatter_spec.rb index c99ddadec34..d3eaca56013 100644 --- a/spec/rubocop/formatter/clang_style_formatter_spec.rb +++ b/spec/rubocop/formatter/clang_style_formatter_spec.rb @@ -3,6 +3,7 @@ RSpec.describe RuboCop::Formatter::ClangStyleFormatter, :config do subject(:formatter) { described_class.new(output) } + let(:cop_class) { RuboCop::Cop::Cop } let(:output) { StringIO.new } describe '#report_file' do diff --git a/spec/rubocop/formatter/emacs_style_formatter_spec.rb b/spec/rubocop/formatter/emacs_style_formatter_spec.rb index ad07752566a..79a2c92f42d 100644 --- a/spec/rubocop/formatter/emacs_style_formatter_spec.rb +++ b/spec/rubocop/formatter/emacs_style_formatter_spec.rb @@ -3,6 +3,7 @@ RSpec.describe RuboCop::Formatter::EmacsStyleFormatter, :config do subject(:formatter) { described_class.new(output) } + let(:cop_class) { RuboCop::Cop::Cop } let(:source) { %w[a b cdefghi].join("\n") } let(:output) { StringIO.new } diff --git a/spec/rubocop/formatter/file_list_formatter_spec.rb b/spec/rubocop/formatter/file_list_formatter_spec.rb index 88c52153d98..47299378068 100644 --- a/spec/rubocop/formatter/file_list_formatter_spec.rb +++ b/spec/rubocop/formatter/file_list_formatter_spec.rb @@ -4,7 +4,7 @@ subject(:formatter) { described_class.new(output) } let(:output) { StringIO.new } - + let(:cop_class) { RuboCop::Cop::Cop } let(:source) { %w[a b cdefghi].join("\n") } describe '#file_finished' do diff --git a/tasks/cops_documentation.rake b/tasks/cops_documentation.rake index 75a2d80a8fd..9d49f1c9493 100644 --- a/tasks/cops_documentation.rake +++ b/tasks/cops_documentation.rake @@ -47,7 +47,7 @@ task generate_cops_documentation: :yard_for_generate_documentation do 'Enabled by default', 'Safe', 'Supports autocorrection', 'VersionAdded', 'VersionChanged' ] - autocorrect = if cop_instance.support_autocorrect? + autocorrect = if cop_instance.class.support_autocorrect? "Yes#{' (Unsafe)' unless cop_instance.safe_autocorrect?}" else 'No'