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

Add specs for reserved keywords #1187

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
155 changes: 155 additions & 0 deletions language/reserved_keywords.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
require_relative '../spec_helper'

describe "Ruby's reserved keywords" do
# Copied from Prism::Translation::Ripper
keywords = [
Comment on lines +4 to +5
Copy link
Author

Choose a reason for hiding this comment

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

TODO: It would be nice to have one canonical list that can be used in all these places:

  1. spec/ruby/language/hash_spec.rb
  2. spec/ruby/language/reserved_keywords.rb
  3. lib/prism/translation/ripper.rb
  4. Ideally, it would also be nice to generate doc/keywords.rdoc from that list, to ensure that all keywords are documented.

Where should such a canonical list live?

Copy link
Member

Choose a reason for hiding this comment

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

There is no way to avoid duplication there, since it's files in different repositories.
So for ruby/spec let's have it as a new file under language/fixtures.
I think a %w[] array would be nice for this BTW.

FWIW in ruby/ruby there is also defs/keywords.

"alias",
"and",
"begin",
"BEGIN",
"break",
"case",
"class",
"def",
"defined?",
"do",
"else",
"elsif",
"end",
"END",
"ensure",
"false",
"for",
"if",
"in",
"module",
"next",
"nil",
"not",
"or",
"redo",
"rescue",
"retry",
"return",
"self",
"super",
"then",
"true",
"undef",
"unless",
"until",
"when",
"while",
"yield",
"__ENCODING__",
"__FILE__",
"__LINE__"
]

keywords.each do |kw|
describe "keyword '#{kw}'" do
it "can't be used as local variable name" do
expect_syntax_error <<~RUBY
#{kw} = "a local variable named '#{kw}'"
RUBY
end

invalid_ivar_names = ["defined?"]

if invalid_ivar_names.include?(kw)
Copy link
Member

Choose a reason for hiding this comment

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

if outside examples like this are pretty much an anti-pattern in ruby/spec.
Same for looping around it.
Instead, the loops should be inside the it.
That also makes it easier to read, e.g. (keywords - %w[defined?]).each.

it "can't be used as an instance variable name" do
expect_syntax_error <<~RUBY
@#{kw} = "an instance variable named '#{kw}'"
RUBY
end
else
it "can be used as an instance variable name" do
result = sandboxed_eval <<~RUBY
@#{kw} = "an instance variable named '#{kw}'"
@#{kw}
RUBY

result.should == "an instance variable named '#{kw}'"
end
end

invalid_class_var_names = ["defined?"]

if invalid_class_var_names.include?(kw)
it "can't be used as a class variable name" do
expect_syntax_error <<~RUBY
@@#{kw} = "a class variable named '#{kw}'"
RUBY
end
else
it "can be used as a class variable name" do
result = sandboxed_eval <<~RUBY
@@#{kw} = "a class variable named '#{kw}'"
@@#{kw}
RUBY

result.should == "a class variable named '#{kw}'"
end
end

invalid_global_var_names = ["defined?"]

if invalid_global_var_names.include?(kw)
it "can't be used as a global variable name" do
expect_syntax_error <<~RUBY
$#{kw} = "a global variable named '#{kw}'"
RUBY
end
else
it "can be used as a global variable name" do
result = sandboxed_eval <<~RUBY
$#{kw} = "a global variable named '#{kw}'"
$#{kw}
RUBY

result.should == "a global variable named '#{kw}'"
end
end

it "can't be used as a positional parameter name" do
expect_syntax_error <<~RUBY
def x(#{kw}); end
RUBY
end

invalid_kw_param_names = ["BEGIN","END","defined?"]

if invalid_kw_param_names.include?(kw)
it "can't be used a keyword parameter name" do
expect_syntax_error <<~RUBY
def m(#{kw}:); end
RUBY
end
else
it "can be used a keyword parameter name" do
result = sandboxed_eval <<~RUBY
def m(#{kw}:)
binding.local_variable_get(:#{kw})
end

m(#{kw}: "an argument to '#{kw}'")
RUBY

result.should == "an argument to '#{kw}'"
end
end

it "can be used as a method name" do
result = sandboxed_eval <<~RUBY
def #{kw}
"a method named '#{kw}'"
end

send(:#{kw})
RUBY

result.should == "a method named '#{kw}'"
end
end
end
end
14 changes: 14 additions & 0 deletions spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,17 @@ def report_on_exception=(value)
ARGV.unshift $0
MSpecRun.main
end

def expect_syntax_error(ruby_src)
-> { eval(ruby_src) }.should raise_error(SyntaxError)
end

# Evaluates the given Ruby source in a temporary Module, to prevent
# the surrounding context from being polluted with the new methods.
def sandboxed_eval(ruby_src)
Copy link
Member

@eregon eregon Jul 31, 2024

Choose a reason for hiding this comment

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

This seems pretty expensive (creating a module for each keyword * spec examples).
Since in ruby/spec every file has its own separate Object instance as top-level self, I think we don't need to worry about this can just define on the self.

Module.new do
# Allows instance methods defined by `ruby_src` to be called directly.
extend self
end
.class_eval(ruby_src)
end