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:

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

Reek is run with these arguments:

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

Actual behavior

RuboCop is run with these arguments:

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

Reek is run with these arguments:

[
  "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:

   #!/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:

   #!/bin/sh
   set -x
   exec bundle exec reek "$@"

   Make sure to chmod +x it.

3. Use this configuration:

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

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

5. 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
   

6. 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:

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

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

  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:

  def bad
    "rubocop will complain about double quotes"
  end

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

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

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

  Style/FrozenStringLiteralComment:
    Enabled: false
  Style/StringLiterals:
    Exclude:
      - sub dir/rubyfile_good.rb

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

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

Test steps

• In a VSCode instance with PR Pass valid filenames to RuboCop and Reek #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]

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.

[github-actions]

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

[FooBarWidget]

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

[github-actions]

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

[ghost]

Ping

…

On Sat, Sep 25, 2021, 5:41 PM github-actions[bot] ***@***.***> wrote: This issue has not had activity for 30 days. It will be automatically closed in 7 days. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#719 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APRKQYUQYRZCDAWBNLT5DGTUDZT47ANCNFSM4ZZLQG2Q> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.