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

Interesting interaction with RSpec let inside a block #1184

Closed
jdickey opened this issue Jun 30, 2014 · 3 comments
Closed

Interesting interaction with RSpec let inside a block #1184

jdickey opened this issue Jun 30, 2014 · 3 comments

Comments

@jdickey
Copy link

jdickey commented Jun 30, 2014

Given the following RSpec spec file (which passes):

require 'spec_helper'

require 'cco/blog_cco2'

# Second-generation CCO for Blogs. Does not (presently) subclass Base.
module CCO
  describe BlogCCO2 do
    let(:from) { BlogCCO2.public_method :from_entity }
    let(:to) { BlogCCO2.public_method :to_entity }

    describe 'has a .from_entity method that' do
      let(:p) { BlogCCO2.public_method :from_entity }

      it 'is a class method' do
        expect(from.receiver).to be BlogCCO2
        expect(p.receiver).to be BlogCCO2
      end

      it 'takes two parameters' do
        expect(p.arity.abs).to eq 2
      end

      describe 'takes two parameters, where' do
        it 'the first parameter is required' do
          param = p.parameters.first
          expect(param[0]).to be :req
        end

        it 'the second parameter is optional' do
          param = p.parameters.second
          expect(param[0]).to be :opt
        end
      end # describe 'takes two parameters, where'
    end # describe 'has a .from_entity method that'

    describe 'has a .to_entity method that' do
      let(:p) { BlogCCO2.public_method :to_entity }

      it 'is a class method' do
        expect(p.receiver).to be BlogCCO2
      end

      it 'takes one required parameter' do
        expect(p.arity).to be 1
        expect(p.parameters.first[0]).to be :req
      end
    end # describe 'has a .to_entity method that'

  end # describe CCO::BlogCCO2
end # module CCO

running RuboCop with the command line rubocop -R spec/models/cco/blog_cco2_spec.rb yields the output

Inspecting 1 file
C

Offenses:

spec/models/cco/blog_cco2_spec.rb:17:16: C: Do not write to stdout. Use Rails' logger if you want to log.
        expect(p.receiver).to be BlogCCO2
               ^
spec/models/cco/blog_cco2_spec.rb:21:16: C: Do not write to stdout. Use Rails' logger if you want to log.
        expect(p.arity.abs).to eq 2
               ^
spec/models/cco/blog_cco2_spec.rb:26:19: C: Do not write to stdout. Use Rails' logger if you want to log.
          param = p.parameters.first
                  ^
spec/models/cco/blog_cco2_spec.rb:31:19: C: Do not write to stdout. Use Rails' logger if you want to log.
          param = p.parameters.second
                  ^
spec/models/cco/blog_cco2_spec.rb:41:16: C: Do not write to stdout. Use Rails' logger if you want to log.
        expect(p.receiver).to be BlogCCO2
               ^
spec/models/cco/blog_cco2_spec.rb:45:16: C: Do not write to stdout. Use Rails' logger if you want to log.
        expect(p.arity).to be 1
               ^
spec/models/cco/blog_cco2_spec.rb:46:16: C: Do not write to stdout. Use Rails' logger if you want to log.
        expect(p.parameters.first[0]).to be :req
               ^

1 file inspected, 7 offenses detected

Note that every reference to p triggers a RuboCop offense, including the first (at line 17), but the expect call at line 16 (which checks from.receiver rather than p.receiver) is not flagged.

Why? Again, RSpec gives me a green bar; no failures.

Behaviour observed in Rubocop 0.23.0 and 0.24.0, with Rails 4.1.2 under Ruby 2.0.0p481. The RSpec version is 2.99.1.

@bbatsov
Copy link
Collaborator

bbatsov commented Jun 30, 2014

That's a bug that we'll have to fix.

@yujinakayama
Copy link
Collaborator

I think the underlying problem is that the Output cop (it's a Rails cop) does not have Include option to restrict the target only to app/**/* by default.

Also, it's difficult to differentiate Kernel#p and any p defined by user (RSpec's let actually defines an instance method).

@jdickey
Copy link
Author

jdickey commented Jul 1, 2014

@yujinakayama Good point; I'd forgotten about Kernel#p. I still think it's a Rubocop bug, and I'm glad that @bbatsov apparently agrees.

@bbatsov bbatsov closed this as completed in ee06209 Aug 4, 2014
koic added a commit to koic/rubocop-rails that referenced this issue Mar 29, 2023
Fixes rubocop#934.

This PR fixes a false negative for `Rails/Output`
when print methods without arguments.

rubocop/rubocop#1184 reported the following problem,
but it seems that it was mistakenly updated to allow no argument cases.

```console
expect(p.arity).to be 1
       ^
spec/models/cco/blog_cco2_spec.rb:46:16: C: Do not write to stdout. Use Rails' logger if you want to log.
```

It seems that rubocop/rubocop#6829 has since used that mistake.
So this PR steers `Rails/Output` back to its original intent.
koic added a commit to koic/rubocop-rails that referenced this issue Mar 29, 2023
Fixes rubocop#934.

This PR fixes a false negative for `Rails/Output`
when print methods without arguments.

rubocop/rubocop#1184 reported the following problem,
but it seems that it was mistakenly updated to allow no argument cases.

```console
expect(p.arity).to be 1
       ^
spec/models/cco/blog_cco2_spec.rb:46:16: C: Do not write to stdout. Use Rails' logger if you want to log.
```

It seems that rubocop/rubocop#6829 has since used that mistake.
So this PR steers `Rails/Output` back to its original intent.
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

3 participants