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

ruby-beautify ignores Rubocop "Exclude" flags #1773

Closed
4 of 5 tasks
bbugh opened this issue Jul 20, 2017 · 4 comments · Fixed by #1776
Closed
4 of 5 tasks

ruby-beautify ignores Rubocop "Exclude" flags #1773

bbugh opened this issue Jul 20, 2017 · 4 comments · Fixed by #1776
Assignees
Milestone

Comments

@bbugh
Copy link
Contributor

bbugh commented Jul 20, 2017

Description

Given that I have this in my .rubocop.yml file:

Style/BlockDelimiters:
  Exclude:
    - "spec/**/*"

I expect that atom-beautify would not run the Style/BlockDelimiters cop on my spec files.

Unfortunately that's not the case. When running a beautification, it ignores the Exclude setting, even though running rubocop from the command line works properly.

Input Before Beautification

This is what the code looked like before:

it "raises an error" do
  expect {
    get something_path
  }.to raise_error ArgumentError
end

Expected Output

It should not change from the input.

Actual Output

The beautified code actually looked like this:

it "raises an error" do
  expect do
    get something_path
  end.to raise_error ArgumentError
end

This meant that Style/BlockDelimiters ran on the spec. This does not happen when running rubocop from the command line.

Steps to Reproduce

  1. Add this to the project .rubocop.yml

    Style/BlockDelimiters:
      Exclude:
        - "spec/**/*"
  2. Create a file like this under /spec:

    # /spec/example_spec.rb
    describe "something" do
      it "raises an error" do 
        expect {
          # whatever
        }.to raise_error ArgumentError
      end
    end
  3. Run command Atom Beautify: Beautify Editor

  4. This beautified code should not change!

Debug

The Debug file outputs a lot of NDA-violating content, so I can't post it.

Checklist

I have:

  • Tried uninstalling and reinstalling Atom Beautify to ensure it installed properly
  • Reloaded (or restarted) Atom to ensure it is not a caching issue
  • Searched through existing Atom Beautify Issues at https://github.com/Glavin001/atom-beautify/issues
    so I know this is not a duplicate issue
  • Filled out the Input, Expected, and Actual sections above or have edited/removed them in a way that fully describes the issue.
  • Generated debugging information by executing Atom Beautify: Help Debug Editor command in Atom and added link for debug.md Gist to this issue
@bbugh
Copy link
Contributor Author

bbugh commented Jul 20, 2017

I looked at the code, and this is happening because of how the temp file is written - it does not preserve the path and does not set the working directory, so Rubocop does not exclude the files as expected when it is run. Here it is simply writing a temp file:

          # src/beautifiers/rubocop.coffee:54
          "--config", configFile
          tempFile = @tempFile("temp", text, '.rb')
          ], {ignoreReturnCode: true})

However, this should be writing out to the relative path of the file with the correct file name, so that excludes are not ignored. Something like path.join(tempDir, relativePath). Running the command also needs to run on the working directory of the temp directory.

@Glavin001
Copy link
Owner

Glavin001 commented Jul 20, 2017

Thanks for reporting this issue and diving into the code to investigate further! Pull Requests are always welcome and it would be great to improve Rubocop support. Thanks in advance!

@stale
Copy link

stale bot commented Nov 4, 2017

This issue has been automatically marked as stale because it has not had recent activity. If this is still an issue, please add a comment. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 4, 2017
@stale stale bot closed this as completed Nov 11, 2017
@Glavin001 Glavin001 reopened this Mar 2, 2018
@stale stale bot removed the stale label Mar 2, 2018
@Glavin001
Copy link
Owner

Published to v0.32.0

@Glavin001 Glavin001 added this to the v0.32.0 milestone Mar 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants