(GH-538) Define newtypes for `tags` field by michaeltlombardi · Pull Request #1382 · PowerShell/DSC

michaeltlombardi · 2026-02-09T19:58:11Z

PR Summary

This change addresses limitation in representing the tags field by defining two newtypes:

Tag as a wrapper around String that follows the "parse, don't validate" pattern to move pattern validation into the constructor. It also treats comparisons with tags case-insensitively. The defined JSON Schema for this type is marked as inline because it is always and only used to validate items in the tags field for various structs.
TagList as a transparent wrapper around HashSet<Tag>. It provides no additional functionality, it just enables us to define a JSON Schema for this reusable type. The representation here uses HashSet rather than Vec because the items in a tag list must be unique (JSON Schema keyword uniqueItems defined as true).

This change modifies the code for the ResourceManifest and ExtensionManifest structs to use TagList instead of Option<Vec<String>> and updates other code as necessary to address the change in type.

PR Context

Prior to this change, the representation for both resource and extension tags was defined as Option<Vec<String>>. With this construction, we can't define a reusable schema for tag lists (because we don't own the JsonSchema trait or the Option<T> type).

The existing implementation also doesn't adhere to the validation from the canonical JSON Schema for tags, which defines the regex pattern ^\w+$ for tags.

Copilot

Pull request overview

This PR introduces two newtypes (Tag and TagList) to replace the previous Option<Vec<String>> representation for tags in resource and extension manifests. The change implements the "parse, don't validate" pattern by moving tag validation into the constructor and enables case-insensitive tag comparisons while providing canonical JSON schemas for these types.

Changes:

Defines Tag newtype wrapping String with regex validation (^\w+$) and case-insensitive comparison/hashing
Defines TagList newtype wrapping HashSet<Tag> to ensure uniqueness and provide reusable schema
Updates ResourceManifest and ExtensionManifest to use TagList instead of Option<Vec<String>>
Adds comprehensive integration tests following the repository's testing patterns
Updates tag filtering logic in list_resources to leverage case-insensitive comparison

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
lib/dsc-lib/src/types/tag.rs	Defines `Tag` newtype with validation pattern `^\w+$`, case-insensitive comparison/hashing, and extensive trait implementations
lib/dsc-lib/src/types/tag_list.rs	Defines `TagList` newtype wrapping `HashSet<Tag>` with ergonomic trait implementations
lib/dsc-lib/src/types/mod.rs	Exports new `Tag` and `TagList` types
lib/dsc-lib/src/extensions/extension_manifest.rs	Changes `tags` field from `Option<Vec<String>>` to `TagList` with `#[serde(default)]`
lib/dsc-lib/src/dscresources/resource_manifest.rs	Changes `tags` field from `Option<Vec<String>>` to `TagList` with `#[serde(default)]`
lib/dsc-lib/src/dscerror.rs	Adds `InvalidTag` error variant for tag validation failures
lib/dsc-lib/locales/en-us.toml	Adds localized error message keys for invalid tag errors
lib/dsc-lib/locales/schemas.definitions.yaml	Adds comprehensive documentation for `tag` and `tags` schema definitions
dsc/src/subcommand.rs	Updates tag filtering to use `is_empty()` and case-insensitive comparison via `Tag` type
lib/dsc-lib/tests/integration/types/tag.rs	Comprehensive integration tests for `Tag` covering methods, schema, serde, and traits
lib/dsc-lib/tests/integration/types/tag_list.rs	Comprehensive integration tests for `TagList` covering schema, serde, and traits
lib/dsc-lib/tests/integration/types/mod.rs	Registers new test modules for `tag` and `tag_list`

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

lib/dsc-lib/tests/integration/types/tag_list.rs

lib/dsc-lib/tests/integration/types/tag.rs

lib/dsc-lib/src/types/tag_list.rs

lib/dsc-lib/src/types/tag.rs

lib/dsc-lib/src/dscerror.rs

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

lib/dsc-lib/src/types/tag_list.rs

lib/dsc-lib/tests/integration/types/tag_list.rs

lib/dsc-lib/locales/schemas.definitions.yaml

lib/dsc-lib/src/types/tag.rs

dsc/src/subcommand.rs

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

lib/dsc-lib/src/types/tag.rs

lib/dsc-lib/locales/schemas.definitions.yaml

Prior to this change, the representation for both resource and extension tags was defined as `Option<Vec<String>>`. With this construction, we can't define a reusable schema for tag lists (because we don't own the `JsonSchema` trait or the `Option<T>` type). The existing implementation also doesn't adhere to the validation from the canonical JSON Schema for tags, which defines the regex pattern `^\w+$` for tags. This change addresses these limitations by defining two newtypes: - `Tag` as a wrapper around `String` that follows the "parse, don't validate" pattern to move pattern validation into the constructor. It also treats comparisons with tags case-insensitively. The defined JSON Schema for this type is marked as `inline` because it is always and only used to validate items in the `tags` field for various structs. - `TagList` as a transparent wrapper around `HashSet<Tag>`. It provides no additional functionality, it just enables us to define a JSON Schema for this reusable type. The representation here uses `HashSet` rather than `Vec` because the items in a tag list must be unique (JSON Schema keyword `uniqueItems` defined as `true`). We can still skip serializing when the `HashSet` is empty, like we can skip serializing when an `Option` is `None`.

Prior to this change, the resource manifest struct defined the `tags` field as `Option<Vec<String>>`. This change updates the field to use the `TagList` newtype, which wraps `HashSet<Tag>` to provide better semantics and JSON Schema for the type. This change ensures that references to the `tags` field are updated through the code base.

Prior to this change, the extension manifest struct defined the `tags` field as `Option<Vec<String>>`. This change updates the field to use the `TagList` newtype, which wraps `HashSet<Tag>` to provide better semantics and JSON Schema for the type. The `tags` field isn't directly accessed in any existing code.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-12T17:52:57Z

lib/dsc-lib/src/types/tag_list.rs

+#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema, DscRepoSchema)]
+#[schemars(
+    title = t!("schemas.definitions.tags.title"),
+    description = t!("schemas.definitions.tags.description"),
+    extend(
+        "markdownDescription" = t!("schemas.definitions.tags.markdownDescription"),
+    )
+)]
+#[dsc_repo_schema(
+    base_name = "tags",
+    folder_path = "definitions",
+)]
+pub struct TagList(HashSet<Tag>);
+


TagList derives Serialize over a HashSet<Tag>, which means JSON/YAML serialization order will be non-deterministic (HashSet iteration order varies between runs/platforms). If manifests/resources are ever serialized (e.g., CLI JSON output), this can cause unstable output and flaky tests. Consider implementing a custom Serialize for TagList that emits a sorted array (you already have Ord on Tag), or switch the backing collection to BTreeSet<Tag> for deterministic iteration/serialization.

Copilot · 2026-02-12T17:52:58Z

lib/dsc-lib/src/types/tag.rs

+
+impl Hash for Tag {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.0.to_lowercase().hash(state)


Tag hashing lowercases by allocating a new String (to_lowercase()) on every hash call. Because Tag is used in HashSet/HashMap, this can become a hot path (multiple hashes per insert/lookup). Consider hashing by iterating over self.0.chars() and feeding each character’s lowercase mapping into the hasher to avoid allocating, while keeping behavior consistent with your case-insensitive equality.

Suggested change

self.0.to_lowercase().hash(state)

for ch in self.0.chars() {

for lower in ch.to_lowercase() {

lower.hash(state);

}

}

Copilot · 2026-02-12T17:52:58Z

lib/dsc-lib/src/dscerror.rs

+    #[error("{t} '{0}' - {t2}: '{1}'", t = t!("dscerror.invalidTagPrefix"), t2 = t!("dscerror.invalidTagSuffix"))]
+    InvalidTag(String, String),
+
    #[error("{t} '{0}' - {t2}: '{1}'", t = t!("dscerror.invalidTypeNamePrefix"), t2 = t!("dscerror.InvalidTypeNameSuffix"))]


The i18n key for the InvalidTypeName suffix uses dscerror.InvalidTypeNameSuffix (capital I), but the locale file defines dscerror.invalidTypeNameSuffix. This will likely cause the error message to fall back to the raw key instead of the translation. Update the key to match the locale entry (or add the missing key in the locale file).

Suggested change

#[error("{t} '{0}' - {t2}: '{1}'", t = t!("dscerror.invalidTypeNamePrefix"), t2 = t!("dscerror.InvalidTypeNameSuffix"))]

#[error("{t} '{0}' - {t2}: '{1}'", t = t!("dscerror.invalidTypeNamePrefix"), t2 = t!("dscerror.invalidTypeNameSuffix"))]

Copilot · 2026-02-12T17:52:58Z

lib/dsc-lib/tests/integration/types/tag.rs

+        #[test_case("abc_123_DEF" => matches Ok(_); "mixed valid characters string is valid")]
+        #[test_case("a.b" => matches Err(_); "string with invalid character raises error")]
+        #[test_case("" => matches Err(_); "empty string raises error")]
+        fn str(text: &str)-> Result<Tag, DscError> {


Minor formatting nit: keep spacing consistent with the rest of the tests (e.g., add a space before -> in the return type).

Suggested change

fn str(text: &str)-> Result<Tag, DscError> {

fn str(text: &str) -> Result<Tag, DscError> {

michaeltlombardi force-pushed the gh-538/main/tag-newtype branch 5 times, most recently from 6840532 to c3026f6 Compare February 11, 2026 19:06

michaeltlombardi requested a review from Copilot February 11, 2026 23:12

Copilot started reviewing on behalf of michaeltlombardi February 11, 2026 23:13 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

michaeltlombardi force-pushed the gh-538/main/tag-newtype branch from c3026f6 to 329dde3 Compare February 12, 2026 16:29

michaeltlombardi requested a review from Copilot February 12, 2026 16:30

Copilot started reviewing on behalf of michaeltlombardi February 12, 2026 16:31 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

michaeltlombardi force-pushed the gh-538/main/tag-newtype branch from 329dde3 to 7f56f8c Compare February 12, 2026 17:30

michaeltlombardi requested a review from Copilot February 12, 2026 17:31

Copilot started reviewing on behalf of michaeltlombardi February 12, 2026 17:31 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

lib/dsc-lib/src/types/tag.rs Outdated Show resolved Hide resolved

lib/dsc-lib/src/types/tag.rs Show resolved Hide resolved

lib/dsc-lib/locales/schemas.definitions.yaml Outdated Show resolved Hide resolved

michaeltlombardi added 3 commits February 12, 2026 11:43

michaeltlombardi force-pushed the gh-538/main/tag-newtype branch from 7f56f8c to 7434453 Compare February 12, 2026 17:44

michaeltlombardi requested a review from Copilot February 12, 2026 17:44

Copilot started reviewing on behalf of michaeltlombardi February 12, 2026 17:45 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

-        self.0.to_lowercase().hash(state)
+        for ch in self.0.chars() {
+            for lower in ch.to_lowercase() {
+                lower.hash(state);
+            }
+        }

	#[error("{t} '{0}' - {t2}: '{1}'", t = t!("dscerror.invalidTypeNamePrefix"), t2 = t!("dscerror.InvalidTypeNameSuffix"))]
	#[error("{t} '{0}' - {t2}: '{1}'", t = t!("dscerror.invalidTypeNamePrefix"), t2 = t!("dscerror.invalidTypeNameSuffix"))]

	fn str(text: &str)-> Result<Tag, DscError> {
	fn str(text: &str) -> Result<Tag, DscError> {

Conversation

michaeltlombardi commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

michaeltlombardi commented Feb 9, 2026 •

edited

Loading