From 70d2e43846e520a17f469256b1efa9951dd7f1ac Mon Sep 17 00:00:00 2001 From: schneems Date: Mon, 23 May 2022 13:22:29 -0500 Subject: [PATCH] Lazyload DeadEnd internals on syntax error Instead of having to load all dead end code on every invocation of Ruby, we can delay requiring the files until they're actually needed (on SyntaxError). Resolves this comment https://github.com/ruby/ruby/pull/5859#pullrequestreview-976985456 This requirement makes the library a little unusual in that `dead_end/version` no longer defines `DeadEnd::VERSION` but rather a placeholder value in another constant so the gem isn't eagerly loaded when using the project's gemspec in local tests. --- CHANGELOG.md | 1 + dead_end.gemspec | 2 +- exe/dead_end | 2 +- lib/dead_end.rb | 1 - lib/dead_end/api.rb | 4 ++ lib/dead_end/core_ext.rb | 10 +++-- lib/dead_end/version.rb | 6 ++- spec/integration/ruby_command_line_spec.rb | 45 ++++++++++++++++++++++ spec/spec_helper.rb | 2 +- 9 files changed, 65 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 451cfd6..b34e235 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## HEAD (unreleased) +- [Breaking] Lazy load DeadEnd internals only if there is a Syntax error. Use `require "dead_end"; require "dead_end/api"` to load eagerly all internals. Otherwise `require "dead_end"` will set up an autoload for the first time the DeadEnd module is used in code. This should only happen on a syntax error. (https://github.com/zombocom/dead_end/pull/142) - Monkeypatch `SyntaxError#detailed_message` in Ruby 3.2+ instead of `require`, `load`, and `require_relative` (https://github.com/zombocom/dead_end/pull/139) ## 3.1.2 diff --git a/dead_end.gemspec b/dead_end.gemspec index 4268ea7..39bfb50 100644 --- a/dead_end.gemspec +++ b/dead_end.gemspec @@ -8,7 +8,7 @@ end Gem::Specification.new do |spec| spec.name = "dead_end" - spec.version = DeadEnd::VERSION + spec.version = UnloadedDeadEnd::VERSION spec.authors = ["schneems"] spec.email = ["richard.schneeman+foo@gmail.com"] diff --git a/exe/dead_end b/exe/dead_end index 9ae3301..db44d5d 100755 --- a/exe/dead_end +++ b/exe/dead_end @@ -1,6 +1,6 @@ #!/usr/bin/env ruby -require_relative "../lib/dead_end" +require_relative "../lib/dead_end/api" DeadEnd::Cli.new( argv: ARGV diff --git a/lib/dead_end.rb b/lib/dead_end.rb index 9b98b97..1467770 100644 --- a/lib/dead_end.rb +++ b/lib/dead_end.rb @@ -1,4 +1,3 @@ # frozen_string_literal: true -require_relative "dead_end/api" require_relative "dead_end/core_ext" diff --git a/lib/dead_end/api.rb b/lib/dead_end/api.rb index 683a0e4..4085af7 100644 --- a/lib/dead_end/api.rb +++ b/lib/dead_end/api.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require_relative "version" require "tmpdir" @@ -7,6 +9,8 @@ require "timeout" module DeadEnd + VERSION = UnloadedDeadEnd::VERSION + # Used to indicate a default value that cannot # be confused with another input. DEFAULT_VALUE = Object.new.freeze diff --git a/lib/dead_end/core_ext.rb b/lib/dead_end/core_ext.rb index 7a1fe6d..0fff205 100644 --- a/lib/dead_end/core_ext.rb +++ b/lib/dead_end/core_ext.rb @@ -1,8 +1,11 @@ # frozen_string_literal: true +# Allow lazy loading, only load code if/when there's a syntax error +autoload :DeadEnd, "dead_end/api" + # Ruby 3.2+ has a cleaner way to hook into Ruby that doesn't use `require` if SyntaxError.new.respond_to?(:detailed_message) - module DeadEnd + module DeadEndUnloaded class MiniStringIO def initialize(isatty: $stderr.isatty) @string = +"" @@ -10,7 +13,6 @@ def initialize(isatty: $stderr.isatty) end attr_reader :isatty - def puts(value = $/, **) @string << value end @@ -23,7 +25,7 @@ def puts(value = $/, **) def detailed_message(highlight: nil, **) message = super file = DeadEnd::PathnameFromMessage.new(message).call.name - io = DeadEnd::MiniStringIO.new + io = DeadEndUnloaded::MiniStringIO.new if file DeadEnd.call( @@ -47,6 +49,8 @@ def detailed_message(highlight: nil, **) end } else + autoload :Pathname, "pathname" + # Monkey patch kernel to ensure that all `require` calls call the same # method module Kernel diff --git a/lib/dead_end/version.rb b/lib/dead_end/version.rb index 9c6158c..9f80322 100644 --- a/lib/dead_end/version.rb +++ b/lib/dead_end/version.rb @@ -1,5 +1,9 @@ # frozen_string_literal: true -module DeadEnd +# Calling `DeadEnd::VERSION` forces an eager load due to +# an `autoload` on the `DeadEnd` constant. +# +# This is used for gemspec access in tests +module UnloadedDeadEnd VERSION = "3.1.2" end diff --git a/spec/integration/ruby_command_line_spec.rb b/spec/integration/ruby_command_line_spec.rb index cbe6deb..7b779d5 100644 --- a/spec/integration/ruby_command_line_spec.rb +++ b/spec/integration/ruby_command_line_spec.rb @@ -101,5 +101,50 @@ module DeadEnd expect(out).to include('❯ 5 it "flerg"').once end end + + it "does not load internals into memory if no syntax error" do + Dir.mktmpdir do |dir| + tmpdir = Pathname(dir) + script = tmpdir.join("script.rb") + script.write <<~EOM + class Dog + end + + # When a constant is defined through an autoload + # then Object.autoload? will return the name of the + # require only until it has been loaded. + # + # We can use this to detect if DeadEnd internals + # have been fully loaded yet or not. + # + # Example: + # + # Object.autoload?("Cat") # => nil + # autoload :Cat, "animals/cat + # Object.autoload?("Cat") # => "animals/cat + # Object.autoload?("Cat") # => "animals/cat + # + # # Once required, `autoload?` returns falsey + # puts Cat.meow # invoke autoload + # Object.autoload?("Cat") # => nil + # + if Object.autoload?("DeadEnd") + puts "DeadEnd is NOT loaded" + else + puts "DeadEnd is loaded" + end + EOM + + require_rb = tmpdir.join("require.rb") + require_rb.write <<~EOM + load "#{script.expand_path}" + EOM + + out = `ruby -I#{lib_dir} -rdead_end #{require_rb} 2>&1` + + expect($?.success?).to be_truthy + expect(out).to include("DeadEnd is NOT loaded").once + end + end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2f002a3..31c3635 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require "bundler/setup" -require "dead_end" +require "dead_end/api" require "benchmark" require "tempfile"