Improve the inline/columnar combination (#80)

* feat: Improve the inline/columnar combination

Fix again the span computation, make so the compact representation does
not behave weirdly when it needs to go columnar in subqueries.

* feat: Add an option to consider JOINs top level or plain reserved newline

* fix: Make sure UNION ALL is correctly parsed

* fix: SELECT DISTINCT should have priority over SELECT

Author: lu-zero

src/formatter.rs

@@ -4,7 +4,7 @@ use crate::indentation::Indentation;
 use crate::inline_block::InlineBlock;
 use crate::params::Params;
 use crate::tokenizer::{Token, TokenKind};
-use crate::{FormatOptions, QueryParams};
+use crate::{FormatOptions, QueryParams, SpanInfo};
 
 // -- fmt: off
 // -- fmt: on
@@ -101,10 +101,6 @@ pub(crate) fn format(
                 formatter.format_newline_reserved_word(token, &mut formatted_query);
                 formatter.previous_reserved_word = Some(token);
             }
-            TokenKind::Join => {
-                formatter.format_newline_reserved_word(token, &mut formatted_query);
-                formatter.previous_reserved_word = Some(token);
-            }
             TokenKind::Reserved => {
                 formatter.format_with_spaces(token, &mut formatted_query);
                 formatter.previous_reserved_word = Some(token);
@@ -167,14 +163,8 @@ impl<'a> Formatter<'a> {
             indentation: Indentation::new(options),
             inline_block: InlineBlock::new(
                 options.max_inline_block,
-                match (options.max_inline_arguments, options.max_inline_top_level) {
-                    (Some(max_inline_args), Some(max_inline_top)) => {
-                        max_inline_args.min(max_inline_top)
-                    }
-                    (Some(max_inline_args), None) => max_inline_args,
-                    (None, Some(max_inline_top)) => max_inline_top,
-                    (None, None) => 0,
-                },
+                options.max_inline_arguments.unwrap_or(0),
+                options.max_inline_top_level.unwrap_or(0),
             ),
             block_level: 0,
         }
@@ -215,12 +205,12 @@ impl<'a> Formatter<'a> {
         self.add_new_line(query);
     }
 
-    // if we are inside an inline block we decide our behaviour as if we are an inline argument
-    fn top_level_behavior(&self) -> (bool, bool) {
-        let span_len = self.top_level_tokens_span();
+    // if we are inside an inline block we decide our behaviour as if were inline
+    fn top_level_behavior(&self, span_info: &SpanInfo) -> (bool, bool) {
+        let span_len = span_info.full_span;
         let block_len = self.inline_block.cur_len();
         if block_len > 0 {
-            let limit = self.options.max_inline_arguments.unwrap_or(0);
+            let limit = self.options.max_inline_top_level.unwrap_or(0);
             (limit < block_len, limit < span_len)
         } else {
             (
@@ -233,24 +223,25 @@ impl<'a> Formatter<'a> {
     }
 
     fn format_top_level_reserved_word(&mut self, token: &Token<'_>, query: &mut String) {
-        let span_len = self.top_level_tokens_span();
-        let (newline_before, newline_after) = self.top_level_behavior();
+        let span_info = self.top_level_tokens_info();
+        let (newline_before, newline_after) = self.top_level_behavior(&span_info);
 
         if newline_before {
             self.indentation.decrease_top_level();
             self.add_new_line(query);
         }
         query.push_str(&self.equalize_whitespace(&self.format_reserved_word(token.value)));
         if newline_after {
-            self.indentation.increase_top_level(span_len);
+            self.indentation.increase_top_level(span_info);
             self.add_new_line(query);
         } else {
             query.push(' ');
         }
     }
 
     fn format_top_level_reserved_word_no_indent(&mut self, token: &Token<'_>, query: &mut String) {
-        let (newline_before, newline_after) = self.top_level_behavior();
+        let span_info = self.top_level_tokens_info();
+        let (newline_before, newline_after) = self.top_level_behavior(&span_info);
 
         if newline_before {
             self.indentation.decrease_top_level();
@@ -263,10 +254,11 @@ impl<'a> Formatter<'a> {
     }
 
     fn format_newline_reserved_word(&mut self, token: &Token<'_>, query: &mut String) {
-        if self
-            .options
-            .max_inline_arguments
-            .map_or(true, |limit| limit < self.indentation.top_level_span())
+        if !self.inline_block.is_active()
+            && self
+                .options
+                .max_inline_arguments
+                .map_or(true, |limit| limit < self.indentation.span())
         {
             self.add_new_line(query);
         } else {
@@ -406,7 +398,7 @@ impl<'a> Formatter<'a> {
 
         if matches!((self.previous_top_level_reserved_word, self.options.max_inline_arguments),
             (Some(word), Some(limit)) if ["select", "from"].contains(&word.value.to_lowercase().as_str()) &&
-                limit > self.indentation.top_level_span())
+                limit > self.indentation.span())
         {
             return;
         }
@@ -528,28 +520,33 @@ impl<'a> Formatter<'a> {
         }
     }
 
-    fn top_level_tokens_span(&self) -> usize {
+    fn top_level_tokens_info(&self) -> SpanInfo {
         let mut block_level = self.block_level;
+        let mut full_span = 0;
 
-        self.tokens[self.index..]
-            .iter()
-            .skip(1)
-            .take_while(|token| match token.kind {
+        for token in self.tokens[self.index..].iter().skip(1) {
+            match token.kind {
                 TokenKind::OpenParen => {
                     block_level += 1;
-                    true
                 }
                 TokenKind::CloseParen => {
                     block_level = block_level.saturating_sub(1);
-                    block_level > self.block_level
+                    if block_level < self.block_level {
+                        break;
+                    }
                 }
                 TokenKind::ReservedTopLevel | TokenKind::ReservedTopLevelNoIndent => {
-                    block_level != self.block_level
+                    if block_level == self.block_level {
+                        break;
+                    }
                 }
-                _ => true,
-            })
-            .map(|token| token.value.len())
-            .sum()
+                _ => {}
+            }
+
+            full_span += token.value.len();
+        }
+
+        SpanInfo { full_span }
     }
 
     fn format_no_change(&self, token: &Token<'_>, query: &mut String) {

src/indentation.rs

@@ -1,9 +1,9 @@
-use crate::{FormatOptions, Indent};
+use crate::{FormatOptions, Indent, SpanInfo};
 
 pub(crate) struct Indentation<'a> {
     options: &'a FormatOptions<'a>,
     indent_types: Vec<IndentType>,
-    top_level_span: Vec<usize>,
+    top_level_span: Vec<SpanInfo>,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -30,7 +30,7 @@ impl<'a> Indentation<'a> {
         }
     }
 
-    pub fn increase_top_level(&mut self, span: usize) {
+    pub fn increase_top_level(&mut self, span: SpanInfo) {
         self.indent_types.push(IndentType::TopLevel);
         self.top_level_span.push(span);
     }
@@ -60,7 +60,8 @@ impl<'a> Indentation<'a> {
         self.top_level_span.clear();
     }
 
-    pub fn top_level_span(&self) -> usize {
-        self.top_level_span.last().map_or(0, |span| *span)
+    /// The full span between two top level tokens
+    pub fn span(&self) -> usize {
+        self.top_level_span.last().map_or(0, |span| span.full_span)
     }
 }

src/inline_block.rs

@@ -3,13 +3,15 @@ use crate::tokenizer::{Token, TokenKind};
 pub(crate) struct BlockInfo {
     length: usize,
     has_forbidden_tokens: bool,
+    has_reseved_tokens: bool,
     top_level_token_span: usize,
 }
 
 pub(crate) struct InlineBlock {
     level: usize,
     inline_max_length: usize,
-    newline_on_reserved_limit: usize,
+    reserved_limit: usize,
+    reserved_top_limit: usize,
     info: Vec<BlockInfo>,
 }
 
@@ -19,24 +21,27 @@ impl Default for InlineBlock {
             info: Vec::new(),
             level: 0,
             inline_max_length: 50,
-            newline_on_reserved_limit: 0,
+            reserved_limit: 0,
+            reserved_top_limit: 0,
         }
     }
 }
 
 impl InlineBlock {
-    pub fn new(inline_max_length: usize, newline_on_reserved_limit: usize) -> Self {
+    pub fn new(inline_max_length: usize, reserved_limit: usize, reserved_top_limit: usize) -> Self {
         InlineBlock {
             inline_max_length,
-            newline_on_reserved_limit,
+            reserved_limit,
+            reserved_top_limit,
             ..Default::default()
         }
     }
 
     fn is_inline_block(&self, info: &BlockInfo) -> bool {
         !info.has_forbidden_tokens
             && info.length <= self.inline_max_length
-            && info.top_level_token_span <= self.newline_on_reserved_limit
+            && info.top_level_token_span <= self.reserved_top_limit
+            && (!info.has_reseved_tokens || info.length <= self.reserved_limit)
     }
 
     pub fn begin_if_possible(&mut self, tokens: &[Token<'_>], index: usize) {
@@ -74,10 +79,10 @@ impl InlineBlock {
         let mut start_top_level = -1;
         let mut start_span = 0;
         let mut has_forbidden_tokens = false;
+        let mut has_reseved_tokens = false;
 
         for token in &tokens[index..] {
             length += token.value.len();
-
             match token.kind {
                 TokenKind::ReservedTopLevel | TokenKind::ReservedTopLevelNoIndent => {
                     if start_top_level != -1 {
@@ -90,6 +95,9 @@ impl InlineBlock {
                         start_span = length;
                     }
                 }
+                TokenKind::ReservedNewline => {
+                    has_reseved_tokens = true;
+                }
                 TokenKind::OpenParen => {
                     level += 1;
                 }
@@ -111,6 +119,7 @@ impl InlineBlock {
         BlockInfo {
             length,
             has_forbidden_tokens,
+            has_reseved_tokens,
             top_level_token_span,
         }
     }
@@ -119,11 +128,5 @@ impl InlineBlock {
         token.kind == TokenKind::LineComment
             || token.kind == TokenKind::BlockComment
             || token.value == ";"
-            || if self.newline_on_reserved_limit == 0 {
-                token.kind == TokenKind::ReservedTopLevel
-                    || token.kind == TokenKind::ReservedNewline
-            } else {
-                false
-            }
     }
 }