Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rubocop-i18n plugin specs fail on rubocop 1.38.0 #11169

Open
Fryguy opened this issue Nov 9, 2022 · 4 comments
Open

rubocop-i18n plugin specs fail on rubocop 1.38.0 #11169

Fryguy opened this issue Nov 9, 2022 · 4 comments

Comments

@Fryguy
Copy link
Contributor

Fryguy commented Nov 9, 2022

Nearly all of the specs in the rubocop-18n plugin fail with the same error, undefined method 'disabled' for nil:NilClass. This started as of rubocop 1.38.0 and works fine in 1.37.1.

The line in question is https://github.com/rubocop/rubocop/blob/v1.38.0/lib/rubocop/comment_config.rb#L94, which implies the registry is nil at this point.

Expected behavior

This plugin's specs all pass with rubocop 1.37.1, and I would expect them to continue to pass.

Actual behavior

Thousands of spec errors all like the following:

  1) RuboCop::Cop::I18n::GetText::DecorateStringFormattingUsingPercent Ds_ decoration with paramaterized %0s behaves like a_detecting_cop has the correct rubocop warning
     Failure/Error: add_offense(message_section, message: "'#{method_name}' function, message string should not contain sprintf style formatting (ie %s)")

     NoMethodError:
       undefined method `disabled' for nil:NilClass
     Shared Example Group: "a_detecting_cop" called from ./spec/rubocop/cop/i18n/gettext/decorate_string_formatting_using_percent_spec.rb:44
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-1.38.0/lib/rubocop/comment_config.rb:94:in `inject_disabled_cops_directives'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-1.38.0/lib/rubocop/comment_config.rb:78:in `analyze'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-1.38.0/lib/rubocop/comment_config.rb:45:in `cop_disabled_line_ranges'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-1.38.0/lib/rubocop/comment_config.rb:38:in `cop_enabled_at_line?'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-1.38.0/lib/rubocop/cop/base.rb:429:in `enabled_line?'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-1.38.0/lib/rubocop/cop/base.rb:127:in `add_offense'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-1.38.0/lib/rubocop/cop/cop.rb:29:in `add_offense'
     # ./lib/rubocop/cop/i18n/gettext/decorate_string_formatting_using_percent.rb:37:in `on_send'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-1.38.0/lib/rubocop/cop/commissioner.rb:100:in `public_send'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-1.38.0/lib/rubocop/cop/commissioner.rb:100:in `block (2 levels) in trigger_responding_cops'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-1.38.0/lib/rubocop/cop/commissioner.rb:160:in `with_cop_error_handling'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-1.38.0/lib/rubocop/cop/commissioner.rb:99:in `block in trigger_responding_cops'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-1.38.0/lib/rubocop/cop/commissioner.rb:98:in `each'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-1.38.0/lib/rubocop/cop/commissioner.rb:98:in `trigger_responding_cops'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-1.38.0/lib/rubocop/cop/commissioner.rb:69:in `on_send'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-ast-1.23.0/lib/rubocop/ast/traversal.rb:20:in `walk'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rubocop-1.38.0/lib/rubocop/cop/commissioner.rb:86:in `investigate'
     # ./spec/shared_functions.rb:6:in `investigate'
     # ./spec/rubocop/cop/i18n/gettext/decorate_string_formatting_using_percent_spec.rb:7:in `block (2 levels) in <top (required)>'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example.rb:457:in `instance_exec'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example.rb:457:in `instance_exec'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/hooks.rb:365:in `run'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/hooks.rb:529:in `block in run_owned_hooks_for'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/hooks.rb:528:in `each'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/hooks.rb:528:in `run_owned_hooks_for'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/hooks.rb:615:in `block in run_example_hooks_for'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/hooks.rb:614:in `reverse_each'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/hooks.rb:614:in `run_example_hooks_for'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/hooks.rb:484:in `run'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example.rb:505:in `run_before_example'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example.rb:261:in `block in run'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example.rb:511:in `block in with_around_and_singleton_context_hooks'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example.rb:468:in `block in with_around_example_hooks'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/hooks.rb:486:in `block in run'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/hooks.rb:624:in `run_around_example_hooks_for'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/hooks.rb:486:in `run'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example.rb:468:in `with_around_example_hooks'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example.rb:511:in `with_around_and_singleton_context_hooks'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example.rb:259:in `run'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example_group.rb:646:in `block in run_examples'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example_group.rb:642:in `map'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example_group.rb:642:in `run_examples'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example_group.rb:607:in `run'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example_group.rb:608:in `block in run'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example_group.rb:608:in `map'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example_group.rb:608:in `run'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example_group.rb:608:in `block in run'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example_group.rb:608:in `map'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/example_group.rb:608:in `run'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/runner.rb:121:in `block (3 levels) in run_specs'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/runner.rb:121:in `map'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/runner.rb:121:in `block (2 levels) in run_specs'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/configuration.rb:2070:in `with_suite_hooks'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/runner.rb:116:in `block in run_specs'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/reporter.rb:74:in `report'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/runner.rb:115:in `run_specs'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/runner.rb:89:in `run'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/runner.rb:71:in `run'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/lib/rspec/core/runner.rb:45:in `invoke'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/rspec-core-3.12.0/exe/rspec:4:in `<top (required)>'
     # /Users/jfrey/.gem/ruby/2.7.2/bin/rspec:23:in `load'
     # /Users/jfrey/.gem/ruby/2.7.2/bin/rspec:23:in `<top (required)>'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/bundler-2.2.24/lib/bundler/cli/exec.rb:63:in `load'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/bundler-2.2.24/lib/bundler/cli/exec.rb:63:in `kernel_load'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/bundler-2.2.24/lib/bundler/cli/exec.rb:28:in `run'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/bundler-2.2.24/lib/bundler/cli.rb:475:in `exec'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/bundler-2.2.24/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/bundler-2.2.24/lib/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/bundler-2.2.24/lib/bundler/vendor/thor/lib/thor.rb:392:in `dispatch'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/bundler-2.2.24/lib/bundler/cli.rb:31:in `dispatch'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/bundler-2.2.24/lib/bundler/vendor/thor/lib/thor/base.rb:485:in `start'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/bundler-2.2.24/lib/bundler/cli.rb:25:in `start'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/bundler-2.2.24/exe/bundle:49:in `block in <top (required)>'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/bundler-2.2.24/lib/bundler/friendly_errors.rb:128:in `with_friendly_errors'
     # /Users/jfrey/.gem/ruby/2.7.2/gems/bundler-2.2.24/exe/bundle:37:in `<top (required)>'
     # /Users/jfrey/.gem/ruby/2.7.2/bin/bundle:23:in `load'
     # /Users/jfrey/.gem/ruby/2.7.2/bin/bundle:23:in `<main>'

Steps to reproduce the problem

  • Clone rubocop-i18n (current HEAD is puppetlabs/rubocop-i18n@92ef8f4)
  • bundle update
  • bundle exec rake spec
    • or a smaller reproducer with: bundle exec rspec spec/rubocop/cop/i18n/gettext/decorate_string_formatting_using_percent_spec.rb:44

RuboCop version

1.38.0 (using Parser 3.1.2.1, rubocop-ast 1.23.0, running on ruby 2.7.2) [x86_64-darwin19]
@camilopayan
Copy link

We ran into the same problem w/ StandardRb, and traced it back to this PR and the additions made to the processed source module.

I was able to fix it on the standardrb end by adding the same methods to our test helper that were added to the cop_helper in rubocop and that fixes it, but it seems like there should be some change to the module class either in rubocop-ast or in rubocop that would elide the need for the test fix? Not sure, what do you think @tdeo?

@tdeo
Copy link
Contributor

tdeo commented Nov 14, 2022

Hey @camilopayan, I'm wondering if you couldn't be using the CopHelper provided by rubocop directly by importing it via require 'rubocop/rspec/support'?

I believe that's what the subproject like rubocop-rails, rubocop-rspec are doing and should let you keep compatibility with other changes within rubocop's codebase in the future

@splattael
Copy link
Contributor

Using require 'rubocop/rspec/support' works best I believe 👍

There's caveat using it in (large) projects which contain RuboCop related and non-related specs. Because CopHelper is included by default in every spec it might collide with specs which define, for example, let(:configuration).

This happened at GitLab twice already: https://gitlab.com/gitlab-org/gitlab/-/issues/382452#note_1182874406

Possible solution we found so far:

  1. Name the lets differently (example)
  2. Include CopHelper et al only for RuboCop related specs (e.g. tagged with type: :rubocop) (example)
  3. Run RuboCop specs separately from the rest of the test suite to avoid collision (see comment)

@tdeo
Copy link
Contributor

tdeo commented Nov 24, 2022

There's caveat using it in (large) projects which contain RuboCop related and non-related specs. Because CopHelper is included by default in every spec it might collide with specs which define, for example, let(:configuration).

This happened at GitLab twice already: https://gitlab.com/gitlab-org/gitlab/-/issues/382452#note_1182874406

Ouch - I believe rubocop could provide some better documentation about it. I believe that shouldn't be a big deal for rubocop-i18n but for the record, here's how I'm doing it in our project:

In spec/cops_helper.rb (which re require only in spec file for custom cops, but I don't see any problem that would be caused by requiring directly in spec_helper.rb):

require 'rubocop'
require 'rubocop/rspec/cop_helper'
require 'rubocop/rspec/shared_contexts'
require 'rubocop/rspec/expect_offense'
require 'rubocop-rspec'
require 'rubocop/rspec/shared_contexts/default_rspec_language_config_context'

require 'cops' # This is actually `lib/cops/` which contains our custom cops, make sure they're required during cop specs since they're not autoloaded

RSpec.configure do |config|
  # spec/lib/cops/ is the folder containing specs for cops in our repo
  config.define_derived_metadata(file_path: %r{/spec/lib/cops/}) do |meta|
    meta[:cop_spec] = true
  end

  config.include CopHelper, :cop_spec
  config.include RuboCop::RSpec::ExpectOffense, :cop_spec

  config.include_context 'config', :cop_spec
  config.include_context 'with default RSpec/Language config', :cop_spec
end

This way, you don't have the default injections of CopHelper for all your test suite but only for specific files. Credits to rubocop-rspec for the strategy given in their doc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants