Vscode-ruby: Wrong file path passed to RuboCop and Reek

[FooBarWidget]

Your environment

• vscode-ruby version: 0.28.1, and also commit 7b5b602
• Ruby version: 2.7.2
• Ruby version manager (if any): RVM
• VS Code version: 1.54.3
• Operating System: macOS Catalina
• Using language server? (eg useLanguageServer is true in your configuration?) yes

Expected behavior

RuboCop is run with these arguments:

~javascript
[
"bundle",
"exec",
"rubocop",
"-s",
"/path-to-file.rb",
"-f",
"json"
]
~

Reek is run with these arguments:

~javascript
[
"bundle",
"exec",
"reek",
"-f",
"json",
"--stdin-filename",
"/path-to-file.rb"
]
~

Actual behavior

RuboCop is run with these arguments:

~javascript
[
"bundle",
"exec",
"rubocop",
"-s",
"'/path-to-file.rb'", // <--- note the extraneous single quotes!
"-f",
"json"
]
~

Reek is run with these arguments:

~javascript
[
"bundle",
"exec",
"reek",
"-f",
"json",
"--stdin-filename",
"'/path-to-file.rb'" // <--- note the extraneous single quotes!
]
~

To reproduce:

1. Create a shell script ~/fake-rubocop that prints the exact arguments passed:

   ~~~bash

   !/bin/sh

   set -x
   exec bundle exec rubocop "$@"
   ~~~

   Make sure to chmod +x it.

2. Create a shell script ~/fake-reek that prints the exact arguments passed:

   ~~~bash

   !/bin/sh

   set -x
   exec bundle exec reek "$@"
   ~~~

   Make sure to chmod +x it.

   1. Use this configuration:

   ~json
   "ruby.useLanguageServer": true,
   "ruby.lint": {
   "rubocop": {
   "command": "/path-to-home-directory/fake-rubocop"
   },
   "reek": {
   "command": "/path-to-home-directory/fake-reek"
   }
   },
   ~

3. Open a Ruby project directory. Inside that project directory, open a random Ruby file.

4. In the Ruby Language Server output, observe that fake-rubocop logs this:

   ~~~

   • exec bundle exec rubocop -s ''\''/path-to-file.rb'\''' -f json
     ~~~

     We expect no extraneous quotes:

     ~~~

   • exec bundle exec rubocop -s /path-to-file -f json
     ~~~

5. In the Ruby Language Server output, observe that fake-reek logs this:

   ~~~

   • exec bundle exec reek -f json --stdin-filename ''\''/path-to-file.rb'\'''
     ~~~

     We expect no extraneous quotes:

     ~~~

   • exec bundle exec reek -f json --stdin-filename /path-to-file.rb
     ~~~

Comments

Because of the extraneous quotes, we pass invalid filenames to RuboCop and Reek. I don't know whether this causes problems for Reek, but it does cause problems for RuboCop: see #227. Normally, RuboCop checks each level of parent directory in the input file's path, in search for a .rubocop.yml. But because we pass an invalid filename, RuboCop can't find a .rubocop.yml, and ends up ignoring the .rubocop.yml that the project has. The extraneous quotes are deliberately inserted by the code:

• In packages/language-server-ruby/src/linters/RuboCop.ts:

  ~typescript
  let args = ['-s', '${documentPath.fsPath}', '-f', 'json'];
  ~

• In packages/language-server-ruby/src/linters/Reek.ts:

  ~typescript
  return ['-f', 'json', '--stdin-filename', '${documentPath.fsPath}'];
  ~

I suspect that the author meant to prevent any issues with spaces in filenames. However, Linters are executed through cross-spawn, which doesn't execute commands through a shell. There should be no problems with spaces in filenames: each element in the argument array represents exactly one argument, with no shell trying to split up filenames by space.

[wingrunr21]

This change was introduced in #647 by @coneybeare.

@coneybeare would you be able to provide your insight here?

[wingrunr21]

ok. @coneybeare if you don't have any comments here it sounds like #647 caused a regression that I'd like to correct. Absent input to the contrary I'll likely merge #720

Giving this through the end of this week then I'll be merging.

[coneybeare]

oh hm, I actually did comment, not sure where it went.

Here was the original issue that prompted my PR. Despite OPs assertion that There should be no problems with spaces in filenames, I saw that they were necessary, at least at the time of the PR.

As long as the proposed fix doesn't introduce a regression for what I fixed, I am onboard.

[AndreiEres]

Any updates?

[FooBarWidget]

I plan on looking into the issue #641 that @coneybeare referenced. I just haven't had time so far.

[FooBarWidget]

@coneybeare I've tested #641. Just like you, I'm on macOS. I've confirmed that my PR #720 has no problems with spaces in filenames.

Test setup

• Rubocop 1.12.0, Ruby 2.7.2, macOS Catalina.

• Create a project directory ~/ruby project with spaces

• Create a file ~/ruby project with spaces/sub dir/rubyfile_bad.rb with:

  ~ruby
  def bad
  "rubocop will complain about double quotes"
  end
  ~

• Create a file ~/ruby project with spaces/sub dir/rubyfile_good.rb with:

  ~ruby
  def good
  "rubocop will not complain about double quotes"
  end
  ~

• Create a file ~/ruby project with spaces/.rubocop.yml with:

  ~~~yaml
  Style/FrozenStringLiteralComment:
  Enabled: false
  Style/StringLiterals:
  Exclude:

  • sub dir/rubyfile_good.rb
    ~~~

• Create a file ~/ruby project with spaces/.vscode/settings.json with:

  ~json
  {
  "ruby.useLanguageServer": true,
  "ruby.lint": {
  "rubocop": true
  }
  }
  ~

Test steps

• In a VSCode instance with PR #720 enabled, open the project workspace.
• Open ruby_bad.rb and ruby_good.rb.

Expected behavior

• VSCode runs Rubocop.
• Rubocop complains only about ruby_bad.rb, but not about ruby_good.rb.

Actual behavior

Matches expected behavior.

[github-actions[bot]]

This issue has not had activity for 30 days. It will be automatically closed in 7 days.

[FooBarWidget]

Please don't auto-close this PR. It's awaiting review.