8000 Improve bundled gems warning messages · ruby/ruby@c59e936 · GitHub
[go: up one dir, main page]
Skip to content

Commit c59e936

Browse files
deivid-rodriguezdeivid-rodriguez
committed
Improve bundled gems warning messages
Currently evenn if the require actually fails, they suggest that the file was actually loaded, which is confusing. I reworded them to reduce this confusion.
1 parent 90aab32 commit c59e936

File tree

2 files changed

+31
-24
lines changed

2 files changed

+31
-24
lines changed

‎lib/bundled_gems.rb

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -101,16 +101,17 @@ def self.uplevel
101101

102102
def self.warning?(name, specs: nil)
103103
# name can be a feature name or a file path with String or Pathname
104-
feature = File.path(name)
104+
feature = File.path(name).sub(LIBEXT, "")
105105

106106
# The actual checks needed to properly identify the gem being required
107107
# are costly (see [Bug #20641]), so we first do a much cheaper check
108108
# to exclude the vast majority of candidates.
109109
subfeature = if feature.include?("/")
110110
# bootsnap expands `require "csv"` to `require "#{LIBDIR}/csv.rb"`,
111111
# and `require "syslog"` to `require "#{ARCHDIR}/syslog.so"`.
112-
name = feature.delete_prefix(ARCHDIR).delete_prefix(LIBDIR).sub(LIBEXT, "")
113-
segments = name.split("/")
112+
feature.delete_prefix!(ARCHDIR)
113+
feature.delete_prefix!(LIBDIR)
114+
segments = feature.split("/")
114115
name = segments.shift
115116
name = EXACT[name] || name
116117
if !SINCE[name]
@@ -119,8 +120,7 @@ def self.warning?(name, specs: nil)
119120
end
120121
segments.any?
121122
else
122-
name = feature.sub(LIBEXT, "")
123-
name = EXACT[name] || name
123+
name = EXACT[feature] || feature
124124
return unless SINCE[name]
125125
false
126126
end
@@ -130,18 +130,25 @@ def self.warning?(name, specs: nil)
130130
return if WARNED[name]
131131
WARNED[name] = true
132132

133+
level = RUBY_VERSION < SINCE[name] ? "warning" : "error"
134+
133135
if subfeature
134136
"#{feature} is found in #{name}, which"
135137
else
136-
"#{feature} was loaded from the standard library, but"
137-
end + build_message(name)
138+
"#{feature} #{level == "warning" ? "was loaded" : "used to be loaded"} from the standard library, but"
139+
end + build_message(name, level)
138140
end
139141

140-
def self.build_message(name)
141-
msg = " #{RUBY_VERSION < SINCE[name] ? "will no longer be" : "is not"} part of the default gems starting from Ruby #{SINCE[name]}."
142+
def self.build_message(name, level)
143+
msg = if level == "warning"
144+
" will no longer be part of the default gems starting from Ruby #{SINCE[name]}"
145+
else
146+
" is not part of the default gems since Ruby #{SINCE[name]}."
147+
end
142148

143149
if defined?(Bundler)
144-
msg += "\nYou can add #{name} to your Gemfile or gemspec to silence this warning."
150+
motivation = level == "warning" ? "silence this warning" : "fix this error"
151+
msg += "\nYou can add #{name} to your Gemfile or gemspec to #{motivation}."
145152

146153
# We detect the gem name from caller_locations. First we walk until we find `require`
147154
# then take the first frame that's not from `require`.
@@ -230,7 +237,7 @@ def message # :nodoc:
230237

231238
name = path.tr("/", "-")
232239
if !defined?(Bundler) && Gem::BUNDLED_GEMS::SINCE[name] && !Gem::BUNDLED_GEMS::WARNED[name]
233-
warn name + Gem::BUNDLED_GEMS.build_message(name), uplevel: Gem::BUNDLED_GEMS.uplevel
240+
warn name + Gem::BUNDLED_GEMS.build_message(name, "error"), uplevel: Gem::BUNDLED_GEMS.uplevel
234241
end
235242
super
236243
end

‎spec/bundled_gems_spec.rb

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,9 @@ def script(code, options = {})
9090
require "openssl"
9191
RUBY
9292

93-
expect(err).to include(/csv was loaded from (.*) from Ruby 3.4.0/)
93+
expect(err).to include(/csv used to be loaded from (.*) since Ruby 3.4.0/)
9494
expect(err).to include(/-e:15/)
95-
expect(err).to include(/openssl was loaded from (.*) from Ruby #{RUBY_VERSION}/)
95+
expect(err).to include(/openssl used to be loaded from (.*) since Ruby #{RUBY_VERSION}/)
9696
expect(err).to include(/-e:18/)
9797
end
9898

@@ -112,7 +112,7 @@ def script(code, options = {})
112112
require "active_support/all"
113113
RUBY
114114

115-
expect(err).to include(/openssl was loaded from (.*) from Ruby 3.5.0/)
115+
expect(err).to include(/openssl used to be loaded from (.*) since Ruby 3.5.0/)
116116
expect(err).to include(/lib\/active_support\/all\.rb:1/)
117117
end
118118

@@ -128,7 +128,7 @@ def script(code, options = {})
128128
end
129129
RUBY
130130

131-
expect(err).to include(/net\/smtp was loaded from (.*) from Ruby 3.1.0/)
131+
expect(err).to include(/net\/smtp used to be loaded from (.*) since Ruby 3.1.0/)
132132
expect(err).to include(/-e:15/)
133133
expect(err).to include("You can add net-smtp")
134134
end
@@ -144,7 +144,7 @@ def script(code, options = {})
144144
require "openssl/bn"
145145
RUBY
146146

147-
expect(err).to include(/openssl\/bn is found in openssl, (.*) part of the default gems starting from Ruby #{RUBY_VERSION}/)
147+
expect(err).to include(/openssl\/bn is found in openssl, (.*) part of the default gems since Ruby #{RUBY_VERSION}/)
148148
expect(err).to include(/-e:14/)
149149
end
150150

@@ -158,7 +158,7 @@ def script(code, options = {})
158158

159159
bundle "exec ruby script.rb"
160160

161-
expect(err).to include(/openssl was loaded from (.*) from Ruby 3.5.0/)
161+
expect(err).to include(/openssl used to be loaded from (.*) since Ruby 3.5.0/)
162162
expect(err).to include(/script\.rb:8/)
163163
end
164164

@@ -176,7 +176,7 @@ def script(code, options = {})
176176

177177
bundle "exec ./script.rb"
178178

179-
expect(err).to include(/openssl was loaded from (.*) from Ruby 3.5.0/)
179+
expect(err).to include(/openssl used to be loaded from (.*) since Ruby 3.5.0/)
180180
expect(err).to include(/script\.rb:9/)
181181
end
182182

@@ -185,7 +185,7 @@ def script(code, options = {})
185185
create_file("Gemfile", "source 'https://rubygems.org'")
186186
bundle "exec ruby -r./stub -ropenssl -e ''"
187187

188-
expect(err).to include(/openssl was loaded from (.*) from Ruby 3.5.0/)
188+
expect(err).to include(/openssl used to be loaded from (.*) since Ruby 3.5.0/)
189189
end
190190

191191
it "Show warning when warn is not the standard one in the current scope" do
@@ -208,7 +208,7 @@ def my
208208
My.my
209209
RUBY
210210

211-
expect(err).to include(/openssl was loaded from (.*) from Ruby 3.5.0/)
211+
expect(err).to include(/openssl used to be loaded from (.*) since Ruby 3.5.0/)
212212
expect(err).to include(/-e:19/)
213213
end
214214

@@ -250,7 +250,7 @@ def my
250250
require Gem::BUNDLED_GEMS::ARCHDIR + 'openssl'
251251
RUBY
252252

253-
expect(err).to include(/openssl was loaded from (.*) from Ruby 3.5.0/)
253+
expect(err).to include(/openssl used to be loaded from (.*) since Ruby 3.5.0/)
254254
# TODO: We should assert caller location like below:
255255
# test_warn_bootsnap.rb:14: warning: ...
256256
end
@@ -270,7 +270,7 @@ def my
270270
require Gem::BUNDLED_GEMS::ARCHDIR + "openssl"
271271
RUBY
272272

273-
expect(err).to include(/openssl was loaded from (.*) from Ruby #{RUBY_VERSION}/)
273+
expect(err).to include(/openssl used to be loaded from (.*) since Ruby #{RUBY_VERSION}/)
274274
# TODO: We should assert caller location like below:
275275
# test_warn_bootsnap_rubyarchdir_gem.rb:14: warning: ...
276276
end
@@ -299,7 +299,7 @@ def my
299299
require Gem.loaded_specs["fileutils2"].full_gem_path + '/lib/fileutils2'
300300
RUBY
301301

302-
expect(err).to include(/fileutils was loaded from (.*) from Ruby #{RUBY_VERSION}/)
302+
expect(err).to include(/fileutils used to be loaded from (.*) since Ruby #{RUBY_VERSION}/)
303303
# TODO: We should assert caller location like below:
304304
# $GEM_HOME/gems/childprocess-5.0.0/lib/childprocess.rb:7: warning:
305305
end
@@ -319,7 +319,7 @@ def my
319319
create_file("Gemfile", "source 'https://rubygems.org'")
320320
bundle "exec ruby script.rb"
321321

322-
expect(err).to include(/openssl was loaded from (.*) from Ruby 3.5.0/)
322+
expect(err).to include(/openssl used to be loaded from (.*) since Ruby 3.5.0/)
323323
expect(err).to include(/script\.rb:13/)
324324
end
325325

0 commit comments

Comments
 (0)
0