Skip to content
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

Improve command input recognition #805

Closed
wants to merge 4 commits into from
Closed
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
53 changes: 42 additions & 11 deletions lib/irb.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1036,6 +1036,9 @@ def eval_input
rescue Interrupt, Exception => exc
handle_exception(exc)
@context.workspace.local_variable_set(:_, exc)
if statement.warning
warn statement.warning
end
end
end
end
Expand Down Expand Up @@ -1102,24 +1105,52 @@ def each_top_level_statement
end
end

COMMAND_FLAG_REGEXP = /(?<cmd_flag>-[a-zA-Z]+( +\S+)*)/
COMMAND_ARG_REGEXP = /(?<cmd_arg>[^- ]\S*)/
COMMAND_NAME_REGEXP = /(?<cmd_name>\S+)/
SIMPLE_COMMAND_REGEXP = /^#{COMMAND_NAME_REGEXP}\z/
COMMAND_WITH_ARGS_REGEXP = /^#{COMMAND_NAME_REGEXP} +#{COMMAND_ARG_REGEXP}\z/
COMMAND_WITH_FLAGS_REGEXP = /^#{COMMAND_NAME_REGEXP} +#{COMMAND_FLAG_REGEXP}\z/
COMMAND_WITH_ARGS_AND_FLAGS_REGEXP = /^#{COMMAND_NAME_REGEXP} +#{COMMAND_ARG_REGEXP} +#{COMMAND_FLAG_REGEXP}\z/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this still matches to the second line. It should be /^/\A

Command (Last line matches to COMMAND_REGEXP)

show_source = 1..
2# Set#include?

Not a command (Both line does not match to COMMAND_REGEXP)

show_source = 1..
2 # Set#include?


COMMAND_REGEXP = Regexp.union(
SIMPLE_COMMAND_REGEXP,
COMMAND_WITH_ARGS_REGEXP,
COMMAND_WITH_FLAGS_REGEXP,
COMMAND_WITH_ARGS_AND_FLAGS_REGEXP
)

def build_statement(code)
code.force_encoding(@context.io.encoding)
command_or_alias, arg = code.split(/\s/, 2)
# Transform a non-identifier alias (@, $) or keywords (next, break)
command_name = @context.command_aliases[command_or_alias.to_sym]
command = command_name || command_or_alias
command_class = ExtendCommandBundle.load_command(command)

if command_class
Statement::Command.new(code, command, arg, command_class)
code.force_encoding(@context.io.encoding) unless code.frozen?

possible_command_or_alias = code.split.first

possible_command_name = @context.command_aliases[possible_command_or_alias.to_sym]
possible_command_name = possible_command_name || possible_command_or_alias
command_class = ExtendCommandBundle.load_command(possible_command_name)

command_syntax_match = COMMAND_REGEXP.match(code.strip)

if command_class && command_syntax_match
arg = [command_syntax_match[:cmd_arg], command_syntax_match[:cmd_flag]].compact.join(' ')
Statement::Command.new(code, possible_command_name, arg, command_class)
else
if command_class
warning_msg = <<~MSG
The input `#{code.strip}` was recognised as a Ruby expression, but it matched the name of the `#{possible_command_name}` command.
If you intended to run it as a command, please check if the syntax is correct.
MSG
end

is_assignment_expression = @scanner.assignment_expression?(code, local_variables: @context.local_variables)
Statement::Expression.new(code, is_assignment_expression)
Statement::Expression.new(code, is_assignment_expression, warning: warning_msg)
end
end

def single_line_command?(code)
command = code.split(/\s/, 2).first
command_match = COMMAND_REGEXP.match(code)
return unless command_match
command = command_match[:cmd_name]
@context.symbol_alias?(command) || @context.transform_args?(command)
end

Expand Down
2 changes: 1 addition & 1 deletion lib/irb/cmd/ls.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class Ls < Nop
description "Show methods, constants, and variables. `-g [query]` or `-G [query]` allows you to filter out the output."

def self.transform_args(args)
if match = args&.match(/\A(?<args>.+\s|)(-g|-G)\s+(?<grep>[^\s]+)\s*\n\z/)
if match = args&.match(/\A(?<args>.+\s|)(-g|-G)\s+(?<grep>[^\s]+)\s*\z/)
args = match[:args]
"#{args}#{',' unless args.chomp.empty?} grep: /#{match[:grep]}/"
else
Expand Down
8 changes: 6 additions & 2 deletions lib/irb/statement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module IRB
class Statement
attr_reader :code
attr_reader :code, :warning

def is_assignment?
raise NotImplementedError
Expand All @@ -21,9 +21,10 @@ def evaluable_code
end

class Expression < Statement
def initialize(code, is_assignment)
def initialize(code, is_assignment, warning: nil)
@code = code
@is_assignment = is_assignment
@warning = warning
end

def suppresses_echo?
Expand Down Expand Up @@ -66,6 +67,9 @@ def should_be_handled_by_debugger?
end

def evaluable_code
# Because measure command is used as a method, we need to treat it differently
return @code if @command == "measure"
Copy link
Member Author

@st0012 st0012 Dec 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to avoid this once #624 is merged.

And we still need it even with #813 so we can print the warning on measure blocks.


# Hook command-specific transformation to return valid Ruby code
if @command_class.respond_to?(:transform_args)
arg = @command_class.transform_args(@arg)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
require_relative "helper"

module TestIRB
class DebugCommandTest < IntegrationTestCase
class DebugIntegrationTest < IntegrationTestCase
def setup
super

Expand Down Expand Up @@ -434,5 +434,22 @@ def test_multi_irb_commands_are_not_available_after_activating_the_debugger
assert_match(/irb\(main\):001> next/, output)
assert_include(output, "Multi-IRB commands are not available when the debugger is enabled.")
end

def test_locals_assignment_not_being_treated_as_debugging_command
write_ruby <<~'ruby'
binding.irb
ruby

output = run_ruby_file do
type "info = 4"
type "info + 1"
type "quit"
end

assert_match(/=> 5/, output)
# Since neither `info = <val>` nor `info + <val>` are debugging commands, debugger should not be activated in this
# session.
assert_not_match(/irb:rdbg/, output)
end
end
end
104 changes: 104 additions & 0 deletions test/irb/test_irb.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,51 @@ def test_symbol_aliases_dont_affect_ruby_syntax
assert_include output, "=> \"It's a foo\""
assert_include output, "=> \"It's a bar\""
end

def test_errored_input_that_match_a_command_raises_a_syntax_check_reminder
write_ruby <<~'RUBY'
class Foo
end
binding.irb
RUBY

errored_output = run_ruby_file do
type "show_source Foo bar"
type "exit!"
end

# The input should cause an error as it's evaluated as Ruby code
assert_include errored_output, 'undefined local variable or method `bar\''

# Because it starts with the name of a command, it should also raise a warning
assert_include errored_output,
'The input `show_source Foo bar` was recognised as a Ruby expression, but it matched the name of the `show_source` command.'
assert_include errored_output,
'If you intended to run it as a command, please check if the syntax is correct.'

output = run_ruby_file do
type "show_source Foo"
type "exit!"
end

# The input should work as a command
assert_include output, "From: #{@ruby_file.path}:1"
# Therefore, it should not raise a warning
assert_not_include output,
'If you intended to run it as a command, please check if the syntax is correct.'

output = run_ruby_file do
type "show_source = 'foo'"
type "show_source + 'bar'"
type "exit!"
end

# The input should work as Ruby code
assert_include(output, '=> "foobar"')
# Therefore, it should not raise a warning either
assert_not_include output,
'If you intended to run it as a command, please check if the syntax is correct.'
end
end

class IrbIOConfigurationTest < TestCase
Expand Down Expand Up @@ -738,4 +783,63 @@ def build_irb
IRB::Irb.new(workspace, TestInputMethod.new)
end
end

class InputCategorisationTest < TestCase
def test_irb_distinguihes_commands_and_non_commands_correctly
irb = build_irb

[
"show_source Foo#bar",
"show_source Foo#bar -s",
"show_source Foo.bar",
"show_source Foo.bar -s",
"show_source == -s",
"show_source =~ -s",
"ls foo.bar -g baz",
"ls -g foo",
"bt 4",
"bt"
].each do |input|
statement = irb.build_statement(input)
assert_equal(IRB::Statement::Command, statement.class, "Expected #{input} to be a command")
end

[
"info + 1",
"info - 1",
"info = 1",
"info = -1",
"info = /regex/",
"info = a",
"info += a",
"info -= a",
"info *= a",
"info &&= a",
"info ||= a",
"info # comment",
"info if foo",
"info ? foo : bar",
"info ; foo",
"info ,other = expr",
"info -" # when an input is not a command nor Ruby code, we want to make it Ruby so it raises syntax error
].each do |input|
statement = irb.build_statement(input)
assert_equal(IRB::Statement::Expression, statement.class, "Expected #{input} to not be a command")
end
end

private

def build_binding
Object.new.instance_eval { binding }
end

def build_irb
IRB.init_config(nil)
workspace = IRB::WorkSpace.new(build_binding)

IRB.conf[:VERBOSE] = false
IRB::Irb.new(workspace, TestInputMethod.new)
end
end
end