Skip to content

Conversation

SteveL-MSFT
Copy link
Member

PR Summary

Refactor how functions are implemented so that:

  • arg validation happens before function is invoked including remaining args and mandatory args
  • metadata is in a single struct that can be extended in the future
  • returnType is now added and used for listing functions

@SteveL-MSFT SteveL-MSFT requested review from Copilot and tgauth August 6, 2025 00:05
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the function implementation approach by consolidating function metadata into a single FunctionMetadata struct and improving argument validation logic. The refactor enhances type safety and introduces more granular argument type checking.

  • Replaces individual trait methods with a unified get_metadata() method returning FunctionMetadata
  • Transitions from AcceptedArgKind to FunctionArgKind with enhanced type information
  • Implements ordered argument type validation and support for remaining arguments with different type constraints

Reviewed Changes

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

Show a summary per file
File Description
dsc_lib/src/functions/mod.rs Core refactor implementing FunctionMetadata struct and enhanced argument validation logic
dsc_lib/src/functions/*.rs Function implementations updated to use new metadata structure
dsc/src/subcommand.rs Function listing updated to use new metadata approach
dsc_lib/locales/en-us.toml Added localization strings for new error messages
dsc/locales/en-us.toml Updated table header for return types display

@tgauth
Copy link
Collaborator

tgauth commented Aug 6, 2025

I wonder if it is worth further abstracting some of the input arg validation for functions like concat that accept multiple arg types, but expect all args to be of a single type

@SteveL-MSFT
Copy link
Member Author

SteveL-MSFT commented Aug 6, 2025

I wonder if it is worth further abstracting some of the input arg validation for functions like concat that accept multiple arg types, but expect all args to be of a single type

If there is sufficient functions that have same pattern, then I would say yes, but if it's more of a one-off, I'd leave it to the individual function to validate. Many of the comparison functions have similar pattern where both have to be string or number, but I think the rust code doesn't change so it doesn't seem to be saving anything if pre-check.

@SteveL-MSFT SteveL-MSFT added this pull request to the merge queue Aug 6, 2025
Merged via the queue into PowerShell:main with commit 2482bbd Aug 6, 2025
4 checks passed
@SteveL-MSFT SteveL-MSFT deleted the refactor-functions branch August 6, 2025 18:05
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.

2 participants