Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Linters run from file's directory, not from workspace root #717

Closed
FooBarWidget opened this issue Mar 25, 2021 · 16 comments
Closed

Linters run from file's directory, not from workspace root #717

FooBarWidget opened this issue Mar 25, 2021 · 16 comments
Labels
enhancement Improves upon existing functionality help-wanted PRs welcome

Comments

@FooBarWidget
Copy link
Contributor

FooBarWidget commented Mar 25, 2021

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

Linters such as Rubocop are run with a current working directory equal to the workspace root.

Actual behavior

Linters such as Rubocop are run with a current working directory equal to that of the open file.

To reproduce:

  1. Create a shell script ~/fake-rubocop that prints the current working directory as well as executes Rubocop:

    #!/bin/sh
    pwd >&2
    bundle exec rubocop "$@"

    Make sure to chmod +x the script.

  2. Use this configuration:

    "ruby.useLanguageServer": true,
    "ruby.lint": {
        "rubocop": {
            "command": "/path-to-home-directory/fake-rubocop"
        }
    },
  3. Open a Ruby project directory. Inside that project directory, open a Ruby file that's not in the workspace root (e.g. lib/foo.rb).

  4. In the Ruby Language Server output, observe that fake-rubocop logs "/path-to-workspace/lib" as current working directory, instead of "/path-to-workspace".

Comments

The troubleshooting document clearly says that this is not the expected behavior:

All commands are run with a cwd of the workspace root.

However, if we look at packages/language-server-ruby/src/Linter.ts, then we see:

const executionRoot = path.dirname(URI.parse(document.uri).fsPath);

This executionRoot is then used to set the current working directory when executing linters. In packages/language-server-ruby/src/linters/BaseLinter.ts:

return spawn(cmd, args, {
    env: this.config.env,
    cwd: this.config.executionRoot,  // <-------
    stdin: of(this.document.getText()),
})...

The code clearly contradicts the documented behavior.

I believe that this bug is the cause of problems like #227. Rubocop only looks for .rubocop.yml in the current working directory. Update: #719 has been identified as the true cause of #227.

FooBarWidget added a commit to FooBarWidget/vscode-ruby that referenced this issue Mar 25, 2021
FooBarWidget added a commit to FooBarWidget/vscode-ruby that referenced this issue Mar 25, 2021
@wingrunr21
Copy link
Collaborator

Closing due to linking against #719

@FooBarWidget
Copy link
Contributor Author

@wingrunr21 I don't think this issue should be closed. This issue isn't the cause of the Rubocop problem, but it's still wrong behavior that contradicts the documentation.

@FooBarWidget
Copy link
Contributor Author

@wingrunr21 Could I ask your attention for the previous comment? Do you agree or disagree?

@wingrunr21 wingrunr21 reopened this Apr 14, 2021
@wingrunr21
Copy link
Collaborator

What behavior are you seeing around this? c1b87d3 was the original commit that implemented this behavior as people were submitting reports that when RuboCop configs were NOT in the workspace root RuboCop was not finding those configurations due to the cwd of execution. RuboCop and others should look recursively up directory trees to find a valid config so in theory running from the same directory as the file should resolve both issues.

@FooBarWidget
Copy link
Contributor Author

FooBarWidget commented Apr 25, 2021

@wingrunr21 Okay, that sounds like a good reason. But if the intention is to run Rubocop from the file's directory instead of the workspace root, then the docs should not say that linters will be run from the workspace root.

@FooBarWidget
Copy link
Contributor Author

I created a new pull request that fixes the docs to match actual behavior. See above.

@github-actions
Copy link

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

@github-actions github-actions bot added the stale label May 26, 2021
@FooBarWidget
Copy link
Contributor Author

FooBarWidget commented May 26, 2021

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

@github-actions github-actions bot removed the stale label May 27, 2021
@github-actions
Copy link

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

@github-actions github-actions bot added the stale label Jul 27, 2021
@FooBarWidget
Copy link
Contributor Author

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

@github-actions github-actions bot removed the stale label Jul 28, 2021
@tschaub
Copy link

tschaub commented Sep 15, 2021

I'm working on a project that has Gemfiles at multiple levels (for locally developed plugins). rubocop is added as a dependency of the root Gemfile. I'm finding that the extension fails to run rubocop for files whose nearest parent Gemfile doesn't include rubocop. I assume this is due to the cwd issue mentioned here (and not #719).

I would guess that finding the appropriate configuration for rubocop would be fixed by addressing #719. But finding rubocop itself would depend on the cwd.

@wingrunr21
Copy link
Collaborator

@tschaub the behavior you are seeing is due to how bundler works vs this extension. If we were to move back to using workspaceRoot for the execution, then people with your situation with different rubocops in sub-Gemfiles wouldn't work.

I think the solution here is to add a configuration option that allows people to specify where a linter is executed. Either at workspaceRoot or at cwd.

@github-actions
Copy link

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

@github-actions github-actions bot added the stale label Nov 27, 2021
@AlexWayfer
Copy link

Actual.

@github-actions github-actions bot removed the stale label Nov 28, 2021
@github-actions
Copy link

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

@github-actions github-actions bot added the stale label Jan 27, 2022
@ccutrer
Copy link

ccutrer commented Jan 27, 2022

Heh, GitHub actions isn't so good at math. It's been a lot more than 30 days since the last activity. Well, it's still a problem, so let's keep it open...

@wingrunr21 wingrunr21 added enhancement Improves upon existing functionality help-wanted PRs welcome and removed stale labels Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Improves upon existing functionality help-wanted PRs welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants