Skip to content

Commit e535b77

Browse files
vidarhvidarh
committed
Fix case where lone arguments to calls did not get put in an array, coupled with some refactoring to start reducing the coupling between the Shunting Yard parser and the Ruby-specific parser
1 parent 927f173 commit e535b77

File tree

6 files changed

+88
-17
lines changed

6 files changed

+88
-17
lines changed

‎features/parser.feature

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,16 @@ Feature: Parser
1313
Examples:
1414
| expr | tree | notes |
1515
| "1 + 2" | [:do,[:+,1,2]] | The full parser wraps a [:do] around everything |
16-
| "foo { }" | [:do,[:call,:foo,[], [:lambda]]] | Testing empty blocks |
17-
| "foo(1) { }" | [:do,[:call,:foo,1, [:lambda]]] | Testing empty blocks |
18-
| "foo(1) { bar }" | [:do,[:call,:foo,1, [:lambda, [],[:bar]]]] | Testing function calls inside a block |
19-
| "foo(1) { bar 1 }" | [:do,[:call,:foo,1, [:lambda, [],[[:call,:bar,1]]]]] | Testing function calls inside a block |
20-
| "foo { bar[0] }" | [:do,[:call,:foo,[],[:lambda, [],[[:callm,:bar,:[],[0]]]]]] | Testing index operator inside a block |
16+
| "foo(1)" | [:do,[:call,:foo,[1]]] | Simple function/method call |
17+
| "foo(1,2)" | [:do,[:call,:foo,[1,2]]] | Simple function/method call |
18+
| "foo { }" | [:do,[:call,:foo,[], [:proc]]] | Testing empty blocks |
19+
| "foo(1) { }" | [:do,[:call,:foo,1, [:proc]]] | Testing empty blocks |
20+
| "foo(1) { bar }" | [:do,[:call,:foo,1, [:proc, [],[:bar]]]] | Testing function calls inside a block |
21+
| "foo(1) { bar 1 }" | [:do,[:call,:foo,1, [:proc, [],[[:call,:bar,[1]]]]]] | Testing function calls inside a block |
22+
| "foo { bar[0] }" | [:do,[:call,:foo,[],[:proc, [],[[:callm,:bar,:[],[0]]]]]] | Testing index operator inside a block |
2123
| "while foo do end" | [:do, [:while, :foo, [:do]]] | while with "do ... end" instead of just "end" |
2224
| "Keywords=Set[1]"+10.chr+"foo" | [:do,[:assign,:Keywords,[:callm,:Set,:[],[1]]],:foo] | :rp before linefeed should terminate an expression |
23-
| "expect(',') or return args" | [:do,[:or,[:call,:expect,","],[:return,:args]]] | Priority of "or" vs. call/return |
25+
| "expect(',') or return args" | [:do,[:or,[:call,:expect,[","]],[:return,:args]]] | Priority of "or" vs. call/return |
2426
| "require File.dirname() + '/../spec_helper'" | [:do, [:require, [:+, [:callm, :File, :dirname, nil], "/../spec_helper"]]] | |
2527
| "File.dirname() + '/../spec_helper'" | [:do, [:+, [:callm, :File, :dirname, nil], "/../spec_helper"]] | |
2628
| "dirname() + '/../spec_helper'" | [:do, [:+,[:call, :dirname],"/../spec_helper"]] | |
@@ -58,7 +60,7 @@ Feature: Parser
5860
| "{:a => 1, :b => 2}" | [:do,[:hash,[:pair,:":a",1],[:pair,:":b",2]]] | Literal hash with two values |
5961
| "vtable = {}" | [:do,[:assign,:vtable,[:hash]]] | Literal hash |
6062
| "foo = {:a => 1,}" | [:do,[:assign,:foo,[:hash,[:pair,:":a",1]]]] | Literal hash with trailing comma |
61-
| "{:a => foo(1), :b => foo(2)}" | [:do, [:hash, [:pair, :":a", [:call, :foo, 1]], [:pair, :":b", [:call, :foo, 2]]]] | Hash where value is a function call |
63+
| "{:a => foo(1), :b => foo(2)}" | [:do, [:hash, [:pair, :":a", [:call, :foo, [1]]], [:pair, :":b", [:call, :foo, [2]]]]] | Hash where value is a function call |
6264
| "{:a => 1, }" | [:do, [:hash, [:pair,:":a",1]]] | Trailing , |
6365
| "a = {'foo' => :bar}" | [:do, [:assign, :a, [:hash, [:pair, "foo", :":bar"]]]] | |
6466

@@ -99,7 +101,7 @@ Feature: Parser
99101
| "def foo(bar=nil); end" | [:do, [:defm, :foo, [[:bar, :default, :nil]], []]] | Default value for arguments |
100102
| "def foo(bar = nil); end" | [:do, [:defm, :foo, [[:bar, :default, :nil]], []]] | Default value for arguments - with whitespace |
101103
| "def foo(bar = []); end" | [:do, [:defm, :foo, [[:bar, :default, [:array]]], []]] | Default value for arguments - with whitespace |
102-
| "def foo(&bar);end " | [:do, [:defm, :foo, [[:bar,:block]], []]] | Block as named argument |
104+
| "def foo(&bar);end " | [:do, [:defm, :foo, [[:bar,:block]], []]] | Block as named argument |
103105
| "def foo(a = :b, c = :d);end; " | [:do, [:defm, :foo, [[:a,:default, :":b"],[:c,:default, :":d"]], []]] | Second argument following argument with initializer |
104106
| "def foo(a = :b, &bar);end; " | [:do, [:defm, :foo, [[:a,:default, :":b"],[:bar,:block]], []]] | Second argument following argument with initializer |
105107
| "def self.foo;end;" | [:do, [:defm, [:self,:foo], [], []]] | Class method etc. |
@@ -129,7 +131,7 @@ Feature: Parser
129131
| "case foo; when a; end" | [:do, [:case, :foo, [[:when, :a, []]]]] | Basic case structure |
130132
| "case foo; when a; b;when c; d; end" | [:do,[:case, :foo, [[:when,:a,[:b]],[:when,:c,[:d]]]]] | More complicated case |
131133
| "case foo; when ?a..?z, ?A..?Z; end" | [:do, [:case, :foo, [[:when, [[:range, 97, 122], [:range, 65, 90]], []]]]] | "When" with multiple conditions |
132-
| "begin; puts 'foo';rescue Exception => e; end; " | [:do, [:block, [], [[:call, :puts, "foo"]], [:rescue, :Exception, :e, []]]] | begin/rescue |
134+
| "begin; puts 'foo';rescue Exception => e; end; " | [:do, [:block, [], [[:call, :puts, ["foo"]]], [:rescue, :Exception, :e, []]]] | begin/rescue |
133135
| "unless foo; bar; else; baz; end" | [:do, [:unless, :foo, [:do, :bar], [:do, :baz]]] | |
134136
| "if foo; bar; elsif baz; a; else; b; end" | [:do, [:if, :foo, [:do, :bar], [:do, [:if, :baz, [:do, :a], [:do, :b]]]]] | |
135137

@@ -159,4 +161,22 @@ Feature: Parser
159161
| "a && b" | [:do, [:and, :a, :b]] | Simple |
160162

161163

162-
164+
@lambda
165+
Scenario Outline: Lambda and block expressions
166+
Given the expression <expr>
167+
When I parse it with the full parser
168+
Then the parse tree should become <tree>
169+
170+
Examples:
171+
| expr | tree |
172+
| "lambda do end" | [:do, [:lambda]] |
173+
| "lambda do puts 'test'; end" | [:do, [:lambda,[], [[:call, :puts, ["test"]]]]] |
174+
| "lambda do puts 'hello'; puts 'world'; end" | [:do, [:lambda,[], [[:call, :puts, ["hello"]], [:call, :puts, ["world"]]]]] |
175+
| "foo do puts 'hello'; puts 'world'; end" | [:do, [:call, :foo,[], [:proc, [], [[:call, :puts, ["hello"]], [:call, :puts, ["world"]]]]]] |
176+
| "l = lambda do puts 'test'; end" | [:do, [:assign, :l, [:lambda, [], [[:call, :puts, ["test"]]]]]] |
177+
| "l = lambda do puts 'foo'; end; puts 'bar'" | [:do, [:assign, :l, [:lambda, [], [[:call, :puts, ["foo"]]]]], [:call, :puts, ["bar"]]] |
178+
179+
180+
181+
182+

‎features/shunting.feature

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ Feature: Shunting Yard
4444

4545
Examples:
4646
| expr | tree |
47-
| "foo(1)" | [:call,:foo,1] |
47+
| "foo(1)" | [:call,:foo,[1]] |
4848
| "foo(1,2)" | [:call,:foo,[1,2]] |
49-
| "foo 1" | [:call,:foo,1] |
49+
| "foo 1" | [:call,:foo,[1]] |
5050
| "foo 1,2" | [:call,:foo,[1,2]] |
5151
| "self.foo" | [:callm,:self,:foo] |
5252
| "self.foo(1)" | [:callm,:self,:foo,1] |

‎shunting.rb

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,15 @@
22
require 'pp'
33
require 'treeoutput'
44

5+
require 'tokenizeradapter'
6+
57
module OpPrec
68
class ShuntingYard
79
def initialize(output, tokenizer, parser)
810
@out = output
9-
@tokenizer = tokenizer
11+
12+
# FIXME: Pass this in instead of storing it.
13+
@tokenizer = TokenizerAdapter.new(tokenizer,parser)
1014
@parser = parser
1115
end
1216

@@ -45,7 +49,7 @@ def shunt(src, ostack = [], inhibit = [])
4549
src.each do |token,op,keyword|
4650
# Normally we stop when encountering a keyword, but it's ok to encounter
4751
# one as the second operand for an infix operator
48-
if inhibit.include?(token) or keyword && (opstate != :prefix || !ostack.last || ostack.last.type != :infix)
52+
if inhibit.include?(token) or keyword && (opstate != :prefix || !ostack.last || ostack.last.type != :infix || token == :end)
4953
src.unget(token)
5054
break
5155
end
@@ -99,7 +103,7 @@ def shunt(src, ostack = [], inhibit = [])
99103
opstate = :infix_or_postfix # After a non-operator value, any single arity operator would be either postfix,
100104
# so when seeing the next operator we will assume it is either infix or postfix.
101105
end
102-
possible_func = op ? op.type == :lp : !token.is_a?(Numeric)
106+
possible_func = op ? op.type == :lp : (!token.is_a?(Numeric) || !token.is_a?(Array))
103107
lastlp = false
104108
src.ws if lp_on_entry
105109
end

‎tokenizeradapter.rb

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
2+
# The purpose of this class is to serve as an adapter between the
3+
# ShuntingYard component, the Tokenizer, and the Parser.
4+
# The reason for this is that a number of tokens indicate
5+
# a need to call back into the parser, but the ShuntingYard
6+
# class only need to know that what is returned is not an operator
7+
# it should be concerned about
8+
9+
class TokenizerAdapter
10+
11+
def initialize(tokenizer, parser)
12+
@tokenizer = tokenizer
13+
@parser = parser
14+
15+
@escape_tokens = {
16+
:lambda => :parse_defexp
17+
}
18+
end
19+
20+
def each
21+
@tokenizer.each do |token, op, keyword|
22+
if keyword and (m = @escape_tokens[token])
23+
@tokenizer.unget(token)
24+
yield(@parser.send(m), nil, nil)
25+
else
26+
yield(token,op,keyword)
27+
end
28+
end
29+
end
30+
31+
def ws
32+
@tokenizer.ws
33+
end
34+
35+
def unget token
36+
@tokenizer.unget(token)
37+
end
38+
39+
def lasttoken
40+
@tokenizer.lasttoken
41+
end
42+
end
43+
44+
45+

‎tokens.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ module Tokens
66

77
Keywords = Set[
88
:begin, :case, :class, :def, :do, :else, :end, :if, :include,
9-
:module, :require, :rescue, :then, :unless, :when, :elsif
9+
:module, :require, :rescue, :then, :unless, :when, :elsif, :lambda
1010
]
1111

1212
# Methods can end with one of these.

‎treeoutput.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,9 @@ def oper(o)
7474
elsif ra and rightv[0] == :flatten
7575
@vstack << E[o.sym, leftv] + flatten(rightv[1..-1])
7676
else
77-
if o.sym == :call || o.sym == :callm and ra and rightv[0] != :flatten and rightv[0] != :comma
77+
# FIXME This seemingly fixes issue where single argument function call does not get its arguments wrapped.
78+
# FIXME Need to verify that this doesn't fail any other tests than the ones it should
79+
if o.sym == :call || o.sym == :callm and o.type == :prefix and rightv[0] != :flatten and rightv[0] != :comma
7880
rightv = E[rightv]
7981
end
8082
@vstack << E[o.sym, flatten(leftv), rightv].compact

0 commit comments

Comments
 (0)