8000 Avoid some false positive bundled gem warnings · ruby/ruby@37a4551 · GitHub
[go: up one dir, main page]
Skip to content

Commit 37a4551

Browse files
deivid-rodriguezdeivid-rodriguez
committed
Avoid some false positive bundled gem warnings
When a bundled gem has already been removed, a `require` caller is rescuing `LoadError`, no warning/error messages should be displayed. Instead, let the bundled gem message be part of `LoadError`, so that it's not displayed when rescued, but still gets to the user when not rescued.
1 parent d55c463 commit 37a4551

File tree

3 files changed

+41
-27
lines changed

3 files changed

+41
-27
lines changed

‎lib/bundled_gems.rb

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,21 @@ def self.replace_require(specs)
4848
[::Kernel.singleton_class, ::Kernel].each do |kernel_class|
4949
kernel_class.send(:alias_method, :no_warning_require, :require)
5050
kernel_class.send(:define_method, :require) do |name|
51-
if message = ::Gem::BUNDLED_GEMS.warning?(name, specs: spec_names)
51+
warning_info = ::Gem::BUNDLED_GEMS.warning?(name, specs: spec_names)
52+
53+
result = kernel_class.send(:no_warning_require, name)
54+
55+
if warning_info
5256
uplevel = ::Gem::BUNDLED_GEMS.uplevel
57+
message = ::Gem::BUNDLED_GEMS.build_message(warning_info)
5358
if uplevel > 0
5459
Kernel.warn message, uplevel: uplevel
5560
else
5661
Kernel.warn message
5762
end
5863
end
59-
kernel_class.send(:no_warning_require, name)
64+
65+
result
6066
end
6167
if kernel_class == ::Kernel
6268
kernel_class.send(:private, :require)
@@ -93,7 +99,7 @@ def self.uplevel
9399
require_found ? 1 : frame_count - 1
94100
end
95101

96-
def self.warning?(name, specs: nil)
102+
def self.warning?(name, specs: {})
97103
# name can be a feature name or a file path with String or Pathname
98104
feature = File.path(name).sub(LIBEXT, "")
99105

@@ -122,19 +128,26 @@ def self.warning?(name, specs: nil)
122128
return if specs.include?(name)
123129

124130
return if WARNED[name]
131+
132+
{ feature: feature, name: name, subfeature: subfeature }
133+
end
134+
135+
def self.build_message(warning_info)
136+
feature = warning_info[:feature]
137+
name = warning_info[:name]
138+
subfeature = warning_info[:subfeature]
139+
125140
WARNED[name] = true
126141

127142
level = RUBY_VERSION < SINCE[name] ? "warning" : "error"
128143

129-
if subfeature
144+
msg = if subfeature
130145
"#{feature} is found in #{name}, which"
131146
else
132147
"#{feature} #{level == "warning" ? "was loaded" : "used to be loaded"} from the standard library, but"
133-
end + build_message(name, level)
134-
end
148+
end
135149

136-
def self.build_message(name, level)
137-
msg = if level == "warning"
150+
msg += if level == "warning"
138151
" will no longer be part of the default gems starting from Ruby #{SINCE[name]}"
139152
else
140153
" is not part of the default gems since Ruby #{SINCE[name]}."
@@ -228,16 +241,14 @@ def self.force_activate(gem)
228241
end
229242
end
230243

231-
# for RubyGems without Bundler environment.
232244
# If loading library is not part of the default gems and the bundled gems, warn it.
233245
class LoadError
234246
def message # :nodoc:
235247
return super unless path
236248

237-
name = path.tr("/", "-")
238-
if !defined?(Bundler) && Gem::BUNDLED_GEMS::SINCE[name] && !Gem::BUNDLED_GEMS::WARNED[name]
239-
warn name + Gem::BUNDLED_GEMS.build_message(name, "error"), uplevel: Gem::BUNDLED_GEMS.uplevel
240-
end
241-
super
249+
warning_info = ::Gem::BUNDLED_GEMS.warning?(path)
250+
return super unless warning_info
251+
252+
::Gem::BUNDLED_GEMS.build_message(warning_info)
242253
end
243254
end

‎spec/bundled_gems_spec.rb

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ def script(code, options = {})
7878
ruby(code, options)
7979
end
8080

81-
it "Show warning require and LoadError" do
81+
it "Show warning on require, but not when LoadError is rescued" do
8282
script <<-RUBY
8383
gemfile do
8484
source "https://rubygems.org"
@@ -91,8 +91,8 @@ def script(code, options = {})
9191
require "openssl"
9292
RUBY
9393

94-
expect(err).to include(/csv used to be loaded from (.*) since Ruby 3.4.0/)
95-
expect(err).to include(/-e:15/)
94+
expect(err).not_to include(/csv used to be loaded from (.*) since Ruby 3.4.0/)
95+
expect(err).not_to include(/-e:15/)
9696
expect(err).to include(/openssl used to be loaded from (.*) since Ruby #{RUBY_VERSION}/)
9797
expect(err).to include(/-e:18/)
9898
end
@@ -117,20 +117,17 @@ def script(code, options = {})
117117
expect(err).to include(/lib\/active_support\/all\.rb:1/)
118118
end
119119

120-
it "Show warning dash gem like net/smtp" do
121-
script <<-RUBY
120+
it "Show error with dash gem already removed like net/smtp" do
121+
script <<-RUBY, raise_on_error: false
122122
gemfile do
123123
source "https://rubygems.org"
124124
end
125125
126-
begin
127-
require "net/smtp"
128-
rescue LoadError
129-
end
126+
require "net/smtp"
130127
RUBY
131128

132129
expect(err).to include(/net\/smtp used to be loaded from (.*) since Ruby 3.1.0/)
133-
expect(err).to include(/-e:15/)
130+
expect(err).to include(/-e:14/)
134131
expect(err).to include("You can add net-smtp")
135132
end
136133

‎test/test_bundled_gems.rb

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ def teardown
1212
end
1313

1414
def test_warning
15-
assert Gem::BUNDLED_GEMS.warning?("csv", specs: {})
15+
warning_info = Gem::BUNDLED_GEMS.warning?("csv", specs: {})
16+
assert warning_info
17+
Gem::BUNDLED_GEMS.build_message(warning_info)
1618
assert_nil Gem::BUNDLED_GEMS.warning?("csv", specs: {})
1719
end
1820

@@ -23,13 +25,17 @@ def test_no_warning_warning
2325

2426
def test_warning_libdir
2527
path = File.join(::RbConfig::CONFIG.fetch("rubylibdir"), "csv.rb")
26-
assert Gem::BUNDLED_GEMS.warning?(path, specs: {})
28+
warning_info = Gem::BUNDLED_GEMS.warning?(path, specs: {})
29+
assert warning_info
30+
Gem::BUNDLED_GEMS.build_message(warning_info)
2731
assert_nil Gem::BUNDLED_GEMS.warning?(path, specs: {})
2832
end
2933

3034
def test_warning_archdir
3135
path = File.join(::RbConfig::CONFIG.fetch("rubyarchdir"), "syslog.so")
32-
assert Gem::BUNDLED_GEMS.warning?(path, specs: {})
36+
warning_info = Gem::BUNDLED_GEMS.warning?(path, specs: {})
37+
assert warning_info
38+
Gem::BUNDLED_GEMS.build_message(warning_info)
3339
assert_nil Gem::BUNDLED_GEMS.warning?(path, specs: {})
3440
end
3541
end

0 commit comments

Comments
 (0)
0