Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

validator rollup #19692

Merged
merged 1 commit into from
Dec 6, 2018
Merged

validator rollup #19692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion extensions/amp-video-docking/0.1/test/validator-amp-video-docking-no-player.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,6 @@ amp-video-docking/0.1/test/validator-amp-video-docking-no-player.html:29:2 The t
| </head>
| <body>
| </body>
| </html>
| </html>
>> ^~~~~~~~~
amp-video-docking/0.1/test/validator-amp-video-docking-no-player.html:33:6 The extension 'amp-video-docking' was found on this page, but is unused. Please remove this extension. [AMP_TAG_PROBLEM]
9 changes: 4 additions & 5 deletions extensions/amp-video-docking/validator-amp-video-docking.protoascii
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,14 @@
#

tags: { # amp-video-docking
attr_lists: "common-extension-attrs"
html_format: AMP
tag_name: "SCRIPT"
spec_name: "amp-video-docking for amp-video"
extension_spec: {
name: "amp-video-docking"
requires_usage: NONE
version: "0.1"
version: "latest"
}
html_format: AMP
requires_extension: "amp-video"
spec_name: "amp-video-docking for amp-video"
tag_name: "SCRIPT"
attr_lists: "common-extension-attrs"
}
62 changes: 60 additions & 2 deletions validator/engine/validator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,11 @@ class ParsedTagSpec {
* @private
*/
this.shouldRecordTagspecValidated_ = shouldRecordTagspecValidated;
/**
* @type {boolean}
* @private
*/
this.attrsCanSatisfyExtension_ = false;
/**
* ParsedAttributes keyed by name.
* @type {!Object<string, number>}
Expand Down Expand Up @@ -667,6 +672,9 @@ class ParsedTagSpec {
if (spec.valueUrl) {
this.containsUrl_ = true;
}
if (spec.requiresExtension.length > 0) {
this.attrsCanSatisfyExtension_ = true;
}
}
}

Expand Down Expand Up @@ -795,6 +803,11 @@ class ParsedTagSpec {
return this.referencePoints_;
}

/** @return {boolean} */
attrsCanSatisfyExtension() {
return this.attrsCanSatisfyExtension_;
}

/**
* @param {string} name
* @return {boolean}
Expand Down Expand Up @@ -2182,6 +2195,16 @@ class ExtensionsContext {
return out;
}

/**
* Records extensions that are used within the document.
* @param {!Array<string>} extensions
*/
recordUsedExtensions(extensions) {
for (const extension of extensions) {
this.extensionsUsed_[extension] = true;
}
}

/**
* Returns a list of unused extensions which produce validation errors
* when unused.
Expand Down Expand Up @@ -2235,8 +2258,7 @@ class ExtensionsContext {

// Record presence of a tag, such as <amp-foo> which requires the usage
// of an amp extension.
for (const requiredExtension of tagSpec.requiresExtension)
{this.extensionsUsed_[requiredExtension] = true;}
this.recordUsedExtensions(tagSpec.requiresExtension);
}
}

Expand Down Expand Up @@ -2441,10 +2463,46 @@ class Context {
encounteredTag, referencePointResult, tagResult, this.getRules(),
this.getLineCol());

this.recordAttrRequiresExtension_(encounteredTag, referencePointResult);
this.recordAttrRequiresExtension_(encounteredTag, tagResult);
this.updateFromTagResult_(referencePointResult);
this.updateFromTagResult_(tagResult);
}

/**
* Record when an encountered tag's attribute that requires an extension
* that it also satisfies that the requied extension is used.
* @param {!amp.htmlparser.ParsedHtmlTag} encounteredTag
* @param {!ValidateTagResult} tagResult
* @private
*/
recordAttrRequiresExtension_(encounteredTag, tagResult) {
if (tagResult.bestMatchTagSpec === null) {
return;
}
const parsedTagSpec = tagResult.bestMatchTagSpec;
if (!parsedTagSpec.attrsCanSatisfyExtension()) {
return;
}
const attrsByName = parsedTagSpec.getAttrsByName();
const extensionsCtx = this.extensions_;
for (const attr of encounteredTag.attrs()) {
if (attr.name in attrsByName) {
const attrId = attrsByName[attr.name];
// negative attr ids are simple attrs (only name set).
if (attrId < 0) {
continue;
}
const parsedAttrSpec =
this.rules_.getParsedAttrSpecs().getByAttrSpecId(attrId);
if (parsedAttrSpec.getSpec().requiresExtension.length > 0) {
extensionsCtx.recordUsedExtensions(
parsedAttrSpec.getSpec().requiresExtension);
}
}
}
}

/**
* Record document-level conditions which have been satisfied.
* @param {!ParsedTagSpec} parsedTagSpec
Expand Down
2 changes: 1 addition & 1 deletion validator/validator-main.protoascii
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ min_validator_revision_required: 348
# newer versions of the spec file. This is currently a Google internal
# mechanism, validator.js does not use this facility. However, any
# change to this file (validator-main.js) requires updating this revision id.
spec_file_revision: 776
spec_file_revision: 777

styles_spec_url: "https://www.ampproject.org/docs/guides/author-develop/responsive/style_pages"
script_spec_url: "https://www.ampproject.org/docs/reference/spec#html-tags"
Expand Down