Improve errors with timestamptz and avoid code generation when there's errors

.github/workflows/test.yml

@@ -20,7 +20,7 @@ jobs:
       fail-fast: true
       matrix:
         erlang: ["27", "28"]
-        postgres: ["postgres:16", "postgres:17"]
+        postgres: ["postgres:17", "postgres:18"]
 
     env:
       DATABASE_URL: postgres://squirrel_test:postgres_password@localhost:5432/squirrel_test
@@ -49,7 +49,7 @@ jobs:
       - uses: erlef/setup-beam@v1
         with:
           otp-version: ${{ matrix.erlang }}
-          gleam-version: "1.12.0"
+          gleam-version: "1.13.0"
           rebar3-version: "3"
       - run: gleam deps download
       - run: ./integration_test

CHANGELOG.md

@@ -1,5 +1,17 @@
 # CHANGELOG
 
+## Unreleased
+
+- Squirrel will no longer perform code generation if there's any errors in the
+  queries. Before it would still generate code for the queries with no errors.
+  This would usually lead the codebase in a broken state until all the errors
+  were fixed.
+  ([Giacomo Cavalieri](https://github.com/giacomocavalieri))
+
+- When trying to use a `timestamptz`, squirrel will now show a useful hint
+  nudging to use a `timestamp` instead.
+  ([Giacomo Cavalieri](https://github.com/giacomocavalieri))
+
 ## 4.4.2 - 2025-10-06
 
 - Fix a bug where squirrel would generate invalid code for enums with a long

birdie_snapshots/timestampz_has_a_nice_hint_nudging_to_use_a_timestamp.accepted

@@ -0,0 +1,19 @@
+---
+version: 1.4.0
+title: timestampz has a nice hint nudging to use a timestamp
+file: ./test/squirrel_test.gleam
+test_name: timestampz_has_a_nice_hint_nudging_to_use_a_timestamp_test
+---
+Error: Unsupported type
+
+    ╭─ query.sql
+    │
+  1 │ select timestamp with time zone '11 oct 1998'
+    ┆
+
+One of the rows returned by this query has type `timestamptz` which I cannot
+currently generate code for.
+
+Hint: In Postgres a `timestamptz` is converted to a regular `timestamp` using
+the connection's time zone. This is very error prone and should be avoided in
+favour of using regular timestamps.

src/squirrel.gleam

@@ -92,9 +92,13 @@ pub fn main() {
 
       let #(report, status_code) = case mode {
         GenerateCode ->
-          generated_queries
-          |> write_queries
-          |> report_written_queries
+          case ensure_no_errors(generated_queries) {
+            Error(errors) -> report_errors(errors)
+            Ok(generated_queries) ->
+              generated_queries
+              |> write_queries
+              |> report_written_queries
+          }
 
         CheckGeneratedCode ->
           generated_queries
@@ -108,6 +112,21 @@ pub fn main() {
   }
 }
 
+fn ensure_no_errors(
+  generated_queries: Dict(String, #(List(TypedQuery), List(Error))),
+) -> Result(Dict(String, List(TypedQuery)), List(Error)) {
+  let all_errors =
+    dict.fold(generated_queries, [], fn(errors, _directory, queries_and_errors) {
+      let #(_queries, new_errors) = queries_and_errors
+      list.append(new_errors, errors)
+    })
+
+  case all_errors {
+    [_, ..] -> Error(all_errors)
+    [] -> Ok(dict.map_values(generated_queries, fn(_, queries) { queries.0 }))
+  }
+}
+
 // --- CLI ARGS PARSING --------------------------------------------------------
 
 type Mode {
@@ -337,19 +356,15 @@ fn generate_queries(
 
 /// Given the queries generated by `generate_queries`, tries to write those to
 /// their own file and returns a dictionary that - for each file - holds the
-/// number of queries that could be generated and a list of all the errors that
-/// took place.
+/// number of queries that could be generated or the error that occurred trying
+/// to write the query.
 ///
 fn write_queries(
-  queries: Dict(String, #(List(TypedQuery), List(Error))),
-) -> Dict(String, #(Int, List(Error))) {
-  use directory, #(queries, errors) <- dict.map_values(queries)
+  queries: Dict(String, List(TypedQuery)),
+) -> Dict(String, Result(Int, Error)) {
+  use directory, queries <- dict.map_values(queries)
   let output_file = directory_to_output_file(directory)
-  case write_queries_to_file(queries, from: directory, to: output_file) {
-    Ok(n) -> #(n, errors)
-    Error(CannotOverwriteExistingFile(..) as error) -> #(0, [error, ..errors])
-    Error(error) -> #(list.length(queries), [error, ..errors])
-  }
+  write_queries_to_file(queries, from: directory, to: output_file)
 }
 
 fn write_queries_to_file(
@@ -528,25 +543,27 @@ fn term_width() -> Int {
 }
 
 fn report_written_queries(
-  dirs: Dict(String, #(Int, List(Error))),
+  directories: Dict(String, Result(Int, Error)),
 ) -> #(String, Int) {
   let #(ok, errors) = {
-    use acc, _, #(oks, errors) <- dict.fold(dirs, #(0, []))
-    let #(all_ok, all_errors) = acc
-    #(all_ok + oks, errors |> list.append(all_errors))
+    use acc, _, outcome <- dict.fold(directories, #(0, []))
+    let #(generated_queries, errors) = acc
+    case outcome {
+      Error(error) -> #(generated_queries, [error, ..errors])
+      Ok(new_count) -> #(generated_queries + new_count, errors)
+    }
   }
 
-  let errors_doc =
-    list.map(errors, error.to_doc)
-    |> doc.join(with: doc.lines(2))
-
   let status_code = case errors {
     [_, ..] -> 1
     [] -> 0
   }
 
   let report = case ok, errors {
-    0, [_, ..] -> doc.to_string(errors_doc, term_width())
+    0, [_, ..] ->
+      errors_to_doc(errors)
+      |> doc.to_string(term_width())
+
     0, [] ->
       [
         text_with_header(
@@ -577,7 +594,7 @@ under your project's `src`, `test`, and `dev` directories.",
 
     n, [_, ..] ->
       [
-        errors_doc,
+        errors_to_doc(errors),
         doc.lines(2),
         text_with_header(
           "🥜 ",
@@ -595,7 +612,19 @@ under your project's `src`, `test`, and `dev` directories.",
   #(report, status_code)
 }
 
-fn report_checked_queries(dirs: Dict(String, Result(Nil, List(Error)))) {
+fn errors_to_doc(errors: List(Error)) -> Document {
+  list.map(errors, error.to_doc)
+  |> doc.join(with: doc.lines(2))
+}
+
+fn report_errors(errors: List(Error)) -> #(String, Int) {
+  let report = errors_to_doc(errors) |> doc.to_string(term_width())
+  #(report, 1)
+}
+
+fn report_checked_queries(
+  dirs: Dict(String, Result(Nil, List(Error))),
+) -> #(String, Int) {
   let errors = {
     use all_errors, _, result <- dict.fold(dirs, [])
     case result {

src/squirrel/internal/error.gleam

@@ -540,15 +540,38 @@ Gleam type!",
         EnumWithNoVariants -> None
       })
 
-    QueryHasUnsupportedType(file:, name: _, content:, type_:, starting_line:) ->
-      printable_error("Unsupported type")
-      |> add_code_paragraph(file:, content:, point: None, starting_line:)
-      |> add_paragraph(
-        "One of the rows returned by this query has type "
-        <> style_inline_code(type_)
-        <> " which I cannot currently generate code for.",
-      )
-      |> call_to_action(for: "this type to be supported")
+    QueryHasUnsupportedType(file:, name: _, content:, type_:, starting_line:) -> {
+      let base_error =
+        printable_error("Unsupported type")
+        |> add_code_paragraph(file:, content:, point: None, starting_line:)
+        |> add_paragraph(
+          "One of the rows returned by this query has type "
+          <> style_inline_code(type_)
+          <> " which I cannot currently generate code for.",
+        )
+
+      case type_ {
+        // Timestampz usage is highly discouraged as it's only good if your
+        // language is bad with time. In all other cases you're worse off using
+        // it and should favour `timestamp`, so instead of a call to action to
+        // ask for support we point people to an explanation on why this should
+        // be avoided.
+        "timestamptz" ->
+          base_error
+          |> hint(
+            "In Postgres a "
+            <> style_inline_code("timestamptz")
+            <> " is converted to a regular "
+            <> style_inline_code("timestamp")
+            <> " using the connection's time zone. This is very error prone and
+should be avoided in favour of using regular timestamps.",
+          )
+
+        _ ->
+          base_error
+          |> call_to_action(for: "this type to be supported")
+      }
+    }
 
     CannotParseQuery(
       file:,

test/squirrel_test.gleam

@@ -737,6 +737,12 @@ from
   |> birdie.snap(title: "column with invalid name")
 }
 
+pub fn timestampz_has_a_nice_hint_nudging_to_use_a_timestamp_test() {
+  "select timestamp with time zone '11 oct 1998'"
+  |> should_error
+  |> birdie.snap(title: "timestampz has a nice hint nudging to use a timestamp")
+}
+
 pub fn query_with_syntax_error_test() {
   "
 select