Add dialyzer and resolve all findings closes #503 #504 #514 #516

Merged
moritz merged 16 commits from issue/mitgliederverwaltung-514 into main 2026-06-02 13:15:00 +02:00
Owner

Description of the implemented changes [0/1935]

The changes were:

  • Bugfixing
  • New Feature
  • Breaking Change
  • Refactoring

Resolve all Dialyzer findings so the just typecheck CI stage runs clean. Dialyzer was added to CI in a prior change and reported ~107 findings; this branch drives that count to zero by reconciling @specs with their success typings, binding intentionally discarded results, removing provably-unreachable clauses, and fixing several latent bugs the type analysis surfaced, all while preserving existing behavior (full test suite stays green).

What has been changed?

  • Add a Dialyzer typecheck stage to the full CI pipelines
  • Run the full test suite on main, tags and promote (Drone CI)
  • Reconcile @specs with their success typings
  • Bind discarded results of side-effecting release tasks
  • Forbid member-export request without actor instead of falling through
  • Bind intentionally discarded side-effecting results
  • Resolve unknown type references in member and authorization specs
  • Drop unreachable CSV error-formatting path
  • Remove dead catch-all clauses unreachable per success typing
  • Return readable string for unreadable upload errors
  • Match DateTime.from_iso8601 three-tuple when formatting cells
  • Show error for unparseable cycle date instead of crashing
  • Drop guards and clauses that can never succeed
  • Redirect a live-view socket in the user-required guard
  • Gate retry skipping on runtime sandbox flag
  • Define all_tenants/0 as empty for non-multitenant schema

Definition of Done

Code Quality

  • No new technical depths
  • Linting passed
  • Documentation is added were needed

Accessibility

  • New elements are properly defined with html-tags
  • Colour contrast follows WCAG criteria
  • Aria labels are added when needed
  • Everything is accessible by keyboard
  • Tab-Order is comprehensible
  • All interactive elements have a visible focus

Testing

  • Tests for new code are written
  • All tests pass
  • axe-core dev tools show no critical or major issues

Additional Notes

  • This PR also bundles the two CI-setup commits from the branch base (against main): feat(dialyzer): add typecheck stage to full CI pipelines (263857e) and ci(drone): run full test suite on main, tags and promote (ce57d04). The Dialyzer cleanup in this branch is what makes that new typecheck stage pass.
  • Scope was larger than the issue's "~70" estimate: 107 findings were resolved.
  • Behavior was preserved by default; the findings that turned out to be real bugs (member-export actor bypass, import upload-error formatting, PDF DateTime parsing, membership-fee invalid-date crash, the live_user_auth redirect) were each fixed with a covering test.
  • No .dialyzer_ignore.exs entries were added and no mix.exs Dialyzer flags were changed.
  • Two commits carry codebase-wide mechanical diffs (expect larger diffs): "reconcile @specs with their success typings" and remove dead catch-all clauses unreachable per success typing".
## Description of the implemented changes [0/1935] The changes were: - [x] Bugfixing - [x] New Feature - [ ] Breaking Change - [x] Refactoring Resolve all Dialyzer findings so the `just typecheck` CI stage runs clean. Dialyzer was added to CI in a prior change and reported ~107 findings; this branch drives that count to zero by reconciling `@spec`s with their success typings, binding intentionally discarded results, removing provably-unreachable clauses, and fixing several latent bugs the type analysis surfaced, all while preserving existing behavior (full test suite stays green). ## What has been changed? - Add a Dialyzer typecheck stage to the full CI pipelines - Run the full test suite on main, tags and promote (Drone CI) - Reconcile @specs with their success typings - Bind discarded results of side-effecting release tasks - Forbid member-export request without actor instead of falling through - Bind intentionally discarded side-effecting results - Resolve unknown type references in member and authorization specs - Drop unreachable CSV error-formatting path - Remove dead catch-all clauses unreachable per success typing - Return readable string for unreadable upload errors - Match DateTime.from_iso8601 three-tuple when formatting cells - Show error for unparseable cycle date instead of crashing - Drop guards and clauses that can never succeed - Redirect a live-view socket in the user-required guard - Gate retry skipping on runtime sandbox flag - Define all_tenants/0 as empty for non-multitenant schema ## Definition of Done ### Code Quality - [ ] No new technical depths - [x] Linting passed - [ ] Documentation is added were needed ### Accessibility - [ ] New elements are properly defined with html-tags - [ ] Colour contrast follows WCAG criteria - [ ] Aria labels are added when needed - [ ] Everything is accessible by keyboard - [ ] Tab-Order is comprehensible - [ ] All interactive elements have a visible focus ### Testing - [x] Tests for new code are written - [x] All tests pass - [ ] axe-core dev tools show no critical or major issues ## Additional Notes - This PR also bundles the two CI-setup commits from the branch base (against main): `feat(dialyzer): add typecheck stage to full CI pipelines` (263857e) and `ci(drone): run full test suite on main, tags and promote` (ce57d04). The Dialyzer cleanup in this branch is what makes that new typecheck stage pass. - Scope was larger than the issue's "~70" estimate: 107 findings were resolved. - Behavior was preserved by default; the findings that turned out to be real bugs (member-export actor bypass, import upload-error formatting, PDF DateTime parsing, membership-fee invalid-date crash, the live_user_auth redirect) were each fixed with a covering test. - No `.dialyzer_ignore.exs` entries were added and no `mix.exs` Dialyzer flags were changed. - Two commits carry codebase-wide mechanical diffs (expect larger diffs): "reconcile @specs with their success typings" and remove dead catch-all clauses unreachable per success typing".
moritz self-assigned this 2026-06-02 12:47:16 +02:00
moritz added 16 commits 2026-06-02 12:47:17 +02:00
ci(drone): run full test suite on main, tags and promote
Some checks reported errors
continuous-integration/drone/push Build encountered an error
ce57d046b9
feat(dialyzer): add typecheck stage to full CI pipelines
Some checks failed
continuous-integration/drone/push Build is passing
continuous-integration/drone/promote/production Build is failing
263857ee26
The nil-actor guard used a one-armed if and continued into the export path regardless. The CheckPagePermission plug already halts unauthenticated requests before this controller runs, so the corrected early return preserves observable behavior while removing the dead fall-through. The export action is split into per-payload clauses so the guard reads as a flat early return.
consume_and_read_csv/2 and MemberCSV.prepare/2 only ever return {:error, binary()}, so the non-binary error branch and the format_error_message/* helpers it called were unreachable. Removed them and bound the remaining discarded locale/dispatch results.
File.read/1 only yields posix atoms, so the File.Error and bare-reason branches were unreachable, and :file.format_error/1 returns a charlist rather than a String. Normalize the error to a binary so it interpolates correctly in flash messages.
DateTime.from_iso8601/1 returns {:ok, datetime, offset}, so the two-tuple clauses never matched and datetime cells fell through to the naive-parse fallback. Matching the real shape routes them through the intended DateTime path; UTC values render identically.
Date.from_iso8601/1 returns {:error, reason}, so the with else clause matching a bare :error never fired and an invalid date raised a WithClauseError. Match the real date/calendar error reasons so the user sees the validation message.
LiveSession.assign_new_resources/2 is typed to return a Phoenix.Socket, which made the on_mount redirect type-incompatible. The authenticated-routes live_session already assigns current_user, so the guard reads it from socket.assigns directly. Also assign the locale into the socket actually used by the no-user redirect instead of discarding it.
The compile-time Mix.env() comparison folded to an always-false literal under analysis. sql_sandbox?/0 reads runtime config (true only in test) and works in releases where Mix is unavailable, preserving the fast-fail-no-retry behavior in tests.
fix(repo): define all_tenants/0 as empty for non-multitenant schema
Some checks reported errors
continuous-integration/drone/push Build was killed
continuous-integration/drone/promote/production Build is passing
9a14cedc14
moritz merged commit 1ef6ea502e into main 2026-06-02 13:15:00 +02:00
moritz deleted branch issue/mitgliederverwaltung-514 2026-06-02 13:15:00 +02:00
Sign in to join this conversation.
No description provided.