From 5fd075d26743cbe6a23edb9ff62a51fbb0be004b Mon Sep 17 00:00:00 2001 From: Alex Hansen Date: Wed, 3 Jul 2024 10:14:24 -0700 Subject: [PATCH] support imports and exports in the formatter (#1692) This is one of the changes required for the imports/exports feature work, and this is particularly important for the standard library, which will make extensive use of long export lists and needs to be formatted well. This PR adds a new context, similar to the existing open/close delim contexts, that is defined by `import` or `export`...`;`. If there is a newline after the `import` or `export` keyword, then we format with newlines. Otherwise, puts it all on one line. See the tests for examples. before: ```qsharp export Foo, Bar, Baz; ``` after: ```qsharp export Foo, Bar, Baz; ``` OR, in single-line mode, ```qsharp export Foo, Bar, Baz; ``` --- compiler/qsc_formatter/src/formatter.rs | 89 ++++++++-- compiler/qsc_formatter/src/formatter/tests.rs | 159 ++++++++++++++++++ 2 files changed, 232 insertions(+), 16 deletions(-) diff --git a/compiler/qsc_formatter/src/formatter.rs b/compiler/qsc_formatter/src/formatter.rs index b54d2989a1..256bb2a13d 100644 --- a/compiler/qsc_formatter/src/formatter.rs +++ b/compiler/qsc_formatter/src/formatter.rs @@ -44,6 +44,7 @@ pub fn calculate_format_edits(code: &str) -> Vec { delim_newlines_stack: vec![], type_param_state: TypeParameterListState::NoState, spec_decl_state: SpecDeclState::NoState, + import_export_state: ImportExportState::NoState, }; // The sliding window used is over three adjacent tokens @@ -144,6 +145,15 @@ enum TypeParameterListState { InTypeParamList, } +/// Whether or not we are currently handling an import or export statement. +#[derive(Clone, Copy, Debug)] +enum ImportExportState { + /// Yes, this is an import or export statement. + HandlingImportExportStatement, + /// No, this is not an import or export statement. + NoState, +} + /// This is to keep track of whether or not the formatter /// is currently processing a functor specialization /// declaration. @@ -174,9 +184,10 @@ enum Delimiter { impl Delimiter { /// Constructs a Delimiter from a token, given the current type-parameter state. - fn from_concrete_toke_kind( + fn from_concrete_token_kind( kind: &ConcreteTokenKind, type_param_state: TypeParameterListState, + import_export_state: ImportExportState, ) -> Delimiter { match kind { ConcreteTokenKind::Syntax(TokenKind::Open(_)) => Delimiter::Open, @@ -191,6 +202,17 @@ impl Delimiter { { Delimiter::Close } + ConcreteTokenKind::Syntax(TokenKind::Keyword(Keyword::Import | Keyword::Export)) => { + Delimiter::Open + } + ConcreteTokenKind::Syntax(TokenKind::Semi) + if matches!( + import_export_state, + ImportExportState::HandlingImportExportStatement + ) => + { + Delimiter::Close + } _ => Delimiter::NonDelim, } } @@ -202,6 +224,7 @@ struct Formatter<'a> { delim_newlines_stack: Vec, type_param_state: TypeParameterListState, spec_decl_state: SpecDeclState, + import_export_state: ImportExportState, } impl<'a> Formatter<'a> { @@ -222,10 +245,23 @@ impl<'a> Formatter<'a> { let are_newlines_in_spaces = whitespace.contains('\n'); let does_right_required_newline = matches!(&right.kind, Syntax(cooked_right) if is_newline_keyword_or_ampersat(cooked_right)); + // Save the left token's status as a delimiter before updating the delimiter state + let left_delim_state = Delimiter::from_concrete_token_kind( + &left.kind, + self.type_param_state, + self.import_export_state, + ); + self.update_spec_decl_state(&left.kind); + self.update_type_param_state(&left.kind, &right.kind); + self.update_import_export_state(&left.kind); - let (left_delim_state, right_delim_state) = - self.update_type_param_state(&left.kind, &right.kind); + // Save the right token's status as a delimiter after updating the delimiter state + let right_delim_state = Delimiter::from_concrete_token_kind( + &right.kind, + self.type_param_state, + self.import_export_state, + ); let newline_context = self.update_indent_level( left_delim_state, @@ -263,6 +299,16 @@ impl<'a> Formatter<'a> { // to be able to differentiate between the unary `-` and the binary `-` // which would have different spacing rules. } + (_, ClosedBinOp(ClosedBinOp::Star)) + if matches!( + self.import_export_state, + ImportExportState::HandlingImportExportStatement + ) => + { + // if this is a star and we are in an import/export, then it isn't actually a + // binop and it's a glob import + effect_no_space(left, whitespace, right, &mut edits); + } (Semi, _) if matches!(newline_context, NewlineContext::Spaces) => { effect_single_space(left, whitespace, right, &mut edits); } @@ -490,21 +536,36 @@ impl<'a> Formatter<'a> { } } + fn update_import_export_state(&mut self, left_kind: &ConcreteTokenKind) { + use qsc_frontend::keyword::Keyword; + use ConcreteTokenKind::*; + use TokenKind::*; + + match left_kind { + Comment => { + // Comments don't update state + } + Syntax(Keyword(Keyword::Import | Keyword::Export)) => { + self.import_export_state = ImportExportState::HandlingImportExportStatement; + } + Syntax(Semi) => { + self.import_export_state = ImportExportState::NoState; + } + _ => (), + } + } + /// Updates the type_param_state of the FormatterState based - /// on the left and right token kinds. Returns the delimiter - /// state of the left and right tokens. + /// on the left and right token kinds. fn update_type_param_state( &mut self, left_kind: &ConcreteTokenKind, right_kind: &ConcreteTokenKind, - ) -> (Delimiter, Delimiter) { + ) { use qsc_frontend::keyword::Keyword; use ConcreteTokenKind::*; use TokenKind::*; - // Save the left token's status as a delimiter before updating the delimiter state - let left_delim_state = Delimiter::from_concrete_toke_kind(left_kind, self.type_param_state); - // If we are leaving a type param list, reset the state if matches!(left_kind, Syntax(Gt)) && matches!( @@ -512,7 +573,7 @@ impl<'a> Formatter<'a> { TypeParameterListState::InTypeParamList ) { - self.type_param_state = TypeParameterListState::NoState + self.type_param_state = TypeParameterListState::NoState; } match right_kind { @@ -551,12 +612,6 @@ impl<'a> Formatter<'a> { self.type_param_state = TypeParameterListState::NoState; } } - - // Save the right token's status as a delimiter after updating the delimiter state - let right_delim_state = - Delimiter::from_concrete_toke_kind(right_kind, self.type_param_state); - - (left_delim_state, right_delim_state) } /// Updates the indent level and manages the `delim_newlines_stack` @@ -709,6 +764,8 @@ fn is_newline_keyword_or_ampersat(cooked: &TokenKind) -> bool { | Use | Borrow | Fixup + | Import + | Export ) ) } diff --git a/compiler/qsc_formatter/src/formatter/tests.rs b/compiler/qsc_formatter/src/formatter/tests.rs index 236456c939..94b99d828f 100644 --- a/compiler/qsc_formatter/src/formatter/tests.rs +++ b/compiler/qsc_formatter/src/formatter/tests.rs @@ -1223,3 +1223,162 @@ fn sample_has_no_formatting_changes() { "#}; assert!(super::calculate_format_edits(input).is_empty()); } + +#[test] +fn format_export_statement_no_newlines() { + let input = "export Microsoft.Quantum.Diagnostics, Foo.Bar.Baz;"; + + check( + input, + &expect![["export Microsoft.Quantum.Diagnostics, Foo.Bar.Baz;"]], + ); +} + +#[test] +fn format_glob_import() { + let input = "import + Microsoft.Quantum.*, + Foo.Bar.Baz as SomethingElse, + AnotherThing;"; + + check( + input, + &expect![[r#" + import + Microsoft.Quantum.*, + Foo.Bar.Baz as SomethingElse, + AnotherThing;"#]], + ); +} +#[test] +fn no_newlines_glob() { + let input = "import foo, bar, baz.quux.*;"; + + check(input, &expect!["import foo, bar, baz.quux.*;"]); +} + +#[test] +fn format_export_statement_newlines() { + let input = "export + Microsoft.Quantum.Diagnostics, + Foo.Bar.Baz;"; + + check( + input, + &expect![[r#" + export + Microsoft.Quantum.Diagnostics, + Foo.Bar.Baz;"#]], + ); +} + +#[test] +fn export_fmt_within_namespace() { + let input = r#" + +namespace Microsoft.Quantum.Arrays { + + + export + All, + Any, + Chunks, + CircularlyShifted, + ColumnAt, + Count, + Diagonal, + DrawMany, + Enumerated, + Excluding, + Filtered, + FlatMapped, + Flattened, + Fold, + ForEach, + Head, + HeadAndRest, + IndexOf, + IndexRange, + Interleaved, + IsEmpty, + IsRectangularArray, + IsSorted, + IsSquareArray, + Mapped, + MappedByIndex, + MappedOverRange, + Most, + MostAndTail, + Padded, + Partitioned, + Rest, + Reversed, + SequenceI, + SequenceL, + Sorted, + Subarray, + Swapped, + Transposed, + Tail, + Unzipped, + Where, + Windows, + Zipped; +} +"#; + + check( + input, + &expect![[r#" + namespace Microsoft.Quantum.Arrays { + + + export + All, + Any, + Chunks, + CircularlyShifted, + ColumnAt, + Count, + Diagonal, + DrawMany, + Enumerated, + Excluding, + Filtered, + FlatMapped, + Flattened, + Fold, + ForEach, + Head, + HeadAndRest, + IndexOf, + IndexRange, + Interleaved, + IsEmpty, + IsRectangularArray, + IsSorted, + IsSquareArray, + Mapped, + MappedByIndex, + MappedOverRange, + Most, + MostAndTail, + Padded, + Partitioned, + Rest, + Reversed, + SequenceI, + SequenceL, + Sorted, + Subarray, + Swapped, + Transposed, + Tail, + Unzipped, + Where, + Windows, + Zipped; + } + "#]], + ); +}