From 5d4b36517de7f79a2293af0c1d40770ea3d19e79 Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Thu, 28 Dec 2023 11:40:52 +0100 Subject: [PATCH 01/10] Make arguments for Common.location_for_eval explicit --- lib/live_ast/common.rb | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/live_ast/common.rb b/lib/live_ast/common.rb index 81a57c0..bf8639c 100644 --- a/lib/live_ast/common.rb +++ b/lib/live_ast/common.rb @@ -43,17 +43,13 @@ def check_is_binding(obj) raise TypeError, message end - def location_for_eval(*args) - bind, *location = args - + def location_for_eval(bind = nil, filename = nil, lineno = nil) if bind - case location.size - when 0 - bind.source_location - when 1 - [location.first, 1] + if filename + lineno ||= 1 + [filename, lineno] else - location + bind.source_location end else ["(eval)", 1] From 5e91e608c5a2bfd7f3d2a01361771840d937e530 Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Thu, 28 Dec 2023 11:42:47 +0100 Subject: [PATCH 02/10] Make arguments for replacement Kernel#eval explicit --- lib/live_ast/replace_eval.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/live_ast/replace_eval.rb b/lib/live_ast/replace_eval.rb index c7df943..1927ec0 100644 --- a/lib/live_ast/replace_eval.rb +++ b/lib/live_ast/replace_eval.rb @@ -53,12 +53,11 @@ class << self alias live_ast_original_eval eval - def eval(*args) - LiveAST::Common.check_arity(args, 1..4) + def eval(string, binding = nil, filename = nil, lineno = nil) LiveAST.eval( - args[0], - args[1] || Binding.of_caller(1), - *LiveAST::Common.location_for_eval(*args[1..3])) + string, + binding || Binding.of_caller(1), + *LiveAST::Common.location_for_eval(binding, filename, lineno)) end end From 3e38e1e1e81c4af5980ca0fac0a6bd16bf87ddf6 Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Thu, 28 Dec 2023 11:51:44 +0100 Subject: [PATCH 03/10] Make arguments for replacement Binding#eval explicit --- lib/live_ast/replace_eval.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/live_ast/replace_eval.rb b/lib/live_ast/replace_eval.rb index 1927ec0..97c40ca 100644 --- a/lib/live_ast/replace_eval.rb +++ b/lib/live_ast/replace_eval.rb @@ -65,8 +65,8 @@ def eval(string, binding = nil, filename = nil, lineno = nil) class Binding alias live_ast_original_binding_eval eval - def eval(*args) - LiveAST.eval(args[0], self, *args[1..]) + def eval(string, filename = nil, lineno = nil) + LiveAST.eval(string, self, filename, lineno) end end From 726b31c06120323ea3b96a40f5fdbce5c0776af5 Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Thu, 28 Dec 2023 12:17:17 +0100 Subject: [PATCH 04/10] Explain need for args splats in #instance_eval and #module_eval replacements --- lib/live_ast/replace_eval.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/live_ast/replace_eval.rb b/lib/live_ast/replace_eval.rb index 97c40ca..cd6524f 100644 --- a/lib/live_ast/replace_eval.rb +++ b/lib/live_ast/replace_eval.rb @@ -74,6 +74,8 @@ def eval(string, filename = nil, lineno = nil) class BasicObject alias live_ast_original_instance_eval instance_eval + # Arity must be handled in code because the first argument is only required + # if no block is passed. def instance_eval(*args, &block) if block live_ast_original_instance_eval(*args, &block) @@ -91,6 +93,8 @@ def instance_eval(*args, &block) class Module alias live_ast_original_module_eval module_eval + # Arity must be handled in code because the first argument is only required + # if no block is passed. def module_eval(*args, &block) if block live_ast_original_module_eval(*args, &block) From 47d6db5d4a966b418c82b46a8b7234c46a5623d6 Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Thu, 28 Dec 2023 12:20:33 +0100 Subject: [PATCH 05/10] Make arguments for LiveAST::Cache.new explicit --- lib/live_ast/linker.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/live_ast/linker.rb b/lib/live_ast/linker.rb index 47b63cd..cb95c6b 100644 --- a/lib/live_ast/linker.rb +++ b/lib/live_ast/linker.rb @@ -2,8 +2,9 @@ module LiveAST class Cache - def initialize(*args) - @source, @user_line = args + def initialize(file, lineno) + @source = file + @user_line = lineno @asts = nil end From 212bb76ef22ec4c59873fe8f134f3dc6c8303790 Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Thu, 28 Dec 2023 12:48:21 +0100 Subject: [PATCH 06/10] Make arguments to LiveAST::Evaler.eval explicit --- lib/live_ast/ast_eval.rb | 4 +- lib/live_ast/base.rb | 4 +- lib/live_ast/evaler.rb | 5 +-- test/base/noninvasive_error_test.rb | 59 +++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 7 deletions(-) create mode 100644 test/base/noninvasive_error_test.rb diff --git a/lib/live_ast/ast_eval.rb b/lib/live_ast/ast_eval.rb index ec6bb9b..deba61e 100644 --- a/lib/live_ast/ast_eval.rb +++ b/lib/live_ast/ast_eval.rb @@ -7,7 +7,7 @@ module Kernel # The same as +eval+ except that the binding argument is required # and AST-accessible objects are created. - def ast_eval(*args) - LiveAST::Evaler.eval(args[0], *args) + def ast_eval(string, bind, filename = nil, lineno = nil) + LiveAST::Evaler.eval(string, string, bind, filename, lineno) end end diff --git a/lib/live_ast/base.rb b/lib/live_ast/base.rb index e831075..f53a86a 100644 --- a/lib/live_ast/base.rb +++ b/lib/live_ast/base.rb @@ -51,8 +51,8 @@ def flush_cache # # Equivalent to Kernel#ast_eval. # - def eval(*args) # :nodoc: - Evaler.eval(args[0], *args) + def eval(string, bind, filename = nil, lineno = nil) # :nodoc: + Evaler.eval(string, string, bind, filename, lineno) end # diff --git a/lib/live_ast/evaler.rb b/lib/live_ast/evaler.rb index ea5d7f5..ae2c31a 100644 --- a/lib/live_ast/evaler.rb +++ b/lib/live_ast/evaler.rb @@ -5,8 +5,8 @@ module Evaler class << self include Common - def eval(parser_source, *args) - evaler_source, bind, *rest = handle_args(*args) + def eval(parser_source, evaler_source, bind, filename = nil, lineno = nil) + evaler_source, bind, *rest = handle_args(evaler_source, bind, filename, lineno) file, line = location_for_eval(bind, *rest) file = LiveAST.strip_token(file) @@ -23,7 +23,6 @@ def eval(parser_source, *args) def handle_args(*args) args.tap do - check_arity(args, 2..4) args[0] = arg_to_str(args[0]) check_is_binding(args[1]) args[2] = arg_to_str(args[2]) if args[2] diff --git a/test/base/noninvasive_error_test.rb b/test/base/noninvasive_error_test.rb new file mode 100644 index 0000000..0d6c3cb --- /dev/null +++ b/test/base/noninvasive_error_test.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require_relative "../test_helper" + +class NonInvasiveErrorTest < BaseTest + def test_arg_error_too_many + orig = assert_raises ArgumentError do + eval("s", binding, "f", 99, nil) + end + + live = assert_raises ArgumentError do + LiveAST.eval("s", binding, "f", 99, nil) + end + + assert_equal orig.message.sub("1..4", "2..4"), live.message + end + + def test_bad_args + [99, Object.new, File].each do |bad| + orig = assert_raises TypeError do + eval(bad, binding) + end + live = assert_raises TypeError do + LiveAST.eval(bad, binding) + end + + assert_equal orig.message, live.message + + orig = assert_raises TypeError do + eval("3 + 4", binding, bad) + end + live = assert_raises TypeError do + LiveAST.eval("3 + 4", binding, bad) + end + + assert_equal orig.message, live.message + end + end + + def test_bad_binding + orig = assert_raises TypeError do + eval("", "bogus") + end + + live = assert_raises TypeError do + LiveAST.eval("", "bogus") + end + + assert_equal orig.message, live.message + end + + def test_shenanigans + error = assert_raises RuntimeError do + LiveAST.load "foo.rb|ast@4" + end + + assert_match(/revision token/, error.message) + end +end From c58e1979ad9dd60c248a13f8972cd6ad552464ec Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Thu, 28 Dec 2023 12:51:43 +0100 Subject: [PATCH 07/10] Inline argument checking in LiveAST::Evaler.eval --- lib/live_ast/evaler.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/lib/live_ast/evaler.rb b/lib/live_ast/evaler.rb index ae2c31a..2dbb894 100644 --- a/lib/live_ast/evaler.rb +++ b/lib/live_ast/evaler.rb @@ -6,9 +6,11 @@ class << self include Common def eval(parser_source, evaler_source, bind, filename = nil, lineno = nil) - evaler_source, bind, *rest = handle_args(evaler_source, bind, filename, lineno) + evaler_source = arg_to_str evaler_source + check_is_binding bind + filename = arg_to_str filename if filename - file, line = location_for_eval(bind, *rest) + file, line = location_for_eval(bind, filename, lineno) file = LiveAST.strip_token(file) key, = Linker.new_cache_synced(parser_source, file, line, false) @@ -20,14 +22,6 @@ def eval(parser_source, evaler_source, bind, filename = nil, lineno = nil) raise e end end - - def handle_args(*args) - args.tap do - args[0] = arg_to_str(args[0]) - check_is_binding(args[1]) - args[2] = arg_to_str(args[2]) if args[2] - end - end end end end From 20c251be30df5e131115ca58c40e50b529667f12 Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Thu, 28 Dec 2023 13:31:07 +0100 Subject: [PATCH 08/10] Explicitly forward arguments in LiveAST::Linker.new_cache_synced --- lib/live_ast/linker.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/live_ast/linker.rb b/lib/live_ast/linker.rb index cb95c6b..e79c5c5 100644 --- a/lib/live_ast/linker.rb +++ b/lib/live_ast/linker.rb @@ -110,10 +110,8 @@ def new_cache(contents, file, user_line, file_is_key) return key, cache end - def new_cache_synced(*args) - @mutex.synchronize do - new_cache(*args) - end + def new_cache_synced(...) + @mutex.synchronize { new_cache(...) } end def flush_cache From 2ff84a79218f82b18899f924fb68a30838236dea Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Thu, 28 Dec 2023 13:34:52 +0100 Subject: [PATCH 09/10] Explicitly forward arguments in Kernel#caller replacement --- lib/live_ast/replace_caller.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/live_ast/replace_caller.rb b/lib/live_ast/replace_caller.rb index a4a2c76..9ae3621 100644 --- a/lib/live_ast/replace_caller.rb +++ b/lib/live_ast/replace_caller.rb @@ -6,8 +6,9 @@ module Kernel private alias live_ast_original_caller caller - def caller(*args) - c = live_ast_original_caller(*args) + + def caller(...) + c = live_ast_original_caller(...) c.shift c.map { |line| LiveAST.strip_token line } end From 92ece79b42f6f72227ba13b845f3c5172a02c78f Mon Sep 17 00:00:00 2001 From: Matijs van Zuijlen Date: Thu, 28 Dec 2023 13:35:33 +0100 Subject: [PATCH 10/10] Explicitly forward arguments in Kernel#raise replacement --- lib/live_ast/replace_raise.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/live_ast/replace_raise.rb b/lib/live_ast/replace_raise.rb index 29faf9e..3d3c83e 100644 --- a/lib/live_ast/replace_raise.rb +++ b/lib/live_ast/replace_raise.rb @@ -7,9 +7,9 @@ module Kernel alias live_ast_original_raise raise - def raise(*args) + def raise(...) ex = begin - live_ast_original_raise(*args) + live_ast_original_raise(...) rescue Exception => e e end