Skip to content

Commit

Permalink
Refactor arg validation (#2862)
Browse files Browse the repository at this point in the history
  • Loading branch information
@florelis
florelis authored Jan 31, 2023
1 parent 36d4495 commit 5d0e02b
Show file tree
Hide file tree
Showing 25 changed files with 548 additions and 352 deletions.
378 changes: 319 additions & 59 deletions src/AppInstallerCLICore/Argument.cpp
View file

Large diffs are not rendered by default.

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

Large diffs are not rendered by default.

8 changes: 5 additions & 3 deletions src/AppInstallerCLICore/Command.cpp
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,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
View file
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
View file
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
View file
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
View file
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

0 comments on commit 5d0e02b

Please sign in to comment.