Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support asdf's .tool-versions file #9319

Merged
merged 1 commit into from Jan 4, 2021
Merged

Support asdf's .tool-versions file #9319

merged 1 commit into from Jan 4, 2021

Conversation

noon-ng
Copy link
Contributor

@noon-ng noon-ng commented Dec 31, 2020

Hey all,
Happy new year! 馃帀
First time contribution so apologies in advance if I'm going about this the wrong way. I figured this could be a worthwhile feature addition to Rubocop.

For context:

I recently switched to asdf from rbenv on my personal dev environment.

I decided to try out Ruby 3.0 today on a project, and when I started getting errors from Rubocop on Ruby 3.0-specific syntax, I realized that project had a leftover .ruby-version pointing to 2.6.3 which surprisingly Rubocop was picking up.

At that point I felt there was an opportunity to extend the target version detection logic to be aware of asdf's .tool-versions file, so here is a proposed change to support that.

Suggest reading the commit message for detail.

Thanks!


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

@noon-ng noon-ng marked this pull request as ready for review January 1, 2021 02:10
@noon-ng
Copy link
Contributor Author

noon-ng commented Jan 1, 2021

(For the of sake of transparency: I had this on draft with a note eliciting opinions on whether/how to eliminate some duplication in the original commit, but eventually I realized the file detection logic I had for .tool-versions was unnecessary so I reworked the PR to be a bit more concise.)

Augments the TargetRuby class with a ToolVersionsFile class, giving
asdf [1] users the option to specify the target Ruby syntax through its
default config file .tool-versions [2].

The file detection and content matching logic is the same whether
.tool-versions or .ruby-version is in use. The only differences between
the two cases are the file name and pattern used to extract the version
number. So, have ToolVersionsFile subclass RubyVersionFile, and modify
find_version to get file name and pattern from available instance
methods.

Since the author is in the process of replacing rbenv with asdf in their
dev environment, it felt sensible to stipulate that ToolVersionsFile
would be more authoritative than RubyVersionFile (but still less than
RuboCopConfig).

In the absence of a .tool-versions file, or when it doesn't happen to
specify a ruby version, revert to the original logic and allow both
rbenv and asdf (in asdf-ruby legacy mode) to be used.

[1]
asdf is a plugin-based runtime installer/manager - think RVM or
rbenv, but not limited to Ruby: github.com/asdf-vm/asdf

Ruby support for asdf is provided by the asdf-ruby plugin:
github.com/asdf-vm/asdf-ruby

[2]
Each line in .tool-versions identifies the runtimes and the version
needed. For example, here's the contents of the .tool-versions in the
author's home dir in their personal laptop:

```
nodejs 14.9.0
ruby 3.0.0
yarn 1.22.5
```
@bbatsov bbatsov merged commit 1b8388f into rubocop:master Jan 4, 2021
@bbatsov
Copy link
Collaborator

bbatsov commented Jan 4, 2021

Looks good. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants