Skip to content

Mark unreachable panics with #[expect(clippy::missing_panics_doc)] - #10761

Draft
emilk wants to merge 5 commits into
apache:mainfrom
emilk:emilk/panics-expect
Draft

Mark unreachable panics with #[expect(clippy::missing_panics_doc)]#10761
emilk wants to merge 5 commits into
apache:mainfrom
emilk:emilk/panics-expect

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 three PRs is functions whose unwrap cannot fail,
but where removing it would mean restructuring code for no gain. Those get an
#[expect] with a reason, which is the repo's documented convention for a lint
you disagree with on a given item.

With all four merged, clippy::missing_panics_doc is clean across the workspace,
so it can be turned on with the rest of the pedantic lints.

What changes are included in this PR?

  • #[expect(clippy::missing_panics_doc, reason = "...")] on the functions whose
    panic is unreachable, each reason naming the invariant that makes it so, for
    example "an OffsetBuffer is never empty" or "an array always matches the
    concrete type of its data type"
  • file-level #![expect(...)] for tests, benchmarks and the helpers they use,
    since those are free to panic
  • the same for the whole arrow-integration-testing crate, which is
    publish = false and holds only the integration test binaries, so nothing
    downstream reads its docs. arrow-integration-test keeps its # Panics
    sections, since it is published
  • records that convention in CONTRIBUTING.md, next to the existing guidance on
    suppressing lints

The lint itself is still not enabled here; that comes with the rest of the
pedantic lints.

One trivial conflict is expected against #10755: both add something directly
above concat_elements_utf8_many, an # Errors section there and an #[expect]
here. Keep both. It resolves itself when this branch is rebased after #10755
lands.

Are these changes tested?

Verified with cargo clippy --workspace --all-features --all-targets with
missing_panics_doc = "warn" added to [workspace.lints.clippy] locally, on top
of the other three PRs: no warnings, and no unfulfilled expectations.

Are there any user-facing changes?

No.

emilk and others added 5 commits August 19, 2026 21:21
A handful of `unwrap`/`expect`/`assert!` calls cannot fail, but removing
them would need real restructuring. Silence
`clippy::missing_panics_doc` for those with a reason instead of
documenting a panic that cannot happen.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tests, benchmarks and their helpers are free to panic, so silence
`clippy::missing_panics_doc` for those files instead of documenting each
function.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`arrow::util::test_util` and `arrow::util::bench_util` are public, and the
file-level `#![expect(clippy::missing_panics_doc)]` means callers get no panic
documentation at all. State it once in the module docs instead.

Also records the tests-and-benches convention in CONTRIBUTING.md, next to the
existing guidance on suppressing lints.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…many`

The offsets of an array of length `size` always yield `size + 1` values, so
the `peek`/`next` calls over them cannot fail.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The crate is `publish = false` and, in its own words, holds the "binaries used
in the Arrow integration tests". Nothing downstream reads its docs and its
helpers are free to panic, so suppress the lint for the whole crate rather than
documenting a panic per function.

`arrow-integration-test` keeps its `# Panics` sections: it is published, as
support for the Arrow JSON test data format.

Co-Authored-By: Claude Opus 5 (1M context) <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