diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index f4b92806503..29adbb78c7f 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -35,5 +35,5 @@ Include the output of `rubocop -V` or `bundle exec rubocop -V` if using Bundler. ``` $ [bundle exec] rubocop -V -0.87.0 (using Parser 2.5.1.2, running on ruby 2.5.1 x86_64-linux) +0.87.1 (using Parser 2.5.1.2, running on ruby 2.5.1 x86_64-linux) ``` diff --git a/.gitignore b/.gitignore index 35e381ad50c..b90a35666a1 100644 --- a/.gitignore +++ b/.gitignore @@ -65,3 +65,7 @@ TAGS # For bundle binstubs bin/* +!bin/rubocop-profile + +# For stackprof or others +tmp/* diff --git a/CHANGELOG.md b/CHANGELOG.md index 54ae4962dad..6e1794a980c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,38 @@ ### New features * [#8279](https://github.com/rubocop-hq/rubocop/pull/8279): Recognise require method passed as argument in `Lint/NonDeterministicRequireOrder` cop. ([@biinari][]) +* [#7333](https://github.com/rubocop-hq/rubocop/issues/7333): Add new `Style/RedundantFileExtensionInRequire` cop. ([@fatkodima][]) +* [#8316](https://github.com/rubocop-hq/rubocop/pull/8316): Support autocorrect for `Lint/DisjunctiveAssignmentInConstructor` cop. ([@fatkodima][]) +* [#8242](https://github.com/rubocop-hq/rubocop/pull/8242): Internal profiling available with `bin/rubocop-profile` and rake tasks. ([@marcandre][]) +* [#8295](https://github.com/rubocop-hq/rubocop/pull/8295): Add new `Style/ArrayCoercion` cop. ([@fatkodima][]) +* [#8293](https://github.com/rubocop-hq/rubocop/pull/8293): Add new `Lint/DuplicateElsifCondition` cop. ([@fatkodima][]) +* [#7736](https://github.com/rubocop-hq/rubocop/issues/7736): Add new `Style/CaseLikeIf` cop. ([@fatkodima][]) +* [#4286](https://github.com/rubocop-hq/rubocop/issues/4286): Add new `Style/HashAsLastArrayItem` cop. ([@fatkodima][]) +* [#8247](https://github.com/rubocop-hq/rubocop/issues/8247): Add new `Style/HashLikeCase` cop. ([@fatkodima][]) +* [#8286](https://github.com/rubocop-hq/rubocop/issues/8286): Internal method `expect_offense` allows abbreviated offense messages. ([@marcandre][]) ### Bug fixes +* [#8232](https://github.com/rubocop-hq/rubocop/issues/8232): Fix a false positive for `Layout/EmptyLinesAroundAccessModifier` when `end` immediately after access modifier. ([@koic][]) +* [#7777](https://github.com/rubocop-hq/rubocop/issues/7777): Fix crash for `Layout/MultilineArrayBraceLayout` when comment is present after last element. ([@shekhar-patil][]) +* [#7776](https://github.com/rubocop-hq/rubocop/issues/7776): Fix crash for `Layout/MultilineMethodCallBraceLayout` when comment is present before closing braces. ([@shekhar-patil][]) +* [#8282](https://github.com/rubocop-hq/rubocop/issues/8282): Fix `Style/IfUnlessModifier` bad precedence detection. ([@tejasbubane][]) +* [#8289](https://github.com/rubocop-hq/rubocop/issues/8289): Fix `Style/AccessorGrouping` to not register offense for accessor with comment. ([@tejasbubane][]) +* [#8310](https://github.com/rubocop-hq/rubocop/pull/8310): Handle major version requirements in `Gemspec/RequiredRubyVersion`. ([@eugeneius][]) +* [#8315](https://github.com/rubocop-hq/rubocop/pull/8315): Fix crash for `Style/PercentLiteralDelimiters` when the source contains invalid characters. ([@eugeneius][]) + +### Changes + +* [#8021](https://github.com/rubocop-hq/rubocop/issues/8021): Rewrite `Layout/SpaceAroundMethodCallOperator` cop to make it faster. ([@fatkodima][]) +* [#8294](https://github.com/rubocop-hq/rubocop/pull/8294): Add `of` to `AllowedNames` of `MethodParameterName` cop. ([@AlexWayfer][]) + +## 0.87.1 (2020-07-07) + +### Bug fixes + +* [#8189](https://github.com/rubocop-hq/rubocop/issues/8189): Fix an error for `Layout/MultilineBlockLayout` where spaces for a new line where not considered. ([@knejad][]) * [#8252](https://github.com/rubocop-hq/rubocop/issues/8252): Fix a command line option name from `--safe-autocorrect` to `--safe-auto-correct`, which is compatible with RuboCop 0.86 and lower. ([@koic][]) +* [#8259](https://github.com/rubocop-hq/rubocop/issues/8259): Fix false positives for `Style/BisectedAttrAccessor` when accessors have different access modifiers. ([@fatkodima][]) * [#8253](https://github.com/rubocop-hq/rubocop/issues/8253): Fix false positives for `Style/AccessorGrouping` when accessors have different access modifiers. ([@fatkodima][]) * [#8257](https://github.com/rubocop-hq/rubocop/issues/8257): Fix an error for `Style/BisectedAttrAccessor` when using `attr_reader` and `attr_writer` with splat arguments. ([@fatkodima][]) * [#8239](https://github.com/rubocop-hq/rubocop/pull/8239): Don't load `.rubocop.yml` from personal folders to check for exclusions if given a custom configuration file. ([@deivid-rodriguez][]) @@ -4657,3 +4685,5 @@ [@fatkodima]: https://github.com/fatkodima [@karlwithak]: https://github.com/karlwithak [@CamilleDrapier]: https://github.com/CamilleDrapier +[@shekhar-patil]: https://github.com/shekhar-patil +[@knejad]: https://github.com/knejad diff --git a/Gemfile b/Gemfile index c5fde9f4d8f..7cd9ed643ad 100644 --- a/Gemfile +++ b/Gemfile @@ -5,6 +5,7 @@ source 'https://rubygems.org' gemspec gem 'bump', require: false +gem 'memory_profiler', platform: :mri gem 'pry' gem 'rake', '~> 13.0' gem 'rspec', '~> 3.7' @@ -14,6 +15,7 @@ gem 'rubocop-rspec', '~> 1.39.0' # Stop upgrading SimpleCov until the following issue will be resolved. # https://github.com/codeclimate/test-reporter/issues/418 gem 'simplecov', '~> 0.10', '< 0.18' +gem 'stackprof', platform: :mri gem 'test-queue' gem 'yard', '~> 0.9' diff --git a/README.md b/README.md index 03aa1d79a72..1a14e50bdff 100644 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ haven't reached version 1.0 yet). To prevent an unwanted RuboCop update you might want to use a conservative version lock in your `Gemfile`: ```rb -gem 'rubocop', '~> 0.87.0', require: false +gem 'rubocop', '~> 0.87.1', require: false ``` ## Quickstart diff --git a/bin/rubocop-profile b/bin/rubocop-profile new file mode 100755 index 00000000000..1743f7a009a --- /dev/null +++ b/bin/rubocop-profile @@ -0,0 +1,31 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +if ARGV.include?('-h') || ARGV.include?('--help') + puts "Usage: same as main `rubocop` command but gathers profiling info" + puts "Additional option: `--memory` to print memory usage" + exit(0) +end +with_mem = ARGV.delete('--memory') + +require 'stackprof' +if with_mem + require 'memory_profiler' + MemoryProfiler.start +end +StackProf.start +start = Process.clock_gettime(Process::CLOCK_MONOTONIC) +begin + load "#{__dir__}/../exe/rubocop" +ensure + delta = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start + puts "Finished in #{delta.round(1)} seconds" + StackProf.stop + if with_mem + puts "Building memory report..." + report = MemoryProfiler.stop + end + Dir.mkdir('tmp') unless File.exist?('tmp') + StackProf.results('tmp/stackprof.dump') + report&.pretty_print(scale_bytes: true) +end diff --git a/config/default.yml b/config/default.yml index c08228f3652..6d6904557ce 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1397,12 +1397,18 @@ Lint/DisjunctiveAssignmentInConstructor: Enabled: true Safe: false VersionAdded: '0.62' + VersionChanged: '0.88' Lint/DuplicateCaseCondition: Description: 'Do not repeat values in case conditionals.' Enabled: true VersionAdded: '0.45' +Lint/DuplicateElsifCondition: + Description: 'Do not repeat conditions used in if `elsif`.' + Enabled: 'pending' + VersionAdded: '0.88' + Lint/DuplicateHashKey: Description: 'Check for duplicate keys in hash literals.' Enabled: true @@ -2179,17 +2185,18 @@ Naming/MethodParameterName: AllowNamesEndingInNumbers: true # Allowed names that will not register an offense AllowedNames: - - io - - id - - to + - at - by - - 'on' + - db + - id - in - - at + - io - ip - - db + - of + - 'on' - os - pp + - to # Forbidden names that will register an offense ForbiddenNames: [] @@ -2340,6 +2347,14 @@ Style/AndOr: - always - conditionals +Style/ArrayCoercion: + Description: >- + Use Array() instead of explicit Array check or [*var], when dealing + with a variable you want to treat as an Array, but you're not certain it's an array. + StyleGuide: '#array-coercion' + Enabled: 'pending' + VersionAdded: '0.88' + Style/ArrayJoin: Description: 'Use Array#join instead of Array#*.' StyleGuide: '#array-join' @@ -2522,6 +2537,12 @@ Style/CaseEquality: # String === "string" AllowOnConstant: false +Style/CaseLikeIf: + Description: 'This cop identifies places where `if-elsif` constructions can be replaced with `case-when`.' + StyleGuide: '#case-vs-if-else' + Enabled: 'pending' + VersionAdded: '0.88' + Style/CharacterLiteral: Description: 'Checks for uses of character literals.' StyleGuide: '#no-character-literals' @@ -2961,6 +2982,18 @@ Style/GuardClause: # needs to have to trigger this cop MinBodyLength: 1 +Style/HashAsLastArrayItem: + Description: >- + Checks for presence or absence of braces around hash literal as a last + array item depending on configuration. + StyleGuide: '#hash-literal-as-last-array-item' + Enabled: 'pending' + VersionAdded: '0.88' + EnforcedStyle: braces + SupportedStyles: + - braces + - no_braces + Style/HashEachMethods: Description: 'Use Hash#each_key and Hash#each_value.' StyleGuide: '#hash-each' @@ -2968,6 +3001,16 @@ Style/HashEachMethods: VersionAdded: '0.80' Safe: false +Style/HashLikeCase: + Description: >- + Checks for places where `case-when` represents a simple 1:1 + mapping and can be replaced with a hash lookup. + Enabled: 'pending' + VersionAdded: '0.88' + # `MinBranchesCount` defines the number of branches `case` needs to have + # to trigger this cop + MinBranchesCount: 3 + Style/HashSyntax: Description: >- Prefer Ruby 1.9 hash syntax { a: 1, b: 2 } over 1.8 syntax @@ -3665,6 +3708,14 @@ Style/RedundantFetchBlock: SafeForConstants: false VersionAdded: '0.86' +Style/RedundantFileExtensionInRequire: + Description: >- + Checks for the presence of superfluous `.rb` extension in + the filename provided to `require` and `require_relative`. + StyleGuide: '#no-explicit-rb-to-require' + Enabled: 'pending' + VersionAdded: '0.88' + Style/RedundantFreeze: Description: "Checks usages of Object#freeze on immutable objects." Enabled: true diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 08a7310745d..17a9797ca3f 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -193,6 +193,7 @@ In the following section you find all available cops: * xref:cops_lint.adoc#lintdeprecatedopensslconstant[Lint/DeprecatedOpenSSLConstant] * xref:cops_lint.adoc#lintdisjunctiveassignmentinconstructor[Lint/DisjunctiveAssignmentInConstructor] * xref:cops_lint.adoc#lintduplicatecasecondition[Lint/DuplicateCaseCondition] +* xref:cops_lint.adoc#lintduplicateelsifcondition[Lint/DuplicateElsifCondition] * xref:cops_lint.adoc#lintduplicatehashkey[Lint/DuplicateHashKey] * xref:cops_lint.adoc#lintduplicatemethods[Lint/DuplicateMethods] * xref:cops_lint.adoc#linteachwithobjectargument[Lint/EachWithObjectArgument] @@ -316,6 +317,7 @@ In the following section you find all available cops: * xref:cops_style.adoc#styleaccessorgrouping[Style/AccessorGrouping] * xref:cops_style.adoc#stylealias[Style/Alias] * xref:cops_style.adoc#styleandor[Style/AndOr] +* xref:cops_style.adoc#stylearraycoercion[Style/ArrayCoercion] * xref:cops_style.adoc#stylearrayjoin[Style/ArrayJoin] * xref:cops_style.adoc#styleasciicomments[Style/AsciiComments] * xref:cops_style.adoc#styleattr[Style/Attr] @@ -326,6 +328,7 @@ In the following section you find all available cops: * xref:cops_style.adoc#styleblockcomments[Style/BlockComments] * xref:cops_style.adoc#styleblockdelimiters[Style/BlockDelimiters] * xref:cops_style.adoc#stylecaseequality[Style/CaseEquality] +* xref:cops_style.adoc#stylecaselikeif[Style/CaseLikeIf] * xref:cops_style.adoc#stylecharacterliteral[Style/CharacterLiteral] * xref:cops_style.adoc#styleclassandmodulechildren[Style/ClassAndModuleChildren] * xref:cops_style.adoc#styleclasscheck[Style/ClassCheck] @@ -369,7 +372,9 @@ In the following section you find all available cops: * xref:cops_style.adoc#stylefrozenstringliteralcomment[Style/FrozenStringLiteralComment] * xref:cops_style.adoc#styleglobalvars[Style/GlobalVars] * xref:cops_style.adoc#styleguardclause[Style/GuardClause] +* xref:cops_style.adoc#stylehashaslastarrayitem[Style/HashAsLastArrayItem] * xref:cops_style.adoc#stylehasheachmethods[Style/HashEachMethods] +* xref:cops_style.adoc#stylehashlikecase[Style/HashLikeCase] * xref:cops_style.adoc#stylehashsyntax[Style/HashSyntax] * xref:cops_style.adoc#stylehashtransformkeys[Style/HashTransformKeys] * xref:cops_style.adoc#stylehashtransformvalues[Style/HashTransformValues] @@ -439,6 +444,7 @@ In the following section you find all available cops: * xref:cops_style.adoc#styleredundantconditional[Style/RedundantConditional] * xref:cops_style.adoc#styleredundantexception[Style/RedundantException] * xref:cops_style.adoc#styleredundantfetchblock[Style/RedundantFetchBlock] +* xref:cops_style.adoc#styleredundantfileextensioninrequire[Style/RedundantFileExtensionInRequire] * xref:cops_style.adoc#styleredundantfreeze[Style/RedundantFreeze] * xref:cops_style.adoc#styleredundantinterpolation[Style/RedundantInterpolation] * xref:cops_style.adoc#styleredundantparentheses[Style/RedundantParentheses] diff --git a/docs/modules/ROOT/pages/cops_gemspec.adoc b/docs/modules/ROOT/pages/cops_gemspec.adoc index ec8b98b3cb6..619828ca092 100644 --- a/docs/modules/ROOT/pages/cops_gemspec.adoc +++ b/docs/modules/ROOT/pages/cops_gemspec.adoc @@ -187,6 +187,11 @@ end Gem::Specification.new do |spec| spec.required_ruby_version = ['>= 2.5.0', '< 2.7.0'] end + +# good +Gem::Specification.new do |spec| + spec.required_ruby_version = '~> 2.5' +end ---- === Configurable attributes diff --git a/docs/modules/ROOT/pages/cops_lint.adoc b/docs/modules/ROOT/pages/cops_lint.adoc index 7244cc24f70..decf1d39c8a 100644 --- a/docs/modules/ROOT/pages/cops_lint.adoc +++ b/docs/modules/ROOT/pages/cops_lint.adoc @@ -539,9 +539,9 @@ OpenSSL::Digest.digest('SHA256', 'foo') | Enabled | No -| No +| Yes (Unsafe) | 0.62 -| - +| 0.88 |=== This cop checks constructors for disjunctive assignments that should @@ -609,6 +609,39 @@ when 'second' end ---- +== Lint/DuplicateElsifCondition + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| No +| 0.88 +| - +|=== + +This cop checks that there are no repeated conditions used in if 'elsif'. + +=== Examples + +[source,ruby] +---- +# bad +if x == 1 + do_something +elsif x == 1 + do_something_else +end + +# good +if x == 1 + do_something +elsif x == 2 + do_something_else +end +---- + == Lint/DuplicateHashKey |=== diff --git a/docs/modules/ROOT/pages/cops_naming.adoc b/docs/modules/ROOT/pages/cops_naming.adoc index d19836cefa3..57cfeeea848 100644 --- a/docs/modules/ROOT/pages/cops_naming.adoc +++ b/docs/modules/ROOT/pages/cops_naming.adoc @@ -750,7 +750,7 @@ end | Boolean | AllowedNames -| `io`, `id`, `to`, `by`, `on`, `in`, `at`, `ip`, `db`, `os`, `pp` +| `at`, `by`, `db`, `id`, `in`, `io`, `ip`, `of`, `on`, `os`, `pp`, `to` | Array | ForbiddenNames diff --git a/docs/modules/ROOT/pages/cops_style.adoc b/docs/modules/ROOT/pages/cops_style.adoc index dd3fe61c0df..564c88a591b 100644 --- a/docs/modules/ROOT/pages/cops_style.adoc +++ b/docs/modules/ROOT/pages/cops_style.adoc @@ -290,6 +290,39 @@ end * https://rubystyle.guide#no-and-or-or +== Style/ArrayCoercion + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| Yes +| 0.88 +| - +|=== + +This cop enforces the use of `Array()` instead of explicit `Array` check or `[*var]`. + +=== Examples + +[source,ruby] +---- +# bad +paths = [paths] unless paths.is_a?(Array) +paths.each { |path| do_something(path) } + +# bad (always creates a new Array instance) +[*paths].each { |path| do_something(path) } + +# good (and a bit more readable) +Array(paths).each { |path| do_something(path) } +---- + +=== References + +* https://rubystyle.guide#array-coercion + == Style/ArrayJoin |=== @@ -826,6 +859,49 @@ some_string =~ /something/ * https://rubystyle.guide#no-case-equality +== Style/CaseLikeIf + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| Yes +| 0.88 +| - +|=== + +This cop identifies places where `if-elsif` constructions +can be replaced with `case-when`. + +=== Examples + +[source,ruby] +---- +# bad +if status == :active + perform_action +elsif status == :inactive || status == :hibernating + check_timeout +else + final_action +end + +# good +case status +when :active + perform_action +when :inactive, :hibernating + check_timeout +else + final_action +end +---- + +=== References + +* https://rubystyle.guide#case-vs-if-else + == Style/CharacterLiteral |=== @@ -3354,6 +3430,59 @@ ok * https://rubystyle.guide#no-nested-conditionals +== Style/HashAsLastArrayItem + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| Yes +| 0.88 +| - +|=== + +Checks for presence or absence of braces around hash literal as a last +array item depending on configuration. + +=== Examples + +==== EnforcedStyle: braces (default) + +[source,ruby] +---- +# bad +[1, 2, one: 1, two: 2] + +# good +[1, 2, { one: 1, two: 2 }] +---- + +==== EnforcedStyle: no_braces + +[source,ruby] +---- +# bad +[1, 2, { one: 1, two: 2 }] + +# good +[1, 2, one: 1, two: 2] +---- + +=== Configurable attributes + +|=== +| Name | Default value | Configurable values + +| EnforcedStyle +| `braces` +| `braces`, `no_braces` +|=== + +=== References + +* https://rubystyle.guide#hash-literal-as-last-array-item + == Style/HashEachMethods |=== @@ -3389,6 +3518,71 @@ hash.each_value { |v| p v } * https://rubystyle.guide#hash-each +== Style/HashLikeCase + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| No +| 0.88 +| - +|=== + +This cop checks for places where `case-when` represents a simple 1:1 +mapping and can be replaced with a hash lookup. + +=== Examples + +==== MinBranchesCount: 3 (default) + +[source,ruby] +---- +# bad +case country +when 'europe' + 'http://eu.example.com' +when 'america' + 'http://us.example.com' +when 'australia' + 'http://au.example.com' +end + +# good +SITES = { + 'europe' => 'http://eu.example.com', + 'america' => 'http://us.example.com', + 'australia' => 'http://au.example.com' +} +SITES[country] +---- + +==== MinBranchesCount: 4 + +[source,ruby] +---- +# good +case country +when 'europe' + 'http://eu.example.com' +when 'america' + 'http://us.example.com' +when 'australia' + 'http://au.example.com' +end +---- + +=== Configurable attributes + +|=== +| Name | Default value | Configurable values + +| MinBranchesCount +| `3` +| Integer +|=== + == Style/HashSyntax |=== @@ -7233,6 +7427,47 @@ ENV.fetch(:key, VALUE) * https://github.com/JuanitoFatas/fast-ruby#hashfetch-with-argument-vs-hashfetch--block-code +== Style/RedundantFileExtensionInRequire + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| Yes +| 0.88 +| - +|=== + +This cop checks for the presence of superfluous `.rb` extension in +the filename provided to `require` and `require_relative`. + +Note: If the extension is omitted, Ruby tries adding '.rb', '.so', + and so on to the name until found. If the file named cannot be found, + a `LoadError` will be raised. + There is an edge case where `foo.so` file is loaded instead of a `LoadError` + if `foo.so` file exists when `require 'foo.rb'` will be changed to `require 'foo'`, + but that seems harmless. + +=== Examples + +[source,ruby] +---- +# bad +require 'foo.rb' +require_relative '../foo.rb' + +# good +require 'foo' +require 'foo.so' +require_relative '../foo' +require_relative '../foo.so' +---- + +=== References + +* https://rubystyle.guide#no-explicit-rb-to-require + == Style/RedundantFreeze |=== diff --git a/docs/modules/ROOT/pages/development.adoc b/docs/modules/ROOT/pages/development.adoc index 41a576da415..790d7291c99 100644 --- a/docs/modules/ROOT/pages/development.adoc +++ b/docs/modules/ROOT/pages/development.adoc @@ -264,6 +264,28 @@ describe RuboCop::Cop::Style::SimplifyNotEmptyWithAny, :config do end ---- +If your code has variables of different lengths, you can use `%{foo}`, +`^{foo}`, and `_{foo}` to format your template; you can also abbreviate +offense messages with `[...]`: + +[source,ruby] +---- +%w[raise fail].each do |keyword| + expect_offense(<<~RUBY, keyword: keyword) + %{keyword}(RuntimeError, msg) + ^{keyword}^^^^^^^^^^^^^^^^^^^ Redundant `RuntimeError` argument [...] + RUBY + +%w[has_one has_many].each do |type| + expect_offense(<<~RUBY, type: type) + class Book + %{type} :chapter, foreign_key: 'book_id' + _{type} ^^^^^^^^^^^^^^^^^^^^^^ Specifying the default [...] + end + RUBY +end +---- + === Auto-correct The auto-correct can help humans automatically fix offenses that have been detected. diff --git a/docs/modules/ROOT/pages/index.adoc b/docs/modules/ROOT/pages/index.adoc index 408e2f478c8..ee3876a76d1 100644 --- a/docs/modules/ROOT/pages/index.adoc +++ b/docs/modules/ROOT/pages/index.adoc @@ -20,7 +20,7 @@ linter: * Multiple result formatters for both interactive use and for feeding data into other tools * Ability to have different configuration for different parts of your codebase * Ability to disable certain cops only for specific files or parts of files -* Extremely flexible configuration that allows you to adapt RuboCop to prety much every style and preference +* Extremely flexible configuration that allows you to adapt RuboCop to pretty much every style and preference * It's easy to extend RuboCop with custom cops and formatters * A vast number of ready-made extensions (e.g. `rubocop-rails`, `rubocop-rspec`, `rubocop-performance` and `rubocop-minitest`) * Wide editor/IDE support diff --git a/docs/modules/ROOT/pages/installation.adoc b/docs/modules/ROOT/pages/installation.adoc index 1ad27179966..02b65cbc49b 100644 --- a/docs/modules/ROOT/pages/installation.adoc +++ b/docs/modules/ROOT/pages/installation.adoc @@ -21,7 +21,7 @@ might want to use a conservative version locking in your `Gemfile`: [source,rb] ---- -gem 'rubocop', '~> 0.87.0', require: false +gem 'rubocop', '~> 0.87.1', require: false ---- NOTE: You can check out our progress on the road to version 1.0 https://github.com/rubocop-hq/rubocop/milestone/4[here]. diff --git a/docs/modules/ROOT/pages/support.adoc b/docs/modules/ROOT/pages/support.adoc index 3c0236604d8..48bb1515b15 100644 --- a/docs/modules/ROOT/pages/support.adoc +++ b/docs/modules/ROOT/pages/support.adoc @@ -25,16 +25,10 @@ It's not actively monitored by the RuboCop maintainers themselves, but still you can get support from other RuboCop users there. -== Stackoverflow +== StackOverflow We're also encouraging users to ask RuboCop-related questions on StackOverflow. When doing so you should use the https://stackoverflow.com/questions/tagged/rubocop[RuboCop] tag (ideally combined with the tag `ruby`). - -== Bountysource - -If you're willing to pay for some feature to be implemented you can use -https://www.bountysource.com/teams/rubocop/issues[Bountysource] to place a -bounty for the work you want to be done. diff --git a/docs/modules/ROOT/pages/usage/basic_usage.adoc b/docs/modules/ROOT/pages/usage/basic_usage.adoc index 70dac68559b..75e02ecbb9a 100644 --- a/docs/modules/ROOT/pages/usage/basic_usage.adoc +++ b/docs/modules/ROOT/pages/usage/basic_usage.adoc @@ -8,7 +8,7 @@ RuboCop has three primary uses: In the next sections we'll briefly cover all of them. -== 1. Code style checker +== Code style checker Running `rubocop` with no arguments will check all Ruby source files in the current directory: @@ -71,9 +71,11 @@ automatically fix the problems it found in your code: [source,sh] ---- $ rubocop -a +# or +$ rubocop --auto-correct ---- -See xref:usage/auto_correct.adoc[Auto-correct]. +TIP: See xref:usage/auto_correct.adoc[Auto-correct] for more details. === Changing what RuboCop considers to be offenses @@ -85,7 +87,7 @@ project's root directory. For more information, see xref:configuration.adoc[Configuration]. -== 2. RuboCop as a replacement for `ruby -w` +== RuboCop as a replacement for `ruby -w` RuboCop natively implements almost all `ruby -w` lint warning checks, and then some. If you want you can use RuboCop simply as a replacement for `ruby -w`: @@ -93,18 +95,22 @@ simply as a replacement for `ruby -w`: [source,sh] ---- $ rubocop -l +# or +$ rubocop --lint ---- -== 3. RuboCop as a formatter +== RuboCop as a formatter There's a handy shortcut to run auto-correction only on code layout (a.k.a. formatting) offenses: [source,sh] ---- $ rubocop -x +# or +$ rubocop --fix-layout ---- -This option was introduced in RuboCop 0.57.0. +NOTE: This option was introduced in RuboCop 0.57.0. == Command-line flags diff --git a/lib/rubocop.rb b/lib/rubocop.rb index b273a8c6764..9628ad53de5 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -251,6 +251,7 @@ require_relative 'rubocop/cop/lint/deprecated_open_ssl_constant' require_relative 'rubocop/cop/lint/disjunctive_assignment_in_constructor' require_relative 'rubocop/cop/lint/duplicate_case_condition' +require_relative 'rubocop/cop/lint/duplicate_elsif_condition' require_relative 'rubocop/cop/lint/duplicate_hash_key' require_relative 'rubocop/cop/lint/duplicate_methods' require_relative 'rubocop/cop/lint/each_with_object_argument' @@ -360,6 +361,7 @@ require_relative 'rubocop/cop/style/accessor_grouping' require_relative 'rubocop/cop/style/alias' require_relative 'rubocop/cop/style/and_or' +require_relative 'rubocop/cop/style/array_coercion' require_relative 'rubocop/cop/style/array_join' require_relative 'rubocop/cop/style/ascii_comments' require_relative 'rubocop/cop/style/attr' @@ -370,6 +372,7 @@ require_relative 'rubocop/cop/style/block_comments' require_relative 'rubocop/cop/style/block_delimiters' require_relative 'rubocop/cop/style/case_equality' +require_relative 'rubocop/cop/style/case_like_if' require_relative 'rubocop/cop/style/character_literal' require_relative 'rubocop/cop/style/class_and_module_children' require_relative 'rubocop/cop/style/class_check' @@ -413,7 +416,9 @@ require_relative 'rubocop/cop/style/frozen_string_literal_comment' require_relative 'rubocop/cop/style/global_vars' require_relative 'rubocop/cop/style/guard_clause' +require_relative 'rubocop/cop/style/hash_as_last_array_item' require_relative 'rubocop/cop/style/hash_each_methods' +require_relative 'rubocop/cop/style/hash_like_case' require_relative 'rubocop/cop/style/hash_syntax' require_relative 'rubocop/cop/style/hash_transform_keys' require_relative 'rubocop/cop/style/hash_transform_values' @@ -434,6 +439,7 @@ require_relative 'rubocop/cop/style/method_call_with_args_parentheses' require_relative 'rubocop/cop/style/redundant_assignment' require_relative 'rubocop/cop/style/redundant_fetch_block' +require_relative 'rubocop/cop/style/redundant_file_extension_in_require' require_relative 'rubocop/cop/style/method_call_with_args_parentheses/omit_parentheses' require_relative 'rubocop/cop/style/method_call_with_args_parentheses/require_parentheses' require_relative 'rubocop/cop/style/method_called_on_do_end_block' diff --git a/lib/rubocop/cli.rb b/lib/rubocop/cli.rb index 3bbea1b0ad8..646c586a401 100644 --- a/lib/rubocop/cli.rb +++ b/lib/rubocop/cli.rb @@ -75,7 +75,7 @@ def execute_runners def validate_options_vs_config if @options[:parallel] && - !@config_store.for_dir(Dir.pwd).for_all_cops['UseCache'] + !@config_store.for_pwd.for_all_cops['UseCache'] raise OptionArgumentError, '-P/--parallel uses caching to speed up ' \ 'execution, so combining with AllCops: ' \ 'UseCache: false is not allowed.' @@ -121,7 +121,7 @@ def apply_default_formatter if @options[:auto_gen_config] formatter = 'autogenconf' else - cfg = @config_store.for_dir(Dir.pwd).for_all_cops + cfg = @config_store.for_pwd.for_all_cops formatter = cfg['DefaultFormatter'] || 'progress' end [[formatter, @options[:output_path]]] diff --git a/lib/rubocop/cli/command/auto_genenerate_config.rb b/lib/rubocop/cli/command/auto_genenerate_config.rb index d98144924df..dd37a40598a 100644 --- a/lib/rubocop/cli/command/auto_genenerate_config.rb +++ b/lib/rubocop/cli/command/auto_genenerate_config.rb @@ -27,10 +27,10 @@ def run private def maybe_run_line_length_cop - if !line_length_enabled?(@config_store.for_dir(Dir.pwd)) + if !line_length_enabled?(@config_store.for_pwd) skip_line_length_cop(PHASE_1_DISABLED) elsif !same_max_line_length?( - @config_store.for_dir(Dir.pwd), ConfigLoader.default_configuration + @config_store.for_pwd, ConfigLoader.default_configuration ) skip_line_length_cop(PHASE_1_OVERRIDDEN) else diff --git a/lib/rubocop/config_loader.rb b/lib/rubocop/config_loader.rb index d8044bfe3f2..e03b040e4d6 100644 --- a/lib/rubocop/config_loader.rb +++ b/lib/rubocop/config_loader.rb @@ -118,14 +118,13 @@ def possible_new_cops?(config) end def add_excludes_from_files(config, config_file) - found_files = find_files_upwards(DOTFILE, config_file) + exclusion_file = find_last_file_upwards(DOTFILE, config_file) - return if found_files.empty? - return if PathUtil.relative_path(found_files.last) == - PathUtil.relative_path(config_file) + return unless exclusion_file + return if PathUtil.relative_path(exclusion_file) == PathUtil.relative_path(config_file) print 'AllCops/Exclude ' if debug? - config.add_excludes_from_higher_level(load_file(found_files.last)) + config.add_excludes_from_higher_level(load_file(exclusion_file)) end def default_configuration diff --git a/lib/rubocop/config_store.rb b/lib/rubocop/config_store.rb index 65675cc7fc5..018e7b0c949 100644 --- a/lib/rubocop/config_store.rb +++ b/lib/rubocop/config_store.rb @@ -33,6 +33,10 @@ def for_file(file) for_dir(File.dirname(file)) end + def for_pwd + for_dir(Dir.pwd) + end + # If type (file/dir) is known beforehand, # prefer using #for_file or #for_dir for improved performance def for(file_or_dir) diff --git a/lib/rubocop/cop/autocorrect_logic.rb b/lib/rubocop/cop/autocorrect_logic.rb index 206e338b0b9..5339246eea9 100644 --- a/lib/rubocop/cop/autocorrect_logic.rb +++ b/lib/rubocop/cop/autocorrect_logic.rb @@ -73,7 +73,7 @@ def range_by_lines(range) end def max_line_length - config.for_cop('Layout/LineLength')['Max'] || 80 + config.for_cop('Layout/LineLength')['Max'] || 120 end def disable_offense_at_end_of_line(range, eol_comment) diff --git a/lib/rubocop/cop/badge.rb b/lib/rubocop/cop/badge.rb index 6ad6fc89ff9..be1710879c4 100644 --- a/lib/rubocop/cop/badge.rb +++ b/lib/rubocop/cop/badge.rb @@ -58,7 +58,7 @@ def match?(other) end def to_s - qualified? ? "#{department}/#{cop_name}" : cop_name + @to_s ||= qualified? ? "#{department}/#{cop_name}" : cop_name end def qualified? diff --git a/lib/rubocop/cop/base.rb b/lib/rubocop/cop/base.rb index eb88e7b8015..a5d6a07cfdf 100644 --- a/lib/rubocop/cop/base.rb +++ b/lib/rubocop/cop/base.rb @@ -105,7 +105,7 @@ def add_global_offense(message = nil, severity: nil) # 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) + return unless current_offense_locations.add?(range) range_to_pass = callback_argument(range) @@ -271,11 +271,19 @@ def correction_strategy ### Reserved for Commissioner: + def current_offense_locations + @current_offense_locations ||= Set.new + end + + def currently_disabled_lines + @currently_disabled_lines ||= Set.new + end + # Called before any investigation def begin_investigation(processed_source) @current_offenses = [] - @current_offense_locations = Set[] - @currently_disabled_lines = Set[] + @current_offense_locations = nil + @currently_disabled_lines = nil @processed_source = processed_source @current_corrector = Corrector.new(@processed_source) if @processed_source.valid_syntax? end @@ -327,7 +335,7 @@ def attempt_correction(range, corrector) def disable_uncorrectable(range) line = range.line - return unless @currently_disabled_lines.add?(line) + return unless currently_disabled_lines.add?(line) disable_offense(range) end diff --git a/lib/rubocop/cop/cop.rb b/lib/rubocop/cop/cop.rb index 1db7052910c..ac1ee8c0aec 100644 --- a/lib/rubocop/cop/cop.rb +++ b/lib/rubocop/cop/cop.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'uri' -require_relative 'legacy/corrections_proxy.rb' +require_relative 'legacy/corrections_proxy' module RuboCop module Cop diff --git a/lib/rubocop/cop/correctors/multiline_literal_brace_corrector.rb b/lib/rubocop/cop/correctors/multiline_literal_brace_corrector.rb index 35e8fc7b248..7a0d973cfb0 100644 --- a/lib/rubocop/cop/correctors/multiline_literal_brace_corrector.rb +++ b/lib/rubocop/cop/correctors/multiline_literal_brace_corrector.rb @@ -42,8 +42,34 @@ def correct_next_line_brace(corrector) corrector.insert_before( last_element_range_with_trailing_comma(node).end, + content_if_comment_present(corrector, node) + ) + end + + def content_if_comment_present(corrector, node) + range = range_with_surrounding_space( + range: children(node).last.source_range, + side: :right + ).end.resize(1) + if range.source == '#' + select_content_to_be_inserted_after_last_element(corrector, node) + else node.loc.end.source + end + end + + def select_content_to_be_inserted_after_last_element(corrector, node) + range = range_between( + node.loc.end.begin_pos, + range_by_whole_lines(node.loc.expression).end.end_pos ) + + remove_trailing_content_of_comment(corrector, range) + range.source + end + + def remove_trailing_content_of_comment(corrector, range) + corrector.remove(range) end def last_element_range_with_trailing_comma(node) diff --git a/lib/rubocop/cop/gemspec/required_ruby_version.rb b/lib/rubocop/cop/gemspec/required_ruby_version.rb index 906d3dc9b6d..c399bb4ab53 100644 --- a/lib/rubocop/cop/gemspec/required_ruby_version.rb +++ b/lib/rubocop/cop/gemspec/required_ruby_version.rb @@ -35,6 +35,11 @@ module Gemspec # Gem::Specification.new do |spec| # spec.required_ruby_version = ['>= 2.5.0', '< 2.7.0'] # end + # + # # good + # Gem::Specification.new do |spec| + # spec.required_ruby_version = '~> 2.5' + # end class RequiredRubyVersion < Cop MSG = '`required_ruby_version` (%s, ' \ 'declared in %s) and `TargetRubyVersion` ' \ @@ -68,7 +73,7 @@ def extract_ruby_version(required_ruby_version) end end - required_ruby_version.str_content.match(/(\d\.\d)/)[1] + required_ruby_version.str_content.scan(/\d/).first(2).join('.') end def message(required_ruby_version, target_ruby_version) diff --git a/lib/rubocop/cop/layout/empty_lines_around_access_modifier.rb b/lib/rubocop/cop/layout/empty_lines_around_access_modifier.rb index 1e431fe50c4..a9a17c6aefe 100644 --- a/lib/rubocop/cop/layout/empty_lines_around_access_modifier.rb +++ b/lib/rubocop/cop/layout/empty_lines_around_access_modifier.rb @@ -109,6 +109,7 @@ def autocorrect(node) def allowed_only_before_style?(node) if node.special_modifier? + return true if processed_source[node.last_line] == 'end' return false if next_line_empty?(node.last_line) end diff --git a/lib/rubocop/cop/layout/end_alignment.rb b/lib/rubocop/cop/layout/end_alignment.rb index 31e13849e51..be1538f7fda 100644 --- a/lib/rubocop/cop/layout/end_alignment.rb +++ b/lib/rubocop/cop/layout/end_alignment.rb @@ -150,9 +150,10 @@ def check_other_alignment(node) end def alignment_node(node) - if style == :keyword + case style + when :keyword node - elsif style == :variable + when :variable alignment_node_for_variable_style(node) else start_line_range(node) diff --git a/lib/rubocop/cop/layout/multiline_block_layout.rb b/lib/rubocop/cop/layout/multiline_block_layout.rb index e1bd2b2203f..25e26778db6 100644 --- a/lib/rubocop/cop/layout/multiline_block_layout.rb +++ b/lib/rubocop/cop/layout/multiline_block_layout.rb @@ -95,11 +95,22 @@ def args_on_beginning_line?(node) end def line_break_necessary_in_args?(node) - needed_length = node.source_range.column + - node.source.lines.first.length + - block_arg_string(node, node.arguments).length + - PIPE_SIZE - needed_length > max_line_length + needed_length_for_args(node) > max_line_length + end + + def needed_length_for_args(node) + node.source_range.column + + characters_needed_for_space_and_pipes(node) + + node.source.lines.first.chomp.length + + block_arg_string(node, node.arguments).length + end + + def characters_needed_for_space_and_pipes(node) + if node.source.lines.first.end_with?("|\n") + PIPE_SIZE + else + 1 + PIPE_SIZE * 2 + end end def add_offense_for_expression(node, expr, msg) diff --git a/lib/rubocop/cop/layout/space_around_block_parameters.rb b/lib/rubocop/cop/layout/space_around_block_parameters.rb index efdf9b2fad5..02c13d08eb1 100644 --- a/lib/rubocop/cop/layout/space_around_block_parameters.rb +++ b/lib/rubocop/cop/layout/space_around_block_parameters.rb @@ -56,11 +56,12 @@ def style_parameter_name def check_inside_pipes(arguments) opening_pipe, closing_pipe = pipes(arguments) - if style == :no_space + case style + when :no_space check_no_space_style_inside_pipes(arguments.children, opening_pipe, closing_pipe) - elsif style == :space + when :space check_space_style_inside_pipes(arguments.children, opening_pipe, closing_pipe) diff --git a/lib/rubocop/cop/layout/space_around_method_call_operator.rb b/lib/rubocop/cop/layout/space_around_method_call_operator.rb index b07963bc8f8..f0414199990 100644 --- a/lib/rubocop/cop/layout/space_around_method_call_operator.rb +++ b/lib/rubocop/cop/layout/space_around_method_call_operator.rb @@ -34,96 +34,55 @@ module Layout # RuboCop::Cop::Cop # ::RuboCop::Cop # - class SpaceAroundMethodCallOperator < Cop - include SurroundingSpace + class SpaceAroundMethodCallOperator < Base + include RangeHelp + extend AutoCorrector + + SPACES_REGEXP = /\A[ \t]+\z/.freeze MSG = 'Avoid using spaces around a method call operator.' def on_send(node) - return unless dot_or_safe_navigation_operator?(node) + return unless node.dot? || node.safe_navigation? - check_and_add_offense(node) + check_space_before_dot(node) + check_space_after_dot(node) end + alias on_csend on_send def on_const(node) return unless node.loc.double_colon - check_and_add_offense(node, false) - end - - def autocorrect(node) - operator = operator_token(node) - left = left_token_for_auto_correction(node, operator) - right = right_token_for_auto_correction(operator) - - lambda do |corrector| - SpaceCorrector.remove_space( - processed_source, corrector, left, right - ) - end + check_space_after_double_colon(node) end - alias on_csend on_send - private - def check_and_add_offense(node, add_left_offense = true) - operator = operator_token(node) - left = previous_token(operator) - right = next_token(operator) - - if !right.comment? && valid_right_token?(right, operator) - no_space_offenses(node, operator, right, MSG) - end - return unless valid_left_token?(left, operator) - - no_space_offenses(node, left, operator, MSG) if add_left_offense - end - - def operator_token(node) - operator_location = - node.const_type? ? node.loc.double_colon : node.loc.dot - - processed_source.find_token do |token| - token.pos == operator_location - end - end - - def previous_token(current_token) - index = processed_source.tokens.index(current_token) - index.zero? ? nil : processed_source.tokens[index - 1] - end - - def next_token(current_token) - index = processed_source.tokens.index(current_token) - processed_source.tokens[index + 1] - end - - def dot_or_safe_navigation_operator?(node) - node.dot? || node.safe_navigation? + def check_space_before_dot(node) + receiver_pos = node.receiver.source_range.end_pos + dot_pos = node.loc.dot.begin_pos + check_space(receiver_pos, dot_pos) end - def valid_left_token?(left, operator) - left && left.line == operator.line + def check_space_after_dot(node) + dot_pos = node.loc.dot.end_pos + selector_pos = node.loc.selector.begin_pos + check_space(dot_pos, selector_pos) end - def valid_right_token?(right, operator) - right && right.line == operator.line + def check_space_after_double_colon(node) + double_colon_pos = node.loc.double_colon.end_pos + name_pos = node.loc.name.begin_pos + check_space(double_colon_pos, name_pos) end - def left_token_for_auto_correction(node, operator) - left_token = previous_token(operator) - return operator if node.const_type? - return left_token if valid_left_token?(left_token, operator) - - operator - end + def check_space(begin_pos, end_pos) + return if end_pos <= begin_pos - def right_token_for_auto_correction(operator) - right_token = next_token(operator) - return right_token if !right_token.comment? && valid_right_token?(right_token, operator) + range = range_between(begin_pos, end_pos) + return unless range.source.match?(SPACES_REGEXP) - operator + add_offense(range) { |corrector| corrector.remove(range) } end end end diff --git a/lib/rubocop/cop/layout/space_inside_array_literal_brackets.rb b/lib/rubocop/cop/layout/space_inside_array_literal_brackets.rb index 1840acc45fe..ebd658d6047 100644 --- a/lib/rubocop/cop/layout/space_inside_array_literal_brackets.rb +++ b/lib/rubocop/cop/layout/space_inside_array_literal_brackets.rb @@ -142,11 +142,12 @@ def line_and_column_for(token) end def issue_offenses(node, left, right, start_ok, end_ok) - if style == :no_space + case style + when :no_space start_ok = next_to_comment?(node, left) no_space_offenses(node, left, right, MSG, start_ok: start_ok, end_ok: end_ok) - elsif style == :space + when :space space_offenses(node, left, right, MSG, start_ok: start_ok, end_ok: end_ok) else diff --git a/lib/rubocop/cop/lint/disjunctive_assignment_in_constructor.rb b/lib/rubocop/cop/lint/disjunctive_assignment_in_constructor.rb index c5e8dd0a3b5..f74c0799ecf 100644 --- a/lib/rubocop/cop/lint/disjunctive_assignment_in_constructor.rb +++ b/lib/rubocop/cop/lint/disjunctive_assignment_in_constructor.rb @@ -22,7 +22,9 @@ module Lint # def initialize # @x = 1 # end - class DisjunctiveAssignmentInConstructor < Cop + class DisjunctiveAssignmentInConstructor < Base + extend AutoCorrector + MSG = 'Unnecessary disjunctive assignment. Use plain assignment.' def on_def(node) @@ -73,7 +75,11 @@ def check_body_lines(lines) # @param [Node] node a disjunctive assignment def check_disjunctive_assignment(node) lhs = node.child_nodes.first - add_offense(node, location: :operator) if lhs.ivasgn_type? + return unless lhs.ivasgn_type? + + add_offense(node.loc.operator) do |corrector| + corrector.replace(node.loc.operator, '=') + end end end end diff --git a/lib/rubocop/cop/lint/duplicate_elsif_condition.rb b/lib/rubocop/cop/lint/duplicate_elsif_condition.rb new file mode 100644 index 00000000000..0dfb9ad0f8b --- /dev/null +++ b/lib/rubocop/cop/lint/duplicate_elsif_condition.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Lint + # This cop checks that there are no repeated conditions used in if 'elsif'. + # + # @example + # # bad + # if x == 1 + # do_something + # elsif x == 1 + # do_something_else + # end + # + # # good + # if x == 1 + # do_something + # elsif x == 2 + # do_something_else + # end + # + class DuplicateElsifCondition < Base + MSG = 'Duplicate `elsif` condition detected.' + + def on_if(node) + previous = [] + while node.if? || node.elsif? + condition = node.condition + add_offense(condition) if previous.include?(condition) + previous << condition + node = node.else_branch + break unless node&.if_type? + end + end + end + end + end +end diff --git a/lib/rubocop/cop/lint/duplicate_methods.rb b/lib/rubocop/cop/lint/duplicate_methods.rb index d6eb2d4b6a7..fecead95116 100644 --- a/lib/rubocop/cop/lint/duplicate_methods.rb +++ b/lib/rubocop/cop/lint/duplicate_methods.rb @@ -126,7 +126,7 @@ def check_self_receiver(node, name) end def message_for_dup(node, method_name) - format(MSG, method: method_name, defined: @definitions[method_name], + format(MSG, method: method_name, defined: source_location(@definitions[method_name]), current: source_location(node)) end @@ -152,7 +152,7 @@ def found_method(node, method_name) add_offense(node, location: loc, message: message) else - @definitions[method_name] = source_location(node) + @definitions[method_name] = node end end diff --git a/lib/rubocop/cop/lint/implicit_string_concatenation.rb b/lib/rubocop/cop/lint/implicit_string_concatenation.rb index df0fd2647c7..93cbc4d4563 100644 --- a/lib/rubocop/cop/lint/implicit_string_concatenation.rb +++ b/lib/rubocop/cop/lint/implicit_string_concatenation.rb @@ -64,9 +64,10 @@ def each_bad_cons(node) def ending_delimiter(str) # implicit string concatenation does not work with %{}, etc. - if str.source[0] == "'" + case str.source[0] + when "'" "'" - elsif str.source[0] == '"' + when '"' '"' end end diff --git a/lib/rubocop/cop/lint/literal_as_condition.rb b/lib/rubocop/cop/lint/literal_as_condition.rb index f98925dd2ea..39b5d5e2f33 100644 --- a/lib/rubocop/cop/lint/literal_as_condition.rb +++ b/lib/rubocop/cop/lint/literal_as_condition.rb @@ -30,6 +30,8 @@ module Lint # break if condition # end class LiteralAsCondition < Cop + include RangeHelp + MSG = 'Literal `%s` appeared as a condition.' def on_if(node) @@ -57,7 +59,8 @@ def on_case(case_node) case_node.each_when do |when_node| next unless when_node.conditions.all?(&:literal?) - add_offense(when_node) + range = when_conditions_range(when_node) + add_offense(when_node, location: range, message: message(range)) end end end @@ -129,6 +132,13 @@ def condition(node) node.condition end end + + def when_conditions_range(when_node) + range_between( + when_node.conditions.first.source_range.begin_pos, + when_node.conditions.last.source_range.end_pos + ) + end end end end diff --git a/lib/rubocop/cop/lint/nested_method_definition.rb b/lib/rubocop/cop/lint/nested_method_definition.rb index ae574cae6fd..74f6f6c5047 100644 --- a/lib/rubocop/cop/lint/nested_method_definition.rb +++ b/lib/rubocop/cop/lint/nested_method_definition.rb @@ -59,31 +59,25 @@ class NestedMethodDefinition < Cop 'Use `lambda` instead.' def on_def(node) - find_nested_defs(node) do |nested_def_node| - add_offense(nested_def_node) - end - end - alias on_defs on_def - - private + subject, = *node + return if node.defs_type? && subject.lvar_type? - def find_nested_defs(node, &block) - node.each_child_node do |child| - if child.def_type? - yield child - elsif child.defs_type? - subject, = *child - next if subject.lvar_type? + def_ancestor = node.each_ancestor(:def, :defs).first + return unless def_ancestor - yield child - elsif !scoping_method_call?(child) - find_nested_defs(child, &block) + within_scoping_def = + node.each_ancestor(:block, :sclass).any? do |ancestor| + scoping_method_call?(ancestor) end - end + + add_offense(node) if def_ancestor && !within_scoping_def end + alias on_defs on_def + + private def scoping_method_call?(child) - eval_call?(child) || exec_call?(child) || child.sclass_type? || + child.sclass_type? || eval_call?(child) || exec_call?(child) || class_or_module_or_struct_new_call?(child) end diff --git a/lib/rubocop/cop/mixin/statement_modifier.rb b/lib/rubocop/cop/mixin/statement_modifier.rb index 88ea52a0334..6712b185bed 100644 --- a/lib/rubocop/cop/mixin/statement_modifier.rb +++ b/lib/rubocop/cop/mixin/statement_modifier.rb @@ -17,9 +17,9 @@ def single_line_as_modifier?(node) end def non_eligible_node?(node) - node.nonempty_line_count > 3 || - !node.modifier_form? && - processed_source.commented?(node.loc.end) + node.modifier_form? || + node.nonempty_line_count > 3 || + processed_source.commented?(node.loc.end) end def non_eligible_body?(body) diff --git a/lib/rubocop/cop/style/accessor_grouping.rb b/lib/rubocop/cop/style/accessor_grouping.rb index 6aec414f8e7..25290de1841 100644 --- a/lib/rubocop/cop/style/accessor_grouping.rb +++ b/lib/rubocop/cop/style/accessor_grouping.rb @@ -58,6 +58,10 @@ def autocorrect(node) private + def previous_line_comment?(node) + comment_line?(processed_source[node.first_line - 2]) + end + def class_send_elements(class_node) class_def = class_node.body @@ -75,6 +79,8 @@ def accessor?(send_node) end def check(send_node) + return if previous_line_comment?(send_node) + if grouped_style? && sibling_accessors(send_node).size > 1 add_offense(send_node) elsif separated_style? && send_node.arguments.size > 1 @@ -94,7 +100,8 @@ def sibling_accessors(send_node) send_node.parent.each_child_node(:send).select do |sibling| accessor?(sibling) && sibling.method?(send_node.method_name) && - node_visibility(sibling) == node_visibility(send_node) + node_visibility(sibling) == node_visibility(send_node) && + !previous_line_comment?(sibling) end end diff --git a/lib/rubocop/cop/style/array_coercion.rb b/lib/rubocop/cop/style/array_coercion.rb new file mode 100644 index 00000000000..73d75ceead8 --- /dev/null +++ b/lib/rubocop/cop/style/array_coercion.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # This cop enforces the use of `Array()` instead of explicit `Array` check or `[*var]`. + # + # @example + # # bad + # paths = [paths] unless paths.is_a?(Array) + # paths.each { |path| do_something(path) } + # + # # bad (always creates a new Array instance) + # [*paths].each { |path| do_something(path) } + # + # # good (and a bit more readable) + # Array(paths).each { |path| do_something(path) } + # + class ArrayCoercion < Base + extend AutoCorrector + + SPLAT_MSG = 'Use `Array(%s)` instead of `[*%s]`.' + CHECK_MSG = 'Use `Array(%s)` instead of explicit `Array` check.' + + def_node_matcher :array_splat?, <<~PATTERN + (array (splat $_)) + PATTERN + + def_node_matcher :unless_array?, <<~PATTERN + (if + (send + (lvar $_) :is_a? + (const nil? :Array)) nil? + (lvasgn $_ + (array + (lvar $_)))) + PATTERN + + def on_array(node) + return unless node.square_brackets? + + array_splat?(node) do |arg_node| + message = format(SPLAT_MSG, arg: arg_node.source) + add_offense(node, message: message) do |corrector| + corrector.replace(node, "Array(#{arg_node.source})") + end + end + end + + def on_if(node) + unless_array?(node) do |var_a, var_b, var_c| + if var_a == var_b && var_c == var_b + message = format(CHECK_MSG, arg: var_a) + add_offense(node, message: message) do |corrector| + corrector.replace(node, "#{var_a} = Array(#{var_a})") + end + end + end + end + end + end + end +end diff --git a/lib/rubocop/cop/style/auto_resource_cleanup.rb b/lib/rubocop/cop/style/auto_resource_cleanup.rb index 6aabcf6cc8c..96129a20798 100644 --- a/lib/rubocop/cop/style/auto_resource_cleanup.rb +++ b/lib/rubocop/cop/style/auto_resource_cleanup.rb @@ -25,10 +25,11 @@ class AutoResourceCleanup < Cop def on_send(node) TARGET_METHODS.each do |target_class, target_method| - target_receiver = s(:const, nil, target_class) + next if node.method_name != target_method + target_receiver = s(:const, nil, target_class) next if node.receiver != target_receiver - next if node.method_name != target_method + next if cleanup?(node) add_offense(node, diff --git a/lib/rubocop/cop/style/bisected_attr_accessor.rb b/lib/rubocop/cop/style/bisected_attr_accessor.rb index 9eaccfaaaa6..40caa659e03 100644 --- a/lib/rubocop/cop/style/bisected_attr_accessor.rb +++ b/lib/rubocop/cop/style/bisected_attr_accessor.rb @@ -21,15 +21,21 @@ module Style # end # class BisectedAttrAccessor < Cop + include VisibilityHelp + MSG = 'Combine both accessors into `attr_accessor %s`.' def on_class(class_node) - reader_names, writer_names = accessor_names(class_node) + VISIBILITY_SCOPES.each do |visibility| + reader_names, writer_names = accessor_names(class_node, visibility) + next unless reader_names && writer_names - accessor_macroses(class_node).each do |macro| - check(macro, reader_names, writer_names) + accessor_macroses(class_node, visibility).each do |macro| + check(macro, reader_names, writer_names) + end end end + alias on_sclass on_class alias on_module on_class def autocorrect(node) @@ -42,18 +48,18 @@ def autocorrect(node) private - def accessor_names(class_node) - reader_names = Set.new - writer_names = Set.new + def accessor_names(class_node, visibility) + reader_names = nil + writer_names = nil - accessor_macroses(class_node).each do |macro| + accessor_macroses(class_node, visibility).each do |macro| names = macro.arguments.map(&:source) names.each do |name| if attr_reader?(macro) - reader_names.add(name) + (reader_names ||= Set.new).add(name) else - writer_names.add(name) + (writer_names ||= Set.new).add(name) end end end @@ -61,7 +67,7 @@ def accessor_names(class_node) [reader_names, writer_names] end - def accessor_macroses(class_node) + def accessor_macroses(class_node, visibility) class_def = class_node.body return [] if !class_def || class_def.def_type? @@ -72,7 +78,13 @@ def accessor_macroses(class_node) class_def.each_child_node(:send) end - send_nodes.select { |node| node.macro? && (attr_reader?(node) || attr_writer?(node)) } + send_nodes.select { |node| attr_within_visibility_scope?(node, visibility) } + end + + def attr_within_visibility_scope?(node, visibility) + node.macro? && + (attr_reader?(node) || attr_writer?(node)) && + node_visibility(node) == visibility end def attr_reader?(send_node) @@ -95,8 +107,8 @@ def check(macro, reader_names, writer_names) end def replacement(macro, node) - class_node = macro.each_ancestor(:class, :module).first - reader_names, writer_names = accessor_names(class_node) + class_node = macro.each_ancestor(:class, :sclass, :module).first + reader_names, writer_names = accessor_names(class_node, node_visibility(macro)) rest_args = rest_args(macro.arguments, reader_names, writer_names) diff --git a/lib/rubocop/cop/style/case_like_if.rb b/lib/rubocop/cop/style/case_like_if.rb new file mode 100644 index 00000000000..d5e31f18b17 --- /dev/null +++ b/lib/rubocop/cop/style/case_like_if.rb @@ -0,0 +1,217 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # This cop identifies places where `if-elsif` constructions + # can be replaced with `case-when`. + # + # @example + # # bad + # if status == :active + # perform_action + # elsif status == :inactive || status == :hibernating + # check_timeout + # else + # final_action + # end + # + # # good + # case status + # when :active + # perform_action + # when :inactive, :hibernating + # check_timeout + # else + # final_action + # end + # + class CaseLikeIf < Cop + include RangeHelp + + MSG = 'Convert `if-elsif` to `case-when`.' + + def on_if(node) + return unless should_check?(node) + + target = find_target(node.condition) + return unless target + + conditions = [] + convertible = true + + branch_conditions(node).each do |branch_condition| + conditions << [] + convertible = collect_conditions(branch_condition, target, conditions.last) + break unless convertible + end + + add_offense(node) if convertible + end + + def autocorrect(node) + target = find_target(node.condition) + + lambda do |corrector| + corrector.insert_before(node, "case #{target.source}\n#{indent(node)}") + + branch_conditions(node).each do |branch_condition| + conditions = [] + collect_conditions(branch_condition, target, conditions) + + range = correction_range(branch_condition) + branch_replacement = "when #{conditions.map(&:source).join(', ')}" + corrector.replace(range, branch_replacement) + end + end + end + + private + + def should_check?(node) + !node.unless? && !node.elsif? && !node.modifier_form? && !node.ternary? && + node.elsif_conditional? + end + + # rubocop:disable Metrics/MethodLength + def find_target(node) + case node.type + when :begin + find_target(node.children.first) + when :or + find_target(node.lhs) + when :match_with_lvasgn + lhs, rhs = *node + if lhs.regexp_type? + rhs + elsif rhs.regexp_type? + lhs + end + when :send + find_target_in_send_node(node) + end + end + # rubocop:enable Metrics/MethodLength + + def find_target_in_send_node(node) + case node.method_name + when :is_a? + node.receiver + when :==, :eql?, :equal? + find_target_in_equality_node(node) + when :=== + node.arguments.first + when :include?, :cover? + receiver = deparenthesize(node.receiver) + node.arguments.first if receiver.range_type? + when :match, :match? + find_target_in_match_node(node) + end + end + + def find_target_in_equality_node(node) + argument = node.arguments.first + receiver = node.receiver + + if argument.literal? || const_reference?(argument) + receiver + elsif receiver.literal? || const_reference?(receiver) + argument + end + end + + def find_target_in_match_node(node) + argument = node.arguments.first + receiver = node.receiver + + if receiver.regexp_type? + argument + elsif argument.regexp_type? + receiver + end + end + + def collect_conditions(node, target, conditions) + condition = + case node.type + when :begin + return collect_conditions(node.children.first, target, conditions) + when :or + return collect_conditions(node.lhs, target, conditions) && + collect_conditions(node.rhs, target, conditions) + when :match_with_lvasgn + lhs, rhs = *node + condition_from_binary_op(lhs, rhs, target) + when :send + condition_from_send_node(node, target) + end + + conditions << condition if condition + end + + # rubocop:disable Metrics/AbcSize + # rubocop:disable Metrics/CyclomaticComplexity + def condition_from_send_node(node, target) + case node.method_name + when :is_a? + node.arguments.first if node.receiver == target + when :==, :eql?, :equal?, :=~, :match, :match? + lhs, _method, rhs = *node + condition_from_binary_op(lhs, rhs, target) + when :=== + lhs, _method, rhs = *node + lhs if rhs == target + when :include?, :cover? + receiver = deparenthesize(node.receiver) + receiver if receiver.range_type? && node.arguments.first == target + end + end + # rubocop:enable Metrics/CyclomaticComplexity + # rubocop:enable Metrics/AbcSize + + def condition_from_binary_op(lhs, rhs, target) + lhs = deparenthesize(lhs) + rhs = deparenthesize(rhs) + + if lhs == target + rhs + elsif rhs == target + lhs + end + end + + def branch_conditions(node) + conditions = [] + while node&.if_type? + conditions << node.condition + node = node.else_branch + end + conditions + end + + def const_reference?(node) + return false unless node.const_type? + + name = node.children[1].to_s + + # We can no be sure if, e.g. `C`, represents a constant or a class reference + name.length > 1 && + name == name.upcase + end + + def deparenthesize(node) + node = node.children.last while node.begin_type? + node + end + + def correction_range(node) + range_between(node.parent.loc.keyword.begin_pos, node.loc.expression.end_pos) + end + + def indent(node) + ' ' * node.loc.column + end + end + end + end +end diff --git a/lib/rubocop/cop/style/commented_keyword.rb b/lib/rubocop/cop/style/commented_keyword.rb index 555b919a9e7..d11ee5e8874 100644 --- a/lib/rubocop/cop/style/commented_keyword.rb +++ b/lib/rubocop/cop/style/commented_keyword.rb @@ -46,17 +46,20 @@ def investigate(processed_source) private KEYWORDS = %w[begin class def end module].freeze + KEYWORD_REGEXES = KEYWORDS.map { |w| /^\s*#{w}\s/ }.freeze + ALLOWED_COMMENTS = %w[ :nodoc: :yields: rubocop:disable rubocop:todo ].freeze + ALLOWED_COMMENT_REGEXES = ALLOWED_COMMENTS.map { |c| /#\s*#{c}/ }.freeze def offensive?(comment) line = line(comment) - KEYWORDS.any? { |word| /^\s*#{word}\s/.match?(line) } && - ALLOWED_COMMENTS.none? { |c| /#\s*#{c}/.match?(line) } + KEYWORD_REGEXES.any? { |r| r.match?(line) } && + ALLOWED_COMMENT_REGEXES.none? { |r| r.match?(line) } end def message(comment) diff --git a/lib/rubocop/cop/style/conditional_assignment.rb b/lib/rubocop/cop/style/conditional_assignment.rb index bf88d1030f6..42e2488a5a0 100644 --- a/lib/rubocop/cop/style/conditional_assignment.rb +++ b/lib/rubocop/cop/style/conditional_assignment.rb @@ -30,7 +30,7 @@ def expand_when_branches(when_branches) end def tail(branch) - branch.begin_type? ? [*branch].last : branch + branch.begin_type? ? Array(branch).last : branch end # rubocop:disable Metrics/AbcSize diff --git a/lib/rubocop/cop/style/exponential_notation.rb b/lib/rubocop/cop/style/exponential_notation.rb index 31aa6a34143..8ffe2fee2ab 100644 --- a/lib/rubocop/cop/style/exponential_notation.rb +++ b/lib/rubocop/cop/style/exponential_notation.rb @@ -60,6 +60,11 @@ module Style # class ExponentialNotation < Cop include ConfigurableEnforcedStyle + MESSAGES = { + scientific: 'Use a mantissa in [1, 10[.', + engineering: 'Use an exponent divisible by 3 and a mantissa in [0.1, 1000[.', + integral: 'Use an integer as mantissa, without trailing zero.' + }.freeze def on_float(node) add_offense(node) if offense?(node) @@ -104,14 +109,7 @@ def offense?(node) end def message(_node) - case style - when :scientific - 'Use a mantissa in [1, 10[.' - when :engineering - 'Use an exponent divisible by 3 and a mantissa in [0.1, 1000[.' - when :integral - 'Use an integer as mantissa, without trailing zero.' - end + MESSAGES[style] end end end diff --git a/lib/rubocop/cop/style/float_division.rb b/lib/rubocop/cop/style/float_division.rb index d493a3b3ec4..bebe6ee4cc3 100644 --- a/lib/rubocop/cop/style/float_division.rb +++ b/lib/rubocop/cop/style/float_division.rb @@ -41,6 +41,12 @@ module Style # a.fdiv(b) class FloatDivision < Cop include ConfigurableEnforcedStyle + MESSAGES = { + left_coerce: 'Prefer using `.to_f` on the left side.', + right_coerce: 'Prefer using `.to_f` on the right side.', + single_coerce: 'Prefer using `.to_f` on one side only.', + fdiv: 'Prefer using `fdiv` for float divisions.' + }.freeze def_node_matcher :right_coerce?, <<~PATTERN (send _ :/ (send _ :to_f)) @@ -77,16 +83,7 @@ def offense_condition?(node) end def message(_node) - case style - when :left_coerce - 'Prefer using `.to_f` on the left side.' - when :right_coerce - 'Prefer using `.to_f` on the right side.' - when :single_coerce - 'Prefer using `.to_f` on one side only.' - when :fdiv - 'Prefer using `fdiv` for float divisions.' - end + MESSAGES[style] end end end diff --git a/lib/rubocop/cop/style/format_string_token.rb b/lib/rubocop/cop/style/format_string_token.rb index 7dae9130c02..a0076eee3f9 100644 --- a/lib/rubocop/cop/style/format_string_token.rb +++ b/lib/rubocop/cop/style/format_string_token.rb @@ -75,11 +75,11 @@ def message(detected_style) # rubocop:disable Style/FormatStringToken def message_text(style) - case style - when :annotated then 'annotated tokens (like `%s`)' - when :template then 'template tokens (like `%{foo}`)' - when :unannotated then 'unannotated tokens (like `%s`)' - end + { + annotated: 'annotated tokens (like `%s`)', + template: 'template tokens (like `%{foo}`)', + unannotated: 'unannotated tokens (like `%s`)' + }[style] end # rubocop:enable Style/FormatStringToken diff --git a/lib/rubocop/cop/style/hash_as_last_array_item.rb b/lib/rubocop/cop/style/hash_as_last_array_item.rb new file mode 100644 index 00000000000..e14339729aa --- /dev/null +++ b/lib/rubocop/cop/style/hash_as_last_array_item.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # Checks for presence or absence of braces around hash literal as a last + # array item depending on configuration. + # + # @example EnforcedStyle: braces (default) + # # bad + # [1, 2, one: 1, two: 2] + # + # # good + # [1, 2, { one: 1, two: 2 }] + # + # @example EnforcedStyle: no_braces + # # bad + # [1, 2, { one: 1, two: 2 }] + # + # # good + # [1, 2, one: 1, two: 2] + # + class HashAsLastArrayItem < Base + include ConfigurableEnforcedStyle + extend AutoCorrector + + def on_hash(node) + return unless node.parent&.array_type? + + if braces_style? + check_braces(node) + else + check_no_braces(node) + end + end + + private + + def check_braces(node) + return if node.braces? + + add_offense(node, message: 'Wrap hash in `{` and `}`.') do |corrector| + corrector.wrap(node, '{', '}') + end + end + + def check_no_braces(node) + return unless node.braces? + + add_offense(node, message: 'Omit the braces around the hash.') do |corrector| + corrector.remove(node.loc.begin) + corrector.remove(node.loc.end) + end + end + + def braces_style? + style == :braces + end + end + end + end +end diff --git a/lib/rubocop/cop/style/hash_like_case.rb b/lib/rubocop/cop/style/hash_like_case.rb new file mode 100644 index 00000000000..51df79899c5 --- /dev/null +++ b/lib/rubocop/cop/style/hash_like_case.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # This cop checks for places where `case-when` represents a simple 1:1 + # mapping and can be replaced with a hash lookup. + # + # @example MinBranchesCount: 3 (default) + # # bad + # case country + # when 'europe' + # 'http://eu.example.com' + # when 'america' + # 'http://us.example.com' + # when 'australia' + # 'http://au.example.com' + # end + # + # # good + # SITES = { + # 'europe' => 'http://eu.example.com', + # 'america' => 'http://us.example.com', + # 'australia' => 'http://au.example.com' + # } + # SITES[country] + # + # @example MinBranchesCount: 4 + # # good + # case country + # when 'europe' + # 'http://eu.example.com' + # when 'america' + # 'http://us.example.com' + # when 'australia' + # 'http://au.example.com' + # end + # + class HashLikeCase < Base + MSG = 'Consider replacing `case-when` with a hash lookup.' + + def_node_matcher :hash_like_case?, <<~PATTERN + (case + _ + (when + ${str_type? sym_type?} + $[!nil? recursive_basic_literal?])+ nil?) + PATTERN + + def on_case(node) + return if node.when_branches.size < min_branches_count + + hash_like_case?(node) do |condition_nodes, body_nodes| + if nodes_of_same_type?(condition_nodes) && + nodes_of_same_type?(body_nodes) + add_offense(node) + end + end + end + + private + + def nodes_of_same_type?(nodes) + nodes.all? { |node| node.type == nodes.first.type } + end + + def min_branches_count + length = cop_config['MinBranchesCount'] || 3 + return length if length.is_a?(Integer) && length.positive? + + raise 'MinBranchesCount needs to be a positive integer!' + end + end + end + end +end diff --git a/lib/rubocop/cop/style/if_unless_modifier.rb b/lib/rubocop/cop/style/if_unless_modifier.rb index 12216045949..72a1c3e1f7c 100644 --- a/lib/rubocop/cop/style/if_unless_modifier.rb +++ b/lib/rubocop/cop/style/if_unless_modifier.rb @@ -41,11 +41,8 @@ class IfUnlessModifier < Cop MSG_USE_NORMAL = 'Modifier form of `%s` makes the line too long.' - ASSIGNMENT_TYPES = %i[lvasgn casgn cvasgn - gvasgn ivasgn masgn].freeze - def on_if(node) - msg = if eligible_node?(node) + msg = if single_line_as_modifier?(node) MSG_USE_MODIFIER unless named_capture_in_condition?(node) elsif too_long_due_to_modifier?(node) MSG_USE_NORMAL @@ -125,13 +122,15 @@ def named_capture_in_condition?(node) node.condition.match_with_lvasgn_type? end - def eligible_node?(node) - !non_eligible_if?(node) && !node.chained? && - !node.nested_conditional? && single_line_as_modifier?(node) + def non_eligible_node?(node) + non_simple_if_unless?(node) || + node.chained? || + node.nested_conditional? || + super end - def non_eligible_if?(node) - node.ternary? || node.modifier_form? || node.elsif? || node.else? + def non_simple_if_unless?(node) + node.ternary? || node.elsif? || node.else? end def another_statement_on_same_line?(node) @@ -153,8 +152,9 @@ def parenthesize?(node) # Parenthesize corrected expression if changing to modifier-if form # would change the meaning of the parent expression # (due to the low operator precedence of modifier-if) - return false if node.parent.nil? - return true if ASSIGNMENT_TYPES.include?(node.parent.type) + parent = node.parent + return false if parent.nil? + return true if parent.assignment? || parent.operator_keyword? node.parent.send_type? && !node.parent.parenthesized? end diff --git a/lib/rubocop/cop/style/missing_else.rb b/lib/rubocop/cop/style/missing_else.rb index 5b467d3c6c0..9e3c92df646 100644 --- a/lib/rubocop/cop/style/missing_else.rb +++ b/lib/rubocop/cop/style/missing_else.rb @@ -119,17 +119,7 @@ def on_case(node) private def check(node) - return if node.else? - - if empty_else_cop_enabled? - if empty_else_style == :empty - add_offense(node) - elsif empty_else_style == :nil - add_offense(node) - end - end - - add_offense(node) + add_offense(node) unless node.else? end def message(node) diff --git a/lib/rubocop/cop/style/numeric_predicate.rb b/lib/rubocop/cop/style/numeric_predicate.rb index b2c95fa5110..4f7a44bc263 100644 --- a/lib/rubocop/cop/style/numeric_predicate.rb +++ b/lib/rubocop/cop/style/numeric_predicate.rb @@ -54,15 +54,14 @@ class NumericPredicate < Cop }.freeze def on_send(node) + numeric, replacement = check(node) + return unless numeric + return if ignored_method?(node.method_name) || node.each_ancestor(:send, :block).any? do |ancestor| ignored_method?(ancestor.method_name) end - numeric, replacement = check(node) - - return unless numeric - add_offense(node, message: format(MSG, prefer: replacement, diff --git a/lib/rubocop/cop/style/parallel_assignment.rb b/lib/rubocop/cop/style/parallel_assignment.rb index 11a24027499..7633cc67e2a 100644 --- a/lib/rubocop/cop/style/parallel_assignment.rb +++ b/lib/rubocop/cop/style/parallel_assignment.rb @@ -30,7 +30,7 @@ class ParallelAssignment < Cop def on_masgn(node) lhs, rhs = *node lhs_elements = *lhs - rhs_elements = [*rhs].compact # edge case for one constant + rhs_elements = Array(rhs).compact # edge case for one constant return if allowed_lhs?(lhs) || allowed_rhs?(rhs) || allowed_masign?(lhs_elements, rhs_elements) @@ -42,7 +42,7 @@ def autocorrect(node) lambda do |corrector| left, right = *node left_elements = *left - right_elements = [*right].compact + right_elements = Array(right).compact order = find_valid_order(left_elements, right_elements) correction = assignment_corrector(node, order) @@ -69,7 +69,7 @@ def allowed_lhs?(node) def allowed_rhs?(node) # Edge case for one constant - elements = [*node].compact + elements = Array(node).compact # Account for edge case of `Constant::CONSTANT` !node.array_type? || diff --git a/lib/rubocop/cop/style/percent_literal_delimiters.rb b/lib/rubocop/cop/style/percent_literal_delimiters.rb index 0ce7e8ec46e..5911b766a4f 100644 --- a/lib/rubocop/cop/style/percent_literal_delimiters.rb +++ b/lib/rubocop/cop/style/percent_literal_delimiters.rb @@ -102,7 +102,7 @@ def contains_delimiter?(node, delimiters) delimiters_regexp = Regexp.union(delimiters) node .children.map { |n| string_source(n) }.compact - .any? { |s| delimiters_regexp.match?(s) } + .any? { |s| delimiters_regexp.match?(s.scrub) } end def string_source(node) diff --git a/lib/rubocop/cop/style/redundant_file_extension_in_require.rb b/lib/rubocop/cop/style/redundant_file_extension_in_require.rb new file mode 100644 index 00000000000..f1abd515ed6 --- /dev/null +++ b/lib/rubocop/cop/style/redundant_file_extension_in_require.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # This cop checks for the presence of superfluous `.rb` extension in + # the filename provided to `require` and `require_relative`. + # + # Note: If the extension is omitted, Ruby tries adding '.rb', '.so', + # and so on to the name until found. If the file named cannot be found, + # a `LoadError` will be raised. + # There is an edge case where `foo.so` file is loaded instead of a `LoadError` + # if `foo.so` file exists when `require 'foo.rb'` will be changed to `require 'foo'`, + # but that seems harmless. + # + # @example + # # bad + # require 'foo.rb' + # require_relative '../foo.rb' + # + # # good + # require 'foo' + # require 'foo.so' + # require_relative '../foo' + # require_relative '../foo.so' + # + class RedundantFileExtensionInRequire < Cop + MSG = 'Redundant `.rb` file extension detected.' + + def_node_matcher :require_call?, <<~PATTERN + (send nil? {:require :require_relative} $str_type?) + PATTERN + + def on_send(node) + require_call?(node) do |name_node| + add_offense(name_node) if name_node.value.end_with?('.rb') + end + end + + def autocorrect(node) + correction = node.value.sub(/\.rb\z/, '') + + lambda do |corrector| + corrector.replace(node, "'#{correction}'") + end + end + end + end + end +end diff --git a/lib/rubocop/cop/style/redundant_sort.rb b/lib/rubocop/cop/style/redundant_sort.rb index a610c7b0e5b..bcdb0403d87 100644 --- a/lib/rubocop/cop/style/redundant_sort.rb +++ b/lib/rubocop/cop/style/redundant_sort.rb @@ -135,9 +135,10 @@ def base(accessor, arg) end def suffix(sorter) - if sorter == :sort + case sorter + when :sort '' - elsif sorter == :sort_by + when :sort_by '_by' end end diff --git a/lib/rubocop/cop/style/stabby_lambda_parentheses.rb b/lib/rubocop/cop/style/stabby_lambda_parentheses.rb index 993decc58d8..304b3b9fe5d 100644 --- a/lib/rubocop/cop/style/stabby_lambda_parentheses.rb +++ b/lib/rubocop/cop/style/stabby_lambda_parentheses.rb @@ -34,9 +34,10 @@ def on_send(node) end def autocorrect(node) - if style == :require_parentheses + case style + when :require_parentheses missing_parentheses_corrector(node) - elsif style == :require_no_parentheses + when :require_no_parentheses unwanted_parentheses_corrector(node) end end diff --git a/lib/rubocop/cop/style/trailing_method_end_statement.rb b/lib/rubocop/cop/style/trailing_method_end_statement.rb index a49d0d1349f..d4f3309eb20 100644 --- a/lib/rubocop/cop/style/trailing_method_end_statement.rb +++ b/lib/rubocop/cop/style/trailing_method_end_statement.rb @@ -33,8 +33,8 @@ module Style # end # end # - class TrailingMethodEndStatement < Cop - include Alignment + class TrailingMethodEndStatement < Base + extend AutoCorrector MSG = 'Place the end statement of a multi-line method on ' \ 'its own line.' @@ -42,13 +42,11 @@ class TrailingMethodEndStatement < Cop def on_def(node) return unless trailing_end?(node) - add_offense(node, location: end_token(node).pos) - end - - def autocorrect(node) - lambda do |corrector| - break_line_before_end(node, corrector) - remove_semicolon(node, corrector) + add_offense(node.loc.end) do |corrector| + corrector.insert_before( + node.loc.end, + "\n" + ' ' * node.loc.keyword.column + ) end end @@ -60,30 +58,9 @@ def trailing_end?(node) body_and_end_on_same_line?(node) end - def end_token(node) - tokens(node).reverse.find(&:end?) - end - def body_and_end_on_same_line?(node) - end_token(node).line == token_before_end(node).line - end - - def token_before_end(node) - i = tokens(node).index(end_token(node)) - tokens(node)[i - 1] - end - - def break_line_before_end(node, corrector) - corrector.insert_before( - end_token(node).pos, - "\n" + ' ' * configured_indentation_width - ) - end - - def remove_semicolon(node, corrector) - return unless token_before_end(node).semicolon? - - corrector.remove(token_before_end(node).pos) + last_child = node.children.last + last_child.loc.last_line == node.loc.end.last_line end end end diff --git a/lib/rubocop/cop/variable_force/variable.rb b/lib/rubocop/cop/variable_force/variable.rb index c96c94dadb8..3855726f0bd 100644 --- a/lib/rubocop/cop/variable_force/variable.rb +++ b/lib/rubocop/cop/variable_force/variable.rb @@ -42,10 +42,10 @@ def referenced? def reference!(node) reference = Reference.new(node, @scope) @references << reference - consumed_branches = Set.new + consumed_branches = nil @assignments.reverse_each do |assignment| - next if consumed_branches.include?(assignment.branch) + next if consumed_branches&.include?(assignment.branch) assignment.reference!(node) unless assignment.run_exclusively_with?(reference) @@ -58,7 +58,9 @@ def reference!(node) break if !assignment.branch || assignment.branch == reference.branch - consumed_branches << assignment.branch unless assignment.branch.may_run_incompletely? + unless assignment.branch.may_run_incompletely? + (consumed_branches ||= Set.new) << assignment.branch + end end end # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity diff --git a/lib/rubocop/file_finder.rb b/lib/rubocop/file_finder.rb index ed0c16eeab4..05dbb3ccd0a 100644 --- a/lib/rubocop/file_finder.rb +++ b/lib/rubocop/file_finder.rb @@ -20,12 +20,12 @@ def find_file_upwards(filename, start_dir) end end - def find_files_upwards(filename, start_dir) - files = [] + def find_last_file_upwards(filename, start_dir) + last_file = nil traverse_files_upwards(filename, start_dir) do |file| - files << file + last_file = file end - files + last_file end private diff --git a/lib/rubocop/path_util.rb b/lib/rubocop/path_util.rb index d9cde8d55aa..109d5b39c15 100644 --- a/lib/rubocop/path_util.rb +++ b/lib/rubocop/path_util.rb @@ -5,7 +5,7 @@ module RuboCop module PathUtil module_function - def relative_path(path, base_dir = PathUtil.pwd) + def relative_path(path, base_dir = Dir.pwd) # Optimization for the common case where path begins with the base # dir. Just cut off the first part. if path.start_with?(base_dir) @@ -24,7 +24,7 @@ def relative_path(path, base_dir = PathUtil.pwd) def smart_path(path) # Ideally, we calculate this relative to the project root. - base_dir = PathUtil.pwd + base_dir = Dir.pwd if path.start_with? base_dir relative_path(path, base_dir) @@ -54,21 +54,6 @@ def absolute?(path) %r{\A([A-Z]:)?/}i.match?(path) end - def self.pwd - @pwd ||= Dir.pwd - end - - def self.reset_pwd - @pwd = nil - end - - def self.chdir(dir, &block) - reset_pwd - Dir.chdir(dir, &block) - ensure - reset_pwd - end - def hidden_file_in_not_hidden_dir?(pattern, path) File.fnmatch?( pattern, path, diff --git a/lib/rubocop/result_cache.rb b/lib/rubocop/result_cache.rb index 5fb366bd337..50995cdaee3 100644 --- a/lib/rubocop/result_cache.rb +++ b/lib/rubocop/result_cache.rb @@ -33,7 +33,7 @@ class << self def requires_file_removal?(file_count, config_store) file_count > 1 && - file_count > config_store.for_dir('.').for_all_cops['MaxFilesInCache'] + file_count > config_store.for_pwd.for_all_cops['MaxFilesInCache'] end def remove_oldest_files(files, dirs, cache_root, verbose) @@ -60,7 +60,7 @@ def remove_files(files, dirs, remove_count) end def self.cache_root(config_store) - root = config_store.for_dir('.').for_all_cops['CacheRootDirectory'] + root = config_store.for_pwd.for_all_cops['CacheRootDirectory'] root ||= if ENV.key?('XDG_CACHE_HOME') # Include user ID in the path to make sure the user has write # access. @@ -72,7 +72,7 @@ def self.cache_root(config_store) end def self.allow_symlinks_in_cache_location?(config_store) - config_store.for_dir('.').for_all_cops['AllowSymlinksInCacheRootDirectory'] + config_store.for_pwd.for_all_cops['AllowSymlinksInCacheRootDirectory'] end def initialize(file, team, options, config_store, cache_root = nil) @@ -158,6 +158,7 @@ class << self end # The checksum of the rubocop program running the inspection. + # rubocop:disable Metrics/AbcSize def rubocop_checksum ResultCache.source_checksum ||= begin @@ -168,13 +169,16 @@ def rubocop_checksum # exe directory. A change to any of them could affect the cop output # so we include them in the cache hash. source_files = $LOADED_FEATURES + Find.find(exe_root).to_a - sources = source_files - .select { |path| File.file?(path) } - .sort - .map { |path| IO.read(path, encoding: Encoding::UTF_8) } - Digest::SHA1.hexdigest(sources.join) + + digest = Digest::SHA1.new + source_files + .select { |path| File.file?(path) } + .sort! + .each { |path| digest << File.mtime(path).to_s } + digest.hexdigest end end + # rubocop:enable Metrics/AbcSize # Return a hash of the options given at invocation, minus the ones that have # no effect on which offenses and disabled line ranges are found, and thus diff --git a/lib/rubocop/rspec/expect_offense.rb b/lib/rubocop/rspec/expect_offense.rb index e305aeca1c8..b44eb1b664e 100644 --- a/lib/rubocop/rspec/expect_offense.rb +++ b/lib/rubocop/rspec/expect_offense.rb @@ -73,19 +73,20 @@ module RSpec # expect_no_corrections # # If your code has variables of different lengths, you can use `%{foo}`, - # `^{foo}`, and `_{foo}` to format your template: + # `^{foo}`, and `_{foo}` to format your template; you can also abbreviate + # offense messages with `[...]`: # # %w[raise fail].each do |keyword| # expect_offense(<<~RUBY, keyword: keyword) # %{keyword}(RuntimeError, msg) - # ^{keyword}^^^^^^^^^^^^^^^^^^^ Redundant `RuntimeError` argument can be removed. + # ^{keyword}^^^^^^^^^^^^^^^^^^^ Redundant `RuntimeError` argument [...] # RUBY # # %w[has_one has_many].each do |type| # expect_offense(<<~RUBY, type: type) # class Book # %{type} :chapter, foreign_key: 'book_id' - # _{type} ^^^^^^^^^^^^^^^^^^^^^^ Specifying the default value is redundant. + # _{type} ^^^^^^^^^^^^^^^^^^^^^^ Specifying the default [...] # end # RUBY # end @@ -102,6 +103,7 @@ module RSpec module ExpectOffense def format_offense(source, **replacements) replacements.each do |keyword, value| + value = value.to_s source = source.gsub("%{#{keyword}}", value) .gsub("^{#{keyword}}", '^' * value.size) .gsub("_{#{keyword}}", ' ' * value.size) @@ -132,7 +134,7 @@ def expect_offense(source, file = nil, severity: nil, **replacements) actual_annotations = expected_annotations.with_offense_annotations(offenses) - expect(actual_annotations.to_s).to eq(expected_annotations.to_s) + expect(actual_annotations).to eq(expected_annotations) expect(offenses.map(&:severity).uniq).to eq([severity]) if severity end @@ -188,6 +190,7 @@ def expect_no_offenses(source, file = nil) # Parsed representation of code annotated with the `^^^ Message` style class AnnotatedSource ANNOTATION_PATTERN = /\A\s*(\^+|\^{}) /.freeze + ABBREV = "[...]\n" # @param annotated_source [String] string passed to the matchers # @@ -206,6 +209,7 @@ def self.parse(annotated_source) source << source_line end end + annotations.each { |a| a[0] = 1 } if source.empty? new(source, annotations) end @@ -221,6 +225,27 @@ def initialize(lines, annotations) @annotations = annotations.sort.freeze end + def ==(other) + other.is_a?(self.class) && + other.lines == lines && + match_annotations?(other) + end + + # Dirty hack: expectations with [...] are rewritten when they match + # This way the diff is clean. + def match_annotations?(other) + annotations.zip(other.annotations) do |(_actual_line, actual_annotation), + (_expected_line, expected_annotation)| + if expected_annotation&.end_with?(ABBREV) + if actual_annotation.start_with?(expected_annotation[0...-ABBREV.length]) + expected_annotation.replace(actual_annotation) + end + end + end + + annotations == other.annotations + end + # Construct annotated source string (like what we parse) # # Reconstruct a deterministic annotated source string. This is @@ -253,6 +278,7 @@ def to_s reconstructed.join end + alias inspect to_s # Return the plain source code without annotations # @@ -279,7 +305,7 @@ def with_offense_annotations(offenses) self.class.new(lines, offense_annotations) end - private + protected attr_reader :lines, :annotations end diff --git a/lib/rubocop/rspec/shared_contexts.rb b/lib/rubocop/rspec/shared_contexts.rb index 026468cc52a..d3fb2081d60 100644 --- a/lib/rubocop/rspec/shared_contexts.rb +++ b/lib/rubocop/rspec/shared_contexts.rb @@ -24,7 +24,7 @@ working_dir = File.join(tmpdir, 'work') Dir.mkdir(working_dir) - RuboCop::PathUtil.chdir(working_dir) do + Dir.chdir(working_dir) do example.run end ensure diff --git a/lib/rubocop/runner.rb b/lib/rubocop/runner.rb index ea5970014b3..c1d8c3a9032 100644 --- a/lib/rubocop/runner.rb +++ b/lib/rubocop/runner.rb @@ -118,8 +118,7 @@ def process_file(file) def file_offenses(file) file_offense_cache(file) do - source = get_processed_source(file) - source, offenses = do_inspection_loop(file, source) + source, offenses = do_inspection_loop(file) offenses = add_redundant_disables(file, offenses.compact.sort, source) offenses.sort.reject(&:disabled?).freeze end @@ -159,8 +158,7 @@ def add_redundant_disables(file, offenses, source) # 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)) + _source, new_offenses = do_inspection_loop(file) offenses |= new_offenses end end @@ -213,7 +211,7 @@ def cached_run? @cached_run ||= (@options[:cache] == 'true' || @options[:cache] != 'false' && - @config_store.for_dir(Dir.pwd).for_all_cops['UseCache']) && + @config_store.for_pwd.for_all_cops['UseCache']) && # When running --auto-gen-config, there's some processing done in the # cops related to calculating the Max parameters for Metrics cops. We # need to do that processing and cannot use caching. @@ -232,7 +230,8 @@ def save_in_cache(cache, offenses) cache.save(offenses) end - def do_inspection_loop(file, processed_source) + def do_inspection_loop(file) + processed_source = get_processed_source(file) offenses = [] # When running with --auto-correct, we need to inspect the file (which diff --git a/lib/rubocop/target_finder.rb b/lib/rubocop/target_finder.rb index 224c86fcdb3..486fddfb24d 100644 --- a/lib/rubocop/target_finder.rb +++ b/lib/rubocop/target_finder.rb @@ -132,7 +132,7 @@ def ruby_filenames end def all_cops_include - @config_store.for('.').for_all_cops['Include'].map(&:to_s) + @config_store.for_pwd.for_all_cops['Include'].map(&:to_s) end def ruby_executable?(file) @@ -160,7 +160,7 @@ def ruby_file?(file) end def configured_include?(file) - @config_store.for('.').file_to_include?(file) + @config_store.for_pwd.file_to_include?(file) end def included_file?(file) diff --git a/lib/rubocop/version.rb b/lib/rubocop/version.rb index 256d34cd39e..935f4268e20 100644 --- a/lib/rubocop/version.rb +++ b/lib/rubocop/version.rb @@ -3,7 +3,7 @@ module RuboCop # This module holds the RuboCop version information. module Version - STRING = '0.87.0' + STRING = '0.87.1' MSG = '%s (using Parser %s, '\ 'rubocop-ast %s, ' \ diff --git a/relnotes/v0.87.1.md b/relnotes/v0.87.1.md new file mode 100644 index 00000000000..cc6115a9e65 --- /dev/null +++ b/relnotes/v0.87.1.md @@ -0,0 +1,14 @@ +### Bug fixes + +* [#8252](https://github.com/rubocop-hq/rubocop/issues/8252): Fix a command line option name from `--safe-autocorrect` to `--safe-auto-correct`, which is compatible with RuboCop 0.86 and lower. ([@koic][]) +* [#8259](https://github.com/rubocop-hq/rubocop/issues/8259): Fix false positives for `Style/BisectedAttrAccessor` when accessors have different access modifiers. ([@fatkodima][]) +* [#8253](https://github.com/rubocop-hq/rubocop/issues/8253): Fix false positives for `Style/AccessorGrouping` when accessors have different access modifiers. ([@fatkodima][]) +* [#8257](https://github.com/rubocop-hq/rubocop/issues/8257): Fix an error for `Style/BisectedAttrAccessor` when using `attr_reader` and `attr_writer` with splat arguments. ([@fatkodima][]) +* [#8239](https://github.com/rubocop-hq/rubocop/pull/8239): Don't load `.rubocop.yml` from personal folders to check for exclusions if given a custom configuration file. ([@deivid-rodriguez][]) +* [#8256](https://github.com/rubocop-hq/rubocop/issues/8256): Fix an error for `--auto-gen-config` when running a cop who do not support auto-correction. ([@koic][]) +* [#8262](https://github.com/rubocop-hq/rubocop/pull/8262): Fix `Lint/DeprecatedOpenSSLConstant` auto-correction of `OpenSSL::Cipher` to use lower case, as some Linux-based systems do not accept upper cased cipher names. ([@bdewater][]) + +[@koic]: https://github.com/koic +[@fatkodima]: https://github.com/fatkodima +[@deivid-rodriguez]: https://github.com/deivid-rodriguez +[@bdewater]: https://github.com/bdewater diff --git a/spec/rubocop/cli/cli_auto_gen_config_spec.rb b/spec/rubocop/cli/cli_auto_gen_config_spec.rb index 5f794a483cc..65becd178d4 100644 --- a/spec/rubocop/cli/cli_auto_gen_config_spec.rb +++ b/spec/rubocop/cli/cli_auto_gen_config_spec.rb @@ -497,7 +497,7 @@ def fooBar; end Layout/LineLength: Max: 95 YAML - RuboCop::PathUtil.chdir('dir') do + Dir.chdir('dir') do expect(cli.run(%w[--auto-gen-config])).to eq(0) end expect($stderr.string).to eq('') diff --git a/spec/rubocop/cli/cli_autocorrect_spec.rb b/spec/rubocop/cli/cli_autocorrect_spec.rb index d63617bd881..410a83b08fb 100644 --- a/spec/rubocop/cli/cli_autocorrect_spec.rb +++ b/spec/rubocop/cli/cli_autocorrect_spec.rb @@ -554,6 +554,7 @@ def verify_section EnforcedStyle: #{style} YAML expect(cli.run(['--auto-correct-all'])).to eq(1) + # rubocop:disable Style/HashLikeCase corrected = case style when :semantic <<~RUBY @@ -589,6 +590,7 @@ def verify_section end.baz RUBY end + # rubocop:enable Style/HashLikeCase expect($stderr.string).to eq('') expect(IO.read('example.rb')).to eq(corrected) end diff --git a/spec/rubocop/cli_spec.rb b/spec/rubocop/cli_spec.rb index 4f5918e7837..d3f3814ccfd 100644 --- a/spec/rubocop/cli_spec.rb +++ b/spec/rubocop/cli_spec.rb @@ -25,7 +25,7 @@ x = 1 RUBY create_empty_file('other/empty') - RuboCop::PathUtil.chdir('other') do + Dir.chdir('other') do expect(cli.run(['--format', 'simple', checked_path])).to eq(1) end expect($stdout.string).to eq(<<~RESULT) diff --git a/spec/rubocop/config_loader_spec.rb b/spec/rubocop/config_loader_spec.rb index e9cef8f3d97..d08b2ea6c26 100644 --- a/spec/rubocop/config_loader_spec.rb +++ b/spec/rubocop/config_loader_spec.rb @@ -1463,7 +1463,7 @@ def cop_enabled?(cop_class) it 'uses paths relative to the .rubocop.yml, not cwd' do config_path = described_class.configuration_file_for('.') - RuboCop::PathUtil.chdir '..' do + Dir.chdir '..' do described_class.configuration_from_file(config_path) expect(defined?(MyClass)).to be_truthy end @@ -1481,7 +1481,7 @@ def cop_enabled?(cop_class) it 'works without a starting .' do config_path = described_class.configuration_file_for('.') $LOAD_PATH.unshift(File.dirname(config_path)) - RuboCop::PathUtil.chdir '..' do + Dir.chdir '..' do described_class.configuration_from_file(config_path) expect(defined?(MyClass)).to be_truthy end diff --git a/spec/rubocop/cop/bundler/insecure_protocol_source_spec.rb b/spec/rubocop/cop/bundler/insecure_protocol_source_spec.rb index 07b26672ef6..131c03c5887 100644 --- a/spec/rubocop/cop/bundler/insecure_protocol_source_spec.rb +++ b/spec/rubocop/cop/bundler/insecure_protocol_source_spec.rb @@ -4,7 +4,7 @@ it 'registers an offense when using `source :gemcutter`' do expect_offense(<<~RUBY) source :gemcutter - ^^^^^^^^^^ The source `:gemcutter` is deprecated because HTTP requests are insecure. Please change your source to 'https://rubygems.org' if possible, or 'http://rubygems.org' if not. + ^^^^^^^^^^ The source `:gemcutter` is deprecated [...] RUBY expect_correction(<<~RUBY) @@ -15,7 +15,7 @@ it 'registers an offense when using `source :rubygems`' do expect_offense(<<~RUBY) source :rubygems - ^^^^^^^^^ The source `:rubygems` is deprecated because HTTP requests are insecure. Please change your source to 'https://rubygems.org' if possible, or 'http://rubygems.org' if not. + ^^^^^^^^^ The source `:rubygems` is deprecated [...] RUBY expect_correction(<<~RUBY) @@ -26,7 +26,7 @@ it 'registers an offense when using `source :rubyforge`' do expect_offense(<<~RUBY) source :rubyforge - ^^^^^^^^^^ The source `:rubyforge` is deprecated because HTTP requests are insecure. Please change your source to 'https://rubygems.org' if possible, or 'http://rubygems.org' if not. + ^^^^^^^^^^ The source `:rubyforge` is deprecated [...] RUBY expect_correction(<<~RUBY) diff --git a/spec/rubocop/cop/gemspec/required_ruby_version_spec.rb b/spec/rubocop/cop/gemspec/required_ruby_version_spec.rb index 54b976fd34d..44d7d4abe5f 100644 --- a/spec/rubocop/cop/gemspec/required_ruby_version_spec.rb +++ b/spec/rubocop/cop/gemspec/required_ruby_version_spec.rb @@ -2,8 +2,7 @@ RSpec.describe RuboCop::Cop::Gemspec::RequiredRubyVersion, :config do context 'target ruby version > 2.7', :ruby27 do - it 'registers an offense when `required_ruby_version` is lower than ' \ - '`TargetRubyVersion`' do + it 'registers an offense when `required_ruby_version` is specified with >= and is lower than `TargetRubyVersion`' do expect_offense(<<~RUBY, '/path/to/foo.gemspec') Gem::Specification.new do |spec| spec.required_ruby_version = '>= 2.6.0' @@ -12,6 +11,15 @@ RUBY end + it 'registers an offense when `required_ruby_version` is specified with ~> and is lower than `TargetRubyVersion`' do + expect_offense(<<~RUBY, '/path/to/foo.gemspec') + Gem::Specification.new do |spec| + spec.required_ruby_version = '~> 2.6.0' + ^^^^^^^^^^ `required_ruby_version` (2.6, declared in foo.gemspec) and `TargetRubyVersion` (2.7, which may be specified in .rubocop.yml) should be equal. + end + RUBY + end + describe 'false negatives' do it 'does not register an offense when `required_ruby_version` ' \ 'is assigned as a variable (string literal)' do @@ -37,8 +45,7 @@ end context 'target ruby version > 2.5', :ruby25 do - it 'registers an offense when `required_ruby_version` is higher than ' \ - '`TargetRubyVersion`' do + it 'registers an offense when `required_ruby_version` is specified with >= and is higher than `TargetRubyVersion`' do expect_offense(<<~RUBY, '/path/to/bar.gemspec') Gem::Specification.new do |spec| spec.required_ruby_version = '>= 2.6.0' @@ -46,11 +53,19 @@ end RUBY end + + it 'registers an offense when `required_ruby_version` is specified with ~> and is higher than `TargetRubyVersion`' do + expect_offense(<<~RUBY, '/path/to/bar.gemspec') + Gem::Specification.new do |spec| + spec.required_ruby_version = '~> 2.6.0' + ^^^^^^^^^^ `required_ruby_version` (2.6, declared in bar.gemspec) and `TargetRubyVersion` (2.5, which may be specified in .rubocop.yml) should be equal. + end + RUBY + end end context 'target ruby version > 2.6', :ruby26 do - it 'does not register an offense when `required_ruby_version` equals ' \ - '`TargetRubyVersion`' do + it 'does not register an offense when `required_ruby_version` is specified with >= and equals `TargetRubyVersion`' do expect_no_offenses(<<~RUBY) Gem::Specification.new do |spec| spec.required_ruby_version = '>= 2.6.0' @@ -58,8 +73,16 @@ RUBY end - it 'does not register an offense when `required_ruby_version` ' \ - '(omit patch version) equals `TargetRubyVersion`' do + it 'does not register an offense when `required_ruby_version` is specified with ~> and equals `TargetRubyVersion`' do + expect_no_offenses(<<~RUBY) + Gem::Specification.new do |spec| + spec.required_ruby_version = '~> 2.6.0' + end + RUBY + end + + it 'does not register an offense when `required_ruby_version` is specified with >= without a patch version and ' \ + 'equals `TargetRubyVersion`' do expect_no_offenses(<<~RUBY) Gem::Specification.new do |spec| spec.required_ruby_version = '>= 2.6' @@ -67,6 +90,15 @@ RUBY end + it 'does not register an offense when `required_ruby_version` is specified with ~> without a patch version and ' \ + 'equals `TargetRubyVersion`' do + expect_no_offenses(<<~RUBY) + Gem::Specification.new do |spec| + spec.required_ruby_version = '~> 2.6' + end + RUBY + end + it 'does not register an offense when lowest version of ' \ '`required_ruby_version` equals `TargetRubyVersion`' do expect_no_offenses(<<~RUBY) @@ -75,5 +107,25 @@ end RUBY end + + it 'registers an offense when `required_ruby_version` is specified with >= without a minor version and is lower ' \ + 'than `TargetRubyVersion`' do + expect_offense(<<~RUBY, 'bar.gemspec') + Gem::Specification.new do |spec| + spec.required_ruby_version = '>= 2' + ^^^^^^ `required_ruby_version` (2, declared in bar.gemspec) and `TargetRubyVersion` (2.6, which may be specified in .rubocop.yml) should be equal. + end + RUBY + end + + it 'registers an offense when `required_ruby_version` is specified with ~> without a minor version and is lower ' \ + 'than `TargetRubyVersion`' do + expect_offense(<<~RUBY, 'bar.gemspec') + Gem::Specification.new do |spec| + spec.required_ruby_version = '~> 2' + ^^^^^^ `required_ruby_version` (2, declared in bar.gemspec) and `TargetRubyVersion` (2.6, which may be specified in .rubocop.yml) should be equal. + end + RUBY + end end end diff --git a/spec/rubocop/cop/generator/require_file_injector_spec.rb b/spec/rubocop/cop/generator/require_file_injector_spec.rb index 8949e84e0cc..bef5d39e0ed 100644 --- a/spec/rubocop/cop/generator/require_file_injector_spec.rb +++ b/spec/rubocop/cop/generator/require_file_injector_spec.rb @@ -13,7 +13,7 @@ around do |example| Dir.mktmpdir('rubocop-require_file_injector_spec-') do |dir| - RuboCop::PathUtil.chdir(dir) do + Dir.chdir(dir) do Dir.mkdir('lib') example.run end diff --git a/spec/rubocop/cop/layout/empty_lines_around_access_modifier_spec.rb b/spec/rubocop/cop/layout/empty_lines_around_access_modifier_spec.rb index 20b56ddcee2..bca605caa63 100644 --- a/spec/rubocop/cop/layout/empty_lines_around_access_modifier_spec.rb +++ b/spec/rubocop/cop/layout/empty_lines_around_access_modifier_spec.rb @@ -397,6 +397,14 @@ def test; end end RUBY end + + it "does not register an offense when `end` immediately after #{access_modifier}" do + expect_no_offenses(<<~RUBY) + class Test + #{access_modifier} + end + RUBY + end end %w[public module_function].each do |access_modifier| diff --git a/spec/rubocop/cop/layout/multiline_array_brace_layout_spec.rb b/spec/rubocop/cop/layout/multiline_array_brace_layout_spec.rb index 8715583ba84..9eb0a66aadf 100644 --- a/spec/rubocop/cop/layout/multiline_array_brace_layout_spec.rb +++ b/spec/rubocop/cop/layout/multiline_array_brace_layout_spec.rb @@ -34,4 +34,26 @@ let(:open) { '[' } let(:close) { ']' } end + + context 'when comment present before closing brace' do + it 'corrects closing brace without crashing' do + expect_offense(<<~RUBY) + { + key1: [a, # comment 1 + b # comment 2 + ], + ^ The closing array brace must be on the same line as the last array element when the opening brace is on the same line as the first array element. + key2: 'foo' + } + RUBY + + expect_correction(<<~RUBY) + { + key1: [a, # comment 1 + b], # comment 2 + key2: 'foo' + } + RUBY + end + end end diff --git a/spec/rubocop/cop/layout/multiline_block_layout_spec.rb b/spec/rubocop/cop/layout/multiline_block_layout_spec.rb index 4ed60d77881..cc923aeb688 100644 --- a/spec/rubocop/cop/layout/multiline_block_layout_spec.rb +++ b/spec/rubocop/cop/layout/multiline_block_layout_spec.rb @@ -78,15 +78,49 @@ RUBY end - it 'does not register offenses when there are too many parameters to fit ' \ - 'on one line' do + it 'does not register offenses when there are too many parameters to fit on one line' do expect_no_offenses(<<~RUBY) some_result = lambda do | so_many, parameters, it_will, be_too_long, - for_one_line| + for_one_line, + line_length, + has_increased, + add_3_more| + do_something + end + RUBY + end + + it 'registers offenses when there are not too many parameters to fit on one line' do + expect_offense(<<~RUBY) + some_result = lambda do | + ^ Block argument expression is not on the same line as the block start. + so_many, + parameters, + it_will, + be_too_long, + for_one_line, + line_length, + has_increased, + add_3_mor| + do_something + end + RUBY + + expect_correction(<<~RUBY) + some_result = lambda do |so_many, parameters, it_will, be_too_long, for_one_line, line_length, has_increased, add_3_mor| + do_something + end + RUBY + end + + it 'considers the extra space required to join the lines together' do + expect_no_offenses(<<~RUBY) + some_result = lambda do + |so_many, parameters, it_will, be_too_long, for_one_line, line_length, has_increased, add_3_more| do_something end RUBY diff --git a/spec/rubocop/cop/layout/multiline_method_call_brace_layout_spec.rb b/spec/rubocop/cop/layout/multiline_method_call_brace_layout_spec.rb index 961ea1be578..27fc260e2e0 100644 --- a/spec/rubocop/cop/layout/multiline_method_call_brace_layout_spec.rb +++ b/spec/rubocop/cop/layout/multiline_method_call_brace_layout_spec.rb @@ -61,4 +61,20 @@ RUBY end end + + context 'when comment present before closing brace' do + it 'corrects closing brace without crashing' do + expect_offense(<<~RUBY) + super(bar(baz, + ham # comment + )) + ^ Closing method call brace must be on the same line as the last argument when opening brace is on the same line as the first argument. + RUBY + + expect_correction(<<~RUBY) + super(bar(baz, + ham)) # comment + RUBY + end + end end diff --git a/spec/rubocop/cop/lint/debugger_spec.rb b/spec/rubocop/cop/lint/debugger_spec.rb index 513883ec66d..66bf92da4de 100644 --- a/spec/rubocop/cop/lint/debugger_spec.rb +++ b/spec/rubocop/cop/lint/debugger_spec.rb @@ -1,64 +1,160 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Lint::Debugger, :config do - shared_examples_for 'debugger' do |name, src| - it "reports an offense for a #{name} call" do - inspect_source(src) - expect(cop.offenses.size).to eq(1) - expect(cop.messages) - .to eq(["Remove debugger entry point `#{src}`."]) - expect(cop.highlights).to eq([src]) + it 'reports an offense for a debugger call' do + expect_offense(<<~RUBY) + debugger + ^^^^^^^^ Remove debugger entry point `debugger`. + RUBY + end + + it 'reports an offense for a byebug call' do + expect_offense(<<~RUBY) + byebug + ^^^^^^ Remove debugger entry point `byebug`. + RUBY + end + + it 'reports an offense for a pry binding call' do + expect_offense(<<~RUBY) + binding.pry + ^^^^^^^^^^^ Remove debugger entry point `binding.pry`. + RUBY + end + + it 'reports an offense for a remote_pry binding call' do + expect_offense(<<~RUBY) + binding.remote_pry + ^^^^^^^^^^^^^^^^^^ Remove debugger entry point `binding.remote_pry`. + RUBY + end + + it 'reports an offense for a pry_remote binding call' do + expect_offense(<<~RUBY) + binding.pry_remote + ^^^^^^^^^^^^^^^^^^ Remove debugger entry point `binding.pry_remote`. + RUBY + end + + context 'with capybara debug method call' do + it 'reports an offense for save_and_open_page' do + expect_offense(<<~RUBY) + save_and_open_page + ^^^^^^^^^^^^^^^^^^ Remove debugger entry point `save_and_open_page`. + RUBY + end + + it 'reports an offense for save_and_open_screenshot' do + expect_offense(<<~RUBY) + save_and_open_screenshot + ^^^^^^^^^^^^^^^^^^^^^^^^ Remove debugger entry point `save_and_open_screenshot`. + RUBY + end + + it 'reports an offense for save_screenshot' do + expect_offense(<<~RUBY) + save_screenshot + ^^^^^^^^^^^^^^^ Remove debugger entry point `save_screenshot`. + RUBY end + + context 'with an argument' do + it 'reports an offense for save_and_open_page' do + expect_offense(<<~RUBY) + save_and_open_page foo + ^^^^^^^^^^^^^^^^^^^^^^ Remove debugger entry point `save_and_open_page foo`. + RUBY + end + + it 'reports an offense for save_and_open_screenshot' do + expect_offense(<<~RUBY) + save_and_open_screenshot foo + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Remove debugger entry point `save_and_open_screenshot foo`. + RUBY + end + + it 'reports an offense for save_screenshot' do + expect_offense(<<~RUBY) + save_screenshot foo + ^^^^^^^^^^^^^^^^^^^ Remove debugger entry point `save_screenshot foo`. + RUBY + end + end + end + + it 'reports an offense for a debugger with an argument call' do + expect_offense(<<~RUBY) + debugger foo + ^^^^^^^^^^^^ Remove debugger entry point `debugger foo`. + RUBY + end + + it 'reports an offense for a byebug with an argument call' do + expect_offense(<<~RUBY) + byebug foo + ^^^^^^^^^^ Remove debugger entry point `byebug foo`. + RUBY + end + + it 'reports an offense for a pry binding with an argument call' do + expect_offense(<<~RUBY) + binding.pry foo + ^^^^^^^^^^^^^^^ Remove debugger entry point `binding.pry foo`. + RUBY end - include_examples 'debugger', 'debugger', 'debugger' - include_examples 'debugger', 'byebug', 'byebug' - include_examples 'debugger', 'pry binding', - 'binding.pry' - include_examples 'debugger', 'pry binding', - 'binding.remote_pry' - include_examples 'debugger', 'pry binding', - 'binding.pry_remote' - include_examples 'debugger', - 'capybara debug method', - 'save_and_open_page' - include_examples 'debugger', - 'capybara debug method', - 'save_and_open_screenshot' - include_examples 'debugger', - 'capybara debug method', - 'save_screenshot' - include_examples 'debugger', 'debugger with an argument', 'debugger foo' - include_examples 'debugger', 'byebug with an argument', 'byebug foo' - include_examples 'debugger', - 'pry binding with an argument', - 'binding.pry foo' - include_examples 'debugger', - 'pry binding with an argument', - 'binding.remote_pry foo' - include_examples 'debugger', - 'pry binding with an argument', - 'binding.pry_remote foo' - include_examples 'debugger', - 'capybara debug method with an argument', - 'save_and_open_page foo' - include_examples 'debugger', - 'capybara debug method with an argument', - 'save_and_open_screenshot foo' - include_examples 'debugger', - 'capybara debug method with an argument', - 'save_screenshot foo' - - include_examples 'debugger', 'remote_byebug', 'remote_byebug' - include_examples 'debugger', 'web console binding', 'binding.console' + it 'reports an offense for a remote_pry binding with an argument call' do + expect_offense(<<~RUBY) + binding.remote_pry foo + ^^^^^^^^^^^^^^^^^^^^^^ Remove debugger entry point `binding.remote_pry foo`. + RUBY + end + + it 'reports an offense for a pry_remote binding with an argument call' do + expect_offense(<<~RUBY) + binding.pry_remote foo + ^^^^^^^^^^^^^^^^^^^^^^ Remove debugger entry point `binding.pry_remote foo`. + RUBY + end + + it 'reports an offense for a remote_byebug call' do + expect_offense(<<~RUBY) + remote_byebug + ^^^^^^^^^^^^^ Remove debugger entry point `remote_byebug`. + RUBY + end + + it 'reports an offense for a web console binding call' do + expect_offense(<<~RUBY) + binding.console + ^^^^^^^^^^^^^^^ Remove debugger entry point `binding.console`. + RUBY + end it 'does not report an offense for a non-pry binding' do expect_no_offenses('binding.pirate') end - include_examples 'debugger', 'debugger with Kernel', 'Kernel.debugger' - include_examples 'debugger', 'debugger with ::Kernel', '::Kernel.debugger' - include_examples 'debugger', 'binding.pry with Kernel', 'Kernel.binding.pry' + it 'reports an offense for a debugger with Kernel call' do + expect_offense(<<~RUBY) + Kernel.debugger + ^^^^^^^^^^^^^^^ Remove debugger entry point `Kernel.debugger`. + RUBY + end + + it 'reports an offense for a debugger with ::Kernel call' do + expect_offense(<<~RUBY) + ::Kernel.debugger + ^^^^^^^^^^^^^^^^^ Remove debugger entry point `::Kernel.debugger`. + RUBY + end + + it 'reports an offense for a binding.pry with Kernel call' do + expect_offense(<<~RUBY) + Kernel.binding.pry + ^^^^^^^^^^^^^^^^^^ Remove debugger entry point `Kernel.binding.pry`. + RUBY + end it 'does not report an offense for save_and_open_page with Kernel' do expect_no_offenses('Kernel.save_and_open_page') @@ -86,6 +182,17 @@ def method RUBY end - include_examples 'debugger', 'irb binding', 'binding.irb' - include_examples 'debugger', 'binding.irb with Kernel', 'Kernel.binding.irb' + it 'reports an offense for a irb binding call' do + expect_offense(<<~RUBY) + binding.irb + ^^^^^^^^^^^ Remove debugger entry point `binding.irb`. + RUBY + end + + it 'reports an offense for a binding.irb with Kernel call' do + expect_offense(<<~RUBY) + Kernel.binding.irb + ^^^^^^^^^^^^^^^^^^ Remove debugger entry point `Kernel.binding.irb`. + RUBY + end end diff --git a/spec/rubocop/cop/lint/disjunctive_assignment_in_constructor_spec.rb b/spec/rubocop/cop/lint/disjunctive_assignment_in_constructor_spec.rb index 50be552a04b..a7808dc65cf 100644 --- a/spec/rubocop/cop/lint/disjunctive_assignment_in_constructor_spec.rb +++ b/spec/rubocop/cop/lint/disjunctive_assignment_in_constructor_spec.rb @@ -43,7 +43,7 @@ def initialize end context 'LHS is ivar' do - it 'registers an offense' do + it 'registers an offense and corrects' do expect_offense(<<~RUBY) class Banana def initialize @@ -52,10 +52,18 @@ def initialize end end RUBY + + expect_correction(<<~RUBY) + class Banana + def initialize + @delicious = true + end + end + RUBY end context 'constructor calls super after assignment' do - it 'registers an offense' do + it 'registers an offense and corrects' do expect_offense(<<~RUBY) class Banana def initialize @@ -65,6 +73,15 @@ def initialize end end RUBY + + expect_correction(<<~RUBY) + class Banana + def initialize + @delicious = true + super + end + end + RUBY end end diff --git a/spec/rubocop/cop/lint/duplicate_elsif_condition_spec.rb b/spec/rubocop/cop/lint/duplicate_elsif_condition_spec.rb new file mode 100644 index 00000000000..9bc424c96b8 --- /dev/null +++ b/spec/rubocop/cop/lint/duplicate_elsif_condition_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Lint::DuplicateElsifCondition do + subject(:cop) { described_class.new } + + it 'registers an offense for repeated elsif conditions' do + expect_offense(<<~RUBY) + if x == 1 + elsif x == 2 + elsif x == 1 + ^^^^^^ Duplicate `elsif` condition detected. + end + RUBY + end + + it 'registers an offense for subsequent repeated elsif conditions' do + expect_offense(<<~RUBY) + if x == 1 + elsif x == 2 + elsif x == 2 + ^^^^^^ Duplicate `elsif` condition detected. + end + RUBY + end + + it 'registers multiple offenses for multiple repeated elsif conditions' do + expect_offense(<<~RUBY) + if x == 1 + elsif x == 2 + elsif x == 1 + ^^^^^^ Duplicate `elsif` condition detected. + elsif x == 2 + ^^^^^^ Duplicate `elsif` condition detected. + end + RUBY + end + + it 'does not register an offense for non-repeated elsif conditions' do + expect_no_offenses(<<~RUBY) + if x == 1 + elsif x == 2 + else + end + RUBY + end + + it 'does not register an offense for partially repeated elsif conditions' do + expect_no_offenses(<<~RUBY) + if x == 1 + elsif x == 1 && x == 2 + end + RUBY + end +end diff --git a/spec/rubocop/cop/lint/duplicate_hash_key_spec.rb b/spec/rubocop/cop/lint/duplicate_hash_key_spec.rb index fe9786bb387..908cc5a851f 100644 --- a/spec/rubocop/cop/lint/duplicate_hash_key_spec.rb +++ b/spec/rubocop/cop/lint/duplicate_hash_key_spec.rb @@ -42,11 +42,10 @@ shared_examples 'duplicated literal key' do |key| it "registers an offense for duplicated `#{key}` hash keys" do - inspect_source("hash = { #{key} => 1, #{key} => 4}") - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.first.message) - .to eq('Duplicated key in hash literal.') - expect(cop.highlights).to eq [key] + expect_offense(<<~RUBY, key: key) + hash = { %{key} => 1, %{key} => 4} + _{key} ^{key} Duplicated key in hash literal. + RUBY end end diff --git a/spec/rubocop/cop/lint/duplicate_methods_spec.rb b/spec/rubocop/cop/lint/duplicate_methods_spec.rb index 3c7eaa30816..71da331b618 100644 --- a/spec/rubocop/cop/lint/duplicate_methods_spec.rb +++ b/spec/rubocop/cop/lint/duplicate_methods_spec.rb @@ -168,7 +168,7 @@ def some_method it 'registers an offense for a duplicate class method in separate ' \ "#{type} blocks" do - inspect_source(<<~RUBY) + expect_offense(<<~RUBY, 'test.rb') #{opening_line} def self.some_method implement 1 @@ -176,15 +176,15 @@ def self.some_method end #{opening_line} def self.some_method + ^^^^^^^^^^^^^^^^^^^^ Method `A.some_method` is defined at both test.rb:2 and test.rb:7. implement 2 end end RUBY - expect(cop.offenses.size).to eq(1) end it 'registers offense for a duplicate instance method in separate files' do - inspect_source(<<~RUBY, 'first.rb') + expect_no_offenses(<<~RUBY, 'first.rb') #{opening_line} def some_method implement 1 @@ -240,19 +240,19 @@ def self.another it 'registers an offense when class << exp is used' do pending - inspect_source(<<~RUBY, 'test.rb') + expect_offense(<<~RUBY, 'test.rb') #{opening_line} class << blah def some_method implement 1 end def some_method + ^^^^^^^^^^^^^^^ Method `A#some_method` is defined at both test.rb:3 and test.rb:6. implement 2 end end end RUBY - expect(cop.offenses.empty?).to be(false) end it "registers an offense for duplicate alias in #{type}" do diff --git a/spec/rubocop/cop/lint/empty_when_spec.rb b/spec/rubocop/cop/lint/empty_when_spec.rb index c95388d80d3..8cd562ed774 100644 --- a/spec/rubocop/cop/lint/empty_when_spec.rb +++ b/spec/rubocop/cop/lint/empty_when_spec.rb @@ -1,176 +1,187 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Lint::EmptyWhen, :config do - before do - inspect_source(source) - end + let(:cop_config) { { 'AllowComments' => false } } - shared_examples 'code with offense' do |code, expected = nil| - context "when checking #{code}" do - let(:source) { code } + context 'when a `when` body is missing' do + it 'registers an offense for a missing when body' do + expect_offense(<<~RUBY) + case foo + when :bar then 1 + when :baz # nothing + ^^^^^^^^^ Avoid `when` branches without a body. + end + RUBY + expect_no_corrections + end - it 'registers an offense' do - expect(cop.offenses.size).to eq(1) - expect(cop.messages).to eq([message]) - end + it 'registers an offense for missing when body followed by else' do + expect_offense(<<~RUBY) + case foo + when :bar then 1 + when :baz # nothing + ^^^^^^^^^ Avoid `when` branches without a body. + else 3 + end + RUBY + expect_no_corrections + end + + it 'registers an offense for missing when ... then body' do + expect_offense(<<~RUBY) + case foo + when :bar then 1 + when :baz then # nothing + ^^^^^^^^^ Avoid `when` branches without a body. + end + RUBY + expect_no_corrections + end - if expected - it 'auto-corrects' do - expect(autocorrect_source(code)).to eq(expected) + it 'registers an offense for missing when ... then body followed by else' do + expect_offense(<<~RUBY) + case foo + when :bar then 1 + when :baz then # nothing + ^^^^^^^^^ Avoid `when` branches without a body. + else 3 end - else - it 'does not auto-correct' do - expect(autocorrect_source(code)).to eq(code) + RUBY + expect_no_corrections + end + + it 'registers an offense for missing when body with a comment' do + expect_offense(<<~RUBY) + case foo + when :bar + 1 + when :baz + ^^^^^^^^^ Avoid `when` branches without a body. + # nothing end - end + RUBY + expect_no_corrections end - end - shared_examples 'code without offense' do |code| - let(:source) { code } + it 'registers an offense for missing when body with a comment ' \ + 'followed by else' do + expect_offense(<<~RUBY) + case foo + when :bar + 1 + when :baz + ^^^^^^^^^ Avoid `when` branches without a body. + # nothing + else + 3 + end + RUBY + expect_no_corrections + end - it 'does not register an offense' do - expect(cop.offenses.empty?).to be(true) + it 'registers an offense when case line has no expression' do + expect_offense(<<~RUBY) + case + when :bar + 1 + when :baz + ^^^^^^^^^ Avoid `when` branches without a body. + # nothing + else + 3 + end + RUBY + expect_no_corrections end end - let(:cop_config) { { 'AllowComments' => false } } + context 'when a `when` body is present' do + it 'accepts case with when ... then statements' do + expect_no_offenses(<<~RUBY) + case foo + when :bar then 1 + when :baz then 2 + end + RUBY + end - let(:message) { 'Avoid `when` branches without a body.' } + it 'accepts case with when ... then statements and else clause' do + expect_no_offenses(<<~RUBY) + case foo + when :bar then 1 + when :baz then 2 + else 3 + end + RUBY + end - context 'when a `when` body is missing' do - it_behaves_like 'code with offense', <<~RUBY - case foo - when :bar then 1 - when :baz # nothing - end - RUBY - - it_behaves_like 'code with offense', <<~RUBY - case foo - when :bar then 1 - when :baz # nothing - else 3 - end - RUBY - - it_behaves_like 'code with offense', <<~RUBY - case foo - when :bar then 1 - when :baz then # nothing - end - RUBY - - it_behaves_like 'code with offense', <<~RUBY - case foo - when :bar then 1 - when :baz then # nothing - else 3 - end - RUBY - - it_behaves_like 'code with offense', <<~RUBY - case foo - when :bar - 1 - when :baz - # nothing - end - RUBY - - it_behaves_like 'code with offense', <<~RUBY - case foo - when :bar - 1 - when :baz - # nothing - else - 3 - end - RUBY - - it_behaves_like 'code with offense', <<~RUBY - case - when :bar - 1 - when :baz - # nothing - else - 3 - end - RUBY - end + it 'accepts case with when bodies' do + expect_no_offenses(<<~RUBY) + case foo + when :bar + 1 + when :baz + 2 + end + RUBY + end - context 'when a `when` body is present' do - it_behaves_like 'code without offense', <<~RUBY - case foo - when :bar then 1 - when :baz then 2 - end - RUBY - - it_behaves_like 'code without offense', <<~RUBY - case foo - when :bar then 1 - when :baz then 2 - else 3 - end - RUBY - - it_behaves_like 'code without offense', <<~RUBY - case foo - when :bar - 1 - when :baz - 2 - end - RUBY - - it_behaves_like 'code without offense', <<~RUBY - case foo - when :bar - 1 - when :baz - 2 - else - 3 - end - RUBY - it_behaves_like 'code without offense', <<~RUBY - case - when :bar - 1 - when :baz - 2 - else - 3 - end - RUBY + it 'accepts case with when bodies and else clause' do + expect_no_offenses(<<~RUBY) + case foo + when :bar + 1 + when :baz + 2 + else + 3 + end + RUBY + end + + it 'accepts with no case line expression' do + expect_no_offenses(<<~RUBY) + case + when :bar + 1 + when :baz + 2 + else + 3 + end + RUBY + end end context 'when `AllowComments: true`' do let(:cop_config) { { 'AllowComments' => true } } - it_behaves_like 'code without offense', <<~RUBY - case condition - when foo - do_something - when bar - # do nothing - end - RUBY + it 'accepts an empty when body with a comment' do + expect_no_offenses(<<~RUBY) + case condition + when foo + do_something + when bar + # do nothing + end + RUBY + end end context 'when `AllowComments: false`' do let(:cop_config) { { 'AllowComments' => false } } - it_behaves_like 'code with offense', <<~RUBY - case condition - when foo - do_something - when bar - # do nothing - end - RUBY + it 'registers an offense for empty when body with a comment' do + expect_offense(<<~RUBY) + case condition + when foo + do_something + when bar + ^^^^^^^^ Avoid `when` branches without a body. + # do nothing + end + RUBY + expect_no_corrections + end end end diff --git a/spec/rubocop/cop/lint/implicit_string_concatenation_spec.rb b/spec/rubocop/cop/lint/implicit_string_concatenation_spec.rb index 71556c76ceb..8ac200a45ac 100644 --- a/spec/rubocop/cop/lint/implicit_string_concatenation_spec.rb +++ b/spec/rubocop/cop/lint/implicit_string_concatenation_spec.rb @@ -33,11 +33,21 @@ class B; 'ghi' 'jkl'; end context 'when the string literals contain newlines' do it 'registers an offense' do - inspect_source(<<~RUBY) - def method; "ab\nc" "de\nf"; end + expect_offense(<<~'RUBY') + def method + "ab + ^^^ Combine "ab\nc" and "de\nf" into a single string literal, [...] + c" "de + f" + end RUBY + end - expect(cop.offenses.size).to eq(1) + it 'does not register an offense for a single string' do + expect_no_offenses(<<~RUBY) + 'abc + def' + RUBY end end diff --git a/spec/rubocop/cop/lint/literal_as_condition_spec.rb b/spec/rubocop/cop/lint/literal_as_condition_spec.rb index 225d5f48722..a60d1789ae7 100644 --- a/spec/rubocop/cop/lint/literal_as_condition_spec.rb +++ b/spec/rubocop/cop/lint/literal_as_condition_spec.rb @@ -5,67 +5,67 @@ %w(1 2.0 [1] {} :sym :"#{a}").each do |lit| it "registers an offense for literal #{lit} in if" do - inspect_source(<<~RUBY) - if #{lit} + expect_offense(<<~RUBY, lit: lit) + if %{lit} + ^{lit} Literal `#{lit}` appeared as a condition. top end RUBY - expect(cop.offenses.size).to eq(1) end it "registers an offense for literal #{lit} in while" do - inspect_source(<<~RUBY) - while #{lit} + expect_offense(<<~RUBY, lit: lit) + while %{lit} + ^{lit} Literal `#{lit}` appeared as a condition. top end RUBY - expect(cop.offenses.size).to eq(1) end it "registers an offense for literal #{lit} in post-loop while" do - inspect_source(<<~RUBY) + expect_offense(<<~RUBY, lit: lit) begin top - end while(#{lit}) + end while(%{lit}) + ^{lit} Literal `#{lit}` appeared as a condition. RUBY - expect(cop.offenses.size).to eq(1) end it "registers an offense for literal #{lit} in until" do - inspect_source(<<~RUBY) - until #{lit} + expect_offense(<<~RUBY, lit: lit) + until %{lit} + ^{lit} Literal `#{lit}` appeared as a condition. top end RUBY - expect(cop.offenses.size).to eq(1) end it "registers an offense for literal #{lit} in post-loop until" do - inspect_source(<<~RUBY) + expect_offense(<<~RUBY, lit: lit) begin top - end until #{lit} + end until %{lit} + ^{lit} Literal `#{lit}` appeared as a condition. RUBY - expect(cop.offenses.size).to eq(1) end it "registers an offense for literal #{lit} in case" do - inspect_source(<<~RUBY) - case #{lit} + expect_offense(<<~RUBY, lit: lit) + case %{lit} + ^{lit} Literal `#{lit}` appeared as a condition. when x then top end RUBY - expect(cop.offenses.size).to eq(1) end it "registers an offense for literal #{lit} in a when " \ 'of a case without anything after case keyword' do - inspect_source(<<~RUBY) + expect_offense(<<~RUBY, lit: lit) case - when #{lit} then top + when %{lit} then top + ^{lit} Literal `#{lit}` appeared as a condition. end RUBY - expect(cop.offenses.size).to eq(1) end it "accepts literal #{lit} in a when of a case with " \ @@ -78,39 +78,39 @@ end it "registers an offense for literal #{lit} in &&" do - inspect_source(<<~RUBY) - if x && #{lit} + expect_offense(<<~RUBY, lit: lit) + if x && %{lit} + ^{lit} Literal `#{lit}` appeared as a condition. top end RUBY - expect(cop.offenses.size).to eq(1) end it "registers an offense for literal #{lit} in complex cond" do - inspect_source(<<~RUBY) - if x && !(a && #{lit}) && y && z + expect_offense(<<~RUBY, lit: lit) + if x && !(a && %{lit}) && y && z + ^{lit} Literal `#{lit}` appeared as a condition. top end RUBY - expect(cop.offenses.size).to eq(1) end it "registers an offense for literal #{lit} in !" do - inspect_source(<<~RUBY) - if !#{lit} + expect_offense(<<~RUBY, lit: lit) + if !%{lit} + ^{lit} Literal `#{lit}` appeared as a condition. top end RUBY - expect(cop.offenses.size).to eq(1) end it "registers an offense for literal #{lit} in complex !" do - inspect_source(<<~RUBY) - if !(x && (y && #{lit})) + expect_offense(<<~RUBY, lit: lit) + if !(x && (y && %{lit})) + ^{lit} Literal `#{lit}` appeared as a condition. top end RUBY - expect(cop.offenses.size).to eq(1) end it "accepts literal #{lit} if it's not an and/or operand" do @@ -130,17 +130,17 @@ end it "registers an offense for `!#{lit}`" do - inspect_source(<<~RUBY) - !#{lit} + expect_offense(<<~RUBY, lit: lit) + !%{lit} + ^{lit} Literal `#{lit}` appeared as a condition. RUBY - expect(cop.offenses.size).to eq(1) end it "registers an offense for `not #{lit}`" do - inspect_source(<<~RUBY) - not(#{lit}) + expect_offense(<<~RUBY, lit: lit) + not(%{lit}) + ^{lit} Literal `#{lit}` appeared as a condition. RUBY - expect(cop.offenses.size).to eq(1) end end diff --git a/spec/rubocop/cop/lint/literal_in_interpolation_spec.rb b/spec/rubocop/cop/lint/literal_in_interpolation_spec.rb index 63704170b4d..dd4b2ddabe4 100644 --- a/spec/rubocop/cop/lint/literal_in_interpolation_spec.rb +++ b/spec/rubocop/cop/lint/literal_in_interpolation_spec.rb @@ -20,54 +20,69 @@ end shared_examples 'literal interpolation' do |literal, expected = literal| - it "registers an offense for #{literal} in interpolation" do - inspect_source(%("this is the \#{#{literal}}")) - expect(cop.offenses.size).to eq(1) - end - - it "has #{literal} as the message highlight" do - inspect_source(%("this is the \#{#{literal}}")) - expect(cop.highlights).to eq([literal.to_s]) - end - - it "removes interpolation around #{literal}" do - corrected = autocorrect_source(%("this is the \#{#{literal}}")) - expect(corrected).to eq(%("this is the #{expected}")) + it "registers an offense for #{literal} in interpolation " \ + 'and removes interpolation around it' do + expect_offense(<<~'RUBY', literal: literal) + "this is the #{%{literal}}" + ^{literal} Literal interpolation detected. + RUBY + expect_correction(<<~RUBY) + "this is the #{expected}" + RUBY end it "removes interpolation around #{literal} when there is more text" do - corrected = - autocorrect_source(%("this is the \#{#{literal}} literally")) - expect(corrected).to eq(%("this is the #{expected} literally")) + expect_offense(<<~'RUBY', literal: literal) + "this is the #{%{literal}} literally" + ^{literal} Literal interpolation detected. + RUBY + expect_correction(<<~RUBY) + "this is the #{expected} literally" + RUBY end it "removes interpolation around multiple #{literal}" do - corrected = - autocorrect_source(%("some \#{#{literal}} with \#{#{literal}} too")) - expect(corrected).to eq(%("some #{expected} with #{expected} too")) + expect_offense(<<~'RUBY', literal: literal) + "some #{%{literal}} with #{%{literal}} too" + ^{literal} Literal interpolation detected. + _{literal} ^{literal} Literal interpolation detected. + RUBY + expect_correction(<<~RUBY) + "some #{expected} with #{expected} too" + RUBY end context 'when there is non-literal and literal interpolation' do context 'when literal interpolation is before non-literal' do - it 'only remove interpolation around literal' do - corrected = - autocorrect_source(%("this is \#{#{literal}} with \#{a} now")) - expect(corrected).to eq(%("this is #{expected} with \#{a} now")) + it 'only removes interpolation around literal' do + expect_offense(<<~'RUBY', literal: literal) + "this is #{%{literal}} with #{a} now" + ^{literal} Literal interpolation detected. + RUBY + expect_correction(<<~RUBY) + "this is #{expected} with \#{a} now" + RUBY end end context 'when literal interpolation is after non-literal' do - it 'only remove interpolation around literal' do - corrected = - autocorrect_source(%("this is \#{a} with \#{#{literal}} now")) - expect(corrected).to eq(%("this is \#{a} with #{expected} now")) + it 'only removes interpolation around literal' do + expect_offense(<<~'RUBY', literal: literal) + "this is #{a} with #{%{literal}} now" + ^{literal} Literal interpolation detected. + RUBY + expect_correction(<<~RUBY) + "this is \#{a} with #{expected} now" + RUBY end end end it "registers an offense only for final #{literal} in interpolation" do - inspect_source(%("this is the \#{#{literal};#{literal}}")) - expect(cop.offenses.size).to eq(1) + expect_offense(<<~'RUBY', literal: literal) + "this is the #{%{literal};%{literal}}" + _{literal} ^{literal} Literal interpolation detected. + RUBY end end @@ -99,9 +114,14 @@ it_behaves_like('literal interpolation', '%i[ s1 s2 ]', '[\"s1\", \"s2\"]') it 'handles nested interpolations when auto-correction' do - corrected = autocorrect_source(%("this is \#{"\#{1}"} silly")) + expect_offense(<<~'RUBY') + "this is #{"#{1}"} silly" + ^ Literal interpolation detected. + RUBY # next iteration fixes this - expect(corrected).to eq %("this is \#{"1"} silly") + expect_correction(<<~'RUBY', loop: false) + "this is #{"1"} silly" + RUBY end shared_examples 'special keywords' do |keyword| @@ -111,20 +131,14 @@ RUBY end - it "does not try to autocorrect strings like #{keyword}" do - corrected = autocorrect_source(%("this is the \#{#{keyword}} silly")) - - expect(corrected).to eq(%("this is the \#{#{keyword}} silly")) - end - - it "registers an offense for interpolation after #{keyword}" do - inspect_source(%("this is the \#{#{keyword}} \#{1}")) - expect(cop.offenses.size).to eq(1) - end - - it "auto-corrects literal interpolation after #{keyword}" do - corrected = autocorrect_source(%("this is the \#{#{keyword}} \#{1}")) - expect(corrected).to eq(%("this is the \#{#{keyword}} 1")) + it "registers an offense and autocorrects interpolation after #{keyword}" do + expect_offense(<<~'RUBY', keyword: keyword) + "this is the #{%{keyword}} #{1}" + _{keyword} ^ Literal interpolation detected. + RUBY + expect_correction(<<~RUBY) + "this is the \#{#{keyword}} 1" + RUBY end end @@ -134,19 +148,16 @@ it_behaves_like('special keywords', '__ENCODING__') shared_examples 'non-special string literal interpolation' do |string| - it "registers an offense for #{string}" do - inspect_source(%("this is the \#{#{string}}")) - expect(cop.offenses.size).to eq(1) - end - - it "has #{string} in the message highlight" do - inspect_source(%("this is the \#{#{string}}")) - expect(cop.highlights).to eq([string]) - end + it "registers an offense for #{string} and removes the interpolation " \ + "and quotes around #{string}" do + expect_offense(<<~'RUBY', string: string) + "this is the #{%{string}}" + ^{string} Literal interpolation detected. + RUBY - it "removes the interpolation and quotes around #{string}" do - corrected = autocorrect_source(%("this is the \#{#{string}}")) - expect(corrected).to eq(%("this is the #{string.gsub(/'|"/, '')}")) + expect_correction(<<~RUBY) + "this is the #{string.gsub(/'|"/, '')}" + RUBY end end @@ -206,18 +217,33 @@ let(:expected) { '42' } it 'removes interpolation in symbols' do - corrected = autocorrect_source(%(:"this is the \#{#{literal}}")) - expect(corrected).to eq(%(:"this is the #{expected}")) + expect_offense(<<~'RUBY', literal: literal) + :"this is the #{%{literal}}" + ^{literal} Literal interpolation detected. + RUBY + expect_correction(<<~RUBY) + :"this is the #{expected}" + RUBY end it 'removes interpolation in backticks' do - corrected = autocorrect_source(%(\`this is the \#{#{literal}}\`)) - expect(corrected).to eq(%(\`this is the #{expected}\`)) + expect_offense(<<~'RUBY', literal: literal) + `this is the #{%{literal}}` + ^{literal} Literal interpolation detected. + RUBY + expect_correction(<<~RUBY) + \`this is the #{expected}\` + RUBY end it 'removes interpolation in regular expressions' do - corrected = autocorrect_source(%(/this is the \#{#{literal}}/)) - expect(corrected).to eq(%(/this is the #{expected}/)) + expect_offense(<<~'RUBY', literal: literal) + /this is the #{%{literal}}/ + ^{literal} Literal interpolation detected. + RUBY + expect_correction(<<~RUBY) + /this is the #{expected}/ + RUBY end end end diff --git a/spec/rubocop/cop/lint/multiple_comparison_spec.rb b/spec/rubocop/cop/lint/multiple_comparison_spec.rb index 490090ba9af..3b16c7a6303 100644 --- a/spec/rubocop/cop/lint/multiple_comparison_spec.rb +++ b/spec/rubocop/cop/lint/multiple_comparison_spec.rb @@ -6,23 +6,15 @@ let(:config) { RuboCop::Config.new } shared_examples 'Check to use two comparison operator' do |operator1, operator2| - bad_source = "x #{operator1} y #{operator2} z" - good_source = "x #{operator1} y && y #{operator2} z" - - it "registers an offense for #{bad_source}" do - inspect_source(bad_source) - expect(cop.offenses.size).to eq(1) - expect(cop.messages) - .to eq(['Use the `&&` operator to compare multiple values.']) - end - - it 'autocorrects' do - new_source = autocorrect_source(bad_source) - expect(new_source).to eq(good_source) - end - - it "accepts for #{good_source}" do - expect_no_offenses(good_source) + it "registers an offense for x #{operator1} y #{operator2} z" do + expect_offense(<<~RUBY, operator1: operator1, operator2: operator2) + x %{operator1} y %{operator2} z + ^^^{operator1}^^^^{operator2}^^ Use the `&&` operator to compare multiple values. + RUBY + + expect_correction(<<~RUBY) + x #{operator1} y && y #{operator2} z + RUBY end end diff --git a/spec/rubocop/cop/lint/next_without_accumulator_spec.rb b/spec/rubocop/cop/lint/next_without_accumulator_spec.rb index eea8dce56d3..40625f9ca9a 100644 --- a/spec/rubocop/cop/lint/next_without_accumulator_spec.rb +++ b/spec/rubocop/cop/lint/next_without_accumulator_spec.rb @@ -3,50 +3,37 @@ RSpec.describe RuboCop::Cop::Lint::NextWithoutAccumulator do subject(:cop) { described_class.new } - def code_without_accumulator(method_name) - <<-RUBY - (1..4).#{method_name}(0) do |acc, i| - next if i.odd? - acc + i - end - RUBY - end - - def code_with_accumulator(method_name) - <<-RUBY - (1..4).#{method_name}(0) do |acc, i| - next acc if i.odd? - acc + i - end - RUBY - end - - def code_with_nested_block(method_name) - <<-RUBY - [(1..3), (4..6)].#{method_name}(0) do |acc, elems| - elems.each_with_index do |elem, i| - next if i == 1 - acc << elem - end - acc - end - RUBY - end - shared_examples 'reduce/inject' do |reduce_alias| context "given a #{reduce_alias} block" do it 'registers an offense for a bare next' do - inspect_source(code_without_accumulator(reduce_alias)) - expect(cop.offenses.size).to eq(1) - expect(cop.highlights).to eq(['next']) + expect_offense(<<~RUBY) + (1..4).#{reduce_alias}(0) do |acc, i| + next if i.odd? + ^^^^ Use `next` with an accumulator argument in a `reduce`. + acc + i + end + RUBY end it 'accepts next with a value' do - expect_no_offenses(code_with_accumulator(reduce_alias)) + expect_no_offenses(<<~RUBY) + (1..4).#{reduce_alias}(0) do |acc, i| + next acc if i.odd? + acc + i + end + RUBY end it 'accepts next within a nested block' do - expect_no_offenses(code_with_nested_block(reduce_alias)) + expect_no_offenses(<<~RUBY) + [(1..3), (4..6)].#{reduce_alias}(0) do |acc, elems| + elems.each_with_index do |elem, i| + next if i == 1 + acc << elem + end + acc + end + RUBY end end end diff --git a/spec/rubocop/cop/lint/non_local_exit_from_iterator_spec.rb b/spec/rubocop/cop/lint/non_local_exit_from_iterator_spec.rb index bd881173fe7..20d52b91eff 100644 --- a/spec/rubocop/cop/lint/non_local_exit_from_iterator_spec.rb +++ b/spec/rubocop/cop/lint/non_local_exit_from_iterator_spec.rb @@ -3,64 +3,46 @@ RSpec.describe RuboCop::Cop::Lint::NonLocalExitFromIterator do subject(:cop) { described_class.new } - context 'inspection' do - before do - inspect_source(source) - end - - let(:message) do - 'Non-local exit from iterator, without return value. ' \ - '`next`, `break`, `Array#find`, `Array#any?`, etc. is preferred.' - end - - shared_examples_for 'offense detector' do + context 'when block is followed by method chain' do + context 'and has single argument' do it 'registers an offense' do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.first.message).to eq(message) - expect(cop.offenses.first.severity.name).to eq(:warning) - expect(cop.highlights).to eq(['return']) - end - end - - context 'when block is followed by method chain' do - context 'and has single argument' do - let(:source) { <<-RUBY } + expect_offense(<<~RUBY) items.each do |item| return if item.stock == 0 + ^^^^^^ Non-local exit from iterator, [...] item.update!(foobar: true) end RUBY - - it_behaves_like('offense detector') - it { expect(cop.offenses.first.line).to eq(2) } end + end - context 'and has multiple arguments' do - let(:source) { <<-RUBY } + context 'and has multiple arguments' do + it 'registers an offense' do + expect_offense(<<~RUBY) items.each_with_index do |item, i| return if item.stock == 0 + ^^^^^^ Non-local exit from iterator, [...] item.update!(foobar: true) end RUBY - - it_behaves_like('offense detector') - it { expect(cop.offenses.first.line).to eq(2) } end + end - context 'and has no argument' do - let(:source) { <<-RUBY } + context 'and has no argument' do + it 'allows' do + expect_no_offenses(<<~RUBY) item.with_lock do return if item.stock == 0 item.update!(foobar: true) end RUBY - - it { expect(cop.offenses.empty?).to be(true) } end end + end - context 'when block is not followed by method chain' do - let(:source) { <<-RUBY } + context 'when block is not followed by method chain' do + it 'allows' do + expect_no_offenses(<<~RUBY) transaction do return unless update_necessary? find_each do |item| @@ -69,12 +51,12 @@ end end RUBY - - it { expect(cop.offenses.empty?).to be(true) } end + end - context 'when block is lambda' do - let(:source) { <<-RUBY } + context 'when block is lambda' do + it 'allows' do + expect_no_offenses(<<~RUBY) items.each(lambda do |item| return if item.stock == 0 item.update!(foobar: true) @@ -84,12 +66,12 @@ item.update!(foobar: true) } RUBY - - it { expect(cop.offenses.empty?).to be(true) } end + end - context 'when lambda is inside of block followed by method chain' do - let(:source) { <<-RUBY } + context 'when lambda is inside of block followed by method chain' do + it 'allows' do + expect_no_offenses(<<~RUBY) RSpec.configure do |config| # some configuration @@ -106,97 +88,78 @@ end end RUBY - - it { expect(cop.offenses.empty?).to be(true) } end + end - context 'when block in middle of nest is followed by method chain' do - let(:source) { <<-RUBY } + context 'when block in middle of nest is followed by method chain' do + it 'registers offenses' do + expect_offense(<<~RUBY) transaction do return unless update_necessary? items.each do |item| return if item.nil? + ^^^^^^ Non-local exit from iterator, [...] item.with_lock do return if item.stock == 0 + ^^^^^^ Non-local exit from iterator, [...] item.very_complicated_update_operation! end end end RUBY + end + end - it 'registers offenses' do - expect(cop.offenses.size).to eq(2) - expect(cop.offenses[0].message).to eq(message) - expect(cop.offenses[0].severity.name).to eq(:warning) - expect(cop.offenses[0].line).to eq(4) - expect(cop.offenses[1].message).to eq(message) - expect(cop.offenses[1].severity.name).to eq(:warning) - expect(cop.offenses[1].line).to eq(6) - expect(cop.highlights).to eq(%w[return return]) + it 'allows return with value' do + expect_no_offenses(<<~RUBY) + def find_first_sold_out_item(items) + items.each do |item| + return item if item.stock == 0 + item.foobar! + end end - end + RUBY + end - context 'when return with value' do - let(:source) { <<-RUBY } - def find_first_sold_out_item(items) - items.each do |item| - return item if item.stock == 0 - item.foobar! - end + it 'allows return in define_method' do + expect_no_offenses(<<~RUBY) + [:method_one, :method_two].each do |method_name| + define_method(method_name) do + return if predicate? end - RUBY + end + RUBY + end - it { expect(cop.offenses.empty?).to be(true) } - end + it 'allows return in define_singleton_method' do + expect_no_offenses(<<~RUBY) + str = 'foo' + str.define_singleton_method :bar do |baz| + return unless baz + replace baz + end + RUBY + end - context 'when the message is define_method' do - let(:source) { <<-RUBY } - [:method_one, :method_two].each do |method_name| - define_method(method_name) do - return if predicate? + context 'when the return is within a nested method definition' do + it 'allows return in an instance method definition' do + expect_no_offenses(<<~RUBY) + Foo.configure do |c| + def bar + return if baz? end end RUBY - - it { expect(cop.offenses.empty?).to be(true) } end - context 'when the message is define_singleton_method' do - let(:source) { <<-RUBY } - str = 'foo' - str.define_singleton_method :bar do |baz| - return unless baz - replace baz + it 'allows return in a class method definition' do + expect_no_offenses(<<~RUBY) + Foo.configure do |c| + def self.bar + return if baz? + end end RUBY - - it { expect(cop.offenses.empty?).to be(true) } - end - - context 'when the return is within a nested method definition' do - context 'with an instance method definition' do - let(:source) { <<-RUBY } - Foo.configure do |c| - def bar - return if baz? - end - end - RUBY - - it { expect(cop.offenses.empty?).to be(true) } - end - - context 'with a class method definition' do - let(:source) { <<-RUBY } - Foo.configure do |c| - def self.bar - return if baz? - end - end - RUBY - - it { expect(cop.offenses.empty?).to be(true) } - end end end end diff --git a/spec/rubocop/cop/lint/rand_one_spec.rb b/spec/rubocop/cop/lint/rand_one_spec.rb index c253049427f..24f3bd7aa37 100644 --- a/spec/rubocop/cop/lint/rand_one_spec.rb +++ b/spec/rubocop/cop/lint/rand_one_spec.rb @@ -6,14 +6,10 @@ shared_examples 'offenses' do |source| describe source do it 'registers an offense' do - inspect_source(source) - expect(cop.messages).to eq( - [ - "`#{source}` always returns `0`. " \ - 'Perhaps you meant `rand(2)` or `rand`?' - ] - ) - expect(cop.highlights).to eq([source]) + expect_offense(<<~RUBY, source: source) + %{source} + ^{source} `#{source}` always returns `0`. [...] + RUBY end end end diff --git a/spec/rubocop/cop/lint/redundant_splat_expansion_spec.rb b/spec/rubocop/cop/lint/redundant_splat_expansion_spec.rb index da507a23654..0dce85591e2 100644 --- a/spec/rubocop/cop/lint/redundant_splat_expansion_spec.rb +++ b/spec/rubocop/cop/lint/redundant_splat_expansion_spec.rb @@ -3,8 +3,6 @@ RSpec.describe RuboCop::Cop::Lint::RedundantSplatExpansion do subject(:cop) { described_class.new } - let(:message) { 'Replace splat expansion with comma separated values.' } - it 'allows assigning to a splat' do expect_no_offenses('*, rhs = *node') end @@ -32,49 +30,61 @@ RUBY end - shared_examples 'splat literal assignment' do |literal| - it 'registers an offense for ' do - inspect_source("a = *#{literal}") - - expect(cop.messages).to eq([message]) - expect(cop.highlights).to eq(["*#{literal}"]) + shared_examples 'splat literal assignment' do |literal, corrects, as_array: literal| + it "registers an offense and #{corrects}" do + expect_offense(<<~RUBY, literal: literal) + a = *%{literal} + ^^{literal} Replace splat expansion with comma separated values. + RUBY + expect_correction(<<~RUBY) + a = #{as_array} + RUBY end end - shared_examples 'array splat expansion' do |literal| + shared_examples 'array splat expansion' do |literal, as_args: nil| context 'method parameters' do - it 'registers an offense' do - inspect_source("array.push(*#{literal})") - - expect(cop.messages) - .to eq(['Pass array contents as separate arguments.']) - expect(cop.highlights).to eq(["*#{literal}"]) + it 'registers an offense and converts to a list of arguments' do + expect_offense(<<~RUBY, literal: literal) + array.push(*%{literal}) + ^^{literal} Pass array contents as separate arguments. + RUBY + expect_correction(<<~RUBY) + array.push(#{as_args}) + RUBY end end - it_behaves_like 'splat literal assignment', literal + it_behaves_like 'splat literal assignment', literal, + 'removes the splat from array', as_array: literal end - shared_examples 'splat expansion' do |literal| + shared_examples 'splat expansion' do |literal, as_array: literal| context 'method parameters' do - it 'registers an offense' do - inspect_source("array.push(*#{literal})") - - expect(cop.messages).to eq([message]) - expect(cop.highlights).to eq(["*#{literal}"]) + it 'registers an offense and converts to an array' do + expect_offense(<<~RUBY, literal: literal) + array.push(*%{literal}) + ^^{literal} Replace splat expansion with comma separated values. + RUBY + expect_correction(<<~RUBY) + array.push(#{as_array}) + RUBY end end - it_behaves_like 'splat literal assignment', literal + it_behaves_like 'splat literal assignment', literal, + 'converts to an array', as_array: as_array end - it_behaves_like 'array splat expansion', '[1, 2, 3]' - it_behaves_like 'array splat expansion', '%w(one two three)' - it_behaves_like 'array splat expansion', '%W(one #{two} three)' - it_behaves_like 'splat expansion', "'a'" - it_behaves_like 'splat expansion', '"#{a}"' - it_behaves_like 'splat expansion', '1' - it_behaves_like 'splat expansion', '1.1' + it_behaves_like 'array splat expansion', '[1, 2, 3]', as_args: '1, 2, 3' + it_behaves_like 'array splat expansion', '%w(one two three)', + as_args: "'one', 'two', 'three'" + it_behaves_like 'array splat expansion', '%W(one #{two} three)', + as_args: '"one", "#{two}", "three"' + it_behaves_like 'splat expansion', "'a'", as_array: "['a']" + it_behaves_like 'splat expansion', '"#{a}"', as_array: '["#{a}"]' + it_behaves_like 'splat expansion', '1', as_array: '[1]' + it_behaves_like 'splat expansion', '1.1', as_array: '[1.1]' context 'assignment to splat expansion' do it 'registers an offense and corrects an array using a constructor' do @@ -323,74 +333,10 @@ end end - context 'autocorrect' do - context 'assignment to a splat expanded variable' do - it 'removes the splat from an array using []' do - new_source = autocorrect_source('a = *[1, 2, 3]') - - expect(new_source).to eq('a = [1, 2, 3]') - end - - it 'removes the splat from an array using %w' do - new_source = autocorrect_source('a = *%w(one two three)') - - expect(new_source).to eq('a = %w(one two three)') - end - - it 'removes the splat from an array using %W' do - new_source = autocorrect_source('a = *%W(one two three)') - - expect(new_source).to eq('a = %W(one two three)') - end - - it 'converts an expanded string to an array' do - new_source = autocorrect_source("a = *'a'") - - expect(new_source).to eq("a = ['a']") - end - - it 'converts an expanded string with interpolation to an array' do - new_source = autocorrect_source('a = *"#{a}"') - - expect(new_source).to eq('a = ["#{a}"]') - end - - it 'converts an expanded integer to an array' do - new_source = autocorrect_source('a = *1') - - expect(new_source).to eq('a = [1]') - end - - it 'converts an expanded float to an array' do - new_source = autocorrect_source('a = *1.1') - - expect(new_source).to eq('a = [1.1]') - end - end - - context 'splat expansion of method parameters' do - it 'removes the splat and brackets from []' do - new_source = autocorrect_source('foo(*[1, 2, 3])') - - expect(new_source).to eq('foo(1, 2, 3)') - end - - it 'changes %w to a list of words' do - new_source = autocorrect_source('foo(*%w(one two three))') - - expect(new_source).to eq("foo('one', 'two', 'three')") - end - - it 'changes %W to a list of words' do - new_source = autocorrect_source('foo(*%W(#{one} two three))') - - expect(new_source).to eq('foo("#{one}", "two", "three")') - end - end - end - - it_behaves_like 'array splat expansion', '%i(first second)' - it_behaves_like 'array splat expansion', '%I(first second #{third})' + it_behaves_like 'array splat expansion', '%i(first second)', + as_args: ':first, :second' + it_behaves_like 'array splat expansion', '%I(first second #{third})', + as_args: ':"first", :"second", :"#{third}"' context 'arrays being expanded with %i variants using splat expansion' do context 'splat expansion of method parameters' do @@ -418,16 +364,26 @@ end context 'splat expansion inside of an array' do - it 'changes %i to a list of symbols' do - new_source = autocorrect_source('[:a, :b, *%i(c d), :e]') + it 'registers an offense and corrects %i to a list of symbols' do + expect_offense(<<~RUBY) + [:a, :b, *%i(c d), :e] + ^^^^^^^^ Pass array contents as separate arguments. + RUBY - expect(new_source).to eq('[:a, :b, :c, :d, :e]') + expect_correction(<<~RUBY) + [:a, :b, :c, :d, :e] + RUBY end - it 'changes %I to a list of symbols' do - new_source = autocorrect_source('[:a, :b, *%I(#{one} two), :e]') + it 'registers an offense and changes %I to a list of symbols' do + expect_offense(<<~'RUBY') + [:a, :b, *%I(#{one} two), :e] + ^^^^^^^^^^^^^^^ Pass array contents as separate arguments. + RUBY - expect(new_source).to eq('[:a, :b, :"#{one}", :"two", :e]') + expect_correction(<<~'RUBY') + [:a, :b, :"#{one}", :"two", :e] + RUBY end end end diff --git a/spec/rubocop/cop/lint/rescue_type_spec.rb b/spec/rubocop/cop/lint/rescue_type_spec.rb index 966303a3e4c..a84ed4cf2d8 100644 --- a/spec/rubocop/cop/lint/rescue_type_spec.rb +++ b/spec/rubocop/cop/lint/rescue_type_spec.rb @@ -42,29 +42,17 @@ def foobar shared_examples 'offenses' do |rescues| context 'begin rescue' do context "rescuing from #{rescues}" do - let(:source) do - <<-RUBY + it 'registers an offense and auto-corrects' do + expect_offense(<<~RUBY, rescues: rescues) begin foo - rescue #{rescues} + rescue %{rescues} + ^^^^^^^^{rescues} Rescuing from `#{rescues}` will raise a `TypeError` instead of catching the actual exception. bar end RUBY - end - - it 'registers an offense' do - inspect_source(source) - - expect(cop.highlights).to eq(["rescue #{rescues}"]) - expect(cop.messages) - .to eq(["Rescuing from `#{rescues}` will raise a `TypeError` " \ - 'instead of catching the actual exception.']) - end - it 'auto-corrects' do - new_source = autocorrect_source(source) - - expect(new_source).to eq(<<-RUBY) + expect_correction(<<~RUBY) begin foo rescue @@ -75,29 +63,17 @@ def foobar end context "rescuing from #{rescues} before another exception" do - let(:source) do - <<-RUBY + it 'registers an offense and auto-corrects' do + expect_offense(<<~RUBY, rescues: rescues) begin foo - rescue #{rescues}, StandardError + rescue %{rescues}, StandardError + ^^^^^^^^{rescues}^^^^^^^^^^^^^^^ Rescuing from `#{rescues}` will raise a `TypeError` instead of catching the actual exception. bar end RUBY - end - it 'registers an offense' do - inspect_source(source) - - expect(cop.highlights).to eq(["rescue #{rescues}, StandardError"]) - expect(cop.messages) - .to eq(["Rescuing from `#{rescues}` will raise a `TypeError` " \ - 'instead of catching the actual exception.']) - end - - it 'auto-corrects' do - new_source = autocorrect_source(source) - - expect(new_source).to eq(<<-RUBY) + expect_correction(<<~RUBY) begin foo rescue StandardError @@ -108,29 +84,17 @@ def foobar end context "rescuing from #{rescues} after another exception" do - let(:source) do - <<-RUBY + it 'registers an offense and auto-corrects' do + expect_offense(<<~RUBY, rescues: rescues) begin foo - rescue StandardError, #{rescues} + rescue StandardError, %{rescues} + ^^^^^^^^^^^^^^^^^^^^^^^{rescues} Rescuing from `#{rescues}` will raise a `TypeError` instead of catching the actual exception. bar end RUBY - end - - it 'registers an offense' do - inspect_source(source) - - expect(cop.highlights).to eq(["rescue StandardError, #{rescues}"]) - expect(cop.messages) - .to eq(["Rescuing from `#{rescues}` will raise a `TypeError` " \ - 'instead of catching the actual exception.']) - end - - it 'auto-corrects' do - new_source = autocorrect_source(source) - expect(new_source).to eq(<<-RUBY) + expect_correction(<<~RUBY) begin foo rescue StandardError @@ -143,31 +107,19 @@ def foobar context 'begin rescue ensure' do context "rescuing from #{rescues}" do - let(:source) do - <<-RUBY + it 'registers an offense and auto-corrects' do + expect_offense(<<~RUBY, rescues: rescues) begin foo - rescue #{rescues} + rescue %{rescues} + ^^^^^^^^{rescues} Rescuing from `#{rescues}` will raise a `TypeError` instead of catching the actual exception. bar ensure baz end RUBY - end - - it 'registers an offense' do - inspect_source(source) - - expect(cop.highlights).to eq(["rescue #{rescues}"]) - expect(cop.messages) - .to eq(["Rescuing from `#{rescues}` will raise a `TypeError` " \ - 'instead of catching the actual exception.']) - end - it 'auto-corrects' do - new_source = autocorrect_source(source) - - expect(new_source).to eq(<<-RUBY) + expect_correction(<<~RUBY) begin foo rescue @@ -182,29 +134,17 @@ def foobar context 'def rescue' do context "rescuing from #{rescues}" do - let(:source) do - <<-RUBY + it 'registers an offense and auto-corrects' do + expect_offense(<<~RUBY, rescues: rescues) def foobar foo - rescue #{rescues} + rescue %{rescues} + ^^^^^^^^{rescues} Rescuing from `#{rescues}` will raise a `TypeError` instead of catching the actual exception. bar end RUBY - end - it 'registers an offense' do - inspect_source(source) - - expect(cop.highlights).to eq(["rescue #{rescues}"]) - expect(cop.messages) - .to eq(["Rescuing from `#{rescues}` will raise a `TypeError` " \ - 'instead of catching the actual exception.']) - end - - it 'auto-corrects' do - new_source = autocorrect_source(source) - - expect(new_source).to eq(<<-RUBY) + expect_correction(<<~RUBY) def foobar foo rescue @@ -217,31 +157,19 @@ def foobar context 'def rescue ensure' do context "rescuing from #{rescues}" do - let(:source) do - <<-RUBY + it 'registers an offense and auto-corrects' do + expect_offense(<<~RUBY, rescues: rescues) def foobar foo - rescue #{rescues} + rescue %{rescues} + ^^^^^^^^{rescues} Rescuing from `#{rescues}` will raise a `TypeError` instead of catching the actual exception. bar ensure baz end RUBY - end - - it 'registers an offense' do - inspect_source(source) - - expect(cop.highlights).to eq(["rescue #{rescues}"]) - expect(cop.messages) - .to eq(["Rescuing from `#{rescues}` will raise a `TypeError` " \ - 'instead of catching the actual exception.']) - end - - it 'auto-corrects' do - new_source = autocorrect_source(source) - expect(new_source).to eq(<<-RUBY) + expect_correction(<<~RUBY) def foobar foo rescue diff --git a/spec/rubocop/cop/lint/script_permission_spec.rb b/spec/rubocop/cop/lint/script_permission_spec.rb index f7c008133e9..9caaa324b93 100644 --- a/spec/rubocop/cop/lint/script_permission_spec.rb +++ b/spec/rubocop/cop/lint/script_permission_spec.rb @@ -36,6 +36,10 @@ #!/usr/bin/ruby ^^^^^^^^^^^^^^^ Script file #{filename} doesn't have execute permission. RUBY + expect_correction(<<~RUBY) + #!/usr/bin/ruby + RUBY + expect(file.stat.executable?).to be_truthy end end end @@ -71,17 +75,5 @@ expect_no_offenses(source) end end - - unless RuboCop::Platform.windows? - context 'auto-correct' do - it 'adds execute permissions to the file' do - File.write(file.path, source) - - autocorrect_source(file.read, file) - - expect(file.stat.executable?).to be_truthy - end - end - end end # rubocop:enable Style/NumericLiteralPrefix diff --git a/spec/rubocop/cop/lint/unified_integer_spec.rb b/spec/rubocop/cop/lint/unified_integer_spec.rb index d52291f784c..ad07d445778 100644 --- a/spec/rubocop/cop/lint/unified_integer_spec.rb +++ b/spec/rubocop/cop/lint/unified_integer_spec.rb @@ -4,32 +4,28 @@ shared_examples 'registers an offense' do |klass| context "when #{klass}" do context 'without any decorations' do - let(:source) { "1.is_a?(#{klass})" } - - it 'registers an offense' do - inspect_source(source) - expect(cop.offenses.size).to eq(1) - expect(cop.messages).to eq(["Use `Integer` instead of `#{klass}`."]) - end - - it 'autocorrects' do - new_source = autocorrect_source(source) - expect(new_source).to eq('1.is_a?(Integer)') + it 'registers an offense and autocorrects' do + expect_offense(<<~RUBY, klass: klass) + 1.is_a?(%{klass}) + ^{klass} Use `Integer` instead of `#{klass}`. + RUBY + + expect_correction(<<~RUBY) + 1.is_a?(Integer) + RUBY end end context 'when explicitly specified as toplevel constant' do - let(:source) { "1.is_a?(::#{klass})" } - it 'registers an offense' do - inspect_source(source) - expect(cop.offenses.size).to eq(1) - expect(cop.messages).to eq(["Use `Integer` instead of `#{klass}`."]) - end - - it 'autocorrects' do - new_source = autocorrect_source(source) - expect(new_source).to eq('1.is_a?(::Integer)') + expect_offense(<<~RUBY, klass: klass) + 1.is_a?(::%{klass}) + ^^^{klass} Use `Integer` instead of `#{klass}`. + RUBY + + expect_correction(<<~RUBY) + 1.is_a?(::Integer) + RUBY end end diff --git a/spec/rubocop/cop/lint/unused_block_argument_spec.rb b/spec/rubocop/cop/lint/unused_block_argument_spec.rb index ac358974a1f..5357665ce9e 100644 --- a/spec/rubocop/cop/lint/unused_block_argument_spec.rb +++ b/spec/rubocop/cop/lint/unused_block_argument_spec.rb @@ -3,14 +3,6 @@ RSpec.describe RuboCop::Cop::Lint::UnusedBlockArgument, :config do let(:cop_config) { { 'AllowUnusedKeywordArguments' => false } } - shared_examples 'auto-correction' do |name, old_source, new_source| - it "auto-corrects #{name}" do - corrected_source = autocorrect_source(old_source) - - expect(corrected_source).to eq(new_source) - end - end - context 'inspection' do context 'when a block takes multiple arguments' do context 'and an argument is unused' do @@ -25,6 +17,19 @@ puts key end RUBY + expect_correction(<<~RUBY) + hash.each do |key, _value| + puts key + end + RUBY + end + end + + context 'and all arguments are used' do + it 'accepts' do + expect_no_offenses(<<~RUBY) + obj.method { |foo, bar| stuff(foo, bar) } + RUBY end end @@ -51,6 +56,47 @@ key, value = value, 42 end RUBY + expect_correction(<<~RUBY) + hash.each do |_key, value| + key, value = value, 42 + end + RUBY + end + end + + context 'and a splat argument is unused' do + it 'registers an offense and preserves splat' do + message = 'Unused block argument - `bars`. ' \ + "If it's necessary, use `_` or `_bars` as an argument " \ + "name to indicate that it won't be used." + + expect_offense(<<~RUBY) + obj.method { |foo, *bars, baz| stuff(foo, baz) } + ^^^^ #{message} + RUBY + expect_correction(<<~RUBY) + obj.method { |foo, *_bars, baz| stuff(foo, baz) } + RUBY + end + end + + context 'and an argument with default value is unused' do + it 'registers an offense and preserves default value' do + message = 'Unused block argument - `bar`. ' \ + "If it's necessary, use `_` or `_bar` as an argument " \ + "name to indicate that it won't be used." + + expect_offense(<<~RUBY) + obj.method do |foo, bar = baz| + ^^^ #{message} + stuff(foo) + end + RUBY + expect_correction(<<~RUBY) + obj.method do |foo, _bar = baz| + stuff(foo) + end + RUBY end end @@ -69,6 +115,36 @@ puts :something end RUBY + expect_correction(<<~RUBY) + hash = { foo: 'FOO', bar: 'BAR' } + hash.each do |_key, _value| + puts :something + end + RUBY + end + + context 'and unused arguments span multiple lines' do + it 'registers offenses and suggests omitting them' do + key_message, value_message = %w[key value].map do |arg| + "Unused block argument - `#{arg}`. You can omit all the " \ + "arguments if you don't care about them." + end + + expect_offense(<<~RUBY) + hash.each do |key, + ^^^ #{key_message} + value| + ^^^^^ #{value_message} + puts :something + end + RUBY + expect_correction(<<~RUBY) + hash.each do |_key, + _value| + puts :something + end + RUBY + end end end end @@ -85,6 +161,11 @@ puts :something end RUBY + expect_correction(<<~RUBY) + 1.times do |_index| + puts :something + end + RUBY end end @@ -100,6 +181,11 @@ puts 'baz' end RUBY + expect_correction(<<~RUBY) + define_method(:foo) do |_bar| + puts 'baz' + end + RUBY end end end @@ -113,6 +199,11 @@ puts index end RUBY + expect_correction(<<~RUBY) + 1.times do |index; _block_local_variable| + puts index + end + RUBY end end end @@ -134,6 +225,9 @@ ^^^ #{bar_message} ^^^ #{foo_message} RUBY + expect_correction(<<~RUBY) + -> (_foo, _bar) { do_something } + RUBY end end @@ -147,6 +241,9 @@ -> (foo, bar) { puts bar } ^^^ #{message} RUBY + expect_correction(<<~RUBY) + -> (_foo, bar) { puts bar } + RUBY end end end @@ -174,6 +271,7 @@ puts 'bar' end RUBY + expect_no_corrections end context 'when AllowUnusedKeywordArguments set' do @@ -200,6 +298,7 @@ puts 'bar' end RUBY + expect_no_corrections end context 'when AllowUnusedKeywordArguments set' do @@ -260,6 +359,13 @@ def other(a) end end RUBY + expect_correction(<<~RUBY) + test do |_key, _value| + def other(a) + puts something(binding) + end + end + RUBY end end end @@ -279,6 +385,11 @@ def other(a) puts something(binding(:other)) end RUBY + expect_correction(<<~RUBY) + test do |_key, _value| + puts something(binding(:other)) + end + RUBY end end end @@ -295,6 +406,9 @@ def other(a) super { |bar| } ^^^ #{message} RUBY + expect_correction(<<~RUBY) + super { |_bar| } + RUBY end end @@ -308,55 +422,6 @@ def other(a) end end - context 'auto-correct' do - it_behaves_like( - 'auto-correction', - 'fixes single', - 'arr.map { |foo| stuff }', - 'arr.map { |_foo| stuff }' - ) - - it_behaves_like( - 'auto-correction', - 'fixes multiple', - 'hash.map { |key, val| stuff }', - 'hash.map { |_key, _val| stuff }' - ) - - it_behaves_like( - 'auto-correction', - 'preserves whitespace', - <<-SOURCE, - hash.map { |key, - val| stuff } - SOURCE - <<-CORRECTED_SOURCE - hash.map { |_key, - _val| stuff } - CORRECTED_SOURCE - ) - - it_behaves_like( - 'auto-correction', - 'preserves splat', - 'obj.method { |foo, *bars, baz| stuff(foo, baz) }', - 'obj.method { |foo, *_bars, baz| stuff(foo, baz) }' - ) - - it_behaves_like( - 'auto-correction', - 'preserves default', - 'obj.method { |foo, bar = baz| stuff(foo) }', - 'obj.method { |foo, _bar = baz| stuff(foo) }' - ) - - it 'ignores used arguments' do - original_source = 'obj.method { |foo, baz| stuff(foo, baz) }' - - expect(autocorrect_source(original_source)).to eq(original_source) - end - end - context 'when IgnoreEmptyBlocks config parameter is set' do let(:cop_config) { { 'IgnoreEmptyBlocks' => true } } @@ -375,6 +440,9 @@ def other(a) ->(arg) { 1 } ^^^ #{message} RUBY + expect_correction(<<~RUBY) + ->(_arg) { 1 } + RUBY end it 'accepts an empty block with multiple unused parameters' do @@ -396,6 +464,9 @@ def other(a) ^^^^ #{arg2_message} ^^^^ #{arg1_message} RUBY + expect_correction(<<~RUBY) + ->(_arg1, _arg2, *_others) { 1 } + RUBY end end end diff --git a/spec/rubocop/cop/lint/unused_method_argument_spec.rb b/spec/rubocop/cop/lint/unused_method_argument_spec.rb index 0580189ee96..0516131374e 100644 --- a/spec/rubocop/cop/lint/unused_method_argument_spec.rb +++ b/spec/rubocop/cop/lint/unused_method_argument_spec.rb @@ -12,7 +12,7 @@ describe 'inspection' do context 'when a method takes multiple arguments' do context 'and an argument is unused' do - it 'registers an offense' do + it 'registers an offense and adds underscore-prefix' do message = 'Unused method argument - `foo`. ' \ "If it's necessary, use `_` or `_foo` " \ "as an argument name to indicate that it won't be used." @@ -23,6 +23,33 @@ def some_method(foo, bar) puts bar end RUBY + expect_correction(<<~RUBY) + def some_method(_foo, bar) + puts bar + end + RUBY + end + + context 'and there is some whitespace around the unused argument' do + it 'registers an offense and preserves whitespace' do + message = 'Unused method argument - `bar`. ' \ + "If it's necessary, use `_` or `_bar` " \ + "as an argument name to indicate that it won't be used." + + expect_offense(<<~RUBY) + def some_method(foo, + bar) + ^^^ #{message} + puts foo + end + RUBY + expect_correction(<<~RUBY) + def some_method(foo, + _bar) + puts foo + end + RUBY + end end context 'and arguments are swap-assigned' do @@ -48,12 +75,18 @@ def foo(a, b) a, b = b, 42 end RUBY + expect_correction(<<~RUBY) + def foo(_a, b) + a, b = b, 42 + end + RUBY end end end context 'and all the arguments are unused' do - it 'registers offenses and suggests the use of `*`' do + it 'registers offenses and suggests the use of `*` and ' \ + 'auto-corrects to add underscore-prefix to all arguments' do (foo_message, bar_message) = %w[foo bar].map do |arg| "Unused method argument - `#{arg}`. " \ "If it's necessary, use `_` or `_#{arg}` " \ @@ -68,10 +101,55 @@ def some_method(foo, bar) ^^^ #{foo_message} end RUBY + + expect_correction(<<~RUBY) + def some_method(_foo, _bar) + end + RUBY end end end + context 'when a splat argument is unused' do + it 'registers an offense and preserves the splat' do + message = 'Unused method argument - `bar`. ' \ + "If it's necessary, use `_` or `_bar` " \ + "as an argument name to indicate that it won't be used." + + expect_offense(<<~RUBY) + def some_method(foo, *bar) + ^^^ #{message} + puts foo + end + RUBY + expect_correction(<<~RUBY) + def some_method(foo, *_bar) + puts foo + end + RUBY + end + end + + context 'when an argument with a default value is unused' do + it 'registers an offense and preserves the default value' do + message = 'Unused method argument - `bar`. ' \ + "If it's necessary, use `_` or `_bar` " \ + "as an argument name to indicate that it won't be used." + + expect_offense(<<~RUBY) + def some_method(foo, bar = 1) + ^^^ #{message} + puts foo + end + RUBY + expect_correction(<<~RUBY) + def some_method(foo, _bar = 1) + puts foo + end + RUBY + end + end + context 'when a required keyword argument is unused' do it 'registers an offense but does not suggest underscore-prefix' do expect_offense(<<~RUBY) @@ -80,6 +158,7 @@ def self.some_method(foo, bar:) puts foo end RUBY + expect_no_corrections end end @@ -91,6 +170,7 @@ def self.some_method(foo, bar: 1) puts foo end RUBY + expect_no_corrections end context 'and AllowUnusedKeywordArguments set' do @@ -106,6 +186,26 @@ def self.some_method(foo, bar: 1) end end + context 'when a trailing block argument is unused' do + it 'registers an offense and removes the unused block arg' do + message = 'Unused method argument - `block`. ' \ + "If it's necessary, use `_` or `_block` " \ + "as an argument name to indicate that it won't be used." + + expect_offense(<<~RUBY) + def some_method(foo, bar, &block) + ^^^^^ #{message} + foo + bar + end + RUBY + expect_correction(<<~RUBY) + def some_method(foo, bar) + foo + bar + end + RUBY + end + end + context 'when a singleton method argument is unused' do it 'registers an offense' do message = "Unused method argument - `foo`. If it's necessary, use " \ @@ -119,6 +219,10 @@ def self.some_method(foo) ^^^ #{message} end RUBY + expect_correction(<<~RUBY) + def self.some_method(_foo) + end + RUBY end end @@ -196,6 +300,11 @@ def some_method(foo) super(:something) end RUBY + expect_correction(<<~RUBY) + def some_method(_foo) + super(:something) + end + RUBY end end end @@ -229,6 +338,13 @@ def other(a) end end RUBY + expect_correction(<<~RUBY) + def some_method(_foo, _bar) + def other(a) + puts something(binding) + end + end + RUBY end end end @@ -248,135 +364,16 @@ def some_method(foo) binding(:something) end RUBY + expect_correction(<<~RUBY) + def some_method(_foo) + binding(:something) + end + RUBY end end end end - describe 'auto-correction' do - let(:corrected_source) { autocorrect_source(source) } - - context 'when multiple arguments are unused' do - let(:source) { <<-RUBY } - def some_method(foo, bar) - end - RUBY - - let(:expected_source) { <<-RUBY } - def some_method(_foo, _bar) - end - RUBY - - it 'adds underscore-prefix to them' do - expect(corrected_source).to eq(expected_source) - end - end - - context 'when only a part of arguments is unused' do - let(:source) { <<-RUBY } - def some_method(foo, bar) - puts foo - end - RUBY - - let(:expected_source) { <<-RUBY } - def some_method(foo, _bar) - puts foo - end - RUBY - - it 'modifies only the unused one' do - expect(corrected_source).to eq(expected_source) - end - end - - context 'when there is some whitespace around the argument' do - let(:source) { <<-RUBY } - def some_method(foo, - bar) - puts foo - end - RUBY - - let(:expected_source) { <<-RUBY } - def some_method(foo, - _bar) - puts foo - end - RUBY - - it 'preserves the whitespace' do - expect(corrected_source).to eq(expected_source) - end - end - - context 'when a splat argument is unused' do - let(:source) { <<-RUBY } - def some_method(foo, *bar) - puts foo - end - RUBY - - let(:expected_source) { <<-RUBY } - def some_method(foo, *_bar) - puts foo - end - RUBY - - it 'preserves the splat' do - expect(corrected_source).to eq(expected_source) - end - end - - context 'when an unused argument has default value' do - let(:source) { <<-RUBY } - def some_method(foo, bar = 1) - puts foo - end - RUBY - - let(:expected_source) { <<-RUBY } - def some_method(foo, _bar = 1) - puts foo - end - RUBY - - it 'preserves the default value' do - expect(corrected_source).to eq(expected_source) - end - end - - context 'when a keyword argument is unused' do - let(:source) { <<-RUBY } - def some_method(foo, bar: 1) - puts foo - end - RUBY - - it 'ignores that since modifying the name changes the method interface' do - expect(corrected_source).to eq(source) - end - end - - context 'when a trailing block argument is unused' do - let(:source) { <<-RUBY } - def some_method(foo, bar, &block) - foo + bar - end - RUBY - - let(:expected_source) { <<-RUBY } - def some_method(foo, bar) - foo + bar - end - RUBY - - it 'removes the unused block arg' do - expect(corrected_source).to eq(expected_source) - end - end - end - context 'when IgnoreEmptyMethods config parameter is set' do let(:cop_config) { { 'IgnoreEmptyMethods' => true } } @@ -408,6 +405,11 @@ def method(arg) 1 end RUBY + expect_correction(<<~RUBY) + def method(_arg) + 1 + end + RUBY end it 'accepts an empty method with multiple unused parameters' do @@ -434,6 +436,11 @@ def method(a, b, *others) 1 end RUBY + expect_correction(<<~RUBY) + def method(_a, _b, *_others) + 1 + end + RUBY end end @@ -499,6 +506,11 @@ def method(arg) 1 end RUBY + expect_correction(<<~RUBY) + def method(_arg) + 1 + end + RUBY end it 'accepts an empty method with multiple unused parameters' do @@ -526,6 +538,11 @@ def method(a, b, *others) 1 end RUBY + expect_correction(<<~RUBY) + def method(_a, _b, *_others) + 1 + end + RUBY end end end diff --git a/spec/rubocop/cop/lint/useless_access_modifier_spec.rb b/spec/rubocop/cop/lint/useless_access_modifier_spec.rb index 58eed0f4df6..42466abae59 100644 --- a/spec/rubocop/cop/lint/useless_access_modifier_spec.rb +++ b/spec/rubocop/cop/lint/useless_access_modifier_spec.rb @@ -487,19 +487,18 @@ def method shared_examples 'repeated visibility modifiers' do |keyword, modifier| it "registers an offense when `#{modifier}` is repeated" do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) #{keyword} A #{modifier == 'private' ? 'protected' : 'private'} def method1 end - #{modifier} - #{modifier} + %{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. def method2 end end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(1) end end @@ -553,23 +552,22 @@ def method2 shared_examples 'at the end of the body' do |keyword, modifier| it "registers an offense for trailing `#{modifier}`" do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) #{keyword} A def method1 end def method2 end - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(1) end end shared_examples 'nested in a begin..end block' do |keyword, modifier| it "still flags repeated `#{modifier}`" do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) #{keyword} A #{modifier == 'private' ? 'protected' : 'private'} def blah @@ -577,15 +575,14 @@ def blah begin def method1 end - #{modifier} - #{modifier} + %{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. def method2 end end end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(1) end unless modifier == 'public' @@ -734,44 +731,41 @@ def method2 end it 'registers an offense if no method is defined' do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) #{keyword} A class << self - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. end end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(1) end it 'registers an offense if no method is defined after the modifier' do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) #{keyword} A class << self def method1 end - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. end end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(1) end it 'registers an offense even if a non-singleton-class method is ' \ 'defined' do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) #{keyword} A def method1 end class << self - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. end end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(1) end end @@ -787,25 +781,23 @@ class << A end it 'registers an offense if no method is defined' do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) class << A - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(1) end it 'registers an offense if no method is defined after the modifier' do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) class << A def method1 end - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(1) end end end @@ -822,43 +814,41 @@ def method1 end it 'registers an offense if no method is defined' do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) A.class_eval do - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(1) end context 'inside a class' do it 'registers an offense when a modifier is ouside the block and a ' \ 'method is defined only inside the block' do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) class A - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. A.class_eval do def method1 end end end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(1) end it 'registers two offenses when a modifier is inside and outside the ' \ ' block and no method is defined' do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) class A - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. A.class_eval do - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. end end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(2) end end end @@ -875,13 +865,12 @@ def foo end it "registers an offense if no method is defined in #{klass}.new" do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) #{klass}.new do - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(1) end end @@ -897,43 +886,41 @@ def foo end it 'registers an offense if no method is defined' do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) A.instance_eval do - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(1) end context 'inside a class' do it 'registers an offense when a modifier is ouside the block and a ' \ 'method is defined only inside the block' do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) class A - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. self.instance_eval do def method1 end end end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(1) end it 'registers two offenses when a modifier is inside and outside the ' \ ' and no method is defined' do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) class A - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. self.instance_eval do - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. end end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(2) end end end @@ -958,42 +945,40 @@ def method3 context 'unused modifiers' do it "registers an offense with a nested #{keyword}" do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) #{keyword} A - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. #{keyword} B - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. end end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(2) end it "registers an offense when outside a nested #{keyword}" do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) #{keyword} A - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. #{keyword} B def method1 end end end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(1) end it "registers an offense when inside a nested #{keyword}" do - src = <<~RUBY + expect_offense(<<~RUBY, modifier: modifier) #{keyword} A #{keyword} B - #{modifier} + %{modifier} + ^{modifier} Useless `#{modifier}` access modifier. end end RUBY - inspect_source(src) - expect(cop.offenses.size).to eq(1) end end end diff --git a/spec/rubocop/cop/lint/useless_comparison_spec.rb b/spec/rubocop/cop/lint/useless_comparison_spec.rb index de42a14863b..9492eee6c81 100644 --- a/spec/rubocop/cop/lint/useless_comparison_spec.rb +++ b/spec/rubocop/cop/lint/useless_comparison_spec.rb @@ -5,19 +5,21 @@ described_class::OPS.each do |op| it "registers an offense for a simple comparison with #{op}" do - inspect_source(<<~RUBY) - 5 #{op} 5 - a #{op} a + expect_offense(<<~RUBY, op: op) + 5 %{op} 5 + ^{op} Comparison of something with itself detected. + a %{op} a + ^{op} Comparison of something with itself detected. RUBY - expect(cop.offenses.size).to eq(2) end it "registers an offense for a complex comparison with #{op}" do - inspect_source(<<~RUBY) - 5 + 10 * 30 #{op} 5 + 10 * 30 - a.top(x) #{op} a.top(x) + expect_offense(<<~RUBY, op: op) + 5 + 10 * 30 %{op} 5 + 10 * 30 + ^{op} Comparison of something with itself detected. + a.top(x) %{op} a.top(x) + ^{op} Comparison of something with itself detected. RUBY - expect(cop.offenses.size).to eq(2) end end diff --git a/spec/rubocop/cop/lint/void_spec.rb b/spec/rubocop/cop/lint/void_spec.rb index 4b645b7c278..23906f2c62d 100644 --- a/spec/rubocop/cop/lint/void_spec.rb +++ b/spec/rubocop/cop/lint/void_spec.rb @@ -7,12 +7,13 @@ described_class::BINARY_OPERATORS.each do |op| it "registers an offense for void op #{op} if not on last line" do - inspect_source(<<~RUBY) - a #{op} b - a #{op} b - a #{op} b + expect_offense(<<~RUBY, op: op) + a %{op} b + ^{op} Operator `#{op}` used in void context. + a %{op} b + ^{op} Operator `#{op}` used in void context. + a %{op} b RUBY - expect(cop.offenses.size).to eq(2) end end @@ -31,15 +32,31 @@ end end - unary_operators = %i[+ - ~ !] - unary_operators.each do |op| + sign_unary_operators = %i[+ -] + other_unary_operators = %i[~ !] + unary_operators = sign_unary_operators + other_unary_operators + + sign_unary_operators.each do |op| + it "registers an offense for void sign op #{op} if not on last line" do + expect_offense(<<~RUBY, op: op) + %{op}b + ^{op} Operator `#{op}@` used in void context. + %{op}b + ^{op} Operator `#{op}@` used in void context. + %{op}b + RUBY + end + end + + other_unary_operators.each do |op| it "registers an offense for void unary op #{op} if not on last line" do - inspect_source(<<~RUBY) - #{op}b - #{op}b - #{op}b + expect_offense(<<~RUBY, op: op) + %{op}b + ^{op} Operator `#{op}` used in void context. + %{op}b + ^{op} Operator `#{op}` used in void context. + %{op}b RUBY - expect(cop.offenses.size).to eq(2) end end @@ -60,22 +77,22 @@ %w[var @var @@var VAR $var].each do |var| it "registers an offense for void var #{var} if not on last line" do - inspect_source(<<~RUBY) - #{var} = 5 - #{var} + expect_offense(<<~RUBY, var: var) + %{var} = 5 + %{var} + ^{var} Variable `#{var}` used in void context. top RUBY - expect(cop.offenses.size).to eq(1) end end %w(1 2.0 :test /test/ [1] {}).each do |lit| it "registers an offense for void lit #{lit} if not on last line" do - inspect_source(<<~RUBY) - #{lit} + expect_offense(<<~RUBY, lit: lit) + %{lit} + ^{lit} Literal `#{lit}` used in void context. top RUBY - expect(cop.offenses.size).to eq(1) end end diff --git a/spec/rubocop/cop/style/accessor_grouping_spec.rb b/spec/rubocop/cop/style/accessor_grouping_spec.rb index 41fe086683a..59d8b01a300 100644 --- a/spec/rubocop/cop/style/accessor_grouping_spec.rb +++ b/spec/rubocop/cop/style/accessor_grouping_spec.rb @@ -123,6 +123,47 @@ class Foo end RUBY end + + it 'does not register offense for accessors with comments' do + expect_no_offenses(<<~RUBY) + class Foo + # @return [String] value of foo + attr_reader :one, :two + + attr_reader :four + end + RUBY + end + + it 'registers offense and corrects if atleast two separate accessors without comments' do + expect_offense(<<~RUBY) + class Foo + # @return [String] value of foo + attr_reader :one, :two + + # [String] Some bar value return + attr_reader :three + + attr_reader :four + ^^^^^^^^^^^^^^^^^ Group together all `attr_reader` attributes. + attr_reader :five + ^^^^^^^^^^^^^^^^^ Group together all `attr_reader` attributes. + end + RUBY + + expect_correction(<<~RUBY) + class Foo + # @return [String] value of foo + attr_reader :one, :two + + # [String] Some bar value return + attr_reader :three + + attr_reader :four, :five + + end + RUBY + end end context 'when EnforcedStyle is separated' do @@ -236,5 +277,14 @@ class Foo end RUBY end + + it 'does not register an offense for grouped accessors with comments' do + expect_no_offenses(<<~RUBY) + class Foo + # Some comment + attr_reader :one, :two + end + RUBY + end end end diff --git a/spec/rubocop/cop/style/array_coercion_spec.rb b/spec/rubocop/cop/style/array_coercion_spec.rb new file mode 100644 index 00000000000..6a44bab1125 --- /dev/null +++ b/spec/rubocop/cop/style/array_coercion_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::ArrayCoercion do + subject(:cop) { described_class.new } + + it 'registers an offense and corrects when splatting variable into array' do + expect_offense(<<~RUBY) + [*paths].each { |path| do_something(path) } + ^^^^^^^^ Use `Array(paths)` instead of `[*paths]`. + RUBY + + expect_correction(<<~RUBY) + Array(paths).each { |path| do_something(path) } + RUBY + end + + it 'registers an offense and corrects when converting variable into array with check' do + expect_offense(<<~RUBY) + paths = [paths] unless paths.is_a?(Array) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `Array(paths)` instead of explicit `Array` check. + RUBY + + expect_correction(<<~RUBY) + paths = Array(paths) + RUBY + end + + it 'does not register an offense when splat is not in array' do + expect_no_offenses(<<~RUBY) + first_path, rest = *paths + RUBY + end + + it 'does not register an offense when splatting multiple variables into array' do + expect_no_offenses(<<~RUBY) + [*paths, '/root'].each { |path| do_something(path) } + RUBY + end + + it 'does not register an offense when converting variable into other named array variable with check' do + expect_no_offenses(<<~RUBY) + other_paths = [paths] unless paths.is_a?(Array) + RUBY + end +end diff --git a/spec/rubocop/cop/style/bisected_attr_accessor_spec.rb b/spec/rubocop/cop/style/bisected_attr_accessor_spec.rb index b00f9acf77a..d22e35fe707 100644 --- a/spec/rubocop/cop/style/bisected_attr_accessor_spec.rb +++ b/spec/rubocop/cop/style/bisected_attr_accessor_spec.rb @@ -86,6 +86,70 @@ class Foo RUBY end + it 'registers an offense and corrects when both accessors are in the same visibility scope' do + expect_offense(<<~RUBY) + class Foo + attr_reader :bar + ^^^^ Combine both accessors into `attr_accessor :bar`. + attr_writer :bar + ^^^^ Combine both accessors into `attr_accessor :bar`. + + private + + attr_writer :baz + ^^^^ Combine both accessors into `attr_accessor :baz`. + attr_reader :baz + ^^^^ Combine both accessors into `attr_accessor :baz`. + end + RUBY + + expect_correction(<<~RUBY) + class Foo + attr_accessor :bar + + + private + + + attr_accessor :baz + end + RUBY + end + + it 'registers an offense and corrects when withing eigenclass' do + expect_offense(<<~RUBY) + class Foo + attr_reader :bar + + class << self + attr_reader :baz + ^^^^ Combine both accessors into `attr_accessor :baz`. + attr_writer :baz + ^^^^ Combine both accessors into `attr_accessor :baz`. + + private + + attr_reader :quux + end + end + RUBY + + expect_correction(<<~RUBY) + class Foo + attr_reader :bar + + class << self + attr_accessor :baz + + + private + + attr_reader :quux + end + end + RUBY + end + it 'does not register an offense when only one accessor of the name exists' do expect_no_offenses(<<~RUBY) class Foo @@ -95,6 +159,17 @@ class Foo RUBY end + it 'does not register an offense when accessors are withing different visibility scopes' do + expect_no_offenses(<<~RUBY) + class Foo + attr_reader :bar + + private + attr_writer :baz + end + RUBY + end + it 'does not register an offense when using `attr_accessor`' do expect_no_offenses(<<~RUBY) class Foo diff --git a/spec/rubocop/cop/style/case_like_if_spec.rb b/spec/rubocop/cop/style/case_like_if_spec.rb new file mode 100644 index 00000000000..89a75c1bd2e --- /dev/null +++ b/spec/rubocop/cop/style/case_like_if_spec.rb @@ -0,0 +1,312 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::CaseLikeIf do + subject(:cop) { described_class.new } + + it 'registers an offense and corrects when using `===`' do + expect_offense(<<~RUBY) + if Integer === x + ^^^^^^^^^^^^^^^^ Convert `if-elsif` to `case-when`. + elsif /foo/ === x + elsif (1..10) === x + else + end + RUBY + + expect_correction(<<~RUBY) + case x + when Integer + when /foo/ + when (1..10) + else + end + RUBY + end + + it 'registers an offense and corrects when using `==` with literal' do + expect_offense(<<~RUBY) + if x == 1 + ^^^^^^^^^ Convert `if-elsif` to `case-when`. + elsif 'str' == x + else + end + RUBY + + expect_correction(<<~RUBY) + case x + when 1 + when 'str' + else + end + RUBY + end + + it 'registers an offense and corrects when using `==` with constant' do + expect_offense(<<~RUBY) + if x == CONSTANT1 + ^^^^^^^^^^^^^^^^^ Convert `if-elsif` to `case-when`. + elsif CONSTANT2 == x + else + end + RUBY + + expect_correction(<<~RUBY) + case x + when CONSTANT1 + when CONSTANT2 + else + end + RUBY + end + + it 'does not register an offense when using `==` with method call with arguments' do + expect_no_offenses(<<~RUBY) + if x == foo(1) + elsif bar(1) == x + else + end + RUBY + end + + it 'does not register an offense when using `==` with class reference' do + expect_no_offenses(<<~RUBY) + if x == Foo + elsif Bar == x + else + end + RUBY + end + + it 'does not register an offense when using `==` with constant containing 1 letter in name' do + expect_no_offenses(<<~RUBY) + if x == F + elsif B == x + else + end + RUBY + end + + it 'registers an offense and corrects when using `is_a?`' do + expect_offense(<<~RUBY) + if x.is_a?(Foo) + ^^^^^^^^^^^^^^^ Convert `if-elsif` to `case-when`. + elsif x.is_a?(Bar) + else + end + RUBY + + expect_correction(<<~RUBY) + case x + when Foo + when Bar + else + end + RUBY + end + + it 'registers an offense and corrects when using `match?` with regexp' do + expect_offense(<<~RUBY) + if /foo/.match?(x) + ^^^^^^^^^^^^^^^^^^ Convert `if-elsif` to `case-when`. + elsif x.match?(/bar/) + else + end + RUBY + + expect_correction(<<~RUBY) + case x + when /foo/ + when /bar/ + else + end + RUBY + end + + it 'does not register an offense when using `match?` with non regexp' do + expect_no_offenses(<<~RUBY) + if y.match?(x) + elsif x.match?('str') + else + end + RUBY + end + + it 'registers an offense and corrects when using `=~`' do + expect_offense(<<~RUBY) + if /foo/ =~ x + ^^^^^^^^^^^^^ Convert `if-elsif` to `case-when`. + elsif x =~ returns_regexp(arg) + else + end + RUBY + + expect_correction(<<~RUBY) + case x + when /foo/ + when returns_regexp(arg) + else + end + RUBY + end + + it 'does not register an offense when using `=~` in first branch with non regexp' do + expect_no_offenses(<<~RUBY) + if x =~ returns_regexp(arg) + elsif x =~ /foo/ + else + end + RUBY + end + + it 'does not register an offense when using `match?` in first branch with non regexp' do + expect_no_offenses(<<~RUBY) + if returns_regexp(arg).match?(x) + elsif x.match?(/bar/) + else + end + RUBY + end + + it 'registers an offense and corrects when using `match?` with non regexp in other branches except first' do + expect_offense(<<~RUBY) + if /foo/.match?(x) + ^^^^^^^^^^^^^^^^^^ Convert `if-elsif` to `case-when`. + elsif returns_regexp(arg).match?(x) + else + end + RUBY + + expect_correction(<<~RUBY) + case x + when /foo/ + when returns_regexp(arg) + else + end + RUBY + end + + it 'registers an offense and corrects when using `include?` with range' do + expect_offense(<<~RUBY) + if (1..10).include?(x) + ^^^^^^^^^^^^^^^^^^^^^^ Convert `if-elsif` to `case-when`. + elsif (11...100).include?(x) + else + end + RUBY + + expect_correction(<<~RUBY) + case x + when 1..10 + when 11...100 + else + end + RUBY + end + + it 'registers an offense and corrects when using `||` within conditions' do + expect_offense(<<~RUBY) + if Integer === x || x == 2 + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Convert `if-elsif` to `case-when`. + elsif 1 == x || (1..10) === x || x.match?(/foo/) + else + end + RUBY + + expect_correction(<<~RUBY) + case x + when Integer, 2 + when 1, (1..10), /foo/ + else + end + RUBY + end + + it 'does not register an offense when one of `||` subconditions is not convertible' do + expect_no_offenses(<<~RUBY) + if Integer === x || (x == 2 && x == 3) + elsif 1 == x || (1..10) === x || x.match?(/foo/) + else + end + RUBY + end + + it 'registers an offense and corrects when using nested conditions with `||`' do + expect_offense(<<~RUBY) + if Integer === x || ((x == 2) || (3 == x)) || x =~ /foo/ + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Convert `if-elsif` to `case-when`. + elsif 1 == x || (1..10) === x || x.match?(/bar/) + else + end + RUBY + + expect_correction(<<~RUBY) + case x + when Integer, 2, 3, /foo/ + when 1, (1..10), /bar/ + else + end + RUBY + end + + it 'registers an offense and corrects when target is a method call' do + expect_offense(<<~RUBY) + if x.type == 1 || x.type == 2 + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Convert `if-elsif` to `case-when`. + elsif /foo/ === x.type + else + end + RUBY + + expect_correction(<<~RUBY) + case x.type + when 1, 2 + when /foo/ + else + end + RUBY + end + + it 'does not register an offense when not all conditions contain target' do + expect_no_offenses(<<~RUBY) + if x == 2 + elsif 3 == y + else + end + RUBY + end + + it 'does not register an offense when only single `if`' do + expect_no_offenses(<<~RUBY) + if x == 1 + end + RUBY + end + + it 'does not register an offense when only `if-else`' do + expect_no_offenses(<<~RUBY) + if x == 1 + else + end + RUBY + end + + it 'does not register an offense when using `unless`' do + expect_no_offenses(<<~RUBY) + unless x == 1 + else + end + RUBY + end + + it 'does not register an offense when using ternary operator' do + expect_no_offenses(<<~RUBY) + x == 1 ? y : z + RUBY + end + + it 'does not register an offense when using modifier `if`' do + expect_no_offenses(<<~RUBY) + foo if x == 1 + RUBY + end +end diff --git a/spec/rubocop/cop/style/hash_as_last_array_item_spec.rb b/spec/rubocop/cop/style/hash_as_last_array_item_spec.rb new file mode 100644 index 00000000000..ddb329b4880 --- /dev/null +++ b/spec/rubocop/cop/style/hash_as_last_array_item_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::HashAsLastArrayItem, :config do + subject(:cop) { described_class.new(config) } + + context 'when EnforcedStyle is braces' do + let(:cop_config) do + { 'EnforcedStyle' => 'braces' } + end + + it 'registers an offense and corrects when hash without braces' do + expect_offense(<<~RUBY) + [1, 2, one: 1, two: 2] + ^^^^^^^^^^^^^^ Wrap hash in `{` and `}`. + RUBY + + expect_correction(<<~RUBY) + [1, 2, {one: 1, two: 2}] + RUBY + end + + it 'does not register an offense when hash with braces' do + expect_no_offenses(<<~RUBY) + [1, 2, { one: 1, two: 2 }] + RUBY + end + + it 'does not register an offense when hash is not inside array' do + expect_no_offenses(<<~RUBY) + foo(one: 1, two: 2) + RUBY + end + end + + context 'when EnforcedStyle is no_braces' do + let(:cop_config) do + { 'EnforcedStyle' => 'no_braces' } + end + + it 'registers an offense and corrects when hash with braces' do + expect_offense(<<~RUBY) + [1, 2, { one: 1, two: 2 }] + ^^^^^^^^^^^^^^^^^^ Omit the braces around the hash. + RUBY + + expect_correction(<<~RUBY) + [1, 2, one: 1, two: 2 ] + RUBY + end + + it 'does not register an offense when hash without braces' do + expect_no_offenses(<<~RUBY) + [1, 2, one: 1, two: 2] + RUBY + end + + it 'does not register an offense when hash is not inside array' do + expect_no_offenses(<<~RUBY) + foo({ one: 1, two: 2 }) + RUBY + end + end +end diff --git a/spec/rubocop/cop/style/hash_like_case_spec.rb b/spec/rubocop/cop/style/hash_like_case_spec.rb new file mode 100644 index 00000000000..958b564dcab --- /dev/null +++ b/spec/rubocop/cop/style/hash_like_case_spec.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::HashLikeCase, :config do + subject(:cop) { described_class.new(config) } + + context 'MinBranchesCount: 2' do + let(:cop_config) do + { 'MinBranchesCount' => 2 } + end + + it 'registers an offense when using `case-when` with string conditions and literal bodies of the same type' do + expect_offense(<<~RUBY) + case x + ^^^^^^ Consider replacing `case-when` with a hash lookup. + when 'foo' + 'FOO' + when 'bar' + 'BAR' + end + RUBY + end + + it 'registers an offense when using `case-when` with symbol conditions and literal bodies of the same type' do + expect_offense(<<~RUBY) + case x + ^^^^^^ Consider replacing `case-when` with a hash lookup. + when :foo + 'FOO' + when :bar + 'BAR' + end + RUBY + end + + it 'does not register an offense when using `case-when` with literals of different types as conditions' do + expect_no_offenses(<<~RUBY) + case x + when 'foo' + 'FOO' + when :bar + 'BAR' + end + RUBY + end + + it 'does not register an offense when using `case-when` with non-literals in conditions' do + expect_no_offenses(<<~RUBY) + case x + when y + 'FOO' + when z + 'BAR' + end + RUBY + end + + it 'does not register an offense when using `case-when` with literal bodies of different types' do + expect_no_offenses(<<~RUBY) + case x + when 'foo' + 'FOO' + when 'bar' + 2 + end + RUBY + end + + it 'does not register an offense when using `case-when` with non-literal bodies' do + expect_no_offenses(<<~RUBY) + case x + when 'foo' + y + when 'bar' + z + end + RUBY + end + + it 'does not register an offense when `case` has an `else` branch' do + expect_no_offenses(<<~RUBY) + case x + when 'foo' + 'FOO' + when 'bar' + 'BAR' + else + 'BAZ' + end + RUBY + end + end + + context 'MinBranchesCount: 3' do + let(:cop_config) do + { 'MinBranchesCount' => 3 } + end + + it 'does not register an offense when branches count is less than required' do + expect_no_offenses(<<~RUBY) + case x + when 'foo' + 'FOO' + when 'bar' + 'BAR' + end + RUBY + end + end +end diff --git a/spec/rubocop/cop/style/if_unless_modifier_of_if_unless_spec.rb b/spec/rubocop/cop/style/if_unless_modifier_of_if_unless_spec.rb index a20eb0e9f63..4220b2d27cc 100644 --- a/spec/rubocop/cop/style/if_unless_modifier_of_if_unless_spec.rb +++ b/spec/rubocop/cop/style/if_unless_modifier_of_if_unless_spec.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true RSpec.describe RuboCop::Cop::Style::IfUnlessModifierOfIfUnless do - include StatementModifierHelper - subject(:cop) { described_class.new } it 'provides a good error message' do diff --git a/spec/rubocop/cop/style/if_unless_modifier_spec.rb b/spec/rubocop/cop/style/if_unless_modifier_spec.rb index 848dbd73554..5bcb14da619 100644 --- a/spec/rubocop/cop/style/if_unless_modifier_spec.rb +++ b/spec/rubocop/cop/style/if_unless_modifier_spec.rb @@ -1,13 +1,11 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::Style::IfUnlessModifier do - include StatementModifierHelper - - subject(:cop) { described_class.new(config) } - - let(:config) do - RuboCop::Config.new('Layout/LineLength' => line_length_config) - end +###### +# Note: most of these tests probably belong in the shared context "condition modifier cop" +###### +RSpec.describe RuboCop::Cop::Style::IfUnlessModifier, :config do + let(:ignore_cop_directives) { true } + let(:allow_uri) { true } let(:line_length_config) do { 'Enabled' => true, @@ -17,8 +15,11 @@ 'URISchemes' => %w[http https] } end - let(:allow_uri) { true } - let(:ignore_cop_directives) { true } + let(:other_cops) { { 'Layout/LineLength' => line_length_config } } + + extra = ' Another good alternative is the usage of control flow `&&`/`||`.' + it_behaves_like 'condition modifier cop', :if, extra + it_behaves_like 'condition modifier cop', :unless, extra context 'modifier if that does not fit on one line' do let(:spaces) { ' ' * 59 } @@ -297,14 +298,6 @@ def f end end - it "accepts multiline if that doesn't fit on one line" do - check_too_long('if') - end - - it 'accepts multiline if whose body is more than one line' do - check_short_multiline('if') - end - context 'multiline unless that fits on one line' do let(:source) do <<~RUBY @@ -343,11 +336,6 @@ def f RUBY end - it 'accepts an empty condition' do - check_empty('if') - check_empty('unless') - end - it 'accepts if/elsif' do expect_no_offenses(<<~RUBY) if test @@ -457,6 +445,32 @@ def f expect(corrected).to eq "a + (1 if b)\n" end + it 'adds parens in autocorrect when if-end used with `||` operator' do + expect_offense(<<~RUBY) + a || if b + ^^ Favor modifier `if` usage when having a single-line body. Another good alternative is the usage of control flow `&&`/`||`. + 1 + end + RUBY + + expect_correction(<<~RUBY) + a || (1 if b) + RUBY + end + + it 'adds parens in autocorrect when if-end used with `&&` operator' do + expect_offense(<<~RUBY) + a && if b + ^^ Favor modifier `if` usage when having a single-line body. Another good alternative is the usage of control flow `&&`/`||`. + 1 + end + RUBY + + expect_correction(<<~RUBY) + a && (1 if b) + RUBY + end + it 'accepts if-end when used as LHS of binary arithmetic' do expect_no_offenses(<<~RUBY) if test @@ -561,22 +575,22 @@ def f end context 'with Layout/IndentationStyle: IndentationWidth config' do - let(:config) do - RuboCop::Config.new( + let(:other_cops) do + { 'Layout/IndentationStyle' => { 'IndentationWidth' => 2 }, 'Layout/LineLength' => { 'Max' => 10 + 12 } # 12 is indentation - ) + } end it_behaves_like 'with tabs indentation' end context 'with Layout/IndentationWidth: Width config' do - let(:config) do - RuboCop::Config.new( + let(:other_cops) do + { 'Layout/IndentationWidth' => { 'Width' => 3 }, 'Layout/LineLength' => { 'Max' => 10 + 18 } # 18 is indentation - ) + } end it_behaves_like 'with tabs indentation' @@ -584,14 +598,7 @@ def f end context 'when Layout/LineLength is disabled' do - let(:config) do - RuboCop::Config.new( - 'Layout/LineLength' => { - 'Enabled' => false, - 'Max' => 80 - } - ) - end + let(:line_length_config) { { 'Enabled' => false } } it 'registers an offense even for a long modifier statement' do expect_offense(<<~RUBY) diff --git a/spec/rubocop/cop/style/percent_literal_delimiters_spec.rb b/spec/rubocop/cop/style/percent_literal_delimiters_spec.rb index 82a63159cd3..a35cf04b017 100644 --- a/spec/rubocop/cop/style/percent_literal_delimiters_spec.rb +++ b/spec/rubocop/cop/style/percent_literal_delimiters_spec.rb @@ -58,6 +58,10 @@ ^^^^^^^^^^^^^^^ `%`-literals should be delimited by `[` and `]`. RUBY end + + it 'does not crash when the source contains invalid characters' do + expect { inspect_source('%{\x80}') }.not_to raise_error + end end context '`%q` string' do diff --git a/spec/rubocop/cop/style/redundant_file_extension_in_require_spec.rb b/spec/rubocop/cop/style/redundant_file_extension_in_require_spec.rb new file mode 100644 index 00000000000..48f5622100f --- /dev/null +++ b/spec/rubocop/cop/style/redundant_file_extension_in_require_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::RedundantFileExtensionInRequire do + subject(:cop) { described_class.new } + + it 'registers an offense and corrects when requiring filename ending with `.rb`' do + expect_offense(<<~RUBY) + require 'foo.rb' + ^^^^^^^^ Redundant `.rb` file extension detected. + require_relative '../foo.rb' + ^^^^^^^^^^^ Redundant `.rb` file extension detected. + RUBY + + expect_correction(<<~RUBY) + require 'foo' + require_relative '../foo' + RUBY + end + + it 'does not register an offense when requiring filename ending with `.so`' do + expect_no_offenses(<<~RUBY) + require 'foo.so' + require_relative '../foo.so' + RUBY + end + + it 'does not register an offense when requiring filename without an extension' do + expect_no_offenses(<<~RUBY) + require 'foo' + require_relative '../foo' + RUBY + end + + it 'does not register an offense when requiring variable as a filename' do + expect_no_offenses(<<~RUBY) + require name + require_relative name + RUBY + end +end diff --git a/spec/rubocop/cop/style/trailing_method_end_statement_spec.rb b/spec/rubocop/cop/style/trailing_method_end_statement_spec.rb index 58a17b2bd28..1b9abe62d29 100644 --- a/spec/rubocop/cop/style/trailing_method_end_statement_spec.rb +++ b/spec/rubocop/cop/style/trailing_method_end_statement_spec.rb @@ -72,8 +72,8 @@ def some_method RUBY expect(corrected).to eq(<<~RUBY) def some_method - [] - end + []; + end RUBY end @@ -86,8 +86,8 @@ def do_this(x) expect(corrected).to eq(<<~RUBY) def do_this(x) y = x + 5 - y / 2 - end + y / 2; + end RUBY end @@ -101,7 +101,7 @@ def f(x, y) def f(x, y) process(x) process(y) - end # comment + end # comment RUBY end @@ -117,7 +117,7 @@ def d block do foo end - end + end RUBY end @@ -125,9 +125,9 @@ def d corrected = autocorrect_source(<<~RUBY) class Foo def some_method - []; end + [] end def another_method - {}; end + {} end end RUBY expect(corrected).to eq(<<~RUBY) diff --git a/spec/rubocop/cop/style/while_until_modifier_spec.rb b/spec/rubocop/cop/style/while_until_modifier_spec.rb index 9e4b34a42b3..4c034944b15 100644 --- a/spec/rubocop/cop/style/while_until_modifier_spec.rb +++ b/spec/rubocop/cop/style/while_until_modifier_spec.rb @@ -1,132 +1,6 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::Style::WhileUntilModifier do - include StatementModifierHelper - - subject(:cop) { described_class.new(config) } - - let(:config) do - RuboCop::Config.new('Layout/LineLength' => { 'Max' => 80 }) - end - - it "accepts multiline unless that doesn't fit on one line" do - check_too_long('unless') - end - - it 'accepts multiline unless whose body is more than one line' do - check_short_multiline('unless') - end - - context 'multiline while that fits on one line' do - it 'registers an offense' do - check_really_short('while') - end - - it 'does auto-correction' do - autocorrect_really_short('while') - end - end - - it "accepts multiline while that doesn't fit on one line" do - check_too_long('while') - end - - it 'accepts multiline while whose body is more than one line' do - check_short_multiline('while') - end - - it 'accepts oneline while when condition has local variable assignment' do - expect_no_offenses(<<~RUBY) - lines = %w{first second third} - while (line = lines.shift) - puts line - end - RUBY - end - - context 'oneline while when assignment is in body' do - let(:source) do - <<~RUBY - while true - x = 0 - end - RUBY - end - - it 'registers an offense' do - expect_offense(<<~RUBY) - while true - ^^^^^ Favor modifier `while` usage when having a single-line body. - x = 0 - end - RUBY - end - - it 'does auto-correction' do - corrected = autocorrect_source(source) - expect(corrected).to eq "x = 0 while true\n" - end - end - - context 'multiline until that fits on one line' do - it 'registers an offense' do - check_really_short('until') - end - - it 'does auto-correction' do - autocorrect_really_short('until') - end - end - - it "accepts multiline until that doesn't fit on one line" do - check_too_long('until') - end - - it 'accepts multiline until whose body is more than one line' do - check_short_multiline('until') - end - - it 'accepts an empty condition' do - check_empty('while') - check_empty('until') - end - - it 'accepts modifier while' do - expect_no_offenses('ala while bala') - end - - it 'accepts modifier until' do - expect_no_offenses('ala until bala') - end - - # Regression: https://github.com/rubocop-hq/rubocop/issues/4006 - context 'when the modifier condition is multiline' do - it 'registers an offense' do - expect_offense(<<~RUBY) - foo while bar || - ^^^^^ Favor modifier `while` usage when having a single-line body. - baz - RUBY - end - end - - context 'when Layout/LineLength is disabled' do - let(:config) do - RuboCop::Config.new( - 'Layout/LineLength' => { - 'Enabled' => false, - 'Max' => 80 - } - ) - end - - it 'registers an offense even for a long modifier statement' do - expect_offense(<<~RUBY) - while foo - ^^^^^ Favor modifier `while` usage when having a single-line body. - "This string would make the line longer than eighty characters if combined with the statement." - end - RUBY - end - end +RSpec.describe RuboCop::Cop::Style::WhileUntilModifier, :config do + it_behaves_like 'condition modifier cop', :while + it_behaves_like 'condition modifier cop', :until end diff --git a/spec/rubocop/file_finder_spec.rb b/spec/rubocop/file_finder_spec.rb index aad53e470de..5459c277ac8 100644 --- a/spec/rubocop/file_finder_spec.rb +++ b/spec/rubocop/file_finder_spec.rb @@ -23,15 +23,14 @@ end end - describe '#find_files_upwards' do - it 'returns an array of files to be found upwards' do - expect(finder.find_files_upwards('file', 'dir')) - .to eq([File.expand_path('file', 'dir'), - File.expand_path('file')]) + describe '#find_last_file_upwards' do + it 'returns the last file found upwards' do + expect(finder.find_last_file_upwards('file', 'dir')) + .to eq(File.expand_path('file')) end - it 'returns an empty array when file is not found' do - expect(finder.find_files_upwards('xyz', 'dir')).to eq([]) + it 'returns nil when file is not found' do + expect(finder.find_last_file_upwards('xyz', 'dir')).to be(nil) end end end diff --git a/spec/rubocop/formatter/html_formatter_spec.rb b/spec/rubocop/formatter/html_formatter_spec.rb index d568d5bc6da..961c7d7869e 100644 --- a/spec/rubocop/formatter/html_formatter_spec.rb +++ b/spec/rubocop/formatter/html_formatter_spec.rb @@ -7,7 +7,7 @@ project_path = File.join(spec_root, 'fixtures/html_formatter/project') FileUtils.cp_r(project_path, '.') - RuboCop::PathUtil.chdir(File.basename(project_path)) do + Dir.chdir(File.basename(project_path)) do example.run end end diff --git a/spec/rubocop/result_cache_spec.rb b/spec/rubocop/result_cache_spec.rb index d531ca52c7a..cec2b7f3602 100644 --- a/spec/rubocop/result_cache_spec.rb +++ b/spec/rubocop/result_cache_spec.rb @@ -20,7 +20,7 @@ let(:file) { 'example.rb' } let(:options) { {} } let(:config_store) do - instance_double(RuboCop::ConfigStore, for_dir: RuboCop::Config.new) + instance_double(RuboCop::ConfigStore, for_pwd: RuboCop::Config.new) end let(:cache_root) { "#{Dir.pwd}/rubocop_cache" } let(:offenses) do @@ -192,7 +192,7 @@ def abs(path) context 'and symlink attack protection is disabled' do before do - allow(config_store).to receive(:for_dir).with('.').and_return( + allow(config_store).to receive(:for_pwd).and_return( RuboCop::Config.new( 'AllCops' => { 'AllowSymlinksInCacheRootDirectory' => true @@ -302,7 +302,7 @@ def abs(path) describe '.cleanup' do before do cfg = RuboCop::Config.new('AllCops' => { 'MaxFilesInCache' => 1 }) - allow(config_store).to receive(:for_dir).with('.').and_return(cfg) + allow(config_store).to receive(:for_pwd).and_return(cfg) allow(config_store).to receive(:for_file).with('other.rb').and_return(cfg) create_file('other.rb', ['x = 1']) $stdout = StringIO.new @@ -341,7 +341,7 @@ def abs(path) 'AllCops' => { 'CacheRootDirectory' => cache_root_directory } } config = RuboCop::Config.new(all_cops) - allow(config_store).to receive(:for_dir).with('.').and_return(config) + allow(config_store).to receive(:for_pwd).and_return(config) end context 'when CacheRootDirectory not set' do diff --git a/spec/rubocop/target_finder_spec.rb b/spec/rubocop/target_finder_spec.rb index d3e176bcce2..5de5c147cab 100755 --- a/spec/rubocop/target_finder_spec.rb +++ b/spec/rubocop/target_finder_spec.rb @@ -172,7 +172,7 @@ let(:args) { [] } it 'finds files under the current directory' do - RuboCop::PathUtil.chdir('dir1') do + Dir.chdir('dir1') do expect(found_files.empty?).to be(false) found_files.each do |file| expect(file).to include('/dir1/') @@ -186,7 +186,7 @@ let(:args) { ['../dir2'] } it 'finds files under the specified directory' do - RuboCop::PathUtil.chdir('dir1') do + Dir.chdir('dir1') do expect(found_files.empty?).to be(false) found_files.each do |file| expect(file).to include('/dir2/') diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 06b04dec1dc..aeda10f7fde 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -51,8 +51,4 @@ if %w[ruby-head-ascii_spec ruby-head-spec].include? ENV['CIRCLE_STAGE'] config.filter_run_excluding broken_on: :ruby_head end - - config.after do - RuboCop::PathUtil.reset_pwd - end end diff --git a/spec/support/condition_modifier_cop.rb b/spec/support/condition_modifier_cop.rb new file mode 100644 index 00000000000..3ca56274c4c --- /dev/null +++ b/spec/support/condition_modifier_cop.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'condition modifier cop' do |keyword, extra_message = nil| + let(:other_cops) { { 'Layout/LineLength' => { 'Max' => 80 } } } + + context "for a multiline '#{keyword}'" do + it 'accepts it if single line would not fit on one line' do + # This statement is one character too long to fit. + condition = 'a' * (40 - keyword.length) + body = 'b' * 39 + expect("#{body} #{keyword} #{condition}".length).to eq(81) + + expect_no_offenses(<<~RUBY) + #{keyword} #{condition} + #{body} + end + RUBY + end + + context 'when Layout/LineLength is disabled' do + let(:other_cops) { { 'Layout/LineLength' => { 'Enabled' => false } } } + + it 'registers an offense even for a long modifier statement' do + expect_offense(<<~RUBY, keyword: keyword) + %{keyword} foo + ^{keyword} Favor modifier `#{keyword}` usage when having a single-line body.#{extra_message} + "This string would make the line longer than eighty characters if combined with the statement." + end + RUBY + end + end + + it 'accepts it if body spans more than one line' do + expect_no_offenses(<<~RUBY) + #{keyword} some_condition + do_something + do_something_else + end + RUBY + end + + it 'corrects it if result fits in one line' do + expect_offense(<<~RUBY, keyword: keyword) + %{keyword} condition + ^{keyword} Favor modifier `#{keyword}` usage when having a single-line body.#{extra_message} + do_something + end + RUBY + + expect_correction(<<~RUBY) + do_something #{keyword} condition + RUBY + end + + it 'accepts an empty body' do + expect_no_offenses(<<~RUBY) + #{keyword} cond + end + RUBY + end + + it 'accepts it when condition has local variable assignment' do + expect_no_offenses(<<~RUBY) + #{keyword} (var = something) + puts var + end + RUBY + end + + it 'corrects it when assignment is in body' do + expect_offense(<<~RUBY, keyword: keyword) + %{keyword} true + ^{keyword} Favor modifier `%{keyword}` usage when having a single-line body.#{extra_message} + x = 0 + end + RUBY + + expect_correction(<<~RUBY) + x = 0 #{keyword} true + RUBY + end + + # See: https://github.com/rubocop-hq/rubocop/issues/8273 + context 'accepts multiline condition in modifier form' do + it 'registers an offense' do + expect_no_offenses(<<~RUBY) + foo #{keyword} bar || + baz + RUBY + end + end + end +end diff --git a/spec/support/statement_modifier_helper.rb b/spec/support/statement_modifier_helper.rb deleted file mode 100644 index df76829f740..00000000000 --- a/spec/support/statement_modifier_helper.rb +++ /dev/null @@ -1,53 +0,0 @@ -# frozen_string_literal: true - -module StatementModifierHelper - def check_empty(keyword) - expect_no_offenses(<<~RUBY) - #{keyword} cond - end - RUBY - end - - def check_really_short(keyword) - inspect_source(<<~RUBY) - #{keyword} a - b - end - RUBY - expect(cop.messages).to eq( - ["Favor modifier `#{keyword}` usage when having a single-line body."] - ) - expect(cop.offenses.map { |o| o.location.source }).to eq([keyword]) - end - - def autocorrect_really_short(keyword) - corrected = autocorrect_source(<<~RUBY) - #{keyword} a - b - end - RUBY - expect(corrected).to eq "b #{keyword} a\n" - end - - def check_too_long(keyword) - # This statement is one character too long to fit. - condition = 'a' * (40 - keyword.length) - body = 'b' * 37 - expect(" #{body} #{keyword} #{condition}".length).to eq(81) - - expect_no_offenses(<<-RUBY.strip_margin('|')) - | #{keyword} #{condition} - | #{body} - | end - RUBY - end - - def check_short_multiline(keyword) - expect_no_offenses(<<~RUBY) - #{keyword} ENV['COVERAGE'] - require 'simplecov' - SimpleCov.start - end - RUBY - end -end diff --git a/tasks/cut_release.rake b/tasks/cut_release.rake index 469208f60e7..3835576df3e 100644 --- a/tasks/cut_release.rake +++ b/tasks/cut_release.rake @@ -32,7 +32,7 @@ namespace :cut_release do File.open('docs/antora.yml', 'w') do |f| f << antora_metadata.sub( 'version: master', - "version: #{version_sans_patch(new_version)}" + "version: '#{version_sans_patch(new_version)}'" ) end diff --git a/tasks/prof.rake b/tasks/prof.rake new file mode 100644 index 00000000000..d6da3edb473 --- /dev/null +++ b/tasks/prof.rake @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +namespace :prof do + DUMP_PATH = 'tmp/stackprof.dump' + + desc 'Run RuboCop on itself with profiling on' + task :run, [:path] do |_task, args| + path = args.fetch(:path, '.') + cmd = "bin/rubocop-profile #{path}" + system cmd + end + + task :run_if_needed, [:path] do + Rake::Task[:run].run unless File.exist?(DUMP_PATH) + end + + desc 'List the slowest cops' + task slow_cops: :run_if_needed do + method = 'RuboCop::Cop::Commissioner#trigger_responding_cops' + cmd = "stackprof #{DUMP_PATH} --text --method '#{method}'" + puts cmd + output = `#{cmd}` + _header, list, _code = *output + .lines + .grep_v(/RuboCop::Cop::Commissioner/) # ignore internal calls + .slice_when { |line| line.match?(/callees.*:|code:/) } + puts list.first(40) + end + + desc 'Check a particular method by walking through the callstack' + task :walk, [:method] => :run_if_needed do |_task, args| + method = args.fetch(:method) do + warn 'usage: bundle exec rake walk[Class#method]' + exit! + end + cmd = "stackprof #{DUMP_PATH} --walk --method '#{method}'" + puts cmd + system cmd + end +end