docs and fixes

Author: Nyrox

README.md

@@ -100,6 +100,23 @@ similar semantics are represented with the same AST. We welcome PRs to fix such
 issues and distinguish different syntaxes in the AST.
 
 
+## WIP: Extracting source locations from AST nodes
+
+This crate allows recovering source locations from AST nodes via the [Spanned](https://docs.rs/sqlparser/latest/sqlparser/ast/trait.Spanned.html) trait, which can be used for advanced diagnostics tooling. Node that this feature is a work in progress and many nodes report missing or inaccurate spans. Please see [this document](./docs/source_spans.md#source-span-contributing-guidelines) for information on how to contribute missing improvements.
+
+```rust
+use sqlparser::ast::Spanned;
+
+// Parse SQL
+let ast = Parser::parse_sql(&GenericDialect, "SELECT A FROM B").unwrap();
+
+// The source span can be retrieved with start and end locations
+assert_eq!(ast[0].span(), Span {
+  start: Location::of(1, 1),
+  end: Location::of(1, 16),
+});
+```
+
 ## SQL compliance
 
 SQL was first standardized in 1987, and revisions of the standard have been

docs/source_spans.md

@@ -0,0 +1,52 @@
+
+## Breaking Changes
+
+These are the current breaking changes introduced by the source spans feature:
+
+#### Added fields for spans (must be added to any existing pattern matches)
+- `Ident` now stores a `Span`
+- `Select`, `With`, `Cte`, `WildcardAdditionalOptions` now store a `TokenWithLocation` 
+
+#### Misc.
+- `TokenWithLocation` stores a full `Span`, rathern than just a source location. Users relying on `token.location` should use `token.location.start` instead.
+## Source Span Contributing Guidelines
+
+For contributing source spans improvement in addition to the general [contribution guidelines](../README.md#contributing), please make sure to pay attention to the following:
+
+
+### Source Span Design Considerations
+
+- `Ident` always have correct source spans
+- Downstream breaking change impact is to be as minimal as possible
+- To this end, use recursive merging of spans in favor of storing spans on all nodes
+- Any metadata added to compute spans must not change semantics (Eq, Ord, Hash, etc.)
+
+The primary reason for missing and inaccurate source spans at this time is missing spans of keyword tokens and values in many structures, either due to lack of time or because adding them would break downstream significantly.  
+
+When considering adding support for source spans on a type, consider the impact to consumers of that type and whether your change would require a consumer to do non-trivial changes to their code.
+
+f.e. of a trivial change
+```rust
+match node {  
+  ast::Query { 
+    field1,
+    field2, 
+    location: _,  // add a new line to ignored location
+}
+```
+
+If adding source spans to a type would require a significant change like wrapping that type or similar, please open an issue to discuss. 
+
+### AST Node Equality and Hashes
+
+When adding tokens to AST nodes, make sure to wrap them in the [IgnoreField](https://docs.rs/sqlparser/latest/sqlparser/ast/helpers/struct.IgnoreField.html) helper to ensure that semantically equivalent AST nodes always compare as equal and hash to the same value. F.e. `select 5` and `SELECT 5` would compare as different `Select` nodes, if the select token was stored directly. f.e.
+
+```rust
+struct Select {
+    select_token: IgnoreField<TokenWithLocation>, // only used for spans
+    /// remaining fields
+    field1,
+    field2,
+    ...
+}
+```

src/ast/query.rs

@@ -511,7 +511,7 @@ impl fmt::Display for NamedWindowDefinition {
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
 pub struct With {
-    pub with_token: TokenWithLocation,
+    pub with_token: IgnoreField<TokenWithLocation>,
     pub recursive: bool,
     pub cte_tables: Vec<Cte>,
 }
@@ -564,7 +564,7 @@ pub struct Cte {
     pub from: Option<Ident>,
     pub materialized: Option<CteAsMaterialized>,
     // needed for accurate span reporting
-    pub closing_paren_token: TokenWithLocation,
+    pub closing_paren_token: IgnoreField<TokenWithLocation>,
 }
 
 impl fmt::Display for Cte {
@@ -620,7 +620,7 @@ impl fmt::Display for IdentWithAlias {
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
 pub struct WildcardAdditionalOptions {
-    pub wildcard_token: TokenWithLocation,
+    pub wildcard_token: IgnoreField<TokenWithLocation>,
     /// `[ILIKE...]`.
     ///  Snowflake syntax: <https://docs.snowflake.com/en/sql-reference/sql/select#parameters>
     pub opt_ilike: Option<IlikeSelectItem>,
@@ -641,7 +641,7 @@ pub struct WildcardAdditionalOptions {
 impl Default for WildcardAdditionalOptions {
     fn default() -> Self {
         Self {
-            wildcard_token: TokenWithLocation::wrap(Token::Mul),
+            wildcard_token: TokenWithLocation::wrap(Token::Mul).into(),
             opt_ilike: None,
             opt_exclude: None,
             opt_except: None,

src/ast/spans.rs

@@ -100,7 +100,7 @@ impl Spanned for With {
         } = self;
 
         union_spans(
-            core::iter::once(with_token.span).chain(cte_tables.iter().map(|item| item.span())),
+            core::iter::once(with_token.0.span).chain(cte_tables.iter().map(|item| item.span())),
         )
     }
 }
@@ -119,7 +119,7 @@ impl Spanned for Cte {
             core::iter::once(alias.span())
                 .chain(core::iter::once(query.span()))
                 .chain(from.iter().map(|item| item.span))
-                .chain(core::iter::once(closing_paren_token.span)),
+                .chain(core::iter::once(closing_paren_token.0.span)),
         )
     }
 }
@@ -1547,7 +1547,7 @@ impl Spanned for WildcardAdditionalOptions {
         } = self;
 
         union_spans(
-            core::iter::once(wildcard_token.span)
+            core::iter::once(wildcard_token.0.span)
                 .chain(opt_ilike.as_ref().map(|i| i.span()))
                 .chain(opt_exclude.as_ref().map(|i| i.span()))
                 .chain(opt_rename.as_ref().map(|i| i.span()))

src/parser/mod.rs

@@ -8799,7 +8799,7 @@ impl<'a> Parser<'a> {
         let _guard = self.recursion_counter.try_decrease()?;
         let with = if let Some(with_token) = self.parse_keyword_token(Keyword::WITH) {
             Some(With {
-                with_token,
+                with_token: with_token.into(),
                 recursive: self.parse_keyword(Keyword::RECURSIVE),
                 cte_tables: self.parse_comma_separated(Parser::parse_cte)?,
             })
@@ -9070,7 +9070,7 @@ impl<'a> Parser<'a> {
                 query,
                 from: None,
                 materialized: is_materialized,
-                closing_paren_token,
+                closing_paren_token: closing_paren_token.into(),
             }
         } else {
             let columns = self.parse_parenthesized_column_list(Optional, false)?;
@@ -9094,7 +9094,7 @@ impl<'a> Parser<'a> {
                 query,
                 from: None,
                 materialized: is_materialized,
-                closing_paren_token,
+                closing_paren_token: closing_paren_token.into(),
             }
         };
         if self.parse_keyword(Keyword::FROM) {
@@ -11365,7 +11365,7 @@ impl<'a> Parser<'a> {
         };
 
         Ok(WildcardAdditionalOptions {
-            wildcard_token,
+            wildcard_token: wildcard_token.into(),
             opt_ilike,
             opt_exclude,
             opt_except,