From af52def93cc8d36bb48ea02cdad09ae783325132 Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Sat, 22 Jan 2022 13:24:47 -0500 Subject: [PATCH] Fix doc comments for custom commands (#815) --- crates/nu-parser/src/lite_parse.rs | 40 ++++++++++-- crates/nu-parser/src/parse_keywords.rs | 74 ++++++++++++++++++++-- crates/nu-parser/src/parser.rs | 38 ++++++----- crates/nu-parser/tests/test_lite_parser.rs | 7 +- src/tests.rs | 25 ++++++++ src/tests/test_parser.rs | 15 +++++ 6 files changed, 163 insertions(+), 36 deletions(-) diff --git a/crates/nu-parser/src/lite_parse.rs b/crates/nu-parser/src/lite_parse.rs index 0a28e9fc87..922ddd84de 100644 --- a/crates/nu-parser/src/lite_parse.rs +++ b/crates/nu-parser/src/lite_parse.rs @@ -85,23 +85,29 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { let mut curr_pipeline = LiteStatement::new(); let mut curr_command = LiteCommand::new(); - let mut last_token_was_pipe = false; + let mut last_token = TokenContents::Eol; + + let mut curr_comment: Option> = None; for token in tokens.iter() { match &token.contents { TokenContents::Item => { + // If we have a comment, go ahead and attach it + if let Some(curr_comment) = curr_comment.take() { + curr_command.comments = curr_comment; + } curr_command.push(token.span); - last_token_was_pipe = false; + last_token = TokenContents::Item; } TokenContents::Pipe => { if !curr_command.is_empty() { curr_pipeline.push(curr_command); curr_command = LiteCommand::new(); } - last_token_was_pipe = true; + last_token = TokenContents::Pipe; } TokenContents::Eol => { - if !last_token_was_pipe { + if last_token != TokenContents::Pipe { if !curr_command.is_empty() { curr_pipeline.push(curr_command); @@ -114,6 +120,13 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { curr_pipeline = LiteStatement::new(); } } + + if last_token == TokenContents::Eol { + // Clear out the comment as we're entering a new comment + curr_comment = None; + } + + last_token = TokenContents::Eol; } TokenContents::Semicolon => { if !curr_command.is_empty() { @@ -128,10 +141,23 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { curr_pipeline = LiteStatement::new(); } - last_token_was_pipe = false; + last_token = TokenContents::Semicolon; } TokenContents::Comment => { - curr_command.comments.push(token.span); + // Comment is beside something + if last_token != TokenContents::Eol { + curr_command.comments.push(token.span); + curr_comment = None; + } else { + // Comment precedes something + if let Some(curr_comment) = &mut curr_comment { + curr_comment.push(token.span); + } else { + curr_comment = Some(vec![token.span]); + } + } + + last_token = TokenContents::Comment; } } } @@ -144,7 +170,7 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option) { block.push(curr_pipeline); } - if last_token_was_pipe { + if last_token == TokenContents::Pipe { ( block, Some(ParseError::UnexpectedEof( diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 49f3f3c8f2..d4c3eff887 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -11,6 +11,7 @@ use std::collections::HashSet; use crate::{ lex, lite_parse, + lite_parse::LiteCommand, parser::{ check_call, check_name, find_captures_in_block, garbage, garbage_statement, parse, parse_block_expression, parse_internal_call, parse_multispan_value, parse_signature, @@ -173,10 +174,65 @@ pub fn parse_for( ) } +fn build_usage(working_set: &StateWorkingSet, spans: &[Span]) -> String { + let mut usage = String::new(); + + let mut num_spaces = 0; + let mut first = true; + + // Use the comments to build the usage + for comment_part in spans { + let contents = working_set.get_span_contents(*comment_part); + + let comment_line = if first { + // Count the number of spaces still at the front, skipping the '#' + let mut pos = 1; + while pos < contents.len() { + if let Some(b' ') = contents.get(pos) { + // continue + } else { + break; + } + pos += 1; + } + + num_spaces = pos; + + first = false; + + String::from_utf8_lossy(&contents[pos..]).to_string() + } else { + let mut pos = 1; + + while pos < contents.len() && pos < num_spaces { + if let Some(b' ') = contents.get(pos) { + // continue + } else { + break; + } + pos += 1; + } + + String::from_utf8_lossy(&contents[pos..]).to_string() + }; + + if !usage.is_empty() { + usage.push('\n'); + } + usage.push_str(&comment_line); + } + + usage +} + pub fn parse_def( working_set: &mut StateWorkingSet, - spans: &[Span], + lite_command: &LiteCommand, ) -> (Statement, Option) { + let spans = &lite_command.parts[..]; + + let usage = build_usage(working_set, &lite_command.comments); + // Checking that the function is used with the correct name // Maybe this is not necessary but it is a sanity check if working_set.get_span_contents(spans[0]) != b"def" { @@ -260,6 +316,7 @@ pub fn parse_def( let declaration = working_set.get_decl_mut(decl_id); signature.name = name.clone(); + signature.usage = usage; *declaration = signature.clone().into_block_command(block_id); @@ -368,8 +425,9 @@ pub fn parse_alias( pub fn parse_export( working_set: &mut StateWorkingSet, - spans: &[Span], + lite_command: &LiteCommand, ) -> (Statement, Option, Option) { + let spans = &lite_command.parts[..]; let mut error = None; let export_span = if let Some(sp) = spans.get(0) { @@ -420,7 +478,11 @@ pub fn parse_export( let kw_name = working_set.get_span_contents(*kw_span); match kw_name { b"def" => { - let (stmt, err) = parse_def(working_set, &spans[1..]); + let lite_command = LiteCommand { + comments: lite_command.comments.clone(), + parts: spans[1..].to_vec(), + }; + let (stmt, err) = parse_def(working_set, &lite_command); error = error.or(err); let export_def_decl_id = if let Some(id) = working_set.find_decl(b"export def") { @@ -615,7 +677,7 @@ pub fn parse_module_block( let source = working_set.get_span_contents(span); - let (output, err) = lex(source, span.start, &[], &[], true); + let (output, err) = lex(source, span.start, &[], &[], false); error = error.or(err); let (output, err) = lite_parse(&output); @@ -639,7 +701,7 @@ pub fn parse_module_block( let (stmt, err) = match name { b"def" => { - let (stmt, err) = parse_def(working_set, &pipeline.commands[0].parts); + let (stmt, err) = parse_def(working_set, &pipeline.commands[0]); (stmt, err) } @@ -653,7 +715,7 @@ pub fn parse_module_block( // since in the second case, the name of the env var would be $env."foo a". b"export" => { let (stmt, exportable, err) = - parse_export(working_set, &pipeline.commands[0].parts); + parse_export(working_set, &pipeline.commands[0]); if err.is_none() { let name_span = pipeline.commands[0].parts[2]; diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 5251bf2780..8dc452e060 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -1,5 +1,6 @@ use crate::{ lex, lite_parse, + lite_parse::LiteCommand, parse_keywords::{parse_for, parse_source}, type_check::{math_result_type, type_compatible}, LiteBlock, ParseError, Token, TokenContents, @@ -2823,7 +2824,7 @@ pub fn parse_block_expression( let source = working_set.get_span_contents(inner_span); - let (output, err) = lex(source, start, &[], &[], true); + let (output, err) = lex(source, start, &[], &[], false); error = error.or(err); working_set.enter_scope(); @@ -3474,30 +3475,33 @@ pub fn parse_variable( pub fn parse_statement( working_set: &mut StateWorkingSet, - spans: &[Span], + lite_command: &LiteCommand, ) -> (Statement, Option) { - let name = working_set.get_span_contents(spans[0]); + let name = working_set.get_span_contents(lite_command.parts[0]); match name { - b"def" => parse_def(working_set, spans), - b"let" => parse_let(working_set, spans), + b"def" => parse_def(working_set, lite_command), + b"let" => parse_let(working_set, &lite_command.parts), b"for" => { - let (expr, err) = parse_for(working_set, spans); + let (expr, err) = parse_for(working_set, &lite_command.parts); (Statement::Pipeline(Pipeline::from_vec(vec![expr])), err) } - b"alias" => parse_alias(working_set, spans), - b"module" => parse_module(working_set, spans), - b"use" => parse_use(working_set, spans), - b"source" => parse_source(working_set, spans), + b"alias" => parse_alias(working_set, &lite_command.parts), + b"module" => parse_module(working_set, &lite_command.parts), + b"use" => parse_use(working_set, &lite_command.parts), + b"source" => parse_source(working_set, &lite_command.parts), b"export" => ( - garbage_statement(spans), - Some(ParseError::UnexpectedKeyword("export".into(), spans[0])), + garbage_statement(&lite_command.parts), + Some(ParseError::UnexpectedKeyword( + "export".into(), + lite_command.parts[0], + )), ), - b"hide" => parse_hide(working_set, spans), + b"hide" => parse_hide(working_set, &lite_command.parts), #[cfg(feature = "plugin")] - b"register" => parse_register(working_set, spans), + b"register" => parse_register(working_set, &lite_command.parts), _ => { - let (expr, err) = parse_expression(working_set, spans, true); + let (expr, err) = parse_expression(working_set, &lite_command.parts, true); (Statement::Pipeline(Pipeline::from_vec(vec![expr])), err) } } @@ -3632,7 +3636,7 @@ pub fn parse_block( expressions: output, }) } else { - let (stmt, err) = parse_statement(working_set, &pipeline.commands[0].parts); + let (stmt, err) = parse_statement(working_set, &pipeline.commands[0]); if error.is_none() { error = err; @@ -3964,7 +3968,7 @@ pub fn parse( working_set.add_file(name, contents); - let (output, err) = lex(contents, span_offset, &[], &[], true); + let (output, err) = lex(contents, span_offset, &[], &[], false); error = error.or(err); let (output, err) = lite_parse(&output); diff --git a/crates/nu-parser/tests/test_lite_parser.rs b/crates/nu-parser/tests/test_lite_parser.rs index bb6d92e84b..f9c638e892 100644 --- a/crates/nu-parser/tests/test_lite_parser.rs +++ b/crates/nu-parser/tests/test_lite_parser.rs @@ -97,16 +97,11 @@ fn separated_comments_dont_stack() -> Result<(), ParseError> { let lite_block = lite_parse_helper(input)?; assert_eq!(lite_block.block.len(), 1); - assert_eq!(lite_block.block[0].commands[0].comments.len(), 2); + assert_eq!(lite_block.block[0].commands[0].comments.len(), 1); assert_eq!(lite_block.block[0].commands[0].parts.len(), 3); assert_eq!( lite_block.block[0].commands[0].comments[0], - Span { start: 0, end: 19 } - ); - - assert_eq!( - lite_block.block[0].commands[0].comments[1], Span { start: 21, end: 38 } ); diff --git a/src/tests.rs b/src/tests.rs index a34d6a477c..fa930cdf35 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -46,6 +46,31 @@ pub fn run_test(input: &str, expected: &str) -> TestResult { Ok(()) } +#[cfg(test)] +pub fn run_test_contains(input: &str, expected: &str) -> TestResult { + let mut file = NamedTempFile::new()?; + let name = file.path(); + + let mut cmd = Command::cargo_bin("engine-q")?; + cmd.arg(name); + + writeln!(file, "{}", input)?; + + let output = cmd.output()?; + + let stderr = String::from_utf8_lossy(&output.stderr).to_string(); + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + + println!("stdout: {}", stdout); + println!("stderr: {}", stderr); + + assert!(output.status.success()); + + assert!(stdout.contains(expected)); + + Ok(()) +} + #[cfg(test)] pub fn fail_test(input: &str, expected: &str) -> TestResult { let mut file = NamedTempFile::new()?; diff --git a/src/tests/test_parser.rs b/src/tests/test_parser.rs index b5e2ad565d..d3e973a70b 100644 --- a/src/tests/test_parser.rs +++ b/src/tests/test_parser.rs @@ -1,5 +1,7 @@ use crate::tests::{fail_test, run_test, TestResult}; +use super::run_test_contains; + #[test] fn env_shorthand() -> TestResult { run_test("FOO=BAR if $false { 3 } else { 4 }", "4") @@ -169,3 +171,16 @@ fn string_interp_with_equals() -> TestResult { fn recursive_parse() -> TestResult { run_test(r#"def c [] { c }; echo done"#, "done") } + +#[test] +fn commands_have_usage() -> TestResult { + run_test_contains( + r#" + # This is a test + # + # To see if I have cool usage + def foo [] {} + help foo"#, + "cool usage", + ) +}