pr feedback

Author: Nyrox

src/ast/helpers/ignore_field.rs

@@ -19,51 +19,50 @@ use core::cmp::{Eq, Ord, Ordering, PartialEq, PartialOrd};
 use core::fmt::{self, Debug, Formatter};
 use core::hash::{Hash, Hasher};
 
-/// A wrapper type that ignores the field when comparing or hashing.
-pub struct IgnoreField<T>(pub T);
+use crate::tokenizer::TokenWithLocation;
+
+/// A wrapper type for attaching tokens to AST nodes that should be ignored in comparisons and hashing.
+/// This should be used when a token is not relevant for semantics, but is still needed for
+/// accurate source location tracking.
+#[derive(Clone)]
+pub struct AttachedToken(pub TokenWithLocation);
 
 // Conditional Implementations
-impl<T: Debug> Debug for IgnoreField<T> {
+impl Debug for AttachedToken {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         self.0.fmt(f)
     }
 }
 
-impl<T: Clone> Clone for IgnoreField<T> {
-    fn clone(&self) -> Self {
-        IgnoreField(self.0.clone())
-    }
-}
-
 // Blanket Implementations
-impl<T> PartialEq for IgnoreField<T> {
+impl PartialEq for AttachedToken {
     fn eq(&self, _: &Self) -> bool {
         true
     }
 }
 
-impl<T> Eq for IgnoreField<T> {}
+impl Eq for AttachedToken {}
 
-impl<T> PartialOrd for IgnoreField<T> {
-    fn partial_cmp(&self, _: &Self) -> Option<Ordering> {
-        Some(Ordering::Equal)
+impl PartialOrd for AttachedToken {
+    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
+        Some(self.cmp(other))
     }
 }
 
-impl<T: Ord> Ord for IgnoreField<T> {
+impl Ord for AttachedToken {
     fn cmp(&self, _: &Self) -> Ordering {
         Ordering::Equal
     }
 }
 
-impl<T> Hash for IgnoreField<T> {
+impl Hash for AttachedToken {
     fn hash<H: Hasher>(&self, _state: &mut H) {
         // Do nothing
     }
 }
 
-impl<T> From<T> for IgnoreField<T> {
-    fn from(value: T) -> Self {
-        IgnoreField(value)
+impl From<TokenWithLocation> for AttachedToken {
+    fn from(value: TokenWithLocation) -> Self {
+        AttachedToken(value)
     }
 }

src/ast/mod.rs

@@ -157,7 +157,7 @@ impl PartialEq for Ident {
         let Ident {
             value,
             quote_style,
-            // backwards compat
+            // exhaustiveness check; we ignore spans in comparisons
             span: _,
         } = self;
 
@@ -170,7 +170,7 @@ impl core::hash::Hash for Ident {
         let Ident {
             value,
             quote_style,
-            // backwards compat
+            // exhaustiveness check; we ignore spans in hashes
             span: _,
         } = self;
 

src/ast/query.rs

@@ -18,7 +18,7 @@
 #[cfg(not(feature = "std"))]
 use alloc::{boxed::Box, vec::Vec};
 
-use helpers::ignore_field::IgnoreField;
+use helpers::ignore_field::AttachedToken;
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
 
@@ -280,8 +280,8 @@ impl fmt::Display for Table {
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
 pub struct Select {
-    /// SELECT
-    pub select_token: IgnoreField<TokenWithLocation>,
+    /// Token for the `SELECT` keyword
+    pub select_token: AttachedToken,
     pub distinct: Option<Distinct>,
     /// MSSQL syntax: `TOP (<N>) [ PERCENT ] [ WITH TIES ]`
     pub top: Option<Top>,
@@ -511,7 +511,8 @@ 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: IgnoreField<TokenWithLocation>,
+    // Token for the "WITH" keyword
+    pub with_token: AttachedToken,
     pub recursive: bool,
     pub cte_tables: Vec<Cte>,
 }
@@ -563,8 +564,8 @@ pub struct Cte {
     pub query: Box<Query>,
     pub from: Option<Ident>,
     pub materialized: Option<CteAsMaterialized>,
-    // needed for accurate span reporting
-    pub closing_paren_token: IgnoreField<TokenWithLocation>,
+    // Token for the closing parenthesis
+    pub closing_paren_token: AttachedToken,
 }
 
 impl fmt::Display for Cte {
@@ -620,7 +621,8 @@ 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: IgnoreField<TokenWithLocation>,
+    /// The wildcard token `*`
+    pub wildcard_token: AttachedToken,
     /// `[ILIKE...]`.
     ///  Snowflake syntax: <https://docs.snowflake.com/en/sql-reference/sql/select#parameters>
     pub opt_ilike: Option<IlikeSelectItem>,

src/parser/mod.rs

@@ -24,7 +24,7 @@ use core::{
     fmt::{self, Display},
     str::FromStr,
 };
-use helpers::ignore_field::IgnoreField;
+use helpers::ignore_field::AttachedToken;
 
 use log::debug;
 
@@ -9370,7 +9370,7 @@ impl<'a> Parser<'a> {
         };
 
         Ok(Select {
-            select_token: IgnoreField(select_token),
+            select_token: AttachedToken(select_token),
             distinct,
             top,
             top_before_distinct,