microsoft / microsoft/mssql-rs
Reduce removable complexity identified by repository audit
- Dominant language
- Rust
- Stars
- 53
- Forks
- 14
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 137
Description
### Problem statement
A repository-wide, read-only complexity audit identified unused code paths, duplicate tests and fuzz targets, repeated pipeline tasks, and unnecessary direct dependencies. These add maintenance overhead without serving current behavior.
This issue tracks the audit's 13 recommendations, ranked by estimated net lines removable. Scope is **over-engineering and complexity only**, not correctness, security, or performance defects.
### Proposed solution
Revalidate each finding against the current branch before implementation, then address the following independently. Preserve public APIs, supported platforms, error behavior, and meaningful test coverage.
- [ ] **delete: Shadow parameter-encoding fuzzer — approximately 297 lines.** `mssql-tds/fuzz/fuzz_targets/fuzz_parameter_encoding.rs:30` maintains a private parameter-encoding imitation and never calls `mssql_tds`. Remove the target, its entry in `mssql-tds/fuzz/Cargo.toml`, and its five-line invocation in `.pipeline/templates/validation-stages.yml` (around line 598). Any replacement fuzzing should exercise the actual production encoder, not a parallel model.
- [ ] **delete: Unused connection executor framework — approximately 295 lines.** Remove `ConnectionExecutor` and its unused private helper types from `mssql-tds/src/connection/connection_actions.rs:728`. No implementations or consumers were found. Retain the used action-chain model and the provider's existing `execute_action_chain` implementation in `mssql-tds/src/connection_provider/tds_connection_provider.rs:140`.
- [ ] **shrink: Repeated authenticated image imports — approximately 245 lines.** Fifteen task bodies in `.pipeline/sync-container-images.yml:255` differ only in display name, source image, and destination image. Replace their 360 lines with one Azure Pipelines step template and parameterized invocations. Preserve existing job boundaries, conditions, credential handling, display names, and per-task retries; keep anonymous non-Docker-Hub imports separate.
- [ ] **delete: Uncalled mock connection handlers — 225 lines.** Remove `handle_encrypted_connection` at `mssql-mock-tds/src/server.rs:1186–1226` and legacy `handle_connection` at `:1325–1508`. Retain the active TLS-aware connection path. No replacement is needed for these uncalled private functions.
- [ ] **shrink: Repeated Arrow integer tests — approximately 225 lines.** Parameterize the 12 repeated basic, auto-mapping, and non-nullable test bodies in `mssql-py-core/tests/test_bulkcopy_arrow_tinyint.py:16`, `test_bulkcopy_arrow_smallint.py:15`, `test_bulkcopy_arrow_int.py:16`, and `test_bulkcopy_arrow_bigint.py:16`. Use three pytest runners plus case data. Preserve all input arrays, expected rows, SQL/Arrow type pairs, nullable-ID ordering, and independent connection/table lifecycles. Leave distinct narrowing and overflow tests intact.
- [ ] **delete: Unreachable outbound request-header scaffolding — approximately 146 lines.** Remove the unused `TraceActivity` and `QueryNotifications` variants, implementations, dispatch arms, and dedicated self-tests from `mssql-tds/src/message/headers.rs:19`. Neither variant is constructed by production callers. Retain transaction-descriptor headers and required inbound protocol handling.
- [ ] **delete: Duplicate fewer-columns bulk-copy test — 61 lines.** Remove `mssql-py-core/tests/test_bulkcopy_auto_fewer_columns.py`. Its sole test has the same executable AST, integration marker, fixture, SQL, input rows, and assertions as the retained test at `mssql-py-core/tests/test_bulkcopy_column_mismatch.py:197`. Check external filename selectors before removal.
- [ ] **shrink: Duplicate connection-provider fuzz target — approximately 56 lines.** Consolidate `mssql-tds/fuzz/fuzz_targets/fuzz_connection_provider_network.rs:23` into the existing `fuzz_connection_provider` target. Both feed identical input through the same mock transport and connection API with the same defaults. Merge separately maintained corpora, remove the duplicate manifest entry, and remove the five-line CI invocation in `.pipeline/templates/validation-stages.yml` (around line 603). Retarget external invocations as needed.
- [ ] **shrink: Temporary conversion-result bindings — approximately 33 lines.** In `mssql-py-core/src/bulkcopy.rs:242–517`, replace 35 occurrences of `let result = conversion(...)?; Ok(Some(result))` with `conversion(...).map(Some)`. Preserve existing conversions, errors, match structure, and `Ok(None)` fallback; introduce no new abstraction.
- [ ] **yagni: Unused no-mapping adapter mode — approximately 30 lines.** Simplify `PythonRowAdapter` in `mssql-py-core/src/bulkcopy.rs:113` and `:1985` by storing destination metadata and resolved mappings directly instead of in `Option`, and remove sequential fallback serialization. Its only construction, at `mssql-py-core/src/cursor.rs:480`, always supplies metadata and `Some(resolved_mappings)`. Preserve bounds errors, column-count validation, and the separately meaningful optional `expected_source_columns`.
- [ ] **delete: Unused raw-handle helper — 15 lines.** Remove `handle_from_raw_mut` and its documentation/suppression at `mssql-odbc/src/handles/mod.rs:146–160`. No callers were found in source, tests, or fuzz support. It is crate-private and not exported through the C ABI. Update the related helper reference in `.github/instructions/mssql-odbc.instructions.md` if needed.
- [ ] **delete: Unused CLI argument scaffolding and dependency declarations — approximately 13 lines, excluding documentation updates.** Remove never-parsed `Args` and its `clap::Parser` import from `mssql-tds-cli/src/main.rs:28`; remove CLI dependencies `clap`, `tracing-subscriber`, `async-trait`, and `futures` from `mssql-tds-cli/Cargo.toml`. Correct `mssql-tds-cli/README.md` so it no longer advertises the unused configuration-file option. Also remove the unused `async-trait` declaration from `mssql-tds/fuzz/Cargo.toml:15`; the main TDS crate retains its own required dependency.
- [ ] **stdlib: Standard tracing initialization cells — approximately 4 lines.** Replace `once_cell::sync::OnceCell` with `std::sync::OnceLock` in `mssql-js/src/tracing_init.rs:4` and `mssql-py-core/src/tracing_init.rs:5`, and remove `once_cell` from both manifests. The scoped uses require only `new` and `set`; merge imports with existing synchronization imports.
### Affected crate
Not applicable / Multiple
Affected areas: `mssql-tds`, `mssql-mock-tds`, `mssql-py-core`, `mssql-odbc`, `mssql-tds-cli`, `mssql-js`, and Azure Pipelines configuration.
### Alternatives considered
- Leave current implementations unchanged. This avoids immediate refactoring work but retains duplicate maintenance and unused branches.
- Wire speculative branches into new functionality. Do this only when backed by a concrete requirement; this issue does not propose adding features to justify otherwise unused code.
- Avoid a broad framework or general-purpose abstraction. Prefer deletion, existing implementations, small pytest parameterizations, a focused pipeline step template, and standard-library replacements.
### Additional context
Audited revision: `aa2ce0e21bac3a44a901e022c9d7b78fbc787742`. Line references correspond to that revision and may shift.
Estimated total: **1,645 net lines removable and 5 distinct direct dependencies removable from the identified consumers** (`clap`, `tracing-subscriber`, `async-trait`, `futures`, `once_cell`). These are estimates, include adjacent comments and tests, and do not imply those packages disappear from the resolved dependency graph: other crates/transitive consumers still use some of them. Replacement sizes and documentation changes can affect the final total.
Tags: `delete` = unused/redundant code; `shrink` = same behavior with less duplication; `yagni` = “You Aren't Gonna Need It,” flexibility with no current caller; `stdlib` = use a standard-library equivalent.
No implementation changes were made during the audit. For follow-up PRs, preserve intended behavior and coverage, update directly related documentation and CI references, and run the applicable repository checks (`cargo bfmt`, `cargo bclippy`, `cargo btest`, plus excluded-crate checks for `mssql-py-core` and relevant Python/pipeline tests).
Contributor guide
Research direction
Start by selecting one independently scoped checklist item, then inspect its named files and revalidate the audit finding against the current branch. Run the applicable Rust, Python, pipeline, or fuzz checks listed in the issue; done means the selected duplication or unused path is removed while behavior, coverage, CI references, and documentation remain correct.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, rust
- Domain
- backend, ci-cd, testing, tooling
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 38/100