From 2d75aeb2763ee9dbeedd8218d8ef394d181eb881 Mon Sep 17 00:00:00 2001 From: harupy Date: Sun, 11 Dec 2022 12:35:28 +0900 Subject: [PATCH 1/7] Fix end location of nodes containing body --- parser/python.lalrpop | 8 +++++--- parser/src/parser.rs | 3 ++- .../rustpython_parser__parser__tests__parse_class.snap | 4 ++-- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/parser/python.lalrpop b/parser/python.lalrpop index 442a5ed..b4f85b5 100644 --- a/parser/python.lalrpop +++ b/parser/python.lalrpop @@ -492,9 +492,10 @@ WithItem: ast::Withitem = { }; FuncDef: ast::Stmt = { - "def" " Test)?> ":" => { + "def" " Test)?> ":" => { let args = Box::new(args); let returns = r.map(|x| Box::new(x.1)); + let end_location = body.last().unwrap().end_location.unwrap(); let type_comment = None; let node = if is_async.is_some() { ast::StmtKind::AsyncFunctionDef { name, args, body, decorator_list, returns, type_comment } @@ -646,15 +647,16 @@ KwargParameter: Option> = { }; ClassDef: ast::Stmt = { - "class" ":" => { + "class" ":" => { let (bases, keywords) = match a { Some((_, arg, _)) => (arg.args, arg.keywords), None => (vec![], vec![]), }; + let end_location = body.last().unwrap().end_location; ast::Stmt { custom: (), location, - end_location: Some(end_location), + end_location, node: ast::StmtKind::ClassDef { name, bases, diff --git a/parser/src/parser.rs b/parser/src/parser.rs index f120dae..27192b6 100644 --- a/parser/src/parser.rs +++ b/parser/src/parser.rs @@ -173,7 +173,8 @@ class Foo(A, B): def __init__(self): pass def method_with_default(self, arg='default'): - pass"; + pass +"; insta::assert_debug_snapshot!(parse_program(source, "").unwrap()); } diff --git a/parser/src/snapshots/rustpython_parser__parser__tests__parse_class.snap b/parser/src/snapshots/rustpython_parser__parser__tests__parse_class.snap index c74d2eb..22b1541 100644 --- a/parser/src/snapshots/rustpython_parser__parser__tests__parse_class.snap +++ b/parser/src/snapshots/rustpython_parser__parser__tests__parse_class.snap @@ -62,8 +62,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 4, - column: 1, + row: 3, + column: 6, }, ), custom: (), From c79cc34b377ff92b0c5dad7eadb361c38b4b614f Mon Sep 17 00:00:00 2001 From: harupy Date: Mon, 12 Dec 2022 01:10:42 +0900 Subject: [PATCH 2/7] Fix other compound statements --- parser/python.lalrpop | 57 ++++++++++++++----- ...er__parser__tests__parse_if_elif_else.snap | 4 +- 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/parser/python.lalrpop b/parser/python.lalrpop index b4f85b5..205bee0 100644 --- a/parser/python.lalrpop +++ b/parser/python.lalrpop @@ -357,16 +357,24 @@ CompoundStatement: ast::Stmt = { }; IfStatement: ast::Stmt = { - "if" ":" => { + "if" ":" => { // Determine last else: let mut last = s3.map(|s| s.2).unwrap_or_default(); - + let end_location = if let Some(last) = last.last() { + last.end_location + } else { + if let Some(last) = s2.last() { + last.4.last().unwrap().end_location + } else { + body.last().unwrap().end_location + } + }; // handle elif: for i in s2.into_iter().rev() { let x = ast::Stmt { custom: (), location: i.0, - end_location: Some(i.5), + end_location: i.4.last().unwrap().end_location, node: ast::StmtKind::If { test: Box::new(i.2), body: i.4, orelse: last }, }; last = vec![x]; @@ -375,19 +383,24 @@ IfStatement: ast::Stmt = { ast::Stmt { custom: (), location, - end_location: Some(end_location), + end_location, node: ast::StmtKind::If { test: Box::new(test), body, orelse: last } } }, }; WhileStatement: ast::Stmt = { - "while" ":" => { + "while" ":" => { + let end_location = if let Some(ref s) = s2 { + s.2.last().unwrap().end_location + } else { + body.last().unwrap().end_location + }; let orelse = s2.map(|s| s.2).unwrap_or_default(); ast::Stmt { custom: (), location, - end_location: Some(end_location), + end_location, node: ast::StmtKind::While { test: Box::new(test), body, @@ -398,7 +411,12 @@ WhileStatement: ast::Stmt = { }; ForStatement: ast::Stmt = { - "for" "in" ":" => { + "for" "in" ":" => { + let end_location = if let Some(ref s) = s2 { + s.2.last().unwrap().end_location.unwrap() + } else { + body.last().unwrap().end_location.unwrap() + }; let orelse = s2.map(|s| s.2).unwrap_or_default(); let target = Box::new(set_context(target, ast::ExprContext::Store)); let iter = Box::new(iter); @@ -416,10 +434,19 @@ TryStatement: ast::Stmt = { "try" ":" => { let orelse = else_suite.map(|s| s.2).unwrap_or_default(); let finalbody = finally.map(|s| s.2).unwrap_or_default(); + let end_location = if let Some(last) = finalbody.last() { + last.end_location + } else { + if let Some(last) = orelse.last() { + last.end_location + } else { + handlers.last().unwrap().end_location + } + }; ast::Stmt { custom: (), location, - end_location: Some(end_location), + end_location, node: ast::StmtKind::Try { body, handlers, @@ -428,14 +455,15 @@ TryStatement: ast::Stmt = { }, } }, - "try" ":" => { + "try" ":" => { let handlers = vec![]; let orelse = vec![]; let finalbody = finally.2; + let end_location = finalbody.last().unwrap().end_location; ast::Stmt { custom: (), location, - end_location: Some(end_location), + end_location, node: ast::StmtKind::Try { body, handlers, @@ -447,7 +475,8 @@ TryStatement: ast::Stmt = { }; ExceptClause: ast::Excepthandler = { - "except" ":" => { + "except" ":" => { + let end_location = body.last().unwrap().end_location.unwrap(); ast::Excepthandler::new( location, end_location, @@ -458,7 +487,8 @@ ExceptClause: ast::Excepthandler = { }, ) }, - "except" ":" => { + "except" ":" => { + let end_location = body.last().unwrap().end_location.unwrap(); ast::Excepthandler::new( location, end_location, @@ -472,7 +502,8 @@ ExceptClause: ast::Excepthandler = { }; WithStatement: ast::Stmt = { - "with" > ":" => { + "with" > ":" => { + let end_location = body.last().unwrap().end_location.unwrap(); let type_comment = None; let node = if is_async.is_some() { ast::StmtKind::AsyncWith { items, body, type_comment } diff --git a/parser/src/snapshots/rustpython_parser__parser__tests__parse_if_elif_else.snap b/parser/src/snapshots/rustpython_parser__parser__tests__parse_if_elif_else.snap index e65226d..7f493dd 100644 --- a/parser/src/snapshots/rustpython_parser__parser__tests__parse_if_elif_else.snap +++ b/parser/src/snapshots/rustpython_parser__parser__tests__parse_if_elif_else.snap @@ -79,8 +79,8 @@ expression: parse_ast }, end_location: Some( Location { - row: 3, - column: 0, + row: 2, + column: 10, }, ), custom: (), From cfbf5f0f6302eed4b0653f46f1839c5c54232e4f Mon Sep 17 00:00:00 2001 From: harupy Date: Mon, 12 Dec 2022 10:24:00 +0900 Subject: [PATCH 3/7] Use method chaining --- parser/python.lalrpop | 47 +++++++++++++++++-------------------------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/parser/python.lalrpop b/parser/python.lalrpop index 205bee0..ff33fe4 100644 --- a/parser/python.lalrpop +++ b/parser/python.lalrpop @@ -360,15 +360,11 @@ IfStatement: ast::Stmt = { "if" ":" => { // Determine last else: let mut last = s3.map(|s| s.2).unwrap_or_default(); - let end_location = if let Some(last) = last.last() { - last.end_location - } else { - if let Some(last) = s2.last() { - last.4.last().unwrap().end_location - } else { - body.last().unwrap().end_location - } - }; + let end_location = last + .last() + .map(|last| last.end_location) + .or_else(|| s2.last().map(|last| last.4.last().unwrap().end_location)) + .unwrap_or_else(|| body.last().unwrap().end_location); // handle elif: for i in s2.into_iter().rev() { let x = ast::Stmt { @@ -391,11 +387,10 @@ IfStatement: ast::Stmt = { WhileStatement: ast::Stmt = { "while" ":" => { - let end_location = if let Some(ref s) = s2 { - s.2.last().unwrap().end_location - } else { - body.last().unwrap().end_location - }; + let end_location = s2 + .as_ref() + .map(|s| s.2.last().unwrap().end_location) + .unwrap_or_else(|| body.last().unwrap().end_location); let orelse = s2.map(|s| s.2).unwrap_or_default(); ast::Stmt { custom: (), @@ -412,11 +407,11 @@ WhileStatement: ast::Stmt = { ForStatement: ast::Stmt = { "for" "in" ":" => { - let end_location = if let Some(ref s) = s2 { - s.2.last().unwrap().end_location.unwrap() - } else { - body.last().unwrap().end_location.unwrap() - }; + let end_location = s2 + .as_ref() + .map(|s| s.2.last().unwrap().end_location) + .unwrap_or_else(|| body.last().unwrap().end_location) + .unwrap(); let orelse = s2.map(|s| s.2).unwrap_or_default(); let target = Box::new(set_context(target, ast::ExprContext::Store)); let iter = Box::new(iter); @@ -434,15 +429,11 @@ TryStatement: ast::Stmt = { "try" ":" => { let orelse = else_suite.map(|s| s.2).unwrap_or_default(); let finalbody = finally.map(|s| s.2).unwrap_or_default(); - let end_location = if let Some(last) = finalbody.last() { - last.end_location - } else { - if let Some(last) = orelse.last() { - last.end_location - } else { - handlers.last().unwrap().end_location - } - }; + let end_location = finalbody + .last() + .map(|last| last.end_location) + .or_else(|| orelse.last().map(|last| last.end_location)) + .unwrap_or_else(|| handlers.last().unwrap().end_location); ast::Stmt { custom: (), location, From 3d8a2458705aee79cfbcd720a9f2a6065f766989 Mon Sep 17 00:00:00 2001 From: harupy Date: Mon, 12 Dec 2022 22:14:05 +0900 Subject: [PATCH 4/7] Address comments Signed-off-by: harupy --- parser/python.lalrpop | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/parser/python.lalrpop b/parser/python.lalrpop index ff33fe4..3b78efc 100644 --- a/parser/python.lalrpop +++ b/parser/python.lalrpop @@ -361,10 +361,11 @@ IfStatement: ast::Stmt = { // Determine last else: let mut last = s3.map(|s| s.2).unwrap_or_default(); let end_location = last - .last() - .map(|last| last.end_location) - .or_else(|| s2.last().map(|last| last.4.last().unwrap().end_location)) - .unwrap_or_else(|| body.last().unwrap().end_location); + .last() + .or_else(|| s2.last().and_then(|last| last.4.last())) + .or_else(|| body.last()) + .unwrap() + .end_location; // handle elif: for i in s2.into_iter().rev() { let x = ast::Stmt { @@ -387,11 +388,12 @@ IfStatement: ast::Stmt = { WhileStatement: ast::Stmt = { "while" ":" => { - let end_location = s2 - .as_ref() - .map(|s| s.2.last().unwrap().end_location) - .unwrap_or_else(|| body.last().unwrap().end_location); let orelse = s2.map(|s| s.2).unwrap_or_default(); + let end_location = orelse + .last() + .or_else(|| body.last()) + .unwrap() + .end_location; ast::Stmt { custom: (), location, @@ -407,12 +409,13 @@ WhileStatement: ast::Stmt = { ForStatement: ast::Stmt = { "for" "in" ":" => { - let end_location = s2 - .as_ref() - .map(|s| s.2.last().unwrap().end_location) - .unwrap_or_else(|| body.last().unwrap().end_location) - .unwrap(); let orelse = s2.map(|s| s.2).unwrap_or_default(); + let end_location = orelse + .last() + .or_else(|| body.last()) + .unwrap() + .end_location + .unwrap(); let target = Box::new(set_context(target, ast::ExprContext::Store)); let iter = Box::new(iter); let type_comment = None; From 16f2e826e438c01d4ed835bcc0a7e735e3448644 Mon Sep 17 00:00:00 2001 From: harupy Date: Mon, 12 Dec 2022 22:18:26 +0900 Subject: [PATCH 5/7] Format --- parser/python.lalrpop | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/parser/python.lalrpop b/parser/python.lalrpop index 32bf834..3c980b0 100644 --- a/parser/python.lalrpop +++ b/parser/python.lalrpop @@ -364,11 +364,11 @@ IfStatement: ast::Stmt = { // Determine last else: let mut last = s3.map(|s| s.2).unwrap_or_default(); let end_location = last - .last() - .or_else(|| s2.last().and_then(|last| last.4.last())) - .or_else(|| body.last()) - .unwrap() - .end_location; + .last() + .or_else(|| s2.last().and_then(|last| last.4.last())) + .or_else(|| body.last()) + .unwrap() + .end_location; // handle elif: for i in s2.into_iter().rev() { let x = ast::Stmt { From f5ce9517fd09595966b776e5c682ea30a40363fc Mon Sep 17 00:00:00 2001 From: harupy Date: Mon, 12 Dec 2022 22:26:03 +0900 Subject: [PATCH 6/7] Update snapshot --- ...n_parser__with__tests__with_statement.snap | 88 +++++++++---------- 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/parser/src/snapshots/rustpython_parser__with__tests__with_statement.snap b/parser/src/snapshots/rustpython_parser__with__tests__with_statement.snap index e82ee2d..a36af82 100644 --- a/parser/src/snapshots/rustpython_parser__with__tests__with_statement.snap +++ b/parser/src/snapshots/rustpython_parser__with__tests__with_statement.snap @@ -10,8 +10,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 2, - column: 0, + row: 1, + column: 12, }, ), custom: (), @@ -66,8 +66,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 3, - column: 0, + row: 2, + column: 17, }, ), custom: (), @@ -140,8 +140,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 4, - column: 0, + row: 3, + column: 15, }, ), custom: (), @@ -218,8 +218,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 5, - column: 0, + row: 4, + column: 25, }, ), custom: (), @@ -332,8 +332,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 6, - column: 0, + row: 5, + column: 24, }, ), custom: (), @@ -441,8 +441,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 7, - column: 0, + row: 6, + column: 29, }, ), custom: (), @@ -568,8 +568,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 8, - column: 0, + row: 7, + column: 13, }, ), custom: (), @@ -622,8 +622,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 9, - column: 0, + row: 8, + column: 18, }, ), custom: (), @@ -694,8 +694,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 10, - column: 0, + row: 9, + column: 14, }, ), custom: (), @@ -750,8 +750,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 11, - column: 0, + row: 10, + column: 19, }, ), custom: (), @@ -824,8 +824,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 12, - column: 0, + row: 11, + column: 15, }, ), custom: (), @@ -880,8 +880,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 13, - column: 0, + row: 12, + column: 20, }, ), custom: (), @@ -972,8 +972,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 14, - column: 0, + row: 13, + column: 17, }, ), custom: (), @@ -1050,8 +1050,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 15, - column: 0, + row: 14, + column: 22, }, ), custom: (), @@ -1161,8 +1161,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 16, - column: 0, + row: 15, + column: 16, }, ), custom: (), @@ -1249,8 +1249,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 17, - column: 0, + row: 16, + column: 21, }, ), custom: (), @@ -1355,8 +1355,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 18, - column: 0, + row: 17, + column: 18, }, ), custom: (), @@ -1462,8 +1462,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 19, - column: 0, + row: 18, + column: 23, }, ), custom: (), @@ -1587,8 +1587,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 20, - column: 0, + row: 19, + column: 19, }, ), custom: (), @@ -1675,8 +1675,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 21, - column: 0, + row: 20, + column: 24, }, ), custom: (), @@ -1781,8 +1781,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 22, - column: 0, + row: 21, + column: 27, }, ), custom: (), @@ -1923,8 +1923,8 @@ expression: "parse_program(source, \"\").unwrap()" }, end_location: Some( Location { - row: 23, - column: 0, + row: 22, + column: 32, }, ), custom: (), From d13b27dcf05606cf96a96d7f05825a84716ee8b8 Mon Sep 17 00:00:00 2001 From: harupy Date: Mon, 12 Dec 2022 22:36:34 +0900 Subject: [PATCH 7/7] Refactor --- parser/python.lalrpop | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/parser/python.lalrpop b/parser/python.lalrpop index 3c980b0..cca2a46 100644 --- a/parser/python.lalrpop +++ b/parser/python.lalrpop @@ -439,7 +439,8 @@ TryStatement: ast::Stmt = { .last() .map(|last| last.end_location) .or_else(|| orelse.last().map(|last| last.end_location)) - .unwrap_or_else(|| handlers.last().unwrap().end_location); + .or_else(|| handlers.last().map(|last| last.end_location)) + .unwrap(); ast::Stmt { custom: (), location,