Skip to content

Commit

Permalink
Validator Rollup (ampproject#7844)
Browse files Browse the repository at this point in the history 
* Revision bump.

* Revision bump for amp-playbuzz changes in ampproject#7450

* Revision bump.

* Allow filtering by HtmlFormat (AMP, AMP4ADS) in the code generator.

* Revision bump.

* Revision bump due to Github pull.

* Code generation now driven by a variable LIGHT, which is broader than the previous GENERATE_DETAILED_ERRORS distinction. LIGHT implies filtered for specific format (AMP or AMP4ADS, and only amp.validator.validateSaxEvents, and no detailed errors.

* Generate ValidatorRules.directAttrLists and globalAttrs and ampLayoutAttrs, reducing overhead by indirection.
  • Loading branch information
@Gregable
Gregable committed Mar 1, 2017
1 parent c583323 commit 9f4082f
Show file tree
Hide file tree
Showing 11 changed files with 1,095 additions and 906 deletions.
181 changes: 120 additions & 61 deletions validator/build.py
View file
Expand Up @@ -61,11 +61,12 @@ def CheckPrereqs():
'Please feel free to edit the source and fix it to your needs.')

# Ensure source files are available.
for f in ['validator-main.protoascii', 'validator.proto',
'validator_gen_js.py', 'package.json', 'engine/validator.js',
'engine/validator_test.js', 'engine/validator-in-browser.js',
'engine/tokenize-css.js', 'engine/parse-css.js',
'engine/parse-srcset.js', 'engine/parse-url.js']:
for f in [
'validator-main.protoascii', 'validator.proto', 'validator_gen_js.py',
'package.json', 'engine/validator.js', 'engine/validator_test.js',
'engine/validator-in-browser.js', 'engine/tokenize-css.js',
'engine/parse-css.js', 'engine/parse-srcset.js', 'engine/parse-url.js'
]:
if not os.path.exists(f):
Die('%s not found. Must run in amp_validator source directory.' % f)

Expand Down Expand Up @@ -146,8 +147,8 @@ def GenValidatorPb2Py(out_dir):
logging.info('entering ...')
assert re.match(r'^[a-zA-Z_\-0-9]+$', out_dir), 'bad out_dir: %s' % out_dir

subprocess.check_call(['protoc', 'validator.proto',
'--python_out=%s' % out_dir])
subprocess.check_call(
['protoc', 'validator.proto', '--python_out=%s' % out_dir])
open('%s/__init__.py' % out_dir, 'w').close()
logging.info('... done')

Expand Down Expand Up @@ -199,6 +200,8 @@ def GenValidatorGeneratedJs(out_dir):
specfile='%s/validator.protoascii' % out_dir,
validator_pb2=validator_pb2,
text_format=text_format,
html_format=None,
light=False,
descriptor=descriptor,
out=out)
out.append('')
Expand All @@ -208,6 +211,39 @@ def GenValidatorGeneratedJs(out_dir):
logging.info('... done')


def GenValidatorGeneratedLightAmpJs(out_dir):
"""Calls validator_gen_js to generate validator-generated-light-amp.js.
Args:
out_dir: directory name of the output directory. Must not have slashes,
dots, etc.
"""
logging.info('entering ...')
assert re.match(r'^[a-zA-Z_\-0-9]+$', out_dir), 'bad out_dir: %s' % out_dir

# These imports happen late, within this method because they don't necessarily
# exist when the module starts running, and the ones that probably do
# are checked by CheckPrereqs.
from google.protobuf import text_format
from google.protobuf import descriptor
from dist import validator_pb2
import validator_gen_js
out = []
validator_gen_js.GenerateValidatorGeneratedJs(
specfile='%s/validator.protoascii' % out_dir,
validator_pb2=validator_pb2,
text_format=text_format,
html_format=validator_pb2.TagSpec.AMP,
light=True,
descriptor=descriptor,
out=out)
out.append('')
f = open('%s/validator-generated-light-amp.js' % out_dir, 'w')
f.write('\n'.join(out))
f.close()
logging.info('... done')


def GenValidatorGeneratedMd(out_dir):
"""Calls validator_gen_md to generate validator-generated.md.
Expand Down Expand Up @@ -246,14 +282,18 @@ def CompileWithClosure(js_files, closure_entry_points, output_file):
output_file: name of the Javascript output file
"""

cmd = ['java', '-jar', 'node_modules/google-closure-compiler/compiler.jar',
'--language_in=ECMASCRIPT6_STRICT', '--language_out=ES5_STRICT',
'--js_output_file=%s' % output_file, '--only_closure_dependencies']
cmd = [
'java', '-jar', 'node_modules/google-closure-compiler/compiler.jar',
'--language_in=ECMASCRIPT6_STRICT', '--language_out=ES5_STRICT',
'--js_output_file=%s' % output_file, '--only_closure_dependencies'
]
cmd += ['--closure_entry_point=%s' % e for e in closure_entry_points]
cmd += ['node_modules/google-closure-library/closure/**.js',
'!node_modules/google-closure-library/closure/**_test.js',
'node_modules/google-closure-library/third_party/closure/**.js',
'!node_modules/google-closure-library/third_party/closure/**_test.js']
cmd += [
'node_modules/google-closure-library/closure/**.js',
'!node_modules/google-closure-library/closure/**_test.js',
'node_modules/google-closure-library/third_party/closure/**.js',
'!node_modules/google-closure-library/third_party/closure/**_test.js'
]
cmd += js_files
subprocess.check_call(cmd)

Expand All @@ -266,16 +306,19 @@ def CompileValidatorMinified(out_dir):
"""
logging.info('entering ...')
CompileWithClosure(
js_files=['engine/htmlparser.js', 'engine/parse-css.js',
'engine/parse-srcset.js', 'engine/parse-url.js',
'engine/tokenize-css.js',
'%s/validator-generated.js' % out_dir,
'engine/validator-in-browser.js', 'engine/validator.js',
'engine/amp4ads-parse-css.js', 'engine/dom-walker.js',
'engine/htmlparser-interface.js'],
closure_entry_points=['amp.validator.validateString',
'amp.validator.renderValidationResult',
'amp.validator.renderErrorMessage'],
js_files=[
'engine/htmlparser.js', 'engine/parse-css.js',
'engine/parse-srcset.js', 'engine/parse-url.js',
'engine/tokenize-css.js', '%s/validator-generated.js' % out_dir,
'engine/validator-in-browser.js', 'engine/validator.js',
'engine/amp4ads-parse-css.js', 'engine/dom-walker.js',
'engine/htmlparser-interface.js'
],
closure_entry_points=[
'amp.validator.validateString',
'amp.validator.renderValidationResult',
'amp.validator.renderErrorMessage'
],
output_file='%s/validator_minified.js' % out_dir)
logging.info('... done')

Expand All @@ -290,9 +333,11 @@ def RunSmokeTest(out_dir, nodejs_cmd):
logging.info('entering ...')
# Run index.js on the minimum valid amp and observe that it passes.
p = subprocess.Popen(
[nodejs_cmd, 'nodejs/index.js', '--validator_js',
'%s/validator_minified.js' % out_dir,
'testdata/feature_tests/minimum_valid_amp.html'],
[
nodejs_cmd, 'nodejs/index.js', '--validator_js',
'%s/validator_minified.js' % out_dir,
'testdata/feature_tests/minimum_valid_amp.html'
],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
(stdout, stderr) = p.communicate()
Expand All @@ -303,9 +348,11 @@ def RunSmokeTest(out_dir, nodejs_cmd):

# Run index.js on an empty file and observe that it fails.
p = subprocess.Popen(
[nodejs_cmd, 'nodejs/index.js', '--validator_js',
'%s/validator_minified.js' % out_dir,
'testdata/feature_tests/empty.html'],
[
nodejs_cmd, 'nodejs/index.js', '--validator_js',
'%s/validator_minified.js' % out_dir,
'testdata/feature_tests/empty.html'
],
stdout=subprocess.PIPE,
stderr=subprocess.PIPE)
(stdout, stderr) = p.communicate()
Expand Down Expand Up @@ -345,13 +392,14 @@ def CompileValidatorTestMinified(out_dir):
"""
logging.info('entering ...')
CompileWithClosure(
js_files=['engine/htmlparser.js', 'engine/parse-css.js',
'engine/parse-srcset.js', 'engine/parse-url.js',
'engine/tokenize-css.js',
'%s/validator-generated.js' % out_dir,
'engine/validator-in-browser.js', 'engine/validator.js',
'engine/amp4ads-parse-css.js', 'engine/htmlparser-interface.js',
'engine/dom-walker.js', 'engine/validator_test.js'],
js_files=[
'engine/htmlparser.js', 'engine/parse-css.js',
'engine/parse-srcset.js', 'engine/parse-url.js',
'engine/tokenize-css.js', '%s/validator-generated.js' % out_dir,
'engine/validator-in-browser.js', 'engine/validator.js',
'engine/amp4ads-parse-css.js', 'engine/htmlparser-interface.js',
'engine/dom-walker.js', 'engine/validator_test.js'
],
closure_entry_points=['amp.validator.ValidatorTest'],
output_file='%s/validator_test_minified.js' % out_dir)
logging.info('... success')
Expand All @@ -366,12 +414,14 @@ def CompileValidatorLightTestMinified(out_dir):
"""
logging.info('entering ...')
CompileWithClosure(
js_files=['engine/htmlparser.js', 'engine/parse-css.js',
'engine/parse-srcset.js', 'engine/parse-url.js',
'engine/tokenize-css.js', '%s/validator-generated.js' % out_dir,
'engine/validator-in-browser.js', 'engine/validator.js',
'engine/amp4ads-parse-css.js', 'engine/htmlparser-interface.js',
'engine/dom-walker.js', 'engine/validator-light_test.js'],
js_files=[
'engine/htmlparser.js', 'engine/parse-css.js',
'engine/parse-srcset.js', 'engine/parse-url.js',
'engine/tokenize-css.js', '%s/validator-generated-light-amp.js' %
out_dir, 'engine/validator-in-browser.js', 'engine/validator.js',
'engine/amp4ads-parse-css.js', 'engine/htmlparser-interface.js',
'engine/dom-walker.js', 'engine/validator-light_test.js'
],
closure_entry_points=['amp.validator.ValidatorTest'],
output_file='%s/validator-light_test_minified.js' % out_dir)
logging.info('... success')
Expand All @@ -386,8 +436,10 @@ def CompileHtmlparserTestMinified(out_dir):
"""
logging.info('entering ...')
CompileWithClosure(
js_files=['engine/htmlparser.js', 'engine/htmlparser-interface.js',
'engine/htmlparser_test.js'],
js_files=[
'engine/htmlparser.js', 'engine/htmlparser-interface.js',
'engine/htmlparser_test.js'
],
closure_entry_points=['amp.htmlparser.HtmlParserTest'],
output_file='%s/htmlparser_test_minified.js' % out_dir)
logging.info('... success')
Expand All @@ -402,10 +454,12 @@ def CompileParseCssTestMinified(out_dir):
"""
logging.info('entering ...')
CompileWithClosure(
js_files=['engine/parse-css.js', 'engine/parse-url.js',
'engine/tokenize-css.js', 'engine/css-selectors.js',
'engine/json-testutil.js', 'engine/parse-css_test.js',
'%s/validator-generated.js' % out_dir],
js_files=[
'engine/parse-css.js', 'engine/parse-url.js',
'engine/tokenize-css.js', 'engine/css-selectors.js',
'engine/json-testutil.js', 'engine/parse-css_test.js',
'%s/validator-generated.js' % out_dir
],
closure_entry_points=['parse_css.ParseCssTest'],
output_file='%s/parse-css_test_minified.js' % out_dir)
logging.info('... success')
Expand All @@ -420,10 +474,12 @@ def CompileParseUrlTestMinified(out_dir):
"""
logging.info('entering ...')
CompileWithClosure(
js_files=['engine/parse-url.js', 'engine/parse-css.js',
'engine/tokenize-css.js', 'engine/css-selectors.js',
'engine/json-testutil.js', 'engine/parse-url_test.js',
'%s/validator-generated.js' % out_dir],
js_files=[
'engine/parse-url.js', 'engine/parse-css.js',
'engine/tokenize-css.js', 'engine/css-selectors.js',
'engine/json-testutil.js', 'engine/parse-url_test.js',
'%s/validator-generated.js' % out_dir
],
closure_entry_points=['parse_url.ParseURLTest'],
output_file='%s/parse-url_test_minified.js' % out_dir)
logging.info('... success')
Expand All @@ -438,11 +494,12 @@ def CompileAmp4AdsParseCssTestMinified(out_dir):
"""
logging.info('entering ...')
CompileWithClosure(
js_files=['engine/amp4ads-parse-css_test.js', 'engine/parse-css.js',
'engine/parse-url.js', 'engine/amp4ads-parse-css.js',
'engine/tokenize-css.js', 'engine/css-selectors.js',
'engine/json-testutil.js',
'%s/validator-generated.js' % out_dir],
js_files=[
'engine/amp4ads-parse-css_test.js', 'engine/parse-css.js',
'engine/parse-url.js', 'engine/amp4ads-parse-css.js',
'engine/tokenize-css.js', 'engine/css-selectors.js',
'engine/json-testutil.js', '%s/validator-generated.js' % out_dir
],
closure_entry_points=['parse_css.Amp4AdsParseCssTest'],
output_file='%s/amp4ads-parse-css_test_minified.js' % out_dir)
logging.info('... success')
Expand All @@ -457,9 +514,10 @@ def CompileParseSrcsetTestMinified(out_dir):
"""
logging.info('entering ...')
CompileWithClosure(
js_files=['engine/parse-srcset.js', 'engine/json-testutil.js',
'engine/parse-srcset_test.js',
'%s/validator-generated.js' % out_dir],
js_files=[
'engine/parse-srcset.js', 'engine/json-testutil.js',
'engine/parse-srcset_test.js', '%s/validator-generated.js' % out_dir
],
closure_entry_points=['parse_srcset.ParseSrcsetTest'],
output_file='%s/parse-srcset_test_minified.js' % out_dir)
logging.info('... success')
Expand Down Expand Up @@ -527,6 +585,7 @@ def Main():
GenValidatorPb2Py(out_dir='dist')
GenValidatorProtoascii(out_dir='dist')
GenValidatorGeneratedJs(out_dir='dist')
GenValidatorGeneratedLightAmpJs(out_dir='dist')
GenValidatorGeneratedMd(out_dir='dist')
CompileValidatorMinified(out_dir='dist')
RunSmokeTest(out_dir='dist', nodejs_cmd=nodejs_cmd)
Expand Down
23 changes: 23 additions & 0 deletions validator/engine/amp4ads-parse-css.js
View file
Expand Up @@ -18,11 +18,14 @@
goog.provide('parse_css.stripVendorPrefix');
goog.provide('parse_css.validateAmp4AdsCss');

goog.require('amp.validator.LIGHT');
goog.require('amp.validator.ValidationError');
goog.require('parse_css.DelimToken');
goog.require('parse_css.ErrorToken');
goog.require('parse_css.IdentToken');
goog.require('parse_css.RuleVisitor');
goog.require('parse_css.Stylesheet');
goog.require('parse_css.TRIVIAL_ERROR_TOKEN');

/**
* Strips vendor prefixes from identifiers, e.g. property names or names
Expand Down Expand Up @@ -115,6 +118,10 @@ class Amp4AdsVisitor extends parse_css.RuleVisitor {
}
const ident = firstIdent(declaration.value);
if (ident === 'fixed' || ident === 'sticky') {
if (amp.validator.LIGHT) {
this.errors.push(parse_css.TRIVIAL_ERROR_TOKEN);
return;
}
this.errors.push(createParseErrorTokenAt(
declaration, amp.validator.ValidationError.Code
.CSS_SYNTAX_DISALLOWED_PROPERTY_VALUE,
Expand Down Expand Up @@ -142,6 +149,10 @@ class Amp4AdsVisitor extends parse_css.RuleVisitor {
parse_css.stripVendorPrefix(transitionedProperty);
if (transitionedPropertyStripped !== 'opacity' &&
transitionedPropertyStripped !== 'transform') {
if (amp.validator.LIGHT) {
this.errors.push(parse_css.TRIVIAL_ERROR_TOKEN);
return;
}
this.errors.push(createParseErrorTokenAt(
decl, amp.validator.ValidationError.Code
.CSS_SYNTAX_DISALLOWED_PROPERTY_VALUE_WITH_HINT,
Expand All @@ -155,6 +166,10 @@ class Amp4AdsVisitor extends parse_css.RuleVisitor {
// the only properties that may be transitioned are opacity and transform.
if (this.inKeyframes !== null && name !== 'transform' &&
name !== 'opacity') {
if (amp.validator.LIGHT) {
this.errors.push(parse_css.TRIVIAL_ERROR_TOKEN);
return;
}
this.errors.push(createParseErrorTokenAt(
decl, amp.validator.ValidationError.Code
.CSS_SYNTAX_PROPERTY_DISALLOWED_WITHIN_AT_RULE,
Expand All @@ -178,6 +193,10 @@ class Amp4AdsVisitor extends parse_css.RuleVisitor {
if (allowed.indexOf(parse_css.stripVendorPrefix(decl.name)) !== -1) {
continue;
}
if (amp.validator.LIGHT) {
this.errors.push(parse_css.TRIVIAL_ERROR_TOKEN);
return;
}
this.errors.push(createParseErrorTokenAt(
decl, amp.validator.ValidationError.Code
.CSS_SYNTAX_PROPERTY_DISALLOWED_TOGETHER_WITH,
Expand All @@ -188,6 +207,10 @@ class Amp4AdsVisitor extends parse_css.RuleVisitor {
}
// (2) Check that the rule is qualified with .amp-animate.
if (!hasAmpAnimate(qualifiedRule)) {
if (amp.validator.LIGHT) {
this.errors.push(parse_css.TRIVIAL_ERROR_TOKEN);
return;
}
this.errors.push(createParseErrorTokenAt(
qualifiedRule, amp.validator.ValidationError.Code
.CSS_SYNTAX_PROPERTY_REQUIRES_QUALIFICATION,
Expand Down

0 comments on commit 9f4082f

Please sign in to comment.