Skip to content

Enable RuboCop in vscode & jekyll directories #3246

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,10 @@ AllCops:
- "sorbet/rbi/shims/**/*.rbi"
Exclude:
- "vendor/**/*"
- "vscode/**/*"
- "vscode/node_modules/**/*"
- "features/**/*"
- "test/fixtures/**/*"
- "test/expectations/**/*"
- "jekyll/**/*"

Layout/LeadingCommentSpace:
AllowRBSInlineAnnotation: true
Expand All @@ -39,6 +38,10 @@ RubyLsp/UseLanguageServerAliases:
Exclude:
- "test/**/*.rb"

RubyLsp/UseRegisterWithHandlerMethod:
Exclude:
- "jekyll/add-ons.markdown" # this cop doesn't apply to enhancements

Sorbet/FalseSigil:
Enabled: false

Expand Down
1 change: 0 additions & 1 deletion jekyll/Gemfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

# frozen_string_literal: true

source "https://rubygems.org"
Expand Down
12 changes: 6 additions & 6 deletions jekyll/add-ons.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -286,13 +286,13 @@ class MyIndexingEnhancement < RubyIndexer::Enhancement
# Create the array of signatures that this method will accept. Every signatures is composed of a list of
# parameters. The parameter classes represent each type of parameter
signatures = [
RubyIndexer::Entry::Signature.new([RubyIndexer::Entry::RequiredParameter.new(name: :a)])
RubyIndexer::Entry::Signature.new([RubyIndexer::Entry::RequiredParameter.new(name: :a)]),
]

@listener.add_method(
"new_method", # Name of the method
location, # Prism location for the node defining this method
signatures # Signatures available to invoke this method
signatures, # Signatures available to invoke this method
)
end

Expand Down Expand Up @@ -576,7 +576,7 @@ require "ruby_lsp/test_helper"

class MyAddonTest < Minitest::Test
def test_my_addon_works
source = <<~RUBY
source = <<~RUBY
# Some test code that allows you to trigger your add-on's contribution
class Foo
def something
Expand All @@ -595,9 +595,9 @@ class MyAddonTest < Minitest::Test
},
position: {
line: 3,
character: 5
}
}
character: 5,
},
},
)

# Pop the server's response to the definition request
Expand Down
4 changes: 2 additions & 2 deletions jekyll/vscode-extension.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ repository is.
Consider a project where the top level of the repository is a Rails application and a sub-directory called `frontend`
contains a React application that implements the frontend layer.

```
```txt
my_project/
frontend/
Gemfile
Expand Down Expand Up @@ -354,7 +354,7 @@ A possible configuration for the `super_awesome_project` would be this:

Now consider a monorepo where both the client and the server are under sub-directories.

```
```txt
my_project/
client/
server/
Expand Down
4 changes: 4 additions & 0 deletions vscode/.rubocop.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
inherit_from: ../.rubocop.yml

Sorbet:
Enabled: false
Comment on lines +3 to +4
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RuboCop was complaining that vscode/activation.rb should be

# typed: strict

which enforces sigs on all methods. The file currently has no method definitions, but if it ever gained any, we couldn't attach any sigs without sorbet-runtime, so strict is clearly too high.

We could use

# typed: true

to simply benefit from type checking, but then Sorbet complains about

env = ENV.filter_map do |k, v|
  utf_8_value = v.dup.force_encoding(Encoding::UTF_8)
  "#{k}RUBY_LSP_VS#{utf_8_value}" if utf_8_value.valid_encoding?
end
env.unshift(RUBY_VERSION, Gem.path.join(","), !!defined?(RubyVM::YJIT))
#    Expected `String` but found `T::Boolean` ^^^^^^^^^^^^^^^^^^^^^^^^^ 
$stderr.print("RUBY_LSP_ACTIVATION_SEPARATOR#{env.join("RUBY_LSP_FS")}RUBY_LSP_ACTIVATION_SEPARATOR")

because it thinks env must be a T::Array[String]. We could do

env.unshift(RUBY_VERSION, Gem.path.join(","), defined?(RubyVM::YJIT) ? "true" : "false")

but technically, we don't need to (interpolating a boolean works fine), so it seemed simplest to just disable the Sorbet cops.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, all we have under vscode for Ruby are the activation scripts. There's no point in adopting Sorbet, let's please keep it disabled.

4 changes: 2 additions & 2 deletions vscode/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ repository is.
Consider a project where the top level of the repository is a Rails application and a sub-directory called `frontend`
contains a React application that implements the frontend layer.

```
```txt
my_project/
frontend/
Gemfile
Expand Down Expand Up @@ -350,7 +350,7 @@ A possible configuration for the `super_awesome_project` would be this:

Now consider a monorepo where both the client and the server are under sub-directories.

```
```txt
my_project/
client/
server/
Expand Down
4 changes: 3 additions & 1 deletion vscode/activation.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# frozen_string_literal: true

env = ENV.filter_map do |k, v|
utf_8_value = v.dup.force_encoding(Encoding::UTF_8)
"#{k}RUBY_LSP_VS#{utf_8_value}" if utf_8_value.valid_encoding?
end
env.unshift(RUBY_VERSION, Gem.path.join(","), !!defined?(RubyVM::YJIT))
STDERR.print("RUBY_LSP_ACTIVATION_SEPARATOR#{env.join("RUBY_LSP_FS")}RUBY_LSP_ACTIVATION_SEPARATOR")
$stderr.print("RUBY_LSP_ACTIVATION_SEPARATOR#{env.join("RUBY_LSP_FS")}RUBY_LSP_ACTIVATION_SEPARATOR")
Loading