Skip to content

ci: enforce clippy -D warnings and clear the warning backlog (#972) - #1000

Open
tschm wants to merge 9 commits into
sfu-db:mainfrom
tschm:rhiza_fix_972_20261008
Open

tschm wants to merge 9 commits into
sfu-db:mainfrom
tschm:rhiza_fix_972_20261008

Conversation

@tschm

@tschm tschm commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

Both clippy steps in ci.yml exit 0 even when there are warnings. This PR clears the clippy backlog and adds -D warnings, so any new warning fails the build.

There are three commits, in this order:

1. The two real defects

  • fed_rewriter.rs: create_sources checked manual_info.is_some() and then called unwrap(). It now uses if let Some(manual_info) = &db_info.manual_info.
  • sources/trino/mod.rs: the Option<String> producer already matches all six serde_json::Value variants, so its trailing _ => throw!(..) arm could never run. I removed it. The other producers' _ arms are reachable, so they stay.

2. Remaining warnings

  • clippy::result_large_err (32 sites) is allowed crate-wide in lib.rs, with the reason written next to it. Every hit comes from two third-party errors held by value, Trino's (232 B) and BigQuery's (176 B), and they propagate into ConnectorXOutError. Boxing them would change public error types, and the error path is cold.
  • dispatcher.rs: ///! → //! for the module doc. prepare() now returns a named Prepared<'w, S, D> alias (type_complexity).
  • constants.rs: cfg(any(feature = "src_oracle")) → cfg(feature = "src_oracle").
  • mssql/tiberius_impl.rs: decode(..)?.to_owned() cloned the Cow itself, so it is now into_owned(). Two single-arm matches become if let.
  • connectorx-cpp: impl From<Plan> for CXFederatedPlan replaces impl Into, and one let x = ..; x becomes a direct return. The second clippy step uses --all, which covers this crate.

3. CI
Both steps now run cargo clippy ... -- -D warnings --force-warn missing_docs.

#![warn(missing_docs)] (from #986) still reports 662 undocumented items. #986 called out that a blanket -D warnings would conflict with that. --force-warn missing_docs keeps those as visible warnings that -D warnings doesn't promote, so the docs backlog can be worked down separately. Once it's at zero, the flag can go. One side effect is that connectorx-cpp also reports 9 missing_docs warnings in the --all step. They are warnings only.

Verification

On macOS arm64 with clippy 0.1.97 (Rust 1.97):

  • cargo clippy --features all -- -D warnings --force-warn missing_docs exits 0.
  • cargo clippy --features all --all -- -D warnings --force-warn missing_docs exits 0.
  • cargo test --lib --features all: 78 passed, 0 failed.
  • cargo fmt --all --check is clean.

Not addressed

  • Windows-only code (cfg(windows) in the mssql source) isn't linted by this CI, which runs on Linux. I applied the same into_owned() change there anyway for consistency.
  • CI installs the stable toolchain, so a new clippy release that adds lints can turn CI red without any code change. If that turns out to be noisy, pinning the toolchain is the usual fix.
  • The mssql changes touch tiberius_impl.rs, which Phase 4: remove the Tiberius MSSQL backend #962 deletes. Whichever lands second will hit a trivial conflict.

Closes #972

🤖 Generated with Claude Code

tschm and others added 3 commits October 8, 2026 07:28
sfu-db#972)

fed_rewriter::create_sources checked manual_info.is_some() and then
unwrapped it; use if let instead. The Option<String> producer in the
Trino source already matches all six serde_json::Value variants, so
the trailing _ arm could never run.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Allow clippy::result_large_err crate-wide, with the reason: the
  Trino and BigQuery client errors are held by value and boxing them
  would change public error types.
- dispatcher: //! instead of ///! for the module doc; name the
  prepare() return type as Prepared.
- constants: drop single-condition cfg(any(..)).
- mssql: Cow::into_owned instead of cloning the Cow; if let instead of
  single-arm match.
- connectorx-cpp: From<Plan> instead of Into; return the struct
  directly instead of via a let binding.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Add -D warnings to both clippy steps. missing_docs stays a warning via
--force-warn while its backlog (enabled in sfu-db#986) is worked down.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented Oct 8, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.72727% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.12%. Comparing base (118450b) to head (8c60d6e).

Files with missing lines Patch % Lines
connectorx/src/sources/mssql/tiberius_impl.rs 77.77% 2 Missing ⚠️
connectorx/src/fed_rewriter.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1000      +/-   ##
==========================================
- Coverage   45.18%   45.12%   -0.06%     
==========================================
  Files          65       65              
  Lines        9606     9593      -13     
==========================================
- Hits         4340     4329      -11     
+ Misses       5266     5264       -2     
Files with missing lines Coverage Δ
connectorx/src/dispatcher.rs 68.42% <100.00%> (-2.78%) ⬇️
connectorx/src/sources/trino/mod.rs 34.21% <ø> (+0.08%) ⬆️
connectorx/src/fed_rewriter.rs 0.00% <0.00%> (ø)
connectorx/src/sources/mssql/tiberius_impl.rs 91.58% <77.77%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@wangxiaoying

Copy link
Copy Markdown
Contributor

Hi @tschm, thanks for your efforts in maintaining this project! I’ve invited you as a collaborator and hope this makes it easier for you to contribute to ConnectorX.

@tschm
tschm requested a review from saurabh500 October 13, 2026 04:31

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enforce clippy in CI: add -D warnings and clear the 140-warning backlog

3 participants