Skip to content

Clippy lints documented as "Enforced in CI" are not enforced anywhere #86

Description

@grokspice

Problem

.github/copilot-instructions.md states, under a heading that says these are enforced in CI:

Errors: clippy::pedantic, clippy::unwrap_used, clippy::expect_used, clippy::clone_on_ref_ptr

Allowed: clippy::module_name_repetitions, clippy::large_futures

CI does not enforce any of them. .github/workflows/build.yml runs:

- name: Cargo clippy
  run: cargo clippy --all-features

with no -D flags, and there is no [lints] table in Cargo.toml, no clippy.toml, and no crate-level #![warn(...)]/#![deny(...)] attributes. All four of those lints are allow by default (pedantic is a group; the other three are restriction), so none of them fire. The only place the list is actually applied is a developer's local rust-analyzer.check.extraArgs, quoted further down the same file.

Two consequences:

  1. The standard is unenforced. Code violating it merges with green CI. Concretely: fix: keep the API key on the origin it was configured for #85 added an .expect(...) in library code and clippy passed on 1.93.1, beta and nightly; it was caught by a reviewer, not by the build.
  2. Reviewers cite a mechanism that does not exist. The review on that PR said the .expect "will fail the repo's clippy configuration" — reasonable to believe from the docs, but false, which makes it hard to tell a real gate from a convention.

Also worth noting: the clippy job in the build_toolchains matrix sets continue-on-error: true, so even genuine warnings there cannot fail the workflow. The hard clippy gate is the separate cargo clippy --all-features step in build_and_test.

Suggested fix

Make the documented set real, in one place, so local and CI agree:

# Cargo.toml
[lints.clippy]
pedantic = { level = "deny", priority = -1 }
unwrap_used = "deny"
expect_used = "deny"
clone_on_ref_ptr = "deny"
module_name_repetitions = "allow"
large_futures = "allow"

A [lints] table is picked up by cargo clippy, cargo build and rust-analyzer alike, which removes the need for extraArgs in the documented VSCode settings.

Two things to decide as part of this:

  • Tests will need an exemption. expect_used/unwrap_used under a blanket deny would fire across the existing test modules, which use .expect(...) extensively (~50 sites in src/client.rs alone). The usual approach is #![cfg_attr(test, allow(clippy::expect_used, clippy::unwrap_used))], or an allow on each test module.
  • Turning pedantic on will surface a backlog in existing code that has never been checked against it. Might be worth landing the three restriction lints first and pedantic separately.

If the intent is that these are only ever local guidance, the opposite fix works too: drop "Enforced in CI" from the heading so the file stops implying a gate that is not there.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions