SublimeLinter / SublimeLinter-rubocop

SublimeLinter 3 plugin for Ruby, using rubocop.
MIT License
159 stars 40 forks source link

Copying files to /tmp breaks rubocop's Exclude directive #6

Closed borntyping closed 10 years ago

borntyping commented 10 years ago

I have a rubocop configuration that ignores certain cops in specific files:

SingleSpaceBeforeFirstArg:
    Exclude:
      - '**/metadata.rb'

However, when the lintier runs it copies the file to /tmp, as shown by it's console output:

SublimeLinter: rubocop: metadata.rb ['/usr/local/bin/rubocop', '--format', 'emacs', '--config', '/home/sam/.../.rubocop.yml'] 
SublimeLinter: rubocop output:
/tmp/tmpi9kcmc.rb:1:5: C: Put one space between the method name and the first argument.

Since the file now has a randomly generated name, rubocop can't correctly configure directives that use Exclude or similar directives. This could potentially be fixed by creating a temporary directory in /tmp, and then copying the file there (e.g. metadata.rb is copied to /tmp/xyzabc/metadata.rb).

eherot commented 10 years ago

:thumbsup: Yeah this is pretty annoying.

jeffbyrnes commented 10 years ago

:+1:

aparajita commented 10 years ago

I'd love to help, but I'm looking for someone who will take responsibility for maintaining this linter, since I don't use it and I don't have time to do it.

aparajita commented 10 years ago

If you could give me a minimal zip of a directory that contains a .rubocop.yml and a file that is excluded, that would help. I can't get the Exclude directive to work at all.

eherot commented 10 years ago

Are you able to get it to work with plain rubocop? It currently does not work at all with SublimeLinter-Rubocop.

aparajita commented 10 years ago

No, I cannot get Exclude to work with plain rubocop from the command line, that's why I would like a working example.

jeffbyrnes commented 10 years ago

Here's one:

SingleSpaceBeforeFirstArg:
  Exclude:
    - metadata.rb

Place that in a .rubocop.yml file, and for a file named metadata.rb, it will not complain about Ruby that looks like this:

name             'et_rails_app'
maintainer       'EverTrue, Inc.'
maintainer_email 'devops@evertrue.com'
license          'All rights reserved'
description      'Installs/Configures the EverTrue Rails Framework'
long_description IO.read(File.join(File.dirname(__FILE__), 'README.md'))
version          '5.0.3'
aparajita commented 10 years ago

The good news is my fix works. The bad news is rubocop's .rubocop.yml parsing is totally broken. If you use the plain filename metadata.rb it complains that you should use '**/metadata.rb', but of course when you do that it doesn't exclude the file. What a pain in the neck. Please report this glaring bug to the rubocop maintainers.

Anyway, I'll push a new version SublimeLinter sometime in the next few days.

aparajita commented 10 years ago

Grrrr... okay, rubocop is parsing .rubocop.yml correctly, but I'm afraid there is no way to get excludes to work using the non-deprecated syntax, because exclude paths are relative to the .rubocop.yml file, not the file being checked, and since the file is in a temp directory (even when named the same), a path like **/metadata.rb doesn't match.

You should explain the situation to the rubocop maintainers and ask them not to deprecate a plain filename as a path.

jeffbyrnes commented 10 years ago

@aparajita perhaps a dumb question, but I'll ask all the same: why not do the linting on the actual directory; why the need to do it in a temp dir?

aparajita commented 10 years ago

You're right, it's a dumb question. ;-)

Maybe you've forgotten that the main utility of SublimeLinter is that it lints as you type. Which means what's on disk is not what's in the buffer, hence the need for a temp file. So how could I lint the file on disk as you type with the same filename without overwriting the existing file? Yeah, I could rename the existing file, write the temp file to the existing filename, then rename the original. But this would cause all kinds of other problems for other programs that don't expect this kind of thing to happen. For example, in the javascript world it's common to have compilers running in a watcher process that recompile every time the file is modified. If I modified the file on every keystroke, it would be recompiling on every keystroke. Not ideal.

eherot commented 10 years ago

Might a possible approach for this particular problem be to parse the yaml and look for "exclude" lines and then try to match those against files in our directory and run with --force-exclusion against the corresponding file in the temp directory?

aparajita commented 10 years ago

Anything is possible. But who will do it? Not me.