Skip to content

Conversation

@liamjpeters
Copy link
Contributor

PR Summary

Add UseConsistentParameterSetName rule to detect potential parameter set issues

Parameter set names are case-sensitive in PowerShell (unlike most other elements), which can lead to runtime errors and unexpected behavior when not handled correctly.

The rule performs five key validations:

  1. Missing DefaultParameterSetName - Warns when parameter sets are defined but no default is specified
  2. Duplicate parameter declarations - Detects parameters declared multiple times in the same parameter set
  3. DefaultParameterSetName case mismatch - Warns when the DefaultParameterSetName matches the text but not case of a defined parameter set.
  4. ParameterSetName case inconsistency - Validates consistent casing across all references to the same parameter set
  5. Invalid characters - Prevents newline characters in parameter set names.

Fixes #396 with the exception of ParameterSets that don't have at least one unique parameter.

This won't help with Dynamic Parameters.

PR Checklist

Comment on lines +18 to +22
1. **Missing DefaultParameterSetName** - Warns when parameter sets are used but no default is specified
2. **Multiple parameter declarations** - Detects when a parameter is declared multiple times in the same parameter set. This is ultimately a runtime exception - this check helps catch it sooner.
3. **Case mismatch between DefaultParameterSetName and ParameterSetName** - Ensures consistent casing
4. **Case mismatch between different ParameterSetName values** - Ensures all references to the same parameter set use identical casing
5. **Parameter set names containing newlines** - Warns against using newline characters in parameter set names
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to verify our understanding of parameter sets to review this better, we'll get back to it Liam!

@bergmeister
Copy link
Collaborator

@liamjpeters can you resolve merge conflict please so we can run CI?

@liamjpeters liamjpeters requested a review from a team as a code owner January 30, 2026 17:45
@liamjpeters
Copy link
Contributor Author

@liamjpeters can you resolve merge conflict please so we can run CI?

Done. If you approve the CI runs, I'll fix any issues that may have cropped up

@bergmeister
Copy link
Collaborator

@liamjpeters can you resolve merge conflict please so we can run CI?

Done. If you approve the CI runs, I'll fix any issues that may have cropped up

Thanks for quick turnaround, approved just now. You should be able to run the tests in your fork as well btw (at least that was the case when we were in Azure DevOps before we moved CI to GitHub). If not, let me know, we can look into enabling that generally or just for you as a regular contributor

| [UseCompatibleSyntax](./UseCompatibleSyntax.md) | Warning | No | Yes |
| [UseCompatibleTypes](./UseCompatibleTypes.md) | Warning | No | Yes |
| [UseConsistentIndentation](./UseConsistentIndentation.md) | Warning | No | Yes |
| [UseConsistentParameterSetName](./UseConsistentParameterSetName.md) | Warning | Yes | |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since it's a new rule and Andy's comment indicated we aren't 100% sure on everything, might be best to start with not enabled by default in first release and pending user feedback enable it by default later. My experience over the last years is that users find surprisingly many edge cases even in simple rules and had to learn this the hard way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok cool. I'll definitely take that on board and learn from your hard-earned experience. I'll get this changed to an optional rule over the weekend.

@liamjpeters
Copy link
Contributor Author

@liamjpeters can you resolve merge conflict please so we can run CI?

Done. If you approve the CI runs, I'll fix any issues that may have cropped up

Thanks for quick turnaround, approved just now. You should be able to run the tests in your fork as well btw (at least that was the case when we were in Azure DevOps before we moved CI to GitHub). If not, let me know, we can look into enabling that generally or just for you as a regular contributor

The triggers for the CI are a push or PR to main. My fork is old enough that it still has master as the default branch and I'm honestly not too sure how to change it 😅. Even still, I probably wouldn't do little PRs for testing on my fork. I can, and do, run the tests locally (on windows at least). It'd need workflow_dispatch as a trigger to enable the "Run Workflow" option in the GUI.

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.

New Rule Suggestion: ParameterSetName mismatch.

3 participants