Skip to content

Commit fdf1916

Browse files
igrayclaude
andcommitted
Upgrade to ruby-lsp 0.26
Bump the ruby-lsp dependency from `~> 0.17` to `~> 0.26` and raise required_ruby_version to >= 3.0.0 to match what ruby-lsp now requires. Addon changes: - Implement `version`. Since 0.23 the addon base class treats it as abstract and uses it for the `Addon.get(name, constraint)` compatibility check, so without it any addon querying this one raises AbstractMethodInvokedError. - Drop the sorbet-runtime dependency. ruby-lsp no longer depends on it and the only reference left here was a bare require; there is no `T.` or `T::Sig` usage anywhere in the gem. Test changes: the suite hand-rolled `with_server` against internals that have since been removed (`core_ext/uri`, `RubyIndexer::IndexablePath`). It now uses the `RubyLsp::TestHelper` module that ruby-lsp ships for addons, requires `ruby_lsp/internal` instead of a long explicit require list, and uses `pop_result` so notifications in the queue don't break the assertions. The expected reek docs URL is interpolated from `Reek::Version::STRING` rather than hardcoded. Clean up the reek warnings so `rake code_analysis` passes: - `Runner#build_examiner` (FeatureEnvy) is gone. It existed to work around Examiner not accepting a separate source and origin, reaching into two of its instance variables to do so. `SourceCode.from(source, origin:)` takes an explicit origin and Examiner passes a SourceCode through untouched, so the workaround is no longer needed. - `Runner#warning_to_diagnostic` (UtilityFunction) moves to `Reek::Diagnostic.from_warning`, which is what the smell was suggesting. - `Runner#run_formatting` (UtilityFunction) is suppressed in place. The Formatter interface requires an instance method and reek is a linter, so there is nothing to format and no instance state to depend on. The origin behaviour from #5 had no test, so add one covering it: a file under a directory with a `.reek.yml` directory directive is exempt while a file outside it is not. Verified it fails when the origin is dropped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent b1e2e26 commit fdf1916

5 files changed

Lines changed: 117 additions & 110 deletions

File tree

lib/ruby_lsp/reek/addon.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
# frozen_string_literal: true
22

33
require "bundler/setup"
4-
require "sorbet-runtime"
54
require "ruby_lsp/addon"
65
require "ruby_lsp/base_server"
76
require "ruby_lsp/server"
@@ -22,6 +21,11 @@ def name
2221
"Reek: Code smell detector for Ruby"
2322
end
2423

24+
# @return [String] The version of the addon.
25+
def version
26+
::RubyLsp::Reek::VERSION
27+
end
28+
2529
# @param global_state [GlobalState] The global state of the Ruby LSP server.
2630
# @param outgoing_queue [Thread::Queue] The outgoing message queue of the Ruby LSP server.
2731
def activate(global_state, message_queue)

lib/ruby_lsp/reek/diagnostic.rb

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# frozen_string_literal: true
2+
3+
module RubyLsp
4+
module Reek
5+
# Translates a Reek smell warning into the LSP diagnostic that Ruby LSP
6+
# reports back to the editor.
7+
module Diagnostic
8+
# @param warning [Reek::SmellWarning] The warning to convert to a diagnostic.
9+
# @return [RubyLsp::Interface::Diagnostic] The diagnostic.
10+
def self.from_warning(warning)
11+
lines = warning.lines
12+
::RubyLsp::Interface::Diagnostic.new(
13+
range: ::RubyLsp::Interface::Range.new(
14+
start: ::RubyLsp::Interface::Position.new(
15+
line: lines.first - 1,
16+
character: 0
17+
),
18+
end: ::RubyLsp::Interface::Position.new(
19+
line: lines.last - 1,
20+
character: 0
21+
)
22+
),
23+
severity: Constant::DiagnosticSeverity::WARNING,
24+
code: warning.smell_type,
25+
code_description: ::RubyLsp::Interface::CodeDescription.new(href: warning.explanatory_link),
26+
source: "Reek",
27+
message: warning.message
28+
)
29+
end
30+
end
31+
end
32+
end

lib/ruby_lsp/reek/runner.rb

Lines changed: 20 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# frozen_string_literal: true
22

33
require "reek"
4+
require_relative "diagnostic"
45

56
module RubyLsp
67
module Reek
@@ -12,69 +13,41 @@ def initialize
1213
@config = ::Reek::Configuration::AppConfiguration.from_default_path
1314
end
1415

15-
# We are not implementing this method, but it is required by the interface
16+
# We are not implementing this method, but it is required by the
17+
# interface. Reek is a linter, so there is nothing to format and the
18+
# source is handed back untouched.
1619
#
17-
# @param uri [String] The URI of the document to format.
18-
# @param document [RubyLsp::Interface::TextDocumentItem] The document to format.
20+
# :reek:UtilityFunction { enabled: false } - the interface requires an
21+
# instance method, so it cannot depend on instance state.
22+
#
23+
# @param uri [URI::Generic] The URI of the document to format.
24+
# @param document [RubyLsp::RubyDocument] The document to format.
1925
# @return [String] The formatted document.
2026
def run_formatting(_uri, document)
2127
document.source
2228
end
2329

24-
# @param uri [String] The URI of the document to run diagnostics on.
25-
# @param document [RubyLsp::Interface::TextDocumentItem] The document to run diagnostics on.
30+
# @param uri [URI::Generic] The URI of the document to run diagnostics on.
31+
# @param document [RubyLsp::RubyDocument] The document to run diagnostics on.
2632
def run_diagnostic(uri, document)
2733
path = Pathname.new(uri.path)
2834
return [] if path_excluded?(path)
2935

30-
examiner = build_examiner(path, document)
31-
examiner.smells.map { |smell| warning_to_diagnostic(smell) }
36+
# We lint the source as it currently stands in the editor, but Reek
37+
# resolves directory directives from the origin, so the origin has to
38+
# be set explicitly to the file on disk rather than defaulting to
39+
# "string".
40+
examiner = ::Reek::Examiner.new(
41+
::Reek::Source::SourceCode.from(document.source, origin: path.to_s),
42+
configuration: config
43+
)
44+
examiner.smells.map { |smell| Diagnostic.from_warning(smell) }
3245
end
3346

3447
private
3548

3649
attr_reader :config
3750

38-
# Examiner does not allow separate source and origin, but we need to
39-
# lint the string from the editor AND know what the filename of the
40-
# edited file is. This patches the examiner to allow this.
41-
def build_examiner(path, document)
42-
examiner = ::Reek::Examiner.new(document.source, configuration: config)
43-
origin = ::Reek::Source::SourceCode.from(path).origin
44-
examiner.instance_variable_set(:@origin, origin)
45-
examiner.instance_variable_set(
46-
:@detector_repository,
47-
::Reek::DetectorRepository.new(
48-
smell_types: examiner.instance_variable_get(:@smell_types),
49-
configuration: config.directive_for(origin)
50-
)
51-
)
52-
examiner
53-
end
54-
55-
# @param warning [Reek::SmellWarning] The warning to convert to a diagnostic.
56-
# @return [RubyLsp::Interface::Diagnostic] The diagnostic.
57-
def warning_to_diagnostic(warning)
58-
lines = warning.lines
59-
::RubyLsp::Interface::Diagnostic.new(
60-
range: ::RubyLsp::Interface::Range.new(
61-
start: ::RubyLsp::Interface::Position.new(
62-
line: lines.first - 1,
63-
character: 0
64-
),
65-
end: ::RubyLsp::Interface::Position.new(
66-
line: lines.last - 1,
67-
character: 0
68-
)
69-
),
70-
severity: Constant::DiagnosticSeverity::WARNING,
71-
code: warning.smell_type,
72-
code_description: ::RubyLsp::Interface::CodeDescription.new(href: warning.explanatory_link),
73-
source: "Reek",
74-
message: warning.message
75-
)
76-
end
77-
7851
def path_excluded?(path)
7952
path.ascend do |ascendant|
8053
break true if config.path_excluded?(ascendant)

ruby-lsp-reek.gemspec

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ Gem::Specification.new do |spec|
1212
spec.description = "An addon for Ruby LSP that enables linting with reek"
1313
spec.homepage = "https://github.com/igray/ruby-lsp-reek"
1414
spec.license = "MIT"
15-
spec.required_ruby_version = Gem::Requirement.new(">= 2.5.0")
15+
spec.required_ruby_version = Gem::Requirement.new(">= 3.0.0")
1616

1717
spec.metadata["allowed_push_host"] = "https://rubygems.org"
1818
spec.metadata["homepage_uri"] = spec.homepage
@@ -30,8 +30,7 @@ Gem::Specification.new do |spec|
3030
spec.require_paths = ["lib"]
3131

3232
spec.add_dependency("reek", "~> 6.0", ">= 5.0")
33-
spec.add_dependency("ruby-lsp", "~> 0.17", ">= 0.12.0")
34-
spec.add_dependency("sorbet-runtime", "~> 0.5", ">= 0.5.5685")
33+
spec.add_dependency("ruby-lsp", "~> 0.26")
3534

3635
spec.add_development_dependency "minitest", "~> 5.20"
3736
spec.add_development_dependency "pry", "~> 0.14"

test/ruby_lsp_addon_test.rb

Lines changed: 58 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -2,29 +2,15 @@
22

33
require "bundler/setup"
44
require "minitest/autorun"
5-
require "sorbet-runtime"
6-
require "uri"
7-
require "core_ext/uri"
8-
require "language_server-protocol"
9-
require "ruby_indexer/ruby_indexer"
10-
require "ruby_lsp/addon"
11-
require "ruby_lsp/base_server"
12-
require "ruby_lsp/client_capabilities"
13-
require "ruby_lsp/server"
14-
require "ruby_lsp/requests/request"
15-
require "ruby_lsp/requests/diagnostics"
16-
require "ruby_lsp/requests/support/formatter"
17-
require "ruby_lsp/utils"
18-
require "ruby_lsp/store"
19-
require "ruby_lsp/document"
20-
require "ruby_lsp/global_state"
21-
require "ruby_lsp/ruby_document"
22-
require "ruby_lsp/type_inferrer"
23-
require "prism"
5+
require "tmpdir"
6+
require "ruby_lsp/internal"
7+
require "ruby_lsp/test_helper"
248
require "pry"
259
require "ruby_lsp/reek/addon"
2610

2711
class RubyLspAddonTest < Minitest::Test
12+
include RubyLsp::TestHelper
13+
2814
def setup
2915
@addon = RubyLsp::Reek::Addon.new
3016
super
@@ -34,6 +20,10 @@ def test_name
3420
assert_equal "Reek: Code smell detector for Ruby", @addon.name
3521
end
3622

23+
def test_version
24+
assert_equal RubyLsp::Reek::VERSION, @addon.version
25+
end
26+
3727
def test_diagnostic
3828
source = <<~RUBY
3929
def foo
@@ -52,9 +42,8 @@ def foo
5242
}
5343
)
5444

55-
result = server.pop_response
45+
result = pop_result(server)
5646

57-
assert_instance_of(RubyLsp::Result, result)
5847
assert_equal "full", result.response.kind
5948
assert_equal 1, result.response.items.size
6049
item = result.response.items.first
@@ -63,54 +52,64 @@ def foo
6352
assert_equal RubyLsp::Constant::DiagnosticSeverity::WARNING, item.severity
6453
assert_equal "UncommunicativeVariableName", item.code
6554
assert_equal(
66-
"https://github.com/troessner/reek/blob/v6.3.0/docs/Uncommunicative-Variable-Name.md",
55+
"https://github.com/troessner/reek/blob/v#{Reek::Version::STRING}/docs/Uncommunicative-Variable-Name.md",
6756
item.code_description.href
6857
)
6958
assert_equal "Reek", item.source
7059
assert_equal("has the variable name 's'", item.message)
7160
end
7261
end
7362

74-
private
63+
# The source being linted comes from the editor buffer, but Reek resolves
64+
# directory directives from the origin of that source. The origin therefore
65+
# has to follow the file on disk, not the buffer.
66+
def test_diagnostic_resolves_directives_from_the_file_path
67+
Dir.mktmpdir do |tmpdir|
68+
workspace = File.realpath(tmpdir)
69+
models = File.join(workspace, "app", "models")
70+
FileUtils.mkdir_p(models)
71+
FileUtils.mkdir_p(File.join(workspace, "lib"))
72+
File.write(File.join(workspace, ".reek.yml"), <<~YAML)
73+
directories:
74+
"app/models":
75+
UncommunicativeVariableName:
76+
enabled: false
77+
YAML
7578

76-
# Overridden from RubyLsp/TestHelper so that we can override the linters configuration
77-
def with_server(
78-
source = nil,
79-
path = "fake.rb",
80-
stub_no_typechecker: false,
81-
load_addons: true,
82-
&block
83-
)
84-
server = RubyLsp::Server.new(test_mode: true)
85-
uri = Kernel.URI(File.join(server.global_state.workspace_path, path))
86-
server.global_state.instance_variable_set(:@linters, ["reek"])
87-
server.global_state.stubs(:typechecker).returns(false) if stub_no_typechecker
88-
89-
if source
90-
server.process_message(
91-
{
92-
method: "textDocument/didOpen",
93-
params: {
94-
textDocument: {
95-
uri:,
96-
text: source,
97-
version: 1
98-
}
99-
}
100-
}
101-
)
79+
document = Struct.new(:source).new(<<~RUBY)
80+
def foo
81+
s = 'hello'
82+
puts s
83+
end
84+
RUBY
85+
86+
Dir.chdir(workspace) do
87+
runner = RubyLsp::Reek::Runner.new
88+
89+
assert_empty runner.run_diagnostic(
90+
URI::Generic.from_path(path: File.join(models, "user.rb")),
91+
document
92+
)
93+
94+
assert_equal ["UncommunicativeVariableName"], runner.run_diagnostic(
95+
URI::Generic.from_path(path: File.join(workspace, "lib", "user.rb")),
96+
document
97+
).map(&:code)
98+
end
10299
end
100+
end
101+
102+
private
103+
104+
# Overridden from RubyLsp::TestHelper so that we can override the linters
105+
# configuration and build a URI inside the workspace, which Reek needs in
106+
# order to resolve directory directives from .reek.yml.
107+
def with_server(source = nil, path = "fake.rb", **kwargs, &block)
108+
uri = URI::Generic.from_path(path: File.join(Dir.pwd, path))
103109

104-
server.global_state.index.index_single(
105-
RubyIndexer::IndexablePath.new(nil, uri.to_standardized_path),
106-
source
107-
)
108-
server.load_addons if load_addons
109-
block.call(server, uri)
110-
ensure
111-
if load_addons
112-
RubyLsp::Addon.addons.each(&:deactivate)
113-
RubyLsp::Addon.addons.clear
110+
super(source, uri, **kwargs) do |server, server_uri|
111+
server.global_state.instance_variable_set(:@linters, ["reek"])
112+
block.call(server, server_uri)
114113
end
115114
end
116115
end

0 commit comments

Comments
 (0)