From f2fa763d8248bc7b566b2a09aa14bc8efa2d4677 Mon Sep 17 00:00:00 2001 From: Boxiao Cao <9083193+antiphoton@users.noreply.github.com> Date: Fri, 25 Jun 2021 14:29:53 -0700 Subject: [PATCH] Sync for validator/cpp/engine and Validator rollup (#35052) * Simplify regex release version variable naming. If the release version can't be determined then return unknown instead of standard. PiperOrigin-RevId: 380675772 * cl/380675772 Simplify regex release version variable naming Co-authored-by: honeybadgerdontcare --- validator/cpp/engine/validator-internal.cc | 27 ++++++++------------ validator/js/engine/htmlparser-interface.js | 26 ++++++++----------- validator/testdata/feature_tests/regexps.out | 2 ++ 3 files changed, 22 insertions(+), 33 deletions(-) diff --git a/validator/cpp/engine/validator-internal.cc b/validator/cpp/engine/validator-internal.cc index d832170ba65e..c2da5e0168d9 100644 --- a/validator/cpp/engine/validator-internal.cc +++ b/validator/cpp/engine/validator-internal.cc @@ -115,7 +115,13 @@ ABSL_FLAG(bool, allow_module_nomodule, true, namespace amp::validator { -// LTS JavaScript (note these are the same as kNomoduleLtsScriptPathRe): +// Standard and Nomodule JavaScript: +// v0.js +// v0/amp-ad-0.1.js +static const LazyRE2 kStandardScriptPathRe = { + R"re((v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\.js)re"}; + +// LTS and Nomodule LTS JavaScript: // lts/v0.js // lts/v0/amp-ad-0.1.js static const LazyRE2 kLtsScriptPathRe = { @@ -127,24 +133,12 @@ static const LazyRE2 kLtsScriptPathRe = { static const LazyRE2 kModuleScriptPathRe = { R"re((v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\.mjs)re"}; -// Nomodule JavaScript: -// v0.js -// v0/amp-ad-0.1.js -static const LazyRE2 kNomoduleScriptPathRe = { - R"re((v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\.js)re"}; - // Module LTS JavaScript: // lts/v0.mjs // lts/v0/amp-ad-0.1.mjs static const LazyRE2 kModuleLtsScriptPathRe = { R"re(lts/(v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\.mjs)re"}; -// Nomodule LTS JavaScript (note these are the same as kLtsScriptPathRe): -// lts/v0.js -// lts/v0/amp-ad-0.1.js -static const LazyRE2 kNomoduleLtsScriptPathRe = { - R"re(lts/(v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\.js)re"}; - // Runtime JavaScript: // v0.js // v0.mjs @@ -332,17 +326,16 @@ ScriptTag ParseScriptTag(htmlparser::Node* node) { // Determine the release version (LTS, module, standard, etc). if ((has_module_attr && RE2::FullMatch(src, *kModuleLtsScriptPathRe)) || - (has_nomodule_attr && - RE2::FullMatch(src, *kNomoduleLtsScriptPathRe))) { + (has_nomodule_attr && RE2::FullMatch(src, *kLtsScriptPathRe))) { script_tag.release_version = ScriptReleaseVersion::MODULE_NOMODULE_LTS; } else if ((has_module_attr && RE2::FullMatch(src, *kModuleScriptPathRe)) || (has_nomodule_attr && - RE2::FullMatch(src, *kNomoduleScriptPathRe))) { + RE2::FullMatch(src, *kStandardScriptPathRe))) { script_tag.release_version = ScriptReleaseVersion::MODULE_NOMODULE; } else if (RE2::FullMatch(src, *kLtsScriptPathRe)) { script_tag.release_version = ScriptReleaseVersion::LTS; - } else { + } else if (RE2::FullMatch(src, *kStandardScriptPathRe)) { script_tag.release_version = ScriptReleaseVersion::STANDARD; } } diff --git a/validator/js/engine/htmlparser-interface.js b/validator/js/engine/htmlparser-interface.js index 6adb5e8b8e51..94f4fa1bfb97 100644 --- a/validator/js/engine/htmlparser-interface.js +++ b/validator/js/engine/htmlparser-interface.js @@ -89,7 +89,13 @@ exports.ScriptReleaseVersion = ScriptReleaseVersion; // AMP domain const /** string */ ampProjectDomain = 'https://cdn.ampproject.org/'; -// LTS JavaScript: +// Standard and Nomodule JavaScript: +// v0.js +// v0/amp-ad-0.1.js +const /** !RegExp */ standardScriptPathRegex = + new RegExp('(v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\\.js$', 'i'); + +// LTS and Nomodule LTS JavaScript: // lts/v0.js // lts/v0/amp-ad-0.1.js const /** !RegExp */ ltsScriptPathRegex = @@ -101,24 +107,12 @@ const /** !RegExp */ ltsScriptPathRegex = const /** !RegExp */ moduleScriptPathRegex = new RegExp('(v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\\.mjs$', 'i'); -// Nomodule JavaScript: -// v0.js -// v0/amp-ad-0.1.js -const /** !RegExp */ nomoduleScriptPathRegex = - new RegExp('(v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\\.js$', 'i'); - // Module LTS JavaScript: // lts/v0.mjs // lts/v0/amp-ad-0.1.mjs const /** !RegExp */ moduleLtsScriptPathRegex = new RegExp('lts/(v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\\.mjs$', 'i'); -// Nomodule LTS JavaScript: -// lts/v0.js -// lts/v0/amp-ad-0.1.js -const /** !RegExp */ nomoduleLtsScriptPathRegex = - new RegExp('lts/(v0|v0/amp-[a-z0-9-]*-[a-z0-9.]*)\\.js$', 'i'); - // Runtime JavaScript: // v0.js // v0.mjs @@ -193,15 +187,15 @@ const ScriptTag = class { // Determine the release version (LTS, module, standard, etc). if ((is_module && moduleLtsScriptPathRegex.test(path)) || - (is_nomodule && nomoduleLtsScriptPathRegex.test(path))) { + (is_nomodule && ltsScriptPathRegex.test(path))) { this.release_version = ScriptReleaseVersion.MODULE_NOMODULE_LTS; } else if ( (is_module && moduleScriptPathRegex.test(path)) || - (is_nomodule && nomoduleScriptPathRegex.test(path))) { + (is_nomodule && standardScriptPathRegex.test(path))) { this.release_version = ScriptReleaseVersion.MODULE_NOMODULE; } else if (ltsScriptPathRegex.test(path)) { this.release_version = ScriptReleaseVersion.LTS; - } else { + } else if (standardScriptPathRegex.test(path)) { this.release_version = ScriptReleaseVersion.STANDARD; } } diff --git a/validator/testdata/feature_tests/regexps.out b/validator/testdata/feature_tests/regexps.out index 69a48d150611..e12bd64ad4be 100644 --- a/validator/testdata/feature_tests/regexps.out +++ b/validator/testdata/feature_tests/regexps.out @@ -38,6 +38,8 @@ feature_tests/regexps.html:27:2 The mandatory attribute 'amp-custom' is missing | | >> ^~~~~~~~~ +feature_tests/regexps.html:36:2 The script version for 'amp-vine' is a unknown version which mismatches with the first script on the page using the standard version. (see https://amp.dev/documentation/guides-and-tutorials/learn/spec/amphtml#required-markup) +>> ^~~~~~~~~ feature_tests/regexps.html:36:2 The attribute 'src' in tag 'amp-vine extension script' is set to the invalid value 'https://cdn.ampproject.org/v0/amp-vine-0.1.js?foobar'. (see https://amp.dev/documentation/components/amp-vine) | >> ^~~~~~~~~