(MAINT) FIx clippy for Rust projects#1466
Open
michaeltlombardi wants to merge 14 commits intoPowerShell:mainfrom
Open
(MAINT) FIx clippy for Rust projects#1466michaeltlombardi wants to merge 14 commits intoPowerShell:mainfrom
michaeltlombardi wants to merge 14 commits intoPowerShell:mainfrom
Conversation
Prior to this change, the clippy invocation in `Build-RustProject` never actually invoked clippy because the check required _all_ projects to be set to require clippy with this check: ```powershell $Clippy -and !$Project.ClippyUnclean ``` However, this _never_ evaluated to true because the function invokes against the full list of packages. This change addresses the problem by: 1. Defining a new self-contained `Test-Clippy` function, which encapsulates the logic for arranging and invoking clippy against DSC rust projects. It now invokes clippy twice, once for general (non-pedantic) projects and again for pedantic projects. This can potentially be optimized in the future by adding linting configuration to project cargo manifests. 1. Updates the `Build-RustProject` function to use `Test-Clippy`. 1. Defines a new field in `.project.data.json`, `RustPackageName`, which is needed to specify specific packages for `cargo` commands when the containing folder for a crate has a different name from the value of the `package.name` field in a cargo manifest. This change required updating the validating JSON schema for the data files and updating the type definition in build helpers to match. 1. Adds a new instance method, `ToPackageFlags()`, to `DscProjectDefinition` to enable quickly emitting the `-p <package_name>` pair for use in invoking cargo commands. Now that the clippy checks are applying, the build will fail locally and in CI. Follow up changes will be made to separately fix each crate.
Prior to this change, the `exe_exists()` test for the `tryWhich()` configuration function looked for `dsc`, which may not exist when the project has not successfully built yet. This change uses `cargo` instead, which is guaranteed to be available when this test is executed.
e6029b7 to
48b7e4f
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes the build pipeline’s Rust clippy integration so -Clippy actually lints the intended crates, adds per-crate package-name metadata for correct cargo -p <name> targeting, and applies clippy-driven refactors across multiple Rust crates/resources.
Changes:
- Add
RustPackageNameto.project.data.json(and VS Code schema) and exposeDscProjectDefinition.ToPackageFlags()forcargo -ptargeting. - Introduce
Test-Clippyand routeBuild-RustProject -Clippythrough it (including separate pedantic vs non-pedantic runs). - Apply clippy/style fixes across Rust projects (pattern matching, let-chains, minor refactors, trait derivations/impls).
Reviewed changes
Copilot reviewed 54 out of 55 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/dsctest/src/refresh_env.rs | Simplifies registry-value matching logic for env refresh. |
| tools/dsctest/src/copy_resource.rs | Minor iterator tweak (next_back) for clippy/clarity. |
| resources/WindowsUpdate/src/windows_update/set.rs | Error construction + control-flow refactors for clippy. |
| resources/WindowsUpdate/src/windows_update/get.rs | Error construction + control-flow refactors for clippy. |
| resources/WindowsUpdate/src/windows_update/export.rs | Filter logic refactors and minor formatting/flow changes. |
| resources/WindowsUpdate/.project.data.json | Adds RustPackageName for correct cargo package selection. |
| resources/windows_service/src/service.rs | Let-chain refactors and minor flow simplification. |
| resources/windows_firewall/src/firewall.rs | Let-chain refactor in protocol/ports validation. |
| resources/sshdconfig/src/repeat_keyword.rs | Let-chain refactors in JSON parsing/manipulation helpers. |
| resources/sshdconfig/src/parser.rs | Let-chain refactors in parser validation/argument handling. |
| resources/sshdconfig/src/get.rs | Let-chain refactor when collecting inherited defaults. |
| resources/sshdconfig/src/formatter.rs | Let-chain refactor for array-empty validation. |
| resources/sshdconfig/src/export.rs | Let-chain refactor when comparing structured keywords. |
| resources/sshdconfig/.project.data.json | Adds RustPackageName for correct cargo package selection. |
| resources/runcommandonset/.project.data.json | Adds RustPackageName for correct cargo package selection. |
| resources/registry/src/main.rs | Let-chain refactor for what-if delete fast-path. |
| resources/registry/.project.data.json | Adds RustPackageName for correct cargo package selection. |
| resources/process/.project.data.json | Adds RustPackageName for correct cargo package selection. |
| resources/osinfo/.project.data.json | Adds RustPackageName for correct cargo package selection. |
| resources/dscecho/src/main.rs | Let-chain refactor for secure-value detection helper. |
| resources/dscecho/.project.data.json | Adds RustPackageName for correct cargo package selection. |
| resources/dism_dsc/src/optional_feature/export.rs | Let-chain refactor in wildcard filter logic. |
| resources/dism_dsc/src/feature_on_demand/export.rs | Let-chain refactor in wildcard filter logic. |
| lib/dsc-lib/src/types/wildcard_type_name.rs | Minor error-path refactor for clippy; iterator adjustment. |
| lib/dsc-lib/src/types/semantic_version.rs | Trait-derivation/impl adjustments and comparison refactor. |
| lib/dsc-lib/src/types/semantic_version_req.rs | Trait-derivation/impl adjustments; Default derive cleanup. |
| lib/dsc-lib/src/types/resource_version.rs | Hash impl refactor; matches! usage; string-compare tweaks. |
| lib/dsc-lib/src/types/resource_version_req.rs | Hash impl refactor; matches! usage; string-compare tweaks. |
| lib/dsc-lib/src/types/fully_qualified_type_name.rs | Adds Default derive; minor argument passing cleanup. |
| lib/dsc-lib/src/types/exit_codes_map.rs | LazyLock init tweak and default-map comparison change. |
| lib/dsc-lib/src/types/exit_code.rs | Manual Hash impl + related derives adjustments. |
| lib/dsc-lib/src/types/date_version.rs | Manual Hash impl + validation and map-based refactors. |
| lib/dsc-lib/src/lib.rs | Removes #[must_use] attribute on find_resource. |
| lib/dsc-lib/src/functions/try_which.rs | Test now asserts cargo is discoverable instead of dsc. |
| lib/dsc-lib/src/functions/data_uri_to_string.rs | Safer parsing via strip_prefix instead of slicing. |
| lib/dsc-lib/src/functions/contains.rs | Let-chain refactors to reduce nesting in contains checks. |
| lib/dsc-lib/src/dscresources/dscresource.rs | Refactors to avoid redundant borrows and improve flow. |
| lib/dsc-lib/src/dscresources/command_resource.rs | Refactors option handling + what-if selection flow. |
| lib/dsc-lib/src/discovery/mod.rs | Uses std::slice::from_ref and removes #[must_use]. |
| lib/dsc-lib/src/discovery/discovery_trait.rs | Doc comment indentation fix. |
| lib/dsc-lib/src/discovery/command_discovery.rs | Adds clippy allow + minor borrow/refactor adjustments. |
| lib/dsc-lib/src/configure/parameters.rs | Let-chain refactors + minor cleanup in parsing fallback. |
| lib/dsc-lib/src/configure/mod.rs | Multiple let-chain refactors + minor env var set cleanup. |
| lib/dsc-lib/src/configure/depends_on.rs | Let-chain refactor for dependsOn validation gating. |
| lib/dsc-lib-jsonschema/src/vscode/transforms/vscodify_refs_and_defs.rs | Updates API usage to accept &str ids. |
| lib/dsc-lib-jsonschema/src/schema_utility_extensions.rs | Changes defs-key lookup signature to &str + related refactors. |
| lib/dsc-lib-jsonschema/src/dsc_repo/mod.rs | Minor cleanup (early-return simplification, char splits). |
| lib/dsc-lib-jsonschema/src/dsc_repo/dsc_repo_schema.rs | Adds #[must_use] attributes and minor string-split tweaks. |
| lib/dsc-lib-jsonschema/build.rs | Panic formatting refactor for clippy. |
| helpers.build.psm1 | Adds RustPackageName, ToPackageFlags(), and new Test-Clippy; updates Build-RustProject. |
| dsc/src/util.rs | Let-chain refactor for env override handling. |
| dsc/src/subcommand.rs | Let-chain refactor + small functional-style parse mapping. |
| dsc/src/resource_command.rs | Let-chain refactor for passthrough formatting branch. |
| dsc-bicep-ext/src/main.rs | Minor field-init cleanup + connect_info body simplification. |
| .vscode/schemas/definitions.json | Adds RustPackageName schema definition and references. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary
This changeset:
Defines a new self-contained
Test-Clippyfunction, which encapsulates the logic for arranging and invoking clippy against DSC rust projects.It now invokes clippy twice, once for general (non-pedantic) projects and again for pedantic projects. This can potentially be optimized in the future by adding linting configuration to project cargo manifests.
Updates the
Build-RustProjectfunction to useTest-Clippy.Defines a new field in
.project.data.json,RustPackageName, which is needed to specify specific packages forcargocommands when the containing folder for a crate has a different name from the value of thepackage.namefield in a cargo manifest.This change required updating the validating JSON schema for the data files and updating the type definition in build helpers to match.
Adds a new instance method,
ToPackageFlags(), toDscProjectDefinitionto enable quickly emitting the-p <package_name>pair for use in invoking cargo commands.Updates the Rust code across the projects to fix all reported clippy violations.
PR Context
Prior to this change, the clippy invocation in
Build-RustProjectnever actually invoked clippy because the check required all projects to be set to require clippy with this check:However, this never evaluated to true because the function invokes against the full list of packages.
Fixing this check in the build script surfaced dozens of linting violations that will prevent compilation when the checks run as intended. This changeset ensures that clippy actually runs when a developer specifies the
-Clippyflag forbuild.ps1and fixes all discovered violations.