misogi / vscode-ruby-rubocop

Rubocop extension for Visual Studio Code
https://marketplace.visualstudio.com/items/misogi.ruby-rubocop
MIT License
137 stars 68 forks source link

[BUG] Fix autocorrect #68

Closed maschwenk closed 6 years ago

maschwenk commented 6 years ago

Autocorrect was not respecting the .rubocop.yml (I think) because even though the cwd was being set to the project root, the actual file being autocorrected was in a tmp directory.

Repro the bug

The way I repro'd the incorrect behaviour is something like this:

  1. Enforce any arbitrary rule that deviates from the standard rubocop config. For example:
    Style/FrozenStringLiteralComment:
    Enabled: false
  2. Autocorrect the file
  3. Observe that it has inserted the frozen string literal comment

I don't really see the need the need to use a tmp file at all. Rubocop provides a --stdin option specifically for editor integrations so it's better to leverage that instead of creating/inserting/reading/deleting a file every time you autocorrect.

Additions:

  1. Added a format command to package.json so that contributors can easily run yarn run format before they commit
  2. Replace the tmp file usage described above with stdin/stdout buffers

The specs didn't break so I'm wondering how well tested this is. I don't write much Typescript so please be gentle 😄

maschwenk commented 6 years ago

@sergey-kintsel Do you have any thoughts on the above?

sergey-kintsel commented 6 years ago

I like it. I was about to make it using STDIN but failed for some reason. Then I checked the existing solutions for other editors and they all were making a tmp file. Anyways, :+1:

maschwenk commented 6 years ago

@misogi What are your thoughts here?

misogi commented 6 years ago

@maschwenk great!

This solution for PATH seems to be better than tmp file. But I feel this outputs (1st line: JSON, 2nd and after: code) are little unnatural. It may cause problems in future. It is good for just lint. I want to insert autocorrected output into JSON.. 🤔

maschwenk commented 6 years ago

@misogi I'm going to look around at other implementations and how they handle it. At least this one does the same: https://github.com/Glavin001/atom-beautify/pull/1776/files#diff-2cd0d999deee395901e272e2980d8f96R63

maschwenk commented 6 years ago

~Just a side note, another reason this is very important to me is because I use formatOnSave (for prettier/eslint). That means that when I hit save on a file, both the linter and the autocorrect run at the same time. After #63 , I started noticing a big slowdown for Rubocop and I think this is the root cause. I would even go further to say that I think the format on save should be configurable.~ Never mind, I just realized you can do language-sepcific formatOnSave settings, so I'll probably just turn that off. Maybe with using stdin method, this will be fast enough that the double-lint won't matter.

// Set the default
"editor.formatOnSave": false,
// Enable per-language
"[ruby]": {
    "editor.formatOnSave": false
}
misogi commented 6 years ago

@maschwenk I checked out this pull request. I formatted .rb file like this. Document become empty. Would you know something about this behavior?

a   =  [3,2,2]
b = ''
# print hd.join(' ')
print a.join(' ')
if b
  print    ' '
  print b
  print   ' '
else
  print ' '
end
misogi commented 6 years ago

5a00c14aad1826a3027fd39bf4d5e981

maschwenk commented 6 years ago

@misogi

I fixed your issue in 12a5f96 . If there are uncorrectable errors, we need to send along the stdout of the exception so that we can still process the partial autocorrection.

In a7590a6 I found a more failproof way of parsing the autocorrect format

misogi commented 6 years ago

@maschwenk Thanks! It seems fixed.

I will merge and release a new version.