Skip to content

Commit

Permalink
Migrate from cfg = "host" to cfg = "exec"
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 488423550
  • Loading branch information
Googler authored and banaag committed Dec 1, 2023
1 parent 795d3c3 commit 61b5845
Show file tree
Hide file tree
Showing 31 changed files with 90 additions and 134 deletions.
2 changes: 1 addition & 1 deletion validator/cpp/engine/embed_data.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ embed_data = rule(
),
"header_generator": attr.label(
executable = True,
cfg = "host",
cfg = "exec",
allow_files = True,
default = Label(
"//cpp/engine/scripts:filecontents_to_cpp_header",
Expand Down
2 changes: 1 addition & 1 deletion validator/cpp/engine/parse-layout-sizes.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace amp::validator::parse_layout_sizes {

// WARNING: This code is still in development and not ready to be used.

// This is a single representation for the CssSizes object.
// This is a single represenation for the CssSizes object.
// It consists of at least a valid size and a possible media condition.
// See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#Attributes
struct CssSize {
Expand Down
2 changes: 1 addition & 1 deletion validator/cpp/engine/parse-srcset_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ TEST(ParseSrcsetTest, LeadingAndTrailingCommasAndCommasInUrl) {
EqCandidates({{"example.com/,/,/,/,50w", "1x"}}));
}

TEST(ParseSrcsetTest, NoWhitespace) {
TEST(ParseSrcsetTest, NoWhitepsace) {
SrcsetParsingResult result = ParseSourceSet("image 100w,image 50w");
EXPECT_TRUE(result.success);
EXPECT_THAT(result.srcset_images,
Expand Down
2 changes: 0 additions & 2 deletions validator/cpp/engine/testing-utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ const std::map<std::string, TestCase>& TestCases() {
"external/amphtml-extensions/*/*/test/*.html",
&html_files)) << "Test cases file pattern not found.";

CHECK(!html_files.empty()) << "Validator test cases are empty. Will not proceed.";

std::sort(html_files.begin(), html_files.end());
for (const std::string& html_file : html_files) {
if (html_file.find("/js_only/") != std::string::npos) continue;
Expand Down
2 changes: 1 addition & 1 deletion validator/cpp/engine/utf8-util_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ TEST(Utf8UtilTest, Utf16StrLen) {
// It's 34 bytes long and 22 utf-8 characters long. Javascript uses UTF16
// strings and string lengths.
// The chars in Iñtërnâtiônàlizætiøn vary between 1 and 2 byte lengths, all
// javascript 1-char lengths. The ⚡ is a 3-byte length character, with a
// javascript 1-char lenghts. The ⚡ is a 3-byte length character, with a
// 1-char javascript length. Finally the 💩 is a 4-byte length character with
// a 2-char javascript length.
EXPECT_EQ(Utf16StrLen("Iñtërnâtiônàlizætiøn☃💩"), 23);
Expand Down
106 changes: 41 additions & 65 deletions validator/cpp/engine/validator-internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -311,62 +311,42 @@ ScriptTag ParseScriptTag(htmlparser::Node* node) {
}
}

if (src.empty()) {
return script_tag;
}

std::string src_str{src};
// Determine if this has a valid AMP domain and separate the path from the
// attribute 'src'. Consumes the domain making src just the path.
if (absl::ConsumePrefix(&src, kAmpProjectDomain)) {
script_tag.is_amp_domain = true;
script_tag.path = src_str;
} else {
script_tag.is_amp_domain = false;
htmlparser::URL url(src_str);
// Error cases, early exit:
if (!url.is_valid()) return script_tag;
if (!url.has_protocol()) return script_tag;
if (url.protocol() != "https" && url.protocol() != "http")
return script_tag;
if (url.hostname().empty()) return script_tag;

src = url.path_params_fragment().data();
// Trim the "/" prefix as this is what kExtensionPathRe expects.
if (!src.empty() && src[0] == '/') src = src.substr(1);
std::string src_str{src};
script_tag.path = src_str;
}

// Only look at script tags that have attribute 'async'.
if (has_async_attr) {
// Determine if this is the AMP Runtime.
if (!script_tag.is_extension &&
RE2::FullMatch(src, *kRuntimeScriptPathRe)) {
script_tag.is_runtime = true;
script_tag.has_valid_path = true;
}

// For AMP Extensions, validate path and extract name and version.
if (script_tag.is_extension &&
RE2::FullMatch(src, *kExtensionPathRe, &script_tag.extension_name,
&script_tag.extension_version)) {
script_tag.has_valid_path = true;
}

// Determine the release version (LTS, module, standard, etc).
if ((has_module_attr && RE2::FullMatch(src, *kModuleLtsScriptPathRe)) ||
(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, *kStandardScriptPathRe))) {
script_tag.release_version = ScriptReleaseVersion::MODULE_NOMODULE;
} else if (RE2::FullMatch(src, *kLtsScriptPathRe)) {
script_tag.release_version = ScriptReleaseVersion::LTS;
} else if (RE2::FullMatch(src, *kStandardScriptPathRe)) {
script_tag.release_version = ScriptReleaseVersion::STANDARD;
script_tag.path = std::string(src);

// Only look at script tags that have attribute 'async'.
if (has_async_attr) {
// Determine if this is the AMP Runtime.
if (!script_tag.is_extension &&
RE2::FullMatch(src, *kRuntimeScriptPathRe)) {
script_tag.is_runtime = true;
script_tag.has_valid_path = true;
}

// For AMP Extensions, validate path and extract name and version.
if (script_tag.is_extension &&
RE2::FullMatch(src, *kExtensionPathRe, &script_tag.extension_name,
&script_tag.extension_version)) {
script_tag.has_valid_path = true;
}

// Determine the release version (LTS, module, standard, etc).
if ((has_module_attr && RE2::FullMatch(src, *kModuleLtsScriptPathRe)) ||
(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, *kStandardScriptPathRe))) {
script_tag.release_version = ScriptReleaseVersion::MODULE_NOMODULE;
} else if (RE2::FullMatch(src, *kLtsScriptPathRe)) {
script_tag.release_version = ScriptReleaseVersion::LTS;
} else if (RE2::FullMatch(src, *kStandardScriptPathRe)) {
script_tag.release_version = ScriptReleaseVersion::STANDARD;
}
}
}
return script_tag;
Expand Down Expand Up @@ -1192,7 +1172,7 @@ class ParsedTagSpec {

// Whether or not the tag should be recorded via
// Context->RecordTagspecValidated if it was validated
// successfully. For performance, this is only done for tags that
// successfullly. For performance, this is only done for tags that
// are mandatory, unique, or possibly required by some other tag.
RecordValidated ShouldRecordTagspecValidated() const {
return should_record_tagspec_validated_;
Expand Down Expand Up @@ -1284,7 +1264,7 @@ std::string TagSpecUrl(const TagSpec& spec) {
return StrCat(extension_spec_url_prefix, spec.extension_spec().name());
if (spec.requires_extension_size() > 0)
// Return the first |requires_extension|, which should be the most
// representative.
// representitive.
return StrCat(extension_spec_url_prefix, spec.requires_extension(0));

return "";
Expand Down Expand Up @@ -2834,11 +2814,11 @@ class InvalidRuleVisitor : public htmlparser::css::RuleVisitor {
class InvalidDeclVisitor : public htmlparser::css::RuleVisitor {
public:
InvalidDeclVisitor(const ParsedDocCssSpec& css_spec, Context* context,
const std::string& tag_descriptive_name,
const std::string& tag_decriptive_name,
ValidationResult* result)
: css_spec_(css_spec),
context_(context),
tag_descriptive_name_(tag_descriptive_name),
tag_descriptive_name_(tag_decriptive_name),
result_(result) {}

void VisitDeclaration(
Expand Down Expand Up @@ -3996,14 +3976,8 @@ void ValidateAmpScriptSrcAttr(const ParsedHtmlTag& tag,
const TagSpec& tag_spec, const Context& context,
ValidationResult* result) {
if (!tag.IsAmpDomain()) {
bool is_amp_format =
c_find(context.type_identifiers(), TypeIdentifier::kAmp) !=
context.type_identifiers().end();
if (!is_amp_format || context.is_transformed()) {
context.AddError(ValidationError::DISALLOWED_AMP_DOMAIN,
context.line_col(),
/*params=*/{}, /*spec_url=*/"", result);
}
context.AddError(ValidationError::DISALLOWED_AMP_DOMAIN, context.line_col(),
/*params=*/{}, /*spec_url=*/"", result);
}

if (tag.IsExtensionScript() && tag_spec.has_extension_spec()) {
Expand Down Expand Up @@ -5686,7 +5660,9 @@ class ParsedValidatorRulesProvider {
class Validator {
public:
Validator(const ParsedValidatorRules* rules, int max_errors = -1)
: rules_(rules), max_errors_(max_errors), context_(rules_, max_errors_) {}
: rules_(rules),
max_errors_(max_errors),
context_(rules_, max_errors_) {}

ValidationResult Validate(const htmlparser::Document& doc) {
doc_metadata_ = doc.Metadata();
Expand Down
24 changes: 12 additions & 12 deletions validator/cpp/engine/validator_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,8 @@ TEST(ValidatorTest, TestCssLengthAmpEmail) {
":13:2 The author stylesheet specified in tag 'style amp-custom' "
"is too long - document contains 75001 bytes whereas the "
"limit is 75000 "
"bytes. (see https://amp.dev/documentation/guides-and-tutorials/email/learn/"
"spec/amphtml#maximum-size)");
"bytes. (see https://amp.dev/documentation/guides-and-tutorials/learn/"
"spec/amphtml/#maximum-size)");
EXPECT_EQ(expected_output, output) << "test case " << test_case_name;
}

Expand Down Expand Up @@ -527,7 +527,7 @@ TEST(ValidatorTest, TestCssLengthAmpEmail) {
":19:6 The author stylesheet specified in tag 'style amp-custom' "
"and the combined inline styles is too large - document contains 75010 "
"bytes whereas the limit is 75000 bytes. (see https://amp.dev/"
"documentation/guides-and-tutorials/email/learn/spec/amphtml#maximum-size)");
"documentation/guides-and-tutorials/learn/spec/amphtml/#maximum-size)");
EXPECT_EQ(expected_output, output) << "test case " << test_case_name;
}

Expand Down Expand Up @@ -555,7 +555,7 @@ TEST(ValidatorTest, TestCssLengthAmpEmail) {
":7519:6 The author stylesheet specified in tag 'style amp-custom' "
"and the combined inline styles is too large - document contains 75014 "
"bytes whereas the limit is 75000 bytes. (see https://amp.dev/"
"documentation/guides-and-tutorials/email/learn/spec/amphtml#maximum-size)");
"documentation/guides-and-tutorials/learn/spec/amphtml/#maximum-size)");
EXPECT_EQ(expected_output, output) << "test case " << test_case_name;
}

Expand Down Expand Up @@ -601,8 +601,8 @@ TEST(ValidatorTest, TestCssLengthAmpEmail) {
test_case_name,
":17:2 The inline style specified in tag 'div' is too long - it "
"contains 1001 bytes whereas the limit is 1000 bytes. (see "
"https://amp.dev/documentation/guides-and-tutorials/email/learn/spec/"
"amphtml#maximum-size)");
"https://amp.dev/documentation/guides-and-tutorials/learn/spec/"
"amphtml/#maximum-size)");
EXPECT_EQ(expected_output, output) << "test case " << test_case_name;
}
}
Expand Down Expand Up @@ -649,8 +649,8 @@ TEST(ValidatorTest, TestCssLengthAmpEmailStrict) {
":13:2 The author stylesheet specified in tag 'style amp-custom' "
"is too long - document contains 75001 bytes whereas the "
"limit is 75000 "
"bytes. (see https://amp.dev/documentation/guides-and-tutorials/email/learn/"
"spec/amphtml#maximum-size)");
"bytes. (see https://amp.dev/documentation/guides-and-tutorials/learn/"
"spec/amphtml/#maximum-size)");
EXPECT_EQ(expected_output, output) << "test case " << test_case_name;
}

Expand Down Expand Up @@ -681,7 +681,7 @@ TEST(ValidatorTest, TestCssLengthAmpEmailStrict) {
":19:6 The author stylesheet specified in tag 'style amp-custom' "
"and the combined inline styles is too large - document contains 75010 "
"bytes whereas the limit is 75000 bytes. (see https://amp.dev/"
"documentation/guides-and-tutorials/email/learn/spec/amphtml#maximum-size)");
"documentation/guides-and-tutorials/learn/spec/amphtml/#maximum-size)");
EXPECT_EQ(expected_output, output) << "test case " << test_case_name;
}

Expand All @@ -701,7 +701,7 @@ TEST(ValidatorTest, TestCssLengthAmpEmailStrict) {
":3769:6 The author stylesheet specified in tag 'style amp-custom' "
"and the combined inline styles is too large - document contains 75014 "
"bytes whereas the limit is 75000 bytes. (see https://amp.dev/"
"documentation/guides-and-tutorials/email/learn/spec/amphtml#maximum-size)");
"documentation/guides-and-tutorials/learn/spec/amphtml/#maximum-size)");
EXPECT_EQ(expected_output, output) << "test case " << test_case_name;
}

Expand Down Expand Up @@ -734,8 +734,8 @@ TEST(ValidatorTest, TestCssLengthAmpEmailStrict) {
StrCat("FAIL\n", test_case_name,
":17:2 The inline style specified in tag 'div' is too long - it "
"contains 1001 bytes whereas the limit is 1000 bytes. (see "
"https://amp.dev/documentation/guides-and-tutorials/email/learn/spec/"
"amphtml#maximum-size)");
"https://amp.dev/documentation/guides-and-tutorials/learn/spec/"
"amphtml/#maximum-size)");
EXPECT_EQ(expected_output, output) << "test case " << test_case_name;
}
}
Expand Down
6 changes: 0 additions & 6 deletions validator/cpp/engine/wasm/BUILD
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Wraps AMP Validator into a WebAssembly library,
# which can be used by javascript files.

load("@bazel_skylib//rules:build_test.bzl", "build_test")
load("@emsdk//emscripten_toolchain:wasm_rules.bzl", "wasm_cc_binary")
load("@io_bazel_rules_closure//closure:defs.bzl", "closure_js_binary", "closure_js_library")

Expand Down Expand Up @@ -73,8 +72,3 @@ closure_js_binary(
":validator_js_lib",
],
)

build_test(
name = "validator_js_test",
targets = [":validator_js_bin"],
)
2 changes: 1 addition & 1 deletion validator/cpp/engine/wasm/validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ function digitizeValidationErrorFields(error) {
/**
* When transforming validation errors and validation results from jspb to plain
* objects, the protobuf base64 string is also attached to the output.
* Hence when a plain object needs to be transformed back to protobuf,
* Hence when a plain object neeeds to be transformed back to protobuf,
* the attached base64 could be directly used.
*/
const PB_BASE64 = '_PB_BASE64';
Expand Down
4 changes: 2 additions & 2 deletions validator/cpp/htmlparser/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ cc_test(
)

# Similar to go lang's defer statement. Defers the execution of statement
# until in which it is declared goes out of scope.
# until in which it is decalred goes out of scope.
cc_library(
name = "defer",
hdrs = [
Expand All @@ -80,7 +80,7 @@ cc_library(
copts = ["-std=c++17"],
)

# Helper library declares various doctype constants and a utility function to
# Helper library decalres various doctype constants and a utility function to
# parse doctype string and extract various components in it.
cc_library(
name = "doctype",
Expand Down
8 changes: 4 additions & 4 deletions validator/cpp/htmlparser/allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
// is naturally aligned if the address used to identify it has an 8-byte
// alignment.
//
// Following data structure contains members totaling 13 bytes, but it's actual
// Following data struture contains members totaling 13 bytes, but it's actual
// size is 24 bytes due to 8 byte alignment.
//
// Alignment is always equal to the largest sized element in the structure.
Expand Down Expand Up @@ -201,10 +201,10 @@ class Allocator {
Allocator& operator=(const Allocator&) = delete;

// Allocates memory of same size required to construct object of type T.
// Returns nullptr if allocation failed.
// Returns nullptr if alloction failed.
void* Allocate() {
// Checks if remaining bytes in block are less than object size, or
// remaining bytes after alignment is less than object size.
// reamining bytes after alignment is less than object size.
// Add a new block.
if (object_size_ > remaining_ || !AlignFreeAddress()) {
if (!NewBlock()) return nullptr;
Expand Down Expand Up @@ -338,7 +338,7 @@ class Allocator {
}

// If the block's address is not aligned, moves the pointer to the address
// that is multiple of alignment_.
// that is multiple of aligment_.
bool AlignFreeAddress() {
// Checks how many bytes to skip to be at the correct alignment.
if (const std::size_t skip =
Expand Down
2 changes: 1 addition & 1 deletion validator/cpp/htmlparser/css/parse-css-urls.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ void Preprocess(vector<char32_t>* codepoints) {
out.push_back('\n');
last_codepoint_was_cr = true;
break;
case '\f': // also known as form feed (FF)
case '\f': // also knwon as form feed (FF)
out.push_back('\n');
last_codepoint_was_cr = false;
break;
Expand Down
6 changes: 3 additions & 3 deletions validator/cpp/htmlparser/css/parse-css.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ const std::string& Token::StringValue() const {
}

std::string Token::ToString() const {
// The following are overridden in their class: AT_KEYWORD, CLOSE_CURLY,
// The following are overriden in their class: AT_KEYWORD, CLOSE_CURLY,
// CLOSE_PAREN, CLOSE_SQUARE, DELIM, DIMENSION, FUNCTION_TOKEN, IDENT,
// NUMBER, OPEN_CURLY, OPEN_PAREN, OPEN_SQUARE, PERCENTAGE, STRING, URL
switch (Type()) {
Expand Down Expand Up @@ -343,7 +343,7 @@ bool Whitespace(char32_t code) {
char32_t kMaximumallowedcodepoint = 0x10ffff;

// A MarkedPosition object saves position information from the tokenizer
// provided as |line| and |col| to the constructor and can later write that
// rovided as |line| and |col| to the constructor and can later write that
// position back to a Token object.
class MarkedPosition {
public:
Expand Down Expand Up @@ -2471,7 +2471,7 @@ CombinatorType::Code CombinatorTypeForToken(const Token& token) {
if (IsDelim(token, "+")) return CombinatorType::ADJACENT_SIBLING;
if (IsDelim(token, "~")) return CombinatorType::GENERAL_SIBLING;
// CombinatorTypeForToken is only ever called if the token has one of these
// delimiters, so reaching this point is impossible.
// delimitors, so reaching this point is impossible.
CHECK(false) << absl::StrCat(
"not a combinator token - type=", TokenType::Code_Name(token.Type()),
" value=", token.StringValue());
Expand Down
2 changes: 1 addition & 1 deletion validator/cpp/htmlparser/css/parse-css.h
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,7 @@ class Selector : public Token {
virtual void Accept(SelectorVisitor* visitor) const = 0;
};

// This node models type selectors and universal selectors.
// This node models type selectors and universial selectors.
// http://www.w3.org/TR/css3-selectors/#type-selectors
// http://www.w3.org/TR/css3-selectors/#universal-selector
class TypeSelector : public Selector {
Expand Down
Loading

0 comments on commit 61b5845

Please sign in to comment.