From b3f74a0dbb356a981fadb2e42e2516ceddc4a27a Mon Sep 17 00:00:00 2001 From: Yao Sun Date: Fri, 6 May 2022 13:18:01 -0700 Subject: [PATCH] Loc error as warning --- src/AppInstallerCLITests/YamlManifest.cpp | 18 ++++++++++++++++++ .../Manifest/ManifestValidation.cpp | 8 ++++---- .../Public/winget/ManifestValidation.h | 8 +++++++- 3 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/AppInstallerCLITests/YamlManifest.cpp b/src/AppInstallerCLITests/YamlManifest.cpp index eac4c782fe..a63b652b09 100644 --- a/src/AppInstallerCLITests/YamlManifest.cpp +++ b/src/AppInstallerCLITests/YamlManifest.cpp @@ -818,4 +818,22 @@ TEST_CASE("ManifestApplyLocale", "[ManifestValidation]") REQUIRE(manifest.CurrentLocalization.Locale == "fr-FR"); REQUIRE(manifest.CurrentLocalization.Get() == "fr-FR package name"); REQUIRE(manifest.CurrentLocalization.Get() == "es-MX publisher"); +} + +TEST_CASE("ManifestLocalizationValidation", "[ManifestValidation]") +{ + Manifest manifest = YamlParser::CreateFromPath(TestDataFile("Manifest-Good-MultiLocale.yaml")); + + // Set 1 locale to bad value + manifest.Localizations.at(0).Locale = "Invalid"; + + // Full validation should detect as error + auto errors = ValidateManifest(manifest, true); + REQUIRE(errors.size() == 1); + REQUIRE(errors.at(0).ErrorLevel == ValidationError::Level::Error); + + // Not full validation should detect as warning + errors = ValidateManifest(manifest, false); + REQUIRE(errors.size() == 1); + REQUIRE(errors.at(0).ErrorLevel == ValidationError::Level::Warning); } \ No newline at end of file diff --git a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp index d1c3957f00..5c00096220 100644 --- a/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp +++ b/src/AppInstallerCommonCore/Manifest/ManifestValidation.cpp @@ -203,20 +203,20 @@ namespace AppInstaller::Manifest // Validate localizations for (auto const& localization : manifest.Localizations) { - auto locErrors = ValidateManifestLocalization(localization); + auto locErrors = ValidateManifestLocalization(localization, !fullValidation); std::move(locErrors.begin(), locErrors.end(), std::inserter(resultErrors, resultErrors.end())); } return resultErrors; } - std::vector ValidateManifestLocalization(const ManifestLocalization& localization) + std::vector ValidateManifestLocalization(const ManifestLocalization& localization, bool treatErrorAsWarning) { std::vector resultErrors; if (!localization.Locale.empty() && !Locale::IsWellFormedBcp47Tag(localization.Locale)) { - resultErrors.emplace_back(ManifestError::InvalidBcp47Value, "PackageLocale", localization.Locale); + resultErrors.emplace_back(ManifestError::InvalidBcp47Value, "PackageLocale", localization.Locale, treatErrorAsWarning ? ValidationError::Level::Warning : ValidationError::Level::Error); } if (localization.Contains(Localization::Agreements)) @@ -227,7 +227,7 @@ namespace AppInstaller::Manifest // At least one must be present if (agreement.Label.empty() && agreement.AgreementText.empty() && agreement.AgreementUrl.empty()) { - resultErrors.emplace_back(ManifestError::InvalidFieldValue, "Agreements"); + resultErrors.emplace_back(ManifestError::InvalidFieldValue, "Agreements", treatErrorAsWarning ? ValidationError::Level::Warning : ValidationError::Level::Error); } } } diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h index 1d6dff5cc5..667e99c02d 100644 --- a/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h +++ b/src/AppInstallerCommonCore/Public/winget/ManifestValidation.h @@ -79,12 +79,18 @@ namespace AppInstaller::Manifest ValidationError(std::string message, std::string field) : Message(std::move(message)), Field(std::move(field)) {} + ValidationError(std::string message, std::string field, Level level) : + Message(std::move(message)), Field(std::move(field)), ErrorLevel(level) {} + ValidationError(std::string message, std::string field, std::string_view value) : Message(std::move(message)), Field(std::move(field)), Value(value) {} ValidationError(std::string message, std::string field, std::string value) : Message(std::move(message)), Field(std::move(field)), Value(std::move(value)) {} + ValidationError(std::string message, std::string field, std::string value, Level level) : + Message(std::move(message)), Field(std::move(field)), Value(std::move(value)), ErrorLevel(level) {} + ValidationError(std::string message, std::string field, std::string value, size_t line, size_t column) : Message(std::move(message)), Field(std::move(field)), Value(std::move(value)), Line(line), Column(column) {} @@ -207,5 +213,5 @@ namespace AppInstaller::Manifest // fullValidation: bool to set if manifest validation should perform extra validation that is not required for reading a manifest. std::vector ValidateManifest(const Manifest& manifest, bool fullValidation = true); - std::vector ValidateManifestLocalization(const ManifestLocalization& localization); + std::vector ValidateManifestLocalization(const ManifestLocalization& localization, bool treatErrorAsWarning = false); } \ No newline at end of file