Correct position on free-variable errors

quill-core/src/main/scala/io/getquill/context/VerifyFreeVariables.scala

@@ -2,14 +2,28 @@ package io.getquill.context
 
 import scala.reflect.macros.whitebox.{Context => MacroContext}
 import io.getquill.quotation.FreeVariables
-import io.getquill.ast.Ast
+import io.getquill.ast.{Ast, Ident, Pos}
 import io.getquill.util.MacroContextExt._
 
 object VerifyFreeVariables {
 
-  def apply(c: MacroContext)(ast: Ast): Ast =
+  def apply(c: MacroContext)(ast: Ast): Ast = {
+    import c.universe.{Ident => _, _}
+
     FreeVariables.verify(ast) match {
       case Right(ast) => ast
-      case Left(msg)  => c.fail(msg)
+      case Left(err) =>
+        err.freeVars match {
+          // we we have a single position from the encosing context in the same file we can actually fail
+          // at the right position and point the compiler to that location since we can modify the position
+          // by the `point` info that we have from our position
+          case List(Ident.WithPos(_, Pos.Real(fileName, _, _, point, _))) if (c.enclosingPosition.source.path == fileName) =>
+            c.failAtPoint(err.msgNoPos, point)
+
+          case _ =>
+            c.fail(err.msg)
+        }
+
     }
+  }
 }

quill-core/src/main/scala/io/getquill/quotation/Liftables.scala

@@ -124,6 +124,11 @@ trait Liftables extends QuatLiftable {
     case Visibility.Hidden  => q"$pack.Visibility.Hidden"
   }
 
+  implicit val positionLiftable: Liftable[Pos] = Liftable[Pos] {
+    case Pos.Real(a, b, c, d, e) => q"$pack.Pos.Real($a, $b, $c, $d, $e)"
+    case Pos.Synthetic           => q"$pack.Pos.Synthetic"
+  }
+
   implicit val queryLiftable: Liftable[Query] = Liftable[Query] {
     case Entity.Opinionated(a, b, quat, renameable) => q"$pack.Entity.Opinionated($a, $b, $quat, $renameable)"
     case Filter(a, b, c)                            => q"$pack.Filter($a, $b, $c)"
@@ -206,8 +211,8 @@ trait Liftables extends QuatLiftable {
     case CaseClass(n, a)   => q"$pack.CaseClass($n, $a)"
   }
 
-  implicit val identLiftable: Liftable[Ident] = Liftable[Ident] { case Ident(a, quat) =>
-    q"$pack.Ident($a, $quat)"
+  implicit val identLiftable: Liftable[Ident] = Liftable[Ident] { case Ident.Opinionated(a, quat, vis, pos) =>
+    q"$pack.Ident.Opinionated($a, $quat, $vis, $pos)"
   }
   implicit val externalIdentLiftable: Liftable[ExternalIdent] = Liftable[ExternalIdent] { case ExternalIdent(a, quat) =>
     q"$pack.ExternalIdent($a, $quat)"

quill-core/src/main/scala/io/getquill/quotation/Parsing.scala

@@ -27,7 +27,7 @@ trait Parsing extends ValueComputation with QuatMaking with MacroUtilBase {
 
   // Variables that need to be sanitized out in various places due to internal conflicts with the way
   // macros hard handled in MetaDsl
-  private[getquill] val dangerousVariables: Set[IdentName] = Set(IdentName("v"))
+  private[getquill] val dangerousVariables: Set[Ident] = Set(Ident.trivial("v"))
 
   case class Parser[T](p: PartialFunction[Tree, T])(implicit ct: ClassTag[T]) {
 
@@ -81,14 +81,14 @@ trait Parsing extends ValueComputation with QuatMaking with MacroUtilBase {
     case q"{..$exprs}" if exprs.size > 1 => Block(exprs.map(astParser(_)))
   }
 
-  val valParser: Parser[Val] = Parser[Val] { case q"val $name: $typ = $body" =>
+  val valParser: Parser[Val] = Parser[Val] { case wholeExpr @ q"val $name: $typ = $body" =>
     // for some reason inferQuat(typ.tpe) causes a compile hang in scala.reflect.internal
     val bodyAst = astParser(body)
-    Val(ident(name, bodyAst.quat), bodyAst)
+    Val(ident(name, bodyAst.quat, wholeExpr.pos), bodyAst)
   }
 
-  val patMatchValParser: Parser[Val] = Parser[Val] { case q"$mods val $name: $typ = ${patMatchParser(value)}" =>
-    Val(ident(name, inferQuat(q"$typ".tpe)), value)
+  val patMatchValParser: Parser[Val] = Parser[Val] { case wholeExpr @ q"$mods val $name: $typ = ${patMatchParser(value)}" =>
+    Val(ident(name, inferQuat(q"$typ".tpe), wholeExpr.pos), value)
   }
 
   val patMatchParser: Parser[Ast] = Parser[Ast] { case q"$expr match { case ($fields) => $body }" =>
@@ -462,16 +462,28 @@ trait Parsing extends ValueComputation with QuatMaking with MacroUtilBase {
   val identParser: Parser[Ident] = Parser[Ident] {
     // TODO Check to see that all these conditions work
     case t: ValDef =>
-      identClean(Ident(t.name.decodedName.toString, inferQuat(t.symbol.typeSignature)))
-    case id @ c.universe.Ident(TermName(name)) => identClean(Ident(name, inferQuat(id.symbol.typeSignature)))
-    case t @ q"$cls.this.$i"                   => identClean(Ident(i.decodedName.toString, inferQuat(t.symbol.typeSignature)))
+      identClean(t.name.decodedName.toString, inferQuat(t.symbol.typeSignature), t.pos)
+    case id @ c.universe.Ident(TermName(name)) =>
+      identClean(name, inferQuat(id.symbol.typeSignature), id.pos)
+    case t @ q"$cls.this.$i" =>
+      identClean(i.decodedName.toString, inferQuat(t.symbol.typeSignature), t.pos)
     case t @ c.universe.Bind(TermName(name), c.universe.Ident(termNames.WILDCARD)) =>
-      identClean(
-        Ident(name, inferQuat(t.symbol.typeSignature))
-      ) // TODO Verify Quat what is the type of this thing? In what cases does it happen? Do we need to do something more clever with the tree and get a TypeRef?
+      // TODO Verify Quat what is the type of this thing? In what cases does it happen? Do we need to do something more clever with the tree and get a TypeRef?
+      identClean(name, inferQuat(t.symbol.typeSignature), t.pos)
   }
-  private def identClean(x: Ident): Ident           = x.copy(name = x.name.replace("$", ""))
-  private def ident(x: TermName, quat: Quat): Ident = identClean(Ident(x.decodedName.toString, quat))
+  private def identClean(name: String, quat: Quat, pos: Position): Ident =
+    Ident.Opinionated(
+      name.replace("$", ""),
+      quat,
+      Visibility.Visible,
+      if (pos != NoPosition)
+        Pos.Real(pos.source.path, pos.line, pos.column, pos.point, 0)
+      else
+        Pos.Synthetic
+    )
+
+  private def ident(x: TermName, quat: Quat, pos: Position): Ident =
+    identClean(x.decodedName.toString, quat, pos)
 
   /**
    * In order to guarantee consistent behavior across multiple databases, we

quill-core/src/main/scala/io/getquill/quotation/Unliftables.scala

@@ -155,6 +155,11 @@ trait Unliftables extends QuatUnliftable {
     case q"$pack.Visibility.Hidden"  => Visibility.Hidden
   }
 
+  implicit val positionUnliftable: Unliftable[Pos] = Unliftable[Pos] {
+    case q"$pack.Pos.Real.apply(${file: String}, ${row: Int}, ${column: Int}, ${point: Int}, ${width: Int})" => Pos.Real(file, row, column, point, width)
+    case q"$pack.Pos.Synthetic"                                                                              => Pos.Synthetic
+  }
+
   implicit val propertyUnliftable: Unliftable[Property] = Unliftable[Property] {
     case q"$pack.Property.apply(${a: Ast}, ${b: String})" => Property(a, b)
     case q"$pack.Property.Opinionated.apply(${a: Ast}, ${b: String}, ${renameable: Renameable}, ${visibility: Visibility})" =>
@@ -209,7 +214,8 @@ trait Unliftables extends QuatUnliftable {
   }
 
   implicit val identUnliftable: Unliftable[Ident] = Unliftable[Ident] {
-    case q"$pack.Ident.apply(${a: String}, ${quat: Quat})" => Ident(a, quat)
+    case q"$pack.Ident.Opinionated.apply(${a: String}, ${quat: Quat}, ${vis: Visibility}, ${pos: Pos})" =>
+      Ident.Opinionated(a, quat, vis, pos)
   }
   implicit val externalIdentUnliftable: Unliftable[ExternalIdent] = Unliftable[ExternalIdent] {
     case q"$pack.ExternalIdent.apply(${a: String}, ${quat: Quat})" => ExternalIdent(a, quat)

quill-core/src/main/scala/io/getquill/util/MacroContextExt.scala

@@ -5,6 +5,7 @@ import io.getquill.util.IndentUtil._
 import io.getquill.util.Messages.{debugEnabled, errorPrefix, prettyPrint}
 import io.getquill.quat.VerifyNoBranches
 
+import scala.reflect.api.Position
 import scala.reflect.macros.blackbox.{Context => MacroContext}
 
 object MacroContextExt {
@@ -16,6 +17,11 @@ object MacroContextExt {
     def error(msg: String): Unit =
       c.error(c.enclosingPosition, if (errorPrefix) s"[quill] $msg" else msg)
 
+    def failAtPoint(msg: String, point: Int): Nothing = {
+      val errorPos = c.enclosingPosition.withPoint(point)
+      c.abort(errorPos, if (errorPrefix) s"[quill] $msg" else msg)
+    }
+
     def fail(msg: String): Nothing =
       c.abort(c.enclosingPosition, if (errorPrefix) s"[quill] $msg" else msg)
 

quill-core/src/test/scala/io/getquill/quotation/FreeVariablesSpec.scala

@@ -1,12 +1,12 @@
 package io.getquill.quotation
 
-import io.getquill.ast.IdentName
 import io.getquill.base.Spec
 import io.getquill.MirrorContexts.testContext.implicitOrd
 import io.getquill.MirrorContexts.testContext.qr1
 import io.getquill.MirrorContexts.testContext.qr2
 import io.getquill.MirrorContexts.testContext.quote
 import io.getquill.MirrorContexts.testContext.unquote
+import io.getquill.ast.Ident
 
 class FreeVariablesSpec extends Spec {
 
@@ -16,77 +16,77 @@ class FreeVariablesSpec extends Spec {
   "detects references to values outside of the quotation (free variables)" - {
     "ident" in {
       val q = quote(s)
-      FreeVariables(q.ast) mustEqual Set(IdentName("s"))
+      FreeVariables(q.ast) mustEqual Set(Ident.trivial("s"))
     }
     "function" in {
       val q =
         quote { (a: String) =>
           s
         }
-      FreeVariables(q.ast) mustEqual Set(IdentName("s"))
+      FreeVariables(q.ast) mustEqual Set(Ident.trivial("s"))
     }
     "filter" in {
       val q =
         quote {
           qr1.filter(_.s == s)
         }
-      FreeVariables(q.ast) mustEqual Set(IdentName("s"))
+      FreeVariables(q.ast) mustEqual Set(Ident.trivial("s"))
     }
     "map" in {
       val q =
         quote {
           qr1.map(_ => s)
         }
-      FreeVariables(q.ast) mustEqual Set(IdentName("s"))
+      FreeVariables(q.ast) mustEqual Set(Ident.trivial("s"))
     }
     "flatMap" in {
       val q =
         quote {
           qr1.map(_ => s).flatMap(_ => qr2)
         }
-      FreeVariables(q.ast) mustEqual Set(IdentName("s"))
+      FreeVariables(q.ast) mustEqual Set(Ident.trivial("s"))
     }
     "concatMap" in {
       val a = Seq(1, 2)
       val q =
         quote {
           qr1.concatMap(_ => a).flatMap(_ => qr2)
         }
-      FreeVariables(q.ast) mustEqual Set(IdentName("a"))
+      FreeVariables(q.ast) mustEqual Set(Ident.trivial("a"))
     }
     "sortBy" in {
       val q =
         quote {
           qr1.sortBy(_ => s)
         }
-      FreeVariables(q.ast) mustEqual Set(IdentName("s"))
+      FreeVariables(q.ast) mustEqual Set(Ident.trivial("s"))
     }
     "groupBy" in {
       val q =
         quote {
           qr1.groupBy(_ => s)
         }
-      FreeVariables(q.ast) mustEqual Set(IdentName("s"))
+      FreeVariables(q.ast) mustEqual Set(Ident.trivial("s"))
     }
     "take" in {
       val q =
         quote {
           qr1.take(i)
         }
-      FreeVariables(q.ast) mustEqual Set(IdentName("i"))
+      FreeVariables(q.ast) mustEqual Set(Ident.trivial("i"))
     }
     "conditional outer join" in {
       val q =
         quote {
           qr1.leftJoin(qr2).on((a, b) => a.s == s)
         }
-      FreeVariables(q.ast) mustEqual Set(IdentName("s"))
+      FreeVariables(q.ast) mustEqual Set(Ident.trivial("s"))
     }
     "assignment" in {
       val q = quote {
         qr1.insert(_.i -> i)
       }
-      FreeVariables(q.ast) mustEqual Set(IdentName("i"))
+      FreeVariables(q.ast) mustEqual Set(Ident.trivial("i"))
     }
     "join" in {
       val i = 1
@@ -95,37 +95,37 @@ class FreeVariablesSpec extends Spec {
           .join(qr2.filter(_.i == i))
           .on((t1, t2) => t1.i == t2.i)
       }
-      FreeVariables(q.ast) mustEqual Set(IdentName("i"))
+      FreeVariables(q.ast) mustEqual Set(Ident.trivial("i"))
     }
     "option operators" - {
       "map" in {
         val i = 1
         val q = quote {
           qr1.map(_.o.map(_ == i))
         }
-        FreeVariables(q.ast) mustEqual Set(IdentName("i"))
+        FreeVariables(q.ast) mustEqual Set(Ident.trivial("i"))
       }
       "forall" in {
         val i = 1
         val q = quote {
           qr1.filter(_.o.forall(_ == i))
         }
-        FreeVariables(q.ast) mustEqual Set(IdentName("i"))
+        FreeVariables(q.ast) mustEqual Set(Ident.trivial("i"))
 
       }
       "exists" in {
         val i = 1
         val q = quote {
           qr1.filter(_.o.exists(_ == i))
         }
-        FreeVariables(q.ast) mustEqual Set(IdentName("i"))
+        FreeVariables(q.ast) mustEqual Set(Ident.trivial("i"))
       }
       "contains" in {
         val i = 1
         val q = quote {
           qr1.filter(_.o.contains(i))
         }
-        FreeVariables(q.ast) mustEqual Set(IdentName("i"))
+        FreeVariables(q.ast) mustEqual Set(Ident.trivial("i"))
       }
     }
   }

quill-engine/src/main/scala/io/getquill/MirrorIdiom.scala

@@ -270,7 +270,7 @@ trait MirrorIdiomBase extends Idiom {
   }
 
   implicit final val identTokenizer: Tokenizer[Ident] = Tokenizer[Ident] {
-    case Ident.Opinionated(name, _, visibility) =>
+    case Ident.Opinionated(name, _, visibility, _) =>
       stmt"${bracketIfHidden(name, visibility).token}"
   }
 

quill-engine/src/main/scala/io/getquill/MirrorSqlDialect.scala

@@ -1,5 +1,6 @@
 package io.getquill
 
+import io.getquill.ast.Ident
 import io.getquill.context.sql.idiom.{ConcatSupport, QuestionMarkBindVariables, SqlIdiom}
 import io.getquill.context._
 import io.getquill.norm.ProductAggregationToken
@@ -56,7 +57,7 @@ object MirrorSqlDialect extends MirrorSqlDialect {
   trait StrategizeElements extends SqlIdiom with QuestionMarkBindVariables with ConcatSupport with CanReturnField {
 
     override def tokenizeIdentName(strategy: NamingStrategy, name: String): String     = strategy.default(name)
-    override def tokenizeTableAlias(strategy: NamingStrategy, table: String): String   = strategy.default(table)
+    override def tokenizeTableAlias(strategy: NamingStrategy, table: Ident): String    = strategy.default(table.name)
     override def tokenizeColumnAlias(strategy: NamingStrategy, column: String): String = strategy.default(column)
     override def tokenizeFixedColumn(strategy: NamingStrategy, column: String): String = strategy.default(column)
     override def prepareForProbing(string: String)                                     = string

quill-engine/src/main/scala/io/getquill/OracleDialect.scala

@@ -82,8 +82,8 @@ trait OracleDialect
   override protected def tokenizeColumnAlias(strategy: NamingStrategy, column: String): String =
     tokenizeEscapeUnderscores(strategy, column, None)
 
-  override protected def tokenizeTableAlias(strategy: NamingStrategy, column: String): String =
-    tokenizeEscapeUnderscores(strategy, column, None)
+  override protected def tokenizeTableAlias(strategy: NamingStrategy, tableName: Ident): String =
+    tokenizeEscapeUnderscores(strategy, tableName.name, None)
 
   private def tokenizeEscapeUnderscores(
     strategy: NamingStrategy,