diff --git a/CHANGELOG.md b/CHANGELOG.md index e3a5e4018..045a80482 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -651,3 +651,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features. [@dswij]: https://github.com/dswij [@francois-ferrandis]: https://github.com/francois-ferrandis [@r7kamura]: https://github.com/r7kamura +[@leoarnold]: https://github.com/leoarnold diff --git a/changelog/new_add_new_rspecfilefixture_cop.md b/changelog/new_add_new_rspecfilefixture_cop.md new file mode 100644 index 000000000..5c3f749a9 --- /dev/null +++ b/changelog/new_add_new_rspecfilefixture_cop.md @@ -0,0 +1 @@ +* [#1214](https://github.com/rubocop/rubocop-rails/pull/1214): Add new `RSpec/FileFixture` cop. ([@leoarnold][]) diff --git a/config/default.yml b/config/default.yml index 0e62d0022..676aa46e0 100644 --- a/config/default.yml +++ b/config/default.yml @@ -353,6 +353,12 @@ RSpec/ExpectOutput: VersionAdded: '1.10' StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ExpectOutput +RSpec/FileFixture: + Description: Favor the use of `file_fixture`. + Enabled: pending + VersionAdded: 2.7.0 + StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FileFixture + RSpec/FilePath: Description: Checks that spec file paths are consistent and well-formed. Enabled: true diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index dcda9d795..fd24393e4 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -32,6 +32,7 @@ * xref:cops_rspec.adoc#rspecexpectchange[RSpec/ExpectChange] * xref:cops_rspec.adoc#rspecexpectinhook[RSpec/ExpectInHook] * xref:cops_rspec.adoc#rspecexpectoutput[RSpec/ExpectOutput] +* xref:cops_rspec.adoc#rspecfilefixture[RSpec/FileFixture] * xref:cops_rspec.adoc#rspecfilepath[RSpec/FilePath] * xref:cops_rspec.adoc#rspecfocus[RSpec/Focus] * xref:cops_rspec.adoc#rspechookargument[RSpec/HookArgument] diff --git a/docs/modules/ROOT/pages/cops_rspec.adoc b/docs/modules/ROOT/pages/cops_rspec.adoc index 0f681d8ab..5f74cec4c 100644 --- a/docs/modules/ROOT/pages/cops_rspec.adoc +++ b/docs/modules/ROOT/pages/cops_rspec.adoc @@ -1491,6 +1491,61 @@ expect { my_app.print_report }.to output('Hello World').to_stdout * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ExpectOutput +== RSpec/FileFixture + +|=== +| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed + +| Pending +| Yes +| Yes +| 2.7.0 +| - +|=== + +Favor the use of `file_fixture`. + +`RSpec` provides the convenience method + +[source,ruby] +---- +file_fixture('path/some_file.csv') +---- + +which (by default) is equivalent to + +[source,ruby] +---- +Rails.root.join('spec/fixtures/files/path/some_file.csv') +---- + +=== Examples + +[source,ruby] +---- +# bad +"#{Rails.root}/spec/fixtures/path/some_file.csv" +Rails.root.join('spec/fixtures/path/some_file.csv') +Rails.root.join('spec', 'fixtures', 'path', 'some_file.csv') + +"#{Rails.root}/spec/fixtures/files/some_file.csv" +Rails.root.join('spec/fixtures/files/some_file.csv') +Rails.root.join('spec', 'fixtures', 'files', 'some_file.csv') + +# good +file_fixture('../path/some_file.csv').path +file_fixture('../path/some_file.csv') +file_fixture('../path/some_file.csv') + +file_fixture('some_file.csv').path +file_fixture('some_file.csv') +file_fixture('some_file.csv') +---- + +=== References + +* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FileFixture + == RSpec/FilePath |=== diff --git a/lib/rubocop/cop/rspec/file_fixture.rb b/lib/rubocop/cop/rspec/file_fixture.rb new file mode 100644 index 000000000..624034b58 --- /dev/null +++ b/lib/rubocop/cop/rspec/file_fixture.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module RSpec + # Favor the use of `file_fixture`. + # + # `RSpec` provides the convenience method + # + # [source,ruby] + # ---- + # file_fixture('path/some_file.csv') + # ---- + # + # which (by default) is equivalent to + # + # [source,ruby] + # ---- + # Rails.root.join('spec/fixtures/files/path/some_file.csv') + # ---- + # + # @example + # # bad + # "#{Rails.root}/spec/fixtures/path/some_file.csv" + # Rails.root.join('spec/fixtures/path/some_file.csv') + # Rails.root.join('spec', 'fixtures', 'path', 'some_file.csv') + # + # "#{Rails.root}/spec/fixtures/files/some_file.csv" + # Rails.root.join('spec/fixtures/files/some_file.csv') + # Rails.root.join('spec', 'fixtures', 'files', 'some_file.csv') + # + # # good + # file_fixture('../path/some_file.csv').path + # file_fixture('../path/some_file.csv') + # file_fixture('../path/some_file.csv') + # + # file_fixture('some_file.csv').path + # file_fixture('some_file.csv') + # file_fixture('some_file.csv') + # + class FileFixture < Base + extend AutoCorrector + + MSG = 'Use `file_fixture`.' + + RESTRICT_ON_SEND = %i[join root].freeze + + DEFAULT_FILE_FIXTURE_PATTERN = %r{^spec/fixtures/files/}.freeze + DEFAULT_FIXTURE_PATTERN = %r{^spec/fixtures/}.freeze + + # @!method file_io?(node) + def_node_matcher :file_io?, <<~PATTERN + (send + (const {nil? cbase} :File) {:binread :binwrite :open :read :write} $...) + PATTERN + + # @!method rails_root_join(node) + def_node_matcher :rails_root_join, <<~PATTERN + (send + (send + (const {nil? cbase} :Rails) :root) :join (str $_)+) + PATTERN + + # @!method dstr_rails_root(node) + def_node_matcher :dstr_rails_root, <<~PATTERN + (dstr + (begin + (send + (const {nil? cbase} :Rails) :root)) + (str $_)) + PATTERN + + def on_send(node) + return unless (crime_scene, strings, method = evidence(node)) + return unless (new_path = new_path(strings)) + + add_offense(crime_scene) do |corrector| + replacement = "file_fixture('#{new_path}')" + replacement += ".#{method}" if method + + corrector.replace(crime_scene, replacement) + end + end + + private + + def evidence(node) + if !file_io?(node.parent) && (string = rails_root_join(node)) + [node, string, nil] + elsif (string = dstr_rails_root(node.parent.parent)) + [node.parent.parent, string.gsub(%r{^/}, ''), :path] + end + end + + def new_path(strings) + path = File.join(strings) + + if DEFAULT_FILE_FIXTURE_PATTERN.match?(path) + path.gsub(DEFAULT_FILE_FIXTURE_PATTERN, '') + elsif DEFAULT_FIXTURE_PATTERN.match?(path) + path.gsub(DEFAULT_FIXTURE_PATTERN, '../') + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rspec_cops.rb b/lib/rubocop/cop/rspec_cops.rb index ccc3f1579..da456c904 100644 --- a/lib/rubocop/cop/rspec_cops.rb +++ b/lib/rubocop/cop/rspec_cops.rb @@ -45,6 +45,7 @@ require_relative 'rspec/expect_change' require_relative 'rspec/expect_in_hook' require_relative 'rspec/expect_output' +require_relative 'rspec/file_fixture' require_relative 'rspec/file_path' require_relative 'rspec/focus' require_relative 'rspec/hook_argument' diff --git a/spec/rubocop/cop/rspec/file_fixture_spec.rb b/spec/rubocop/cop/rspec/file_fixture_spec.rb new file mode 100644 index 000000000..81bb3a736 --- /dev/null +++ b/spec/rubocop/cop/rspec/file_fixture_spec.rb @@ -0,0 +1,145 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::RSpec::FileFixture, :config do + it 'registers an offense when using ' \ + '`"\#{Rails.root}/spec/fixtures/files/some_file.csv"`' do + expect_offense(<<~RUBY) + "\#{Rails.root}/spec/fixtures/files/some_file.csv" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `file_fixture`. + RUBY + + expect_correction(<<~RUBY) + file_fixture('some_file.csv').path + RUBY + end + + it 'registers an offense when using ' \ + '`"\#{::Rails.root}/spec/fixtures/path/some_file.csv"`' do + expect_offense(<<~RUBY) + "\#{::Rails.root}/spec/fixtures/path/some_file.csv" + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `file_fixture`. + RUBY + + expect_correction(<<~RUBY) + file_fixture('../path/some_file.csv').path + RUBY + end + + it 'registers an offense when using ' \ + "`Rails.root.join('spec/fixtures/files/some_file.csv')`" do + expect_offense(<<~RUBY) + Rails.root.join('spec/fixtures/files/some_file.csv') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `file_fixture`. + RUBY + + expect_correction(<<~RUBY) + file_fixture('some_file.csv') + RUBY + end + + it 'registers an offense when using ' \ + "`::Rails.root.join('spec/fixtures/path/some_file.csv')`" do + expect_offense(<<~RUBY) + ::Rails.root.join('spec/fixtures/path/some_file.csv') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `file_fixture`. + RUBY + + expect_correction(<<~RUBY) + file_fixture('../path/some_file.csv') + RUBY + end + + it 'registers an offense when using ' \ + "`Rails.root.join('spec', 'fixtures/path/some_file.csv')`" do + expect_offense(<<~RUBY) + Rails.root.join('spec', 'fixtures/path/some_file.csv') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `file_fixture`. + RUBY + + expect_correction(<<~RUBY) + file_fixture('../path/some_file.csv') + RUBY + end + + it 'registers an offense when using ' \ + "`Rails.root.join('spec/fixtures', 'path/some_file.csv')`" do + expect_offense(<<~RUBY) + Rails.root.join('spec/fixtures', 'path/some_file.csv') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `file_fixture`. + RUBY + + expect_correction(<<~RUBY) + file_fixture('../path/some_file.csv') + RUBY + end + + it 'registers an offense when using ' \ + "`Rails.root.join('spec/fixtures/path', 'some_file.csv')`" do + expect_offense(<<~RUBY) + Rails.root.join('spec/fixtures/path', 'some_file.csv') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `file_fixture`. + RUBY + + expect_correction(<<~RUBY) + file_fixture('../path/some_file.csv') + RUBY + end + + it 'registers an offense when using ' \ + "`Rails.root.join('spec', 'fixtures', 'path/some_file.csv')`" do + expect_offense(<<~RUBY) + Rails.root.join('spec', 'fixtures', 'path/some_file.csv') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `file_fixture`. + RUBY + + expect_correction(<<~RUBY) + file_fixture('../path/some_file.csv') + RUBY + end + + it 'registers an offense when using ' \ + "`Rails.root.join('spec', 'fixtures/path', 'some_file.csv')`" do + expect_offense(<<~RUBY) + Rails.root.join('spec', 'fixtures/path', 'some_file.csv') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `file_fixture`. + RUBY + + expect_correction(<<~RUBY) + file_fixture('../path/some_file.csv') + RUBY + end + + it 'registers an offense when using ' \ + "`Rails.root.join('spec/fixtures', 'path', 'some_file.csv')`" do + expect_offense(<<~RUBY) + Rails.root.join('spec/fixtures', 'path', 'some_file.csv') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `file_fixture`. + RUBY + + expect_correction(<<~RUBY) + file_fixture('../path/some_file.csv') + RUBY + end + + it 'registers an offense when using ' \ + "`Rails.root.join('spec', 'fixtures', 'path', 'some_file.csv')`" do + expect_offense(<<~RUBY) + Rails.root.join('spec', 'fixtures', 'path', 'some_file.csv') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `file_fixture`. + RUBY + + expect_correction(<<~RUBY) + file_fixture('../path/some_file.csv') + RUBY + end + + %i[binread binwrite open read write].each do |method| + # This is handled by `Rails/RootPathnameMethods` + it 'does not register an offense when using ' \ + "`File.#{method}(Rails.root.join(...))`" do + expect_no_offenses(<<~RUBY) + File.#{method}(Rails.root.join('spec', 'fixtures', 'path', 'some_file.csv')) + RUBY + end + end +end