Skip to content

Commit e1966c5

Browse files
maciejhirszmaciejhirsz
authored
Merge pull request #28 from ratel-rust/parenfix
Parenfix
2 parents 096a4ef + 6c341b9 commit e1966c5

File tree

6 files changed

+120
-60
lines changed

6 files changed

+120
-60
lines changed

‎core/src/codegen.rs‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ impl Generator {
7373

7474
#[inline]
7575
pub fn write_block<T: Code>(&mut self, items: &Vec<T>) {
76+
if items.len() == 0 {
77+
return;
78+
}
79+
7680
self.indent();
7781
for item in items {
7882
self.new_line();
@@ -374,6 +378,11 @@ impl Code for Expression {
374378
},
375379

376380
Expression::Object(ref members) => {
381+
if members.len() == 0 {
382+
gen.write_bytes(b"{}");
383+
return;
384+
}
385+
377386
gen.write_byte(b'{');
378387
gen.indent();
379388

@@ -425,11 +434,14 @@ impl Code for Expression {
425434
},
426435

427436
Expression::Binary {
428-
ref left,
429437
ref operator,
438+
ref left,
430439
ref right,
440+
..
431441
} => {
432-
if left.binding_power() < self.binding_power() {
442+
let bp = self.binding_power();
443+
444+
if left.binding_power() < bp {
433445
gen.write_byte(b'(');
434446
gen.write(left);
435447
gen.write_byte(b')');
@@ -439,7 +451,14 @@ impl Code for Expression {
439451
gen.write_min(b" ", b"");
440452
gen.write(operator);
441453
gen.write_min(b" ", b"");
442-
gen.write(right);
454+
455+
if right.needs_parens(bp) {
456+
gen.write_byte(b'(');
457+
gen.write(right);
458+
gen.write_byte(b')');
459+
} else {
460+
gen.write(right);
461+
}
443462
},
444463

445464
Expression::Prefix {

‎core/src/grammar.rs‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,8 +252,9 @@ pub enum Expression {
252252
arguments: Vec<Expression>,
253253
},
254254
Binary {
255-
left: Box<Expression>,
255+
parenthesized: bool,
256256
operator: OperatorType,
257+
left: Box<Expression>,
257258
right: Box<Expression>,
258259
},
259260
Prefix {
@@ -320,6 +321,7 @@ impl Expression {
320321
#[inline]
321322
pub fn binary<E: Into<Expression>>(left: E, operator: OperatorType, right: E) -> Self {
322323
Expression::Binary {
324+
parenthesized: false,
323325
operator: operator,
324326
left: Box::new(left.into()),
325327
right: Box::new(right.into()),
@@ -341,6 +343,30 @@ impl Expression {
341343
arguments: arguments,
342344
}
343345
}
346+
347+
#[inline]
348+
pub fn parenthesize(mut self) -> Expression {
349+
if let Expression::Binary {
350+
ref mut parenthesized,
351+
..
352+
} = self {
353+
*parenthesized = true;
354+
}
355+
356+
self
357+
}
358+
359+
#[inline]
360+
pub fn needs_parens(&self, bp: u8) -> bool {
361+
match *self {
362+
Expression::Binary {
363+
ref parenthesized,
364+
ref operator,
365+
..
366+
} => *parenthesized && bp >= operator.binding_power(),
367+
_ => false
368+
}
369+
}
344370
}
345371

346372
impl From<&'static str> for Expression {

‎core/src/parser.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,8 @@ impl<'a> Parser<'a> {
130130

131131
loop {
132132
match next!(self) {
133-
ParenClose => break,
134-
token => {
133+
BracketClose => break,
134+
token => {
135135
let expression = try!(self.expression_from_token(token, 0));
136136
list.push(expression);
137137
}
@@ -453,7 +453,7 @@ impl<'a> Parser<'a> {
453453

454454
expect!(self, ParenClose);
455455

456-
Ok(expression)
456+
Ok(expression.parenthesize())
457457
}
458458
}
459459
}
@@ -786,9 +786,10 @@ impl<'a> Parser<'a> {
786786
let expression = try!(self.sequence_or_expression_from_token(token));
787787

788788
if let Expression::Binary {
789-
left,
790789
operator: In,
790+
left,
791791
right,
792+
..
792793
} = expression {
793794
return self.for_in_statement_from_expressions(*left, *right);
794795
}

‎core/src/transformer.rs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,9 +269,10 @@ impl Transformable for Expression {
269269
},
270270

271271
Expression::Binary {
272-
ref mut left,
273272
ref mut operator,
273+
ref mut left,
274274
ref mut right,
275+
..
275276
} => {
276277
left.transform(settings);
277278
right.transform(settings);
@@ -343,7 +344,7 @@ impl Transformable for Expression {
343344
left = Expression::binary(
344345
left,
345346
Addition,
346-
expression
347+
expression.parenthesize()
347348
);
348349

349350
if quasi.len() == 0 {

‎core/tests/codegen.rs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,22 @@ fn template_strings_plain() {
3939
assert_compile!("`foo\nbar`;", "\"foo\\nbar\";");
4040
}
4141

42+
#[test]
43+
fn operator_precedence_and_parens() {
44+
// Preserve parens when necessary
45+
assert_compile!("'foo'+(1+2);", r#"'foo'+(1+2);"#);
46+
47+
// Should strip parens when not necessary
48+
assert_compile!("(1+2)+'foo';", r#"1+2+'foo';"#);
49+
assert_compile!("'foo'+(1*2);", r#"'foo'+1*2;"#);
50+
assert_compile!("(1*2)+'foo';", r#"1*2+'foo';"#);
51+
}
52+
4253
#[test]
4354
fn template_strings_interpolation() {
4455
assert_compile!("`foo${1}bar`;", r#""foo"+1+"bar";"#);
56+
assert_compile!("`foo${1+2}bar`;", r#""foo"+(1+2)+"bar";"#);
57+
assert_compile!("`foo${1*2}bar`;", r#""foo"+1*2+"bar";"#);
4558
assert_compile!("`foo${1}${2**2}bar`;", r#""foo"+1+Math.pow(2,2)+"bar";"#);
4659
assert_compile!("`foo${1}bar${2**2}`;", r#""foo"+1+"bar"+Math.pow(2,2);"#);
4760
}

‎core/tests/parser.rs‎

Lines changed: 50 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -298,46 +298,46 @@ fn template_string_complex() {
298298

299299
#[test]
300300
fn binary_expression() {
301-
assert_expression!("true == 1", Expression::Binary {
302-
left: Box::new(Expression::Literal(Value::True)),
303-
operator: Equality,
304-
right: boxnum!("1")
305-
});
301+
assert_expression!("true == 1", Expression::binary(
302+
Expression::Literal(Value::True),
303+
Equality,
304+
num!("1")
305+
));
306306
}
307307

308308
#[test]
309309
fn binary_expression_less_than() {
310-
assert_expression!("i < 10", Expression::Binary {
311-
left: Box::new(ident!("i")),
312-
operator: Lesser,
313-
right: boxnum!("10")
314-
});
310+
assert_expression!("i < 10", Expression::binary(
311+
ident!("i"),
312+
Lesser,
313+
num!("10")
314+
));
315315
}
316316

317317
#[test]
318318
fn op_precedence_left() {
319-
assert_expression!("1 + 2 * 3", Expression::Binary {
320-
left: boxnum!("1"),
321-
operator: Addition,
322-
right: Box::new(Expression::Binary {
323-
left: boxnum!("2"),
324-
operator: Multiplication,
325-
right: boxnum!("3"),
326-
}),
327-
});
319+
assert_expression!("1 + 2 * 3", Expression::binary(
320+
num!("1"),
321+
Addition,
322+
Expression::binary(
323+
num!("2"),
324+
Multiplication,
325+
num!("3")
326+
),
327+
));
328328
}
329329

330330
#[test]
331331
fn op_precedence_right() {
332-
assert_expression!("1 * 2 + 3", Expression::Binary {
333-
left: Box::new(Expression::Binary {
334-
left: boxnum!("1"),
335-
operator: Multiplication,
336-
right: boxnum!("2"),
337-
}),
338-
operator: Addition,
339-
right: boxnum!("3"),
340-
});
332+
assert_expression!("1 * 2 + 3", Expression::binary(
333+
Expression::binary(
334+
num!("1"),
335+
Multiplication,
336+
num!("2"),
337+
),
338+
Addition,
339+
num!("3"),
340+
));
341341
}
342342

343343
#[test]
@@ -489,19 +489,19 @@ fn if_else_no_block_statement() {
489489
fn for_statement() {
490490
assert_statement!("for (i = 0; i < 10; i++) {}", Statement::For {
491491
init: Some(Box::new(Statement::Expression {
492-
value: Expression::Binary {
493-
left: Box::new(ident!("i")),
494-
operator: OperatorType::Assign,
495-
right: Box::new(num!("0")),
496-
}
492+
value: Expression::binary(
493+
ident!("i"),
494+
Assign,
495+
num!("0"),
496+
)
497497
})),
498-
test: Some(Expression::Binary {
499-
left: Box::new(ident!("i")),
500-
operator: OperatorType::Lesser,
501-
right: Box::new(num!("10")),
502-
}),
498+
test: Some(Expression::binary(
499+
ident!("i"),
500+
Lesser,
501+
num!("10"),
502+
)),
503503
update: Some(Expression::Postfix {
504-
operator: OperatorType::Increment,
504+
operator: Increment,
505505
operand: Box::new(ident!("i")),
506506
}),
507507
body: Box::new(Statement::Block {
@@ -522,13 +522,13 @@ fn for_declare_statement() {
522522
}
523523
],
524524
})),
525-
test: Some(Expression::Binary {
526-
left: Box::new(ident!("i")),
527-
operator: OperatorType::Lesser,
528-
right: Box::new(num!("10")),
529-
}),
525+
test: Some(Expression::binary(
526+
ident!("i"),
527+
Lesser,
528+
num!("10"),
529+
)),
530530
update: Some(Expression::Postfix {
531-
operator: OperatorType::Increment,
531+
operator: Increment,
532532
operand: Box::new(ident!("i")),
533533
}),
534534
body: Box::new(Statement::Block {
@@ -666,11 +666,11 @@ fn arrow_function_shorthand() {
666666
param!("n")
667667
],
668668
body: Box::new(Statement::Expression {
669-
value: Expression::Binary {
670-
left: Box::new(ident!("n")),
671-
operator: Multiplication,
672-
right: Box::new(ident!("n")),
673-
}
669+
value: Expression::binary(
670+
ident!("n"),
671+
Multiplication,
672+
ident!("n")
673+
)
674674
}),
675675
});
676676
}

0 commit comments

Comments
 (0)