Repository: joshtemple/lkml

Issue:
Fix leading whitespace in SQL blocks
In the LookML editor, SQL statements have a leading space (you can see the highlighted blue background). However, this is probably confusing for users, as they wouldn't think to insert a leading space when they write or modify the SQL statements, leading to confusion when the LookML is generated without one.

We could manage this automatically in the `DictParser` class by checking for the presence of leading whitespace and inserting a space if none is found.
Quotes not escaped using lkml.dump(…)
When deserialising a quote (") in a string using `lkml.dump(…)` quotes are added around the whole string, but the quotes inside the string aren't escaped leading to invalid lkml.

Present workaround is to do str.replace("\"", "\\\"") before setting the value of the field.

Discussion:

Patch:
diff --git a/docs/source/advanced.rst b/docs/source/advanced.rst
index 1fd8251..4243ce7 100644
--- a/docs/source/advanced.rst
+++ b/docs/source/advanced.rst
@@ -26,7 +26,7 @@ If we parse this LookML with :py:func:`lkml.load`, we'll lose the comment and an
 
     >>> parsed = lkml.load(text)
     >>> parsed
-    {'dimensions': [{'sql': ' ${TABLE}.days_in_inventory ', 'name': 'days_in_inventory'}]} 
+    {'dimensions': [{'sql': '${TABLE}.days_in_inventory', 'name': 'days_in_inventory'}]} 
 
 Writing this dictionary back to LookML with :py:func:`lkml.dump` yields the following:
 
diff --git a/docs/source/conf.py b/docs/source/conf.py
index 168cd9b..e7e0aa7 100644
--- a/docs/source/conf.py
+++ b/docs/source/conf.py
@@ -17,9 +17,9 @@
 
 # -- Project information -----------------------------------------------------
 
-project = 'lkml'
-copyright = '2021, Josh Temple'
-author = 'Josh Temple'
+project = "lkml"
+copyright = "2021, Josh Temple"
+author = "Josh Temple"
 
 
 # -- General configuration ---------------------------------------------------
@@ -27,39 +27,37 @@ author = 'Josh Temple'
 # Add any Sphinx extension module names here, as strings. They can be
 # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
 # ones.
-extensions = ['sphinx.ext.doctest', 'sphinx.ext.napoleon', 'sphinx.ext.autodoc']
+extensions = ["sphinx.ext.doctest", "sphinx.ext.napoleon", "sphinx.ext.autodoc"]
 
 # Add any paths that contain templates here, relative to this directory.
-templates_path = ['_templates']
+templates_path = ["_templates"]
 
 # List of patterns, relative to source directory, that match files and
 # directories to ignore when looking for source files.
 # This pattern also affects html_static_path and html_extra_path.
 exclude_patterns = []
 
-master_doc = 'index'
+master_doc = "index"
 
 # -- Options for HTML output -------------------------------------------------
 
 # The theme to use for HTML and HTML Help pages.  See the documentation for
 # a list of builtin themes.
 #
-html_theme = 'alabaster'
+html_theme = "alabaster"
 html_theme_options = dict(
-    description='A speedy LookML parser and serializer implemented in pure Python.',
+    description="A speedy LookML parser and serializer implemented in pure Python.",
     github_button=True,
-    github_user='joshtemple',
-    github_repo='lkml',
-    github_type='star',
+    github_user="joshtemple",
+    github_repo="lkml",
+    github_type="star",
 )
 
 # Add any paths that contain custom static files (such as style sheets) here,
 # relative to this directory. They are copied after the builtin static files,
 # so a file named "default.css" will overwrite the builtin "default.css".
-html_static_path = ['_static']
+html_static_path = ["_static"]
 
 # These paths are either relative to html_static_path
 # or fully qualified paths (eg. https://...)
-html_css_files = [
-    'css/custom.css',
-]
\ No newline at end of file
+html_css_files = ["css/custom.css"]
diff --git a/docs/source/simple.rst b/docs/source/simple.rst
index 901a80d..9d011ae 100644
--- a/docs/source/simple.rst
+++ b/docs/source/simple.rst
@@ -26,10 +26,10 @@ Parsing a LookML string into Python is easy. Pass the LookML string into :py:fun
    >>> pprint(result)
    {'dimension_groups': [{'group_label': 'Order Date',
                           'name': 'created',
-                          'sql': ' ${TABLE}.created_at ',
+                          'sql': '${TABLE}.created_at',
                           'timeframes': ['hour', 'date', 'week', 'month', 'year'],
                           'type': 'time'}],
-    'dimensions': [{'name': 'order_id', 'sql': ' ${TABLE}.order_id '}]}
+    'dimensions': [{'name': 'order_id', 'sql': '${TABLE}.order_id'}]}
 
 :py:func:`lkml.load` also supports parsing LookML directly from files::
 
@@ -66,9 +66,9 @@ Fields that can be repeated (e.g. ``view``, ``dimension``, or ``join``) are comb
 
    >>> result = lkml.load(lookml)
    >>> pprint(result)
-   {'dimensions': [{'name': 'order_id', 'sql': ' ${TABLE}.order_id '},
-                   {'name': 'amount', 'sql': ' ${TABLE}.amount '},
-                   {'name': 'status', 'sql': ' ${TABLE}.status '}]}
+   {'dimensions': [{'name': 'order_id', 'sql': '${TABLE}.order_id'},
+                   {'name': 'amount', 'sql': '${TABLE}.amount'},
+                   {'name': 'status', 'sql': '${TABLE}.status'}]}
 
 Here's an example of some LookML that has been parsed into a dictionary. Note that the repeated key ``join`` has been transformed into a plural key ``joins``: a list of dictionaries representing each join::
 
diff --git a/lkml/parser.py b/lkml/parser.py
index 39599c8..58205ab 100644
--- a/lkml/parser.py
+++ b/lkml/parser.py
@@ -7,6 +7,7 @@ from typing import List, Optional, Sequence, Tuple, Type, Union
 
 import lkml.tokens as tokens
 import lkml.tree as tree
+import lkml.utils as utils
 
 # Delimiter character used during logging to show the depth of nesting
 DELIMITER = ". "
@@ -404,6 +405,9 @@ class Parser:
             return tree.QuotedSyntaxToken(value, prefix, suffix)
         elif self.check(tokens.ExpressionBlockToken):
             value = self.consume_token_value()
+            expr_prefix, value, expr_suffix = utils.strip(value)
+            prefix += expr_prefix
+
             if self.check(tokens.ExpressionBlockEndToken):
                 self.advance()
             else:
@@ -411,7 +415,7 @@ class Parser:
             suffix = self.consume_trivia() if parse_suffix else ""
             if self.log_debug:
                 logger.debug("%sSuccessfully parsed value.", self.depth * DELIMITER)
-            return tree.ExpressionSyntaxToken(value, prefix, suffix)
+            return tree.ExpressionSyntaxToken(value, prefix, suffix, expr_suffix)
         else:
             return None
 
diff --git a/lkml/simple.py b/lkml/simple.py
index 77de3f5..1ad2c77 100644
--- a/lkml/simple.py
+++ b/lkml/simple.py
@@ -469,6 +469,6 @@ class DictParser:
         if force_quote or key in QUOTED_LITERAL_KEYS:
             return QuotedSyntaxToken(value, prefix, suffix)
         elif key in EXPR_BLOCK_KEYS:
-            return ExpressionSyntaxToken(value.strip() + " ", prefix, suffix)
+            return ExpressionSyntaxToken(value.strip(), prefix, suffix)
         else:
             return SyntaxToken(value, prefix, suffix)
diff --git a/lkml/tree.py b/lkml/tree.py
index 6f02470..ca5cece 100644
--- a/lkml/tree.py
+++ b/lkml/tree.py
@@ -78,12 +78,19 @@ class DoubleSemicolon(SyntaxToken):
 
 class QuotedSyntaxToken(SyntaxToken):
     def format_value(self) -> str:
-        return '"' + self.value + '"'
+        # Escape double quotes since the whole value is quoted
+        return '"' + self.value.replace(r"\"", '"').replace('"', r"\"") + '"'
 
 
+@dataclass(frozen=True)
 class ExpressionSyntaxToken(SyntaxToken):
-    def format_value(self) -> str:
-        return self.value + ";;"
+    prefix: str = " "
+    expr_suffix: str = " "
+
+    def __str__(self) -> str:
+        return items_to_str(
+            self.prefix, self.format_value(), self.expr_suffix, ";;", self.suffix
+        )
 
 
 class SyntaxNode(ABC):
diff --git a/lkml/utils.py b/lkml/utils.py
new file mode 100644
index 0000000..0779a9e
--- /dev/null
+++ b/lkml/utils.py
@@ -0,0 +1,19 @@
+from typing import Tuple
+
+
+def strip(s: str) -> Tuple[str, str, str]:
+    """Modified str.strip() which returns the stripped whitespace."""
+    start = 0
+    end = -1
+    nchars = len(s)
+
+    while start < nchars and s[start].isspace():
+        start += 1
+    if start != nchars:  # Check for case where only whitespace
+        while s[end].isspace():
+            end -= 1
+
+    lspace = "" if start == 0 else s[:start]
+    rspace = "" if end == -1 else s[end + 1 :]
+
+    return lspace, s[start : end + 1 or None], rspace
diff --git a/setup.cfg b/setup.cfg
index cca9af2..1f69beb 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -21,6 +21,8 @@ exclude = venv,env,.venv
 [tool:pytest]
 addopts = -vvv
 python_files = tests/*.py
+markers =
+    acceptance: test lkml on GitHub-scraped files (deselect with '-m "not acceptance"')
 
 [pydocstyle]
 convention = google
diff --git a/setup.py b/setup.py
index 630bd48..e9c87be 100644
--- a/setup.py
+++ b/setup.py
@@ -2,7 +2,7 @@ from pathlib import Path
 
 from setuptools import find_packages, setup
 
-__version__ = "1.0.0"
+__version__ = "1.0.1"
 
 here = Path(__file__).parent.resolve()
 
@@ -31,13 +31,6 @@ setup(
     packages=find_packages(exclude=["docs", "tests*", "scripts"]),
     include_package_data=True,
     author="Josh Temple",
-    tests_require=[
-        "black",
-        "flake8",
-        "isort",
-        "mypy",
-        "pytest",
-        "pytest-cov",
-    ],
+    tests_require=["black", "flake8", "isort", "mypy", "pytest", "pytest-cov"],
     author_email="",
 )
diff --git a/tox.ini b/tox.ini
index ba9834b..112d0b0 100644
--- a/tox.ini
+++ b/tox.ini
@@ -5,7 +5,8 @@ envlist = py37
 deps = -rrequirements.txt
 commands =
     mypy lkml
+    pytest -m "not acceptance"
+    pytest -m "acceptance"
     flake8 lkml
-    pytest
-    isort -rc --check lkml
-    black --check lkml
\ No newline at end of file
+    black --check lkml
+    isort -rc --check lkml
\ No newline at end of file


Test patch:
diff --git a/tests/test_functional.py b/tests/test_functional.py
index 47887bb..4ad5ef8 100644
--- a/tests/test_functional.py
+++ b/tests/test_functional.py
@@ -1,9 +1,5 @@
 from dataclasses import replace
-from lkml.tree import (
-    ContainerNode,
-    DocumentNode,
-    ListNode,
-)
+from lkml.tree import ContainerNode, DocumentNode, ListNode
 from pathlib import Path
 import pytest
 import lkml
diff --git a/tests/test_github.py b/tests/test_github.py
index e6b8a9f..f83c6a5 100644
--- a/tests/test_github.py
+++ b/tests/test_github.py
@@ -23,6 +23,7 @@ def lookml(request):
     yield text
 
 
+@pytest.mark.acceptance
 def test_round_trip_should_work(lookml):
     # Load the LookML from file, parsing into a tree
     tree = lkml.parse(lookml)
diff --git a/tests/test_parser.py b/tests/test_parser.py
index fbd9154..af62bf6 100644
--- a/tests/test_parser.py
+++ b/tests/test_parser.py
@@ -268,14 +268,14 @@ def test_parse_pair_with_quoted_literal():
     parser = lkml.parser.Parser(stream)
     result = parser.parse_pair()
     assert result == PairNode(
-        type=SyntaxToken("view_label"), value=QuotedSyntaxToken("The View"),
+        type=SyntaxToken("view_label"), value=QuotedSyntaxToken("The View")
     )
     with pytest.raises(AttributeError):
         result.prefix
 
 
 def test_parse_pair_with_sql_block():
-    sql = "SELECT * FROM schema.table"
+    sql = " SELECT * FROM schema.table "
     stream = (
         tokens.LiteralToken("sql", 1),
         tokens.ValueToken(1),
@@ -286,7 +286,9 @@ def test_parse_pair_with_sql_block():
     )
     parser = lkml.parser.Parser(stream)
     result = parser.parse_pair()
-    assert result == PairNode(type=SyntaxToken("sql"), value=ExpressionSyntaxToken(sql))
+    assert result == PairNode(
+        type=SyntaxToken("sql"), value=ExpressionSyntaxToken(sql.strip())
+    )
 
 
 def test_parse_pair_with_bad_key():
diff --git a/tests/test_simple.py b/tests/test_simple.py
index eb1e11c..273def6 100644
--- a/tests/test_simple.py
+++ b/tests/test_simple.py
@@ -264,10 +264,7 @@ def test_parse_top_level_pairs(parser):
 
 def test_resolve_filters_filter_only_field(parser):
     nodes = parser.resolve_filters(
-        [
-            {"name": "filter_a", "type": "string"},
-            {"name": "filter_b", "type": "number"},
-        ]
+        [{"name": "filter_a", "type": "string"}, {"name": "filter_b", "type": "number"}]
     )
     result = "".join(str(node) for node in nodes)
     assert result == (
@@ -294,4 +291,3 @@ def test_resolve_filters_new_filters(parser):
     node = parser.resolve_filters([{"dimension_a": "-NULL"}, {"dimension_b": ">5"}])
     result = str(node)
     assert result == 'filters: [\n  dimension_a: "-NULL",\n  dimension_b: ">5",\n]'
-
diff --git a/tests/test_tree.py b/tests/test_tree.py
index b875ab0..c829748 100644
--- a/tests/test_tree.py
+++ b/tests/test_tree.py
@@ -1,4 +1,3 @@
-from lkml.parser import Syntax
 import pytest
 from lkml.tree import (
     BlockNode,
@@ -11,17 +10,32 @@ from lkml.tree import (
     RightCurlyBrace,
     SyntaxToken,
     QuotedSyntaxToken,
+    ExpressionSyntaxToken,
 )
 from lkml.visitors import LookMlVisitor
 
 
 @pytest.mark.parametrize(
-    "token_class,expected", [(SyntaxToken, "foo"), (QuotedSyntaxToken, '"foo"'),],
+    "token_class,expected", [(SyntaxToken, "foo"), (QuotedSyntaxToken, '"foo"')]
 )
 def test_syntax_token_str_should_return_formatted(token_class, expected):
     assert str(token_class("foo")) == expected
 
 
+def test_quoted_syntax_token_quotes_double_quotes():
+    text = 'This is the "best" dimension'
+    token = QuotedSyntaxToken(text)
+    assert token.format_value() == r'"This is the \"best\" dimension"'
+
+
+def test_expression_syntax_token_with_expression_suffix():
+    sql = "SELECT * FROM orders"
+    token = ExpressionSyntaxToken(
+        sql, prefix=" ", suffix=" # A comment", expr_suffix=" "
+    )
+    assert str(token) == " SELECT * FROM orders ;; # A comment"
+
+
 def test_pair_node_str_should_return_formatted():
     node = PairNode(type=SyntaxToken("foo"), value=SyntaxToken("bar"))
     assert str(node) == "foo: bar"
@@ -42,9 +56,9 @@ def test_list_node_str_should_return_formatted():
         type=SyntaxToken("filters"),
         left_bracket=LeftBracket(),
         items=(
-            PairNode(SyntaxToken("created_date"), QuotedSyntaxToken("7 days"),),
+            PairNode(SyntaxToken("created_date"), QuotedSyntaxToken("7 days")),
             PairNode(
-                SyntaxToken("user.status", prefix=" "), QuotedSyntaxToken("-disabled"),
+                SyntaxToken("user.status", prefix=" "), QuotedSyntaxToken("-disabled")
             ),
         ),
         right_bracket=RightBracket(),