diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 927e3d9..91970b9 100644 --- a/.github/workflows/test.yml +++ b/.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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 39e0fc2..54e1c28 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/birdie_snapshots/timestampz_has_a_nice_hint_nudging_to_use_a_timestamp.accepted b/birdie_snapshots/timestampz_has_a_nice_hint_nudging_to_use_a_timestamp.accepted new file mode 100644 index 0000000..a37470e --- /dev/null +++ b/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. diff --git a/src/squirrel.gleam b/src/squirrel.gleam index 8ae1edb..93af187 100644 --- a/src/squirrel.gleam +++ b/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 { diff --git a/src/squirrel/internal/error.gleam b/src/squirrel/internal/error.gleam index 1acb0c6..1d983ad 100644 --- a/src/squirrel/internal/error.gleam +++ b/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:, diff --git a/test/squirrel_test.gleam b/test/squirrel_test.gleam index 1a0e2f6..311e8ee 100644 --- a/test/squirrel_test.gleam +++ b/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