Skip to content

Document the panics of public functions - #10760

Draft
emilk wants to merge 6 commits into
apache:mainfrom
emilk:emilk/panics-docs
Draft

Document the panics of public functions#10760
emilk wants to merge 6 commits into
apache:mainfrom
emilk:emilk/panics-docs

Conversation

@emilk

@emilk emilk commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

This is one of four PRs splitting up the clippy::missing_panics_doc work.
Each one stands on its own and touches its own set of functions, but they are
meant to land in order, since the later ones assume the earlier ones:

  1. Return errors instead of panicking in fallible functions #10755 - return errors from fallible functions
  2. Remove some unwraps #10759 - remove unreachable panics
  3. Document the panics of public functions #10760 - document the panics that genuinely remain
  4. Mark unreachable panics with #[expect(clippy::missing_panics_doc)] #10761 - #[expect] the unreachable ones, so the lint can be turned on

What is left after the first two PRs is panics that are real and that the
function cannot report any other way. Those get a # Panics section.

What changes are included in this PR?

  • # Panics sections on the public functions that can still panic, including
    the ones that only panic under the force_validate feature
  • two of them say why the panic is not an error, so the reader does not have to
    wonder: try_for_each_valid_idx takes the caller's error type and so has no
    error value to build, and the arrow-integration-test JSON readers parse
    fixtures, where malformed input is a broken test rather than bad data
  • arrow-csv's Writer::into_inner documents its panic honestly. A successful
    write flushes, but a write that returned an error leaves rows buffered, and
    the flush inside into_inner can then fail. It returns W, so there is
    nowhere to report that
  • arrow::util::test_util and arrow::util::bench_util are public, so their
    module docs say the helpers panic on invalid input
  • the # Panics sections go before #[inline], #[deprecated] and
    #[proc_macro_derive], rather than after, so the doc block stays in one piece
  • closes an unterminated doc code fence in PrimitiveDictionaryBuilder

arrow-integration-testing is deliberately left undocumented: it is
publish = false and holds only integration test binaries. #10761 suppresses the
lint for that crate instead.

Are these changes tested?

Documentation only. cargo doc --workspace --all-features is clean.

Are there any user-facing changes?

Documentation only.

emilk and others added 4 commits August 19, 2026 21:20
Add `# Panics` sections to public functions that can panic, in
preparation for enabling the `clippy::missing_panics_doc` lint.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The `#[expect]` reason claimed that `write` always flushes, so `into_inner`
could not fail. That only holds on the success path: `write` flushes at the
very end, so a `write` that returned an error leaves rows buffered, and the
flush inside `into_inner` can then fail and panic.

`into_inner` returns `W`, so there is nowhere to report that. Document it
instead of asserting it cannot happen.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A doc comment placed after `#[inline]`, `#[deprecated]` or
`#[proc_macro_derive]` still renders, but it splits the doc block in two and
reads as if the attribute belongs to the section. Move the new `# Panics`
sections above the attribute, where the rest of the docs are.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two of the documented panics are deliberate, and the docs now say so rather
than leaving the reader to wonder why the function does not report the failure:

* `try_for_each_valid_idx` takes the caller's error type, so it has no error
  value to build
* the `arrow-integration-test` JSON readers parse fixtures, where malformed
  input is a broken test rather than bad data

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
emilk and others added 2 commits August 20, 2026 12:55
`MutableArrayData::try_with_capacities` arrived with apache#10675. It only turns
the dictionary key overflow into an error, and still panics for the other
cases, so say so under a heading the lint recognises.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant