Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor arg validation #2862

Merged
merged 9 commits into from
Jan 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
378 changes: 319 additions & 59 deletions src/AppInstallerCLICore/Argument.cpp

Large diffs are not rendered by default.

212 changes: 131 additions & 81 deletions src/AppInstallerCLICore/Argument.h

Large diffs are not rendered by default.

8 changes: 5 additions & 3 deletions src/AppInstallerCLICore/Command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ namespace AppInstaller::CLI

infoOut << '[';

if (arg.Alias() == Argument::NoAlias)
if (arg.Alias() == ArgumentCommon::NoAlias)
{
infoOut << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << arg.Name();
}
Expand Down Expand Up @@ -704,10 +704,12 @@ namespace AppInstaller::CLI
if (Manifest::ConvertToScopeEnum(execArgs.GetArg(Execution::Args::Type::InstallScope)) == Manifest::ScopeEnum::Unknown)
{
auto validOptions = Utility::Join(", "_liv, std::vector<Utility::LocIndString>{ "user"_lis, "machine"_lis});
throw CommandException(Resource::String::InvalidArgumentValueError(s_ArgumentName_Scope, validOptions));
throw CommandException(Resource::String::InvalidArgumentValueError(ArgumentCommon::ForType(Execution::Args::Type::InstallScope).Name, validOptions));
}
}

Argument::ValidateExclusiveArguments(execArgs);

ValidateArgumentsInternal(execArgs);
}

Expand Down Expand Up @@ -780,7 +782,7 @@ namespace AppInstaller::CLI
{
for (const auto& arg : stateMachine.Arguments())
{
if (arg.Alias() != Argument::NoAlias)
if (arg.Alias() != ArgumentCommon::NoAlias)
{
context.Reporter.Completion() << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << arg.Alias() << std::endl;
}
Expand Down
6 changes: 3 additions & 3 deletions src/AppInstallerCLICore/Commands/CompleteCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ namespace AppInstaller::CLI
std::vector<Argument> CompleteCommand::GetArguments() const
{
return {
Argument{ "word", Argument::NoAlias, Args::Type::Word, Resource::String::WordArgumentDescription, ArgumentType::Standard, Argument::Visibility::Example, true },
Argument{ "commandline", Argument::NoAlias, Args::Type::CommandLine, Resource::String::CommandLineArgumentDescription, ArgumentType::Standard, Argument::Visibility::Example, true },
Argument{ "position", Argument::NoAlias, Args::Type::Position, Resource::String::PositionArgumentDescription, ArgumentType::Standard, Argument::Visibility::Example, true },
Argument{ Args::Type::Word, Resource::String::WordArgumentDescription, ArgumentType::Standard, Argument::Visibility::Example, true },
Argument{ Args::Type::CommandLine, Resource::String::CommandLineArgumentDescription, ArgumentType::Standard, Argument::Visibility::Example, true },
Argument{ Args::Type::Position, Resource::String::PositionArgumentDescription, ArgumentType::Standard, Argument::Visibility::Example, true },
};
}

Expand Down
6 changes: 3 additions & 3 deletions src/AppInstallerCLICore/Commands/ExportCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ namespace AppInstaller::CLI
std::vector<Argument> ExportCommand::GetArguments() const
{
return {
Argument{ "output", 'o', Execution::Args::Type::OutputFile, Resource::String::OutputFileArgumentDescription, ArgumentType::Positional, true },
Argument{ "source", 's', Execution::Args::Type::Source, Resource::String::ExportSourceArgumentDescription, ArgumentType::Standard },
Argument{ "include-versions", Argument::NoAlias, Execution::Args::Type::IncludeVersions, Resource::String::ExportIncludeVersionsArgumentDescription, ArgumentType::Flag },
Argument{ Execution::Args::Type::OutputFile, Resource::String::OutputFileArgumentDescription, ArgumentType::Positional, true },
Argument{ Execution::Args::Type::Source, Resource::String::ExportSourceArgumentDescription, ArgumentType::Standard },
Argument{ Execution::Args::Type::IncludeVersions, Resource::String::ExportIncludeVersionsArgumentDescription, ArgumentType::Flag },
Argument::ForType(Execution::Args::Type::AcceptSourceAgreements),
};
}
Expand Down
6 changes: 3 additions & 3 deletions src/AppInstallerCLICore/Commands/ImportCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ namespace AppInstaller::CLI
std::vector<Argument> ImportCommand::GetArguments() const
{
return {
Argument{ "import-file", 'i', Execution::Args::Type::ImportFile, Resource::String::ImportFileArgumentDescription, ArgumentType::Positional, true },
Argument{ "ignore-unavailable", Argument::NoAlias, Execution::Args::Type::IgnoreUnavailable, Resource::String::ImportIgnoreUnavailableArgumentDescription, ArgumentType::Flag },
Argument{ "ignore-versions", Argument::NoAlias, Execution::Args::Type::IgnoreVersions, Resource::String::ImportIgnorePackageVersionsArgumentDescription, ArgumentType::Flag },
Argument{ Execution::Args::Type::ImportFile, Resource::String::ImportFileArgumentDescription, ArgumentType::Positional, true },
Argument{ Execution::Args::Type::IgnoreUnavailable, Resource::String::ImportIgnoreUnavailableArgumentDescription, ArgumentType::Flag },
Argument{ Execution::Args::Type::IgnoreVersions, Resource::String::ImportIgnorePackageVersionsArgumentDescription, ArgumentType::Flag },
Argument::ForType(Execution::Args::Type::NoUpgrade),
Argument::ForType(Execution::Args::Type::AcceptPackageAgreements),
Argument::ForType(Execution::Args::Type::AcceptSourceAgreements),
Expand Down
18 changes: 2 additions & 16 deletions src/AppInstallerCLICore/Commands/InstallCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ namespace AppInstaller::CLI
Argument::ForType(Args::Type::Version),
Argument::ForType(Args::Type::Channel),
Argument::ForType(Args::Type::Source),
Argument{ s_ArgumentName_Scope, Argument::NoAlias, Args::Type::InstallScope, Resource::String::InstallScopeDescription, ArgumentType::Standard, Argument::Visibility::Help },
Argument{ Args::Type::InstallScope, Resource::String::InstallScopeDescription, ArgumentType::Standard, Argument::Visibility::Help },
Argument::ForType(Args::Type::InstallArchitecture),
Argument::ForType(Args::Type::Exact),
Argument::ForType(Args::Type::Interactive),
Expand Down Expand Up @@ -95,21 +95,7 @@ namespace AppInstaller::CLI

void InstallCommand::ValidateArgumentsInternal(Args& execArgs) const
{
Argument::ValidatePackageSelectionArgumentSupplied(execArgs);

if (execArgs.Contains(Args::Type::Manifest) &&
(execArgs.Contains(Args::Type::Query) ||
execArgs.Contains(Args::Type::Id) ||
execArgs.Contains(Args::Type::Name) ||
execArgs.Contains(Args::Type::Moniker) ||
execArgs.Contains(Args::Type::Version) ||
execArgs.Contains(Args::Type::Channel) ||
execArgs.Contains(Args::Type::Source) ||
execArgs.Contains(Args::Type::Exact)))
{
throw CommandException(Resource::String::BothManifestAndSearchQueryProvided);
}

Argument::ValidateCommonArguments(execArgs);
}

void InstallCommand::ExecuteInternal(Context& context) const
Expand Down
2 changes: 1 addition & 1 deletion src/AppInstallerCLICore/Commands/ListCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace AppInstaller::CLI
Argument::ForType(Execution::Args::Type::Command),
Argument::ForType(Execution::Args::Type::Count),
Argument::ForType(Execution::Args::Type::Exact),
Argument{ s_ArgumentName_Scope, Argument::NoAlias, Execution::Args::Type::InstallScope, Resource::String::InstalledScopeArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help },
Argument{ Execution::Args::Type::InstallScope, Resource::String::InstalledScopeArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help },
Argument::ForType(Execution::Args::Type::CustomHeader),
Argument::ForType(Execution::Args::Type::AcceptSourceAgreements),
};
Expand Down
10 changes: 3 additions & 7 deletions src/AppInstallerCLICore/Commands/PinCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ namespace AppInstaller::CLI
Argument::ForType(Args::Type::Tag),
Argument::ForType(Args::Type::Command),
Argument::ForType(Args::Type::Exact),
Argument{ "version"_liv, 'v', Args::Type::GatedVersion, Resource::String::GatedVersionArgumentDescription, ArgumentType::Standard },
Argument{ Args::Type::GatedVersion, Resource::String::GatedVersionArgumentDescription, ArgumentType::Standard },
Argument::ForType(Args::Type::Source),
Argument::ForType(Args::Type::CustomHeader),
Argument::ForType(Args::Type::AcceptSourceAgreements),
Argument::ForType(Args::Type::Force),
Argument{ "blocking"_liv, Argument::NoAlias, Args::Type::BlockingPin, Resource::String::PinAddBlockingArgumentDescription, ArgumentType::Flag },
Argument{ Args::Type::BlockingPin, Resource::String::PinAddBlockingArgumentDescription, ArgumentType::Flag },
};
}

Expand Down Expand Up @@ -107,11 +107,7 @@ namespace AppInstaller::CLI

void PinAddCommand::ValidateArgumentsInternal(Execution::Args& execArgs) const
{
if (execArgs.Contains(Execution::Args::Type::GatedVersion) && execArgs.Contains(Execution::Args::Type::BlockingPin))
{
throw CommandException(Resource::String::BothGatedVersionAndBlockingFlagProvided);
}

Argument::ValidateCommonArguments(execArgs);
}

void PinAddCommand::ExecuteInternal(Execution::Context& context) const
Expand Down
6 changes: 3 additions & 3 deletions src/AppInstallerCLICore/Commands/RootCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ namespace AppInstaller::CLI
{
return
{
Argument{ "version", 'v', Execution::Args::Type::ListVersions, Resource::String::ToolVersionArgumentDescription, ArgumentType::Flag, Argument::Visibility::Help },
Argument{ "info", Argument::NoAlias, Execution::Args::Type::Info, Resource::String::ToolInfoArgumentDescription, ArgumentType::Flag, Argument::Visibility::Help },
Argument{ Execution::Args::Type::ToolVersion, Resource::String::ToolVersionArgumentDescription, ArgumentType::Flag, Argument::Visibility::Help },
Argument{ Execution::Args::Type::Info, Resource::String::ToolInfoArgumentDescription, ArgumentType::Flag, Argument::Visibility::Help },
};
}

Expand Down Expand Up @@ -211,7 +211,7 @@ namespace AppInstaller::CLI

OutputGroupPolicies(context);
}
else if (context.Args.Contains(Execution::Args::Type::ListVersions))
else if (context.Args.Contains(Execution::Args::Type::ToolVersion))
{
context.Reporter.Info() << 'v' << Runtime::GetClientVersion() << std::endl;
}
Expand Down
2 changes: 1 addition & 1 deletion src/AppInstallerCLICore/Commands/SearchCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ namespace AppInstaller::CLI

void SearchCommand::ValidateArgumentsInternal(Args& execArgs) const
{
Argument::ValidatePackageSelectionArgumentSupplied(execArgs);
Argument::ValidateCommonArguments(execArgs);
}

void SearchCommand::ExecuteInternal(Context& context) const
Expand Down
16 changes: 4 additions & 12 deletions src/AppInstallerCLICore/Commands/SettingsCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@ namespace AppInstaller::CLI

namespace
{
constexpr Utility::LocIndView s_ArgumentName_Enable = "enable"_liv;
constexpr Utility::LocIndView s_ArgumentName_Disable = "disable"_liv;
constexpr Utility::LocIndView s_ArgName_EnableAndDisable = "enable|disable"_liv;
Utility::LocIndView s_SettingsCommand_HelpLink = "https://aka.ms/winget-settings"_liv;
}

Expand All @@ -29,8 +26,8 @@ namespace AppInstaller::CLI
std::vector<Argument> SettingsCommand::GetArguments() const
{
return {
Argument{ s_ArgumentName_Enable, Argument::NoAlias, Execution::Args::Type::AdminSettingEnable, Resource::String::AdminSettingEnableDescription, ArgumentType::Standard, Argument::Visibility::Help },
Argument{ s_ArgumentName_Disable, Argument::NoAlias, Execution::Args::Type::AdminSettingDisable, Resource::String::AdminSettingDisableDescription, ArgumentType::Standard, Argument::Visibility::Help },
Argument{ Execution::Args::Type::AdminSettingEnable, Resource::String::AdminSettingEnableDescription, ArgumentType::Standard, Argument::Visibility::Help },
Argument{ Execution::Args::Type::AdminSettingDisable, Resource::String::AdminSettingDisableDescription, ArgumentType::Standard, Argument::Visibility::Help },
};
}

Expand All @@ -51,11 +48,6 @@ namespace AppInstaller::CLI

void SettingsCommand::ValidateArgumentsInternal(Execution::Args& execArgs) const
{
if (execArgs.Contains(Execution::Args::Type::AdminSettingEnable) && execArgs.Contains(Execution::Args::Type::AdminSettingDisable))
{
throw CommandException(Resource::String::TooManyAdminSettingArgumentsError(s_ArgName_EnableAndDisable));
}

// Get admin setting string for all available options except Unknown
using AdminSetting_t = std::underlying_type_t<AdminSetting>;
std::vector<Utility::LocIndString> adminSettingList;
Expand All @@ -68,12 +60,12 @@ namespace AppInstaller::CLI

if (execArgs.Contains(Execution::Args::Type::AdminSettingEnable) && AdminSetting::Unknown == StringToAdminSetting(execArgs.GetArg(Execution::Args::Type::AdminSettingEnable)))
{
throw CommandException(Resource::String::InvalidArgumentValueError(s_ArgumentName_Enable, validOptions));
throw CommandException(Resource::String::InvalidArgumentValueError(ArgumentCommon::ForType(Execution::Args::Type::AdminSettingEnable).Name, validOptions));
}

if (execArgs.Contains(Execution::Args::Type::AdminSettingDisable) && AdminSetting::Unknown == StringToAdminSetting(execArgs.GetArg(Execution::Args::Type::AdminSettingDisable)))
{
throw CommandException(Resource::String::InvalidArgumentValueError(s_ArgumentName_Disable, validOptions));
throw CommandException(Resource::String::InvalidArgumentValueError(ArgumentCommon::ForType(Execution::Args::Type::AdminSettingDisable).Name, validOptions));
}
}

Expand Down
19 changes: 3 additions & 16 deletions src/AppInstallerCLICore/Commands/ShowCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ namespace AppInstaller::CLI
return {
Argument::ForType(Execution::Args::Type::Query),
// The manifest argument from Argument::ForType can be blocked by Group Policy but we don't want that here
Argument{ "manifest", 'm', Execution::Args::Type::Manifest, Resource::String::ManifestArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help },
Argument{ Execution::Args::Type::Manifest, Resource::String::ManifestArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help },
Argument::ForType(Execution::Args::Type::Id),
Argument::ForType(Execution::Args::Type::Name),
Argument::ForType(Execution::Args::Type::Moniker),
Argument::ForType(Execution::Args::Type::Version),
Argument::ForType(Execution::Args::Type::Channel),
Argument::ForType(Execution::Args::Type::Source),
Argument::ForType(Execution::Args::Type::Exact),
Argument{ s_ArgumentName_Scope, Argument::NoAlias, Args::Type::InstallScope, Resource::String::InstallScopeDescription, ArgumentType::Standard, Argument::Visibility::Help },
Argument{ Args::Type::InstallScope, Resource::String::InstallScopeDescription, ArgumentType::Standard, Argument::Visibility::Help },
Argument::ForType(Execution::Args::Type::InstallArchitecture),
Argument::ForType(Execution::Args::Type::Locale),
Argument::ForType(Execution::Args::Type::ListVersions),
Expand Down Expand Up @@ -56,20 +56,7 @@ namespace AppInstaller::CLI

void ShowCommand::ValidateArgumentsInternal(Args& execArgs) const
{
Argument::ValidatePackageSelectionArgumentSupplied(execArgs);

if (execArgs.Contains(Args::Type::Manifest) &&
(execArgs.Contains(Args::Type::Query) ||
execArgs.Contains(Args::Type::Id) ||
execArgs.Contains(Args::Type::Name) ||
execArgs.Contains(Args::Type::Moniker) ||
execArgs.Contains(Args::Type::Version) ||
execArgs.Contains(Args::Type::Channel) ||
execArgs.Contains(Args::Type::Source) ||
execArgs.Contains(Args::Type::Exact)))
{
throw CommandException(Resource::String::BothManifestAndSearchQueryProvided);
}
Argument::ValidateCommonArguments(execArgs);
}

void ShowCommand::ExecuteInternal(Execution::Context& context) const
Expand Down
2 changes: 1 addition & 1 deletion src/AppInstallerCLICore/Commands/SourceCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ namespace AppInstaller::CLI
{
return {
Argument::ForType(Args::Type::SourceName),
Argument{ "force", Argument::NoAlias, Args::Type::ForceSourceReset, Resource::String::SourceResetForceArgumentDescription, ArgumentType::Flag },
Argument{ Args::Type::ForceSourceReset, Resource::String::SourceResetForceArgumentDescription, ArgumentType::Flag },
};
}

Expand Down
23 changes: 2 additions & 21 deletions src/AppInstallerCLICore/Commands/UninstallCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ namespace AppInstaller::CLI
Argument::ForType(Args::Type::Channel),
Argument::ForType(Args::Type::Source),
Argument::ForType(Args::Type::Exact),
Argument{ s_ArgumentName_Scope, Argument::NoAlias, Execution::Args::Type::InstallScope, Resource::String::InstalledScopeArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help },
Argument{ Execution::Args::Type::InstallScope, Resource::String::InstalledScopeArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help },
Argument::ForType(Args::Type::Interactive),
Argument::ForType(Args::Type::Silent),
Argument::ForType(Args::Type::Force),
Expand Down Expand Up @@ -89,26 +89,7 @@ namespace AppInstaller::CLI

void UninstallCommand::ValidateArgumentsInternal(Execution::Args& execArgs) const
{
Argument::ValidatePackageSelectionArgumentSupplied(execArgs);

if (execArgs.Contains(Execution::Args::Type::Manifest) &&
(execArgs.Contains(Execution::Args::Type::Query) ||
execArgs.Contains(Execution::Args::Type::Id) ||
execArgs.Contains(Execution::Args::Type::Name) ||
execArgs.Contains(Execution::Args::Type::Moniker) ||
execArgs.Contains(Execution::Args::Type::ProductCode) ||
execArgs.Contains(Execution::Args::Type::Version) ||
execArgs.Contains(Execution::Args::Type::Channel) ||
execArgs.Contains(Execution::Args::Type::Source) ||
execArgs.Contains(Execution::Args::Type::Exact)))
{
throw CommandException(Resource::String::BothManifestAndSearchQueryProvided);
}

if (execArgs.Contains(Execution::Args::Type::Purge) && execArgs.Contains(Execution::Args::Type::Preserve))
{
throw CommandException(Resource::String::BothPurgeAndPreserveFlagsProvided);
}
Argument::ValidateCommonArguments(execArgs);
}

void UninstallCommand::ExecuteInternal(Execution::Context& context) const
Expand Down
Loading