Skip to content

Add specs for reserved keywords #1187

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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 = [
Copy link
Contributor 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, done: 0f54929 (#1187)

"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|
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
keywords.each do |kw|
keywords.each do |name|

kw feels too confusing here, especially since there is a spec about keyword arguments below.
Since we are mostly testing names for variables/methods, name seems clearer in usages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mmmmm I disagree, in a spec focused on describing the behaviour of keywords, I would expect "keyword" to a be prominent term. It's an overloaded term, but that's just how Ruby is already. Perhaps keyword is better than kw?

Anyway, I renamed to name

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)
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)
Module.new do
# Allows instance methods defined by `ruby_src` to be called directly.
extend self
end
.class_eval(ruby_src)
end