Skip to content

Backport warning feature for bundled gems from master #11420

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

Merged
merged 15 commits into from
Aug 21, 2024
Merged
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
89 changes: 62 additions & 27 deletions lib/bundled_gems.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,18 @@ module Gem::BUNDLED_GEMS
"resolv-replace" => "3.4.0",
"rinda" => "3.4.0",
"syslog" => "3.4.0",
"ostruct" => "3.5.0",
"pstore" => "3.5.0",
"rdoc" => "3.5.0",
"win32ole" => "3.5.0",
"fiddle" => "3.5.0",
"logger" => "3.5.0",
}.freeze

SINCE_FAST_PATH = SINCE.transform_keys { |g| g.sub(/\A.*\-/, "") }.freeze

EXACT = {
"abbrev" => true,
"base64" => true,
"bigdecimal" => true,
"csv" => true,
"drb" => true,
"getoptlong" => true,
"mutex_m" => true,
"nkf" => true, "kconv" => "nkf",
"observer" => true,
"resolv-replace" => true,
"rinda" => true,
"syslog" => true,
"kconv" => "nkf",
}.freeze

PREFIXED = {
Expand Down Expand Up @@ -70,8 +65,12 @@ def self.replace_require(specs)
[::Kernel.singleton_class, ::Kernel].each do |kernel_class|
kernel_class.send(:alias_method, :no_warning_require, :require)
kernel_class.send(:define_method, :require) do |name|
if message = ::Gem::BUNDLED_GEMS.warning?(name, specs: spec_names) # rubocop:disable Style/HashSyntax
warn message, :uplevel => 1
if message = ::Gem::BUNDLED_GEMS.warning?(name, specs: spec_names)
if ::Gem::BUNDLED_GEMS.uplevel > 0
Kernel.warn message, uplevel: ::Gem::BUNDLED_GEMS.uplevel
else
Kernel.warn message
end
end
kernel_class.send(:no_warning_require, name)
end
Expand All @@ -83,6 +82,36 @@ def self.replace_require(specs)
end
end

def self.uplevel
frame_count = 0
frames_to_skip = 3
uplevel = 0
require_found = false
Thread.each_caller_location do |cl|
frame_count += 1
if frames_to_skip >= 1
frames_to_skip -= 1
next
end
uplevel += 1
if require_found
if cl.base_label != "require"
return uplevel
end
else
if cl.base_label == "require"
require_found = true
end
end
# Don't show script name when bundle exec and call ruby script directly.
if cl.path.end_with?("bundle")
frame_count = 0
break
end
end
require_found ? 1 : frame_count - 1
end

def self.find_gem(path)
if !path
return
Expand All @@ -93,7 +122,7 @@ def self.find_gem(path)
else
return
end
EXACT[n] or PREFIXED[n = n[%r[\A[^/]+(?=/)]]] && n
(EXACT[n] || !!SINCE[n]) or PREFIXED[n = n[%r[\A[^/]+(?=/)]]] && n
end

def self.warning?(name, specs: nil)
Expand All @@ -108,7 +137,7 @@ def self.warning?(name, specs: nil)
# We'll fail to warn requires for files that are not the entry point
# of the gem, e.g. require "logger/formatter.rb" won't warn.
# But that's acceptable because this warning is best effort,
# and in the overwhelming majority of case logger.rb will end
# and in the overwhelming majority of cases logger.rb will end
# up required.
return unless SINCE_FAST_PATH[File.basename(feature, ".*")]
else
Expand Down Expand Up @@ -148,29 +177,35 @@ def self.warning?(name, specs: nil)
end

def self.build_message(gem)
msg = " #{RUBY_VERSION < SINCE[gem] ? "will no longer be" : "is not"} part of the default gems since Ruby #{SINCE[gem]}."
msg = " #{RUBY_VERSION < SINCE[gem] ? "will no longer be" : "is not"} part of the default gems starting from Ruby #{SINCE[gem]}."

if defined?(Bundler)
msg += " Add #{gem} to your Gemfile or gemspec."
msg += "\nYou can add #{gem} to your Gemfile or gemspec to silence this warning."

# We detect the gem name from caller_locations. We need to skip 2 frames like:
# lib/ruby/3.3.0+0/bundled_gems.rb:90:in `warning?'",
# lib/ruby/3.3.0+0/bundler/rubygems_integration.rb:247:in `block (2 levels) in replace_require'",
# We detect the gem name from caller_locations. First we walk until we find `require`
# then take the first frame that's not from `require`.
#
# Additionally, we need to skip Bootsnap and Zeitwerk if present, these
# gems decorate Kernel#require, so they are not really the ones issuing
# the require call users should be warned about. Those are upwards.
frames_to_skip = 2
frames_to_skip = 3
location = nil
require_found = false
Thread.each_caller_location do |cl|
if frames_to_skip >= 1
frames_to_skip -= 1
next
end

if cl.base_label != "require"
location = cl.path
break
if require_found
if cl.base_label != "require"
location = cl.path
break
end
else
if cl.base_label == "require"
require_found = true
end
end
end

Expand All @@ -183,7 +218,7 @@ def self.build_message(gem)
end
end
if caller_gem
msg += " Also contact author of #{caller_gem} to add #{gem} into its gemspec."
msg += "\nAlso please contact the author of #{caller_gem} to request adding #{gem} into its gemspec."
end
end
else
Expand All @@ -204,7 +239,7 @@ def message

name = path.tr("/", "-")
if !defined?(Bundler) && Gem::BUNDLED_GEMS::SINCE[name] && !Gem::BUNDLED_GEMS::WARNED[name]
warn name + Gem::BUNDLED_GEMS.build_message(name)
warn name + Gem::BUNDLED_GEMS.build_message(name), uplevel: Gem::BUNDLED_GEMS.uplevel
end
super
end
Expand Down
4 changes: 4 additions & 0 deletions tool/test_for_warn_bundled_gems/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -51,3 +51,7 @@ echo
echo "* Don't show warning bigdecimal/util when bigdecimal on Gemfile"
ruby test_no_warn_sub_feature.rb
echo

echo "* Show warning when warn is not the standard one in the current scope"
ruby test_warn_redefined.rb
echo
18 changes: 18 additions & 0 deletions tool/test_for_warn_bundled_gems/test_warn_redefined.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
module My
def warn(msg)
end

def my
require "bundler/inline"

gemfile do
source "https://rubygems.org"
end

require "csv"
end

extend self
end

My.my
Loading