Improve diagnostics when an invalid triplet name is supplied. (#1497)

* Improve diagnostics when an invalid triplet name is supplied. This fixes a regression introduced in #1474 where an error message is added but the corresponding text being parsed is not. This was reported as microsoft/vcpkg#41143 * Fix the unit tests for "hypens" 😅 * Fix comment for changed variable name
microsoft · Sep 30, 2024 · 48946f6 · 48946f6
1 parent b267c70
commit 48946f6
Show file tree

Hide file tree

Showing 9 changed files with 87 additions and 44 deletions.
diff --git a/azure-pipelines/end-to-end-tests-dir/cli.ps1 b/azure-pipelines/end-to-end-tests-dir/cli.ps1
@@ -50,7 +50,7 @@ $out = Run-VcpkgAndCaptureOutput -TestArgs @('install', 'this-is-super-not-a-#po
 Throw-IfNotFailed
 
 [string]$expected = @"
-error: expected the end of input parsing a package spec; this usually means the indicated character is not allowed to be in a package spec. Port, triplet, and feature names are all lowercase alphanumeric+hypens.
+error: expected the end of input parsing a package spec; this usually means the indicated character is not allowed to be in a package spec. Port, triplet, and feature names are all lowercase alphanumeric+hyphens.
   on expression: this-is-super-not-a-#port
                                      ^
 
@@ -76,6 +76,21 @@ if (-Not ($out.Replace("`r`n", "`n").EndsWith($expected)))
     throw 'Bad malformed --binarysource output; it was: ' + $out
 }
 
+$out = Run-VcpkgAndCaptureOutput -TestArgs @('install', 'zlib', '--triplet', 'ARM-windows')
+Throw-IfNotFailed
+
+$expected = @"
+error: Invalid triplet name. Triplet names are all lowercase alphanumeric+hyphens.
+  on expression: ARM-windows
+                 ^
+Built-in Triplets:
+"@
+
+if (-Not ($out.Replace("`r`n", "`n").StartsWith($expected)))
+{
+    throw 'Bad malformed triplet output. It was: ' + $out
+}
+
 $out = Run-VcpkgAndCaptureStdErr -TestArgs @('x-package-info', 'zlib#notaport', '--x-json', '--x-installed')
 Throw-IfNotFailed
 $expected = @"

diff --git a/include/vcpkg/base/message-data.inc.h b/include/vcpkg/base/message-data.inc.h
@@ -2207,25 +2207,25 @@ DECLARE_MESSAGE(
     (msg::package_name, msg::url),
     "",
     "\"{package_name}\" is not a valid feature name. "
-    "Feature names must be lowercase alphanumeric+hypens and not reserved (see {url} for more information).")
+    "Feature names must be lowercase alphanumeric+hyphens and not reserved (see {url} for more information).")
 DECLARE_MESSAGE(ParseIdentifierError,
                 (msg::value, msg::url),
                 "{value} is a lowercase identifier like 'boost'",
                 "\"{value}\" is not a valid identifier. "
-                "Identifiers must be lowercase alphanumeric+hypens and not reserved (see {url} for more information).")
+                "Identifiers must be lowercase alphanumeric+hyphens and not reserved (see {url} for more information).")
 DECLARE_MESSAGE(
     ParsePackageNameNotEof,
     (msg::url),
     "",
     "expected the end of input parsing a package name; this usually means the indicated character is not allowed to be "
-    "in a port name. Port names are all lowercase alphanumeric+hypens and not reserved (see {url} for more "
+    "in a port name. Port names are all lowercase alphanumeric+hyphens and not reserved (see {url} for more "
     "information).")
 DECLARE_MESSAGE(
     ParsePackageNameError,
     (msg::package_name, msg::url),
     "",
     "\"{package_name}\" is not a valid package name. "
-    "Package names must be lowercase alphanumeric+hypens and not reserved (see {url} for more information).")
+    "Package names must be lowercase alphanumeric+hyphens and not reserved (see {url} for more information).")
 DECLARE_MESSAGE(ParsePackagePatternError,
                 (msg::package_name, msg::url),
                 "",
@@ -2237,11 +2237,15 @@ DECLARE_MESSAGE(
     (),
     "",
     "expected the end of input parsing a package spec; this usually means the indicated character is not allowed to be "
-    "in a package spec. Port, triplet, and feature names are all lowercase alphanumeric+hypens.")
+    "in a package spec. Port, triplet, and feature names are all lowercase alphanumeric+hyphens.")
 DECLARE_MESSAGE(ParseQualifiedSpecifierNotEofSquareBracket,
                 (msg::version_spec),
                 "",
                 "expected the end of input parsing a package spec; did you mean {version_spec} instead?")
+DECLARE_MESSAGE(ParseTripletNotEof,
+                (),
+                "",
+                "Invalid triplet name. Triplet names are all lowercase alphanumeric+hyphens.")
 DECLARE_MESSAGE(PathMustBeAbsolute,
                 (msg::path),
                 "",

diff --git a/include/vcpkg/base/parse.h b/include/vcpkg/base/parse.h
@@ -21,6 +21,10 @@ namespace vcpkg
         int column;
     };
 
+    void append_caret_line(LocalizedString& res,
+                           const Unicode::Utf8Decoder& it,
+                           const Unicode::Utf8Decoder& start_of_line);
+
     struct ParseMessage
     {
         SourceLoc location = {};

diff --git a/locales/messages.json b/locales/messages.json
@@ -1252,19 +1252,20 @@
   "ParagraphExpectedColonAfterField": "expected ':' after field name",
   "ParagraphExpectedFieldName": "expected field name",
   "ParagraphUnexpectedEndOfLine": "unexpected end of line, to span a blank line use \"  .\"",
-  "ParseFeatureNameError": "\"{package_name}\" is not a valid feature name. Feature names must be lowercase alphanumeric+hypens and not reserved (see {url} for more information).",
+  "ParseFeatureNameError": "\"{package_name}\" is not a valid feature name. Feature names must be lowercase alphanumeric+hyphens and not reserved (see {url} for more information).",
   "_ParseFeatureNameError.comment": "An example of {package_name} is zlib. An example of {url} is https://github.com/microsoft/vcpkg.",
-  "ParseIdentifierError": "\"{value}\" is not a valid identifier. Identifiers must be lowercase alphanumeric+hypens and not reserved (see {url} for more information).",
+  "ParseIdentifierError": "\"{value}\" is not a valid identifier. Identifiers must be lowercase alphanumeric+hyphens and not reserved (see {url} for more information).",
   "_ParseIdentifierError.comment": "{value} is a lowercase identifier like 'boost' An example of {url} is https://github.com/microsoft/vcpkg.",
-  "ParsePackageNameError": "\"{package_name}\" is not a valid package name. Package names must be lowercase alphanumeric+hypens and not reserved (see {url} for more information).",
+  "ParsePackageNameError": "\"{package_name}\" is not a valid package name. Package names must be lowercase alphanumeric+hyphens and not reserved (see {url} for more information).",
   "_ParsePackageNameError.comment": "An example of {package_name} is zlib. An example of {url} is https://github.com/microsoft/vcpkg.",
-  "ParsePackageNameNotEof": "expected the end of input parsing a package name; this usually means the indicated character is not allowed to be in a port name. Port names are all lowercase alphanumeric+hypens and not reserved (see {url} for more information).",
+  "ParsePackageNameNotEof": "expected the end of input parsing a package name; this usually means the indicated character is not allowed to be in a port name. Port names are all lowercase alphanumeric+hyphens and not reserved (see {url} for more information).",
   "_ParsePackageNameNotEof.comment": "An example of {url} is https://github.com/microsoft/vcpkg.",
   "ParsePackagePatternError": "\"{package_name}\" is not a valid package pattern. Package patterns must use only one wildcard character (*) and it must be the last character in the pattern (see {url} for more information).",
   "_ParsePackagePatternError.comment": "An example of {package_name} is zlib. An example of {url} is https://github.com/microsoft/vcpkg.",
-  "ParseQualifiedSpecifierNotEof": "expected the end of input parsing a package spec; this usually means the indicated character is not allowed to be in a package spec. Port, triplet, and feature names are all lowercase alphanumeric+hypens.",
+  "ParseQualifiedSpecifierNotEof": "expected the end of input parsing a package spec; this usually means the indicated character is not allowed to be in a package spec. Port, triplet, and feature names are all lowercase alphanumeric+hyphens.",
   "ParseQualifiedSpecifierNotEofSquareBracket": "expected the end of input parsing a package spec; did you mean {version_spec} instead?",
   "_ParseQualifiedSpecifierNotEofSquareBracket.comment": "An example of {version_spec} is zlib:[email protected].",
+  "ParseTripletNotEof": "Invalid triplet name. Triplet names are all lowercase alphanumeric+hyphens.",
   "PathMustBeAbsolute": "Value of environment variable X_VCPKG_REGISTRIES_CACHE is not absolute: {path}",
   "_PathMustBeAbsolute.comment": "An example of {path} is /foo/bar.",
   "PerformingPostBuildValidation": "Performing post-build validation",

diff --git a/src/vcpkg-test/input.cpp b/src/vcpkg-test/input.cpp
@@ -81,7 +81,7 @@ TEST_CASE ("parse_package_spec forbid illegal characters", "[input][parse_packag
     {
         REQUIRE(
             maybe_parsed.error() ==
-            R"(error: expected the end of input parsing a package spec; this usually means the indicated character is not allowed to be in a package spec. Port, triplet, and feature names are all lowercase alphanumeric+hypens.
+            R"(error: expected the end of input parsing a package spec; this usually means the indicated character is not allowed to be in a package spec. Port, triplet, and feature names are all lowercase alphanumeric+hyphens.
   on expression: zlib#notaport
                      ^)");
     }
@@ -111,10 +111,17 @@ TEST_CASE ("check_triplet rejects malformed triplet", "[input][check_triplet]")
     db.available_triplets.push_back(TripletFile{"invalid.triplet_name", "invalid.triplet_name.cmake"});
     auto maybe_check = check_triplet("invalid.triplet_name", db);
     REQUIRE(!maybe_check.has_value());
-    static constexpr StringLiteral expected_error{
-        "error: expected the end of input parsing a package spec; this usually means the indicated character is not "
-        "allowed to be in a package spec. Port, triplet, and feature names are all lowercase alphanumeric+hypens."};
-    REQUIRE(maybe_check.error() == expected_error);
+    REQUIRE(!maybe_check.has_value());
+    REQUIRE(maybe_check.error().data() ==
+            R"(error: Invalid triplet name. Triplet names are all lowercase alphanumeric+hyphens.
+  on expression: invalid.triplet_name
+                        ^
+Built-in Triplets:
+Community Triplets:
+Overlay Triplets from "invalid.triplet_name.cmake":
+  invalid.triplet_name
+See https://learn.microsoft.com/vcpkg/users/triplets?WT.mc_id=vcpkg_inproduct_cli for more information.
+)");
 }
 
 TEST_CASE ("check_and_get_package_spec validates the triplet", "[input][check_and_get_package_spec]")
@@ -153,7 +160,7 @@ TEST_CASE ("check_and_get_package_spec forbids malformed", "[input][check_and_ge
     REQUIRE(
         maybe_spec.error() ==
         LocalizedString::from_raw(
-            R"(error: expected the end of input parsing a package spec; this usually means the indicated character is not allowed to be in a package spec. Port, triplet, and feature names are all lowercase alphanumeric+hypens.
+            R"(error: expected the end of input parsing a package spec; this usually means the indicated character is not allowed to be in a package spec. Port, triplet, and feature names are all lowercase alphanumeric+hyphens.
   on expression: zlib:x86-windows#
                                  ^)"));
 }
@@ -222,7 +229,7 @@ TEST_CASE ("check_and_get_full_package_spec forbids malformed", "[input][check_a
     REQUIRE(
         maybe_spec.error() ==
         LocalizedString::from_raw(
-            R"(error: expected the end of input parsing a package spec; this usually means the indicated character is not allowed to be in a package spec. Port, triplet, and feature names are all lowercase alphanumeric+hypens.
+            R"(error: expected the end of input parsing a package spec; this usually means the indicated character is not allowed to be in a package spec. Port, triplet, and feature names are all lowercase alphanumeric+hyphens.
   on expression: zlib[core]:x86-windows#
                                        ^)"));
 }

diff --git a/src/vcpkg-test/manifests.cpp b/src/vcpkg-test/manifests.cpp
@@ -1419,7 +1419,7 @@ TEST_CASE ("default-feature-empty errors", "[manifests]")
     REQUIRE(!m_pgh.has_value());
     REQUIRE(m_pgh.error().data() ==
             "<test manifest>: error: $.default-features[0] (a feature name): \"\" is not a valid feature name. Feature "
-            "names must be lowercase alphanumeric+hypens and not reserved (see " +
+            "names must be lowercase alphanumeric+hyphens and not reserved (see " +
                 docs::manifests_url + " for more information).");
 }
 
@@ -1432,7 +1432,7 @@ TEST_CASE ("default-feature-empty-object errors", "[manifests]")
     REQUIRE(!m_pgh.has_value());
     REQUIRE(m_pgh.error().data() ==
             "<test manifest>: error: $.default-features[0].name (a feature name): \"\" is not a valid feature name. "
-            "Feature names must be lowercase alphanumeric+hypens and not reserved (see " +
+            "Feature names must be lowercase alphanumeric+hyphens and not reserved (see " +
                 docs::manifests_url + " for more information).");
 }
 
@@ -1445,7 +1445,7 @@ TEST_CASE ("dependency-name-empty errors", "[manifests]")
     REQUIRE(!m_pgh.has_value());
     REQUIRE(m_pgh.error().data() ==
             "<test manifest>: error: $.dependencies[0] (a package name): \"\" is not a valid package name. Package "
-            "names must be lowercase alphanumeric+hypens and not reserved (see " +
+            "names must be lowercase alphanumeric+hyphens and not reserved (see " +
                 docs::manifests_url + " for more information).");
 }
 
@@ -1458,7 +1458,7 @@ TEST_CASE ("dependency-name-empty-object errors", "[manifests]")
     REQUIRE(!m_pgh.has_value());
     REQUIRE(m_pgh.error().data() ==
             "<test manifest>: error: $.dependencies[0].name (a package name): \"\" is not a valid package name. "
-            "Package names must be lowercase alphanumeric+hypens and not reserved (see " +
+            "Package names must be lowercase alphanumeric+hyphens and not reserved (see " +
                 docs::manifests_url + " for more information).");
 }
 
@@ -1545,7 +1545,7 @@ TEST_CASE ("dependency-feature-name-empty errors", "[manifests]")
     REQUIRE(!m_pgh.has_value());
     REQUIRE(m_pgh.error().data() ==
             "<test manifest>: error: $.dependencies[0].features[0] (a feature name): \"\" is not a valid feature name. "
-            "Feature names must be lowercase alphanumeric+hypens and not reserved (see " +
+            "Feature names must be lowercase alphanumeric+hyphens and not reserved (see " +
                 docs::manifests_url + " for more information).");
 }
 
@@ -1563,6 +1563,6 @@ TEST_CASE ("dependency-feature-name-empty-object errors", "[manifests]")
     REQUIRE(!m_pgh.has_value());
     REQUIRE(m_pgh.error().data() ==
             "<test manifest>: error: $.dependencies[0].features[0].name (a feature name): \"\" is not a valid feature "
-            "name. Feature names must be lowercase alphanumeric+hypens and not reserved (see " +
+            "name. Feature names must be lowercase alphanumeric+hyphens and not reserved (see " +
                 docs::manifests_url + " for more information).");
 }