From 53842533b79e429dc3efca258221587c9aec06e6 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Sat, 29 Apr 2017 15:33:06 -0400 Subject: [PATCH] Enforce use of `throw` stmt in early error tests Previously, test consumers were encouraged to insert a `throw` statement as the first statement of tests for early errors. This recommendation made tests harder to consume, and as an optional transformation, consumers may have ignored it or simply been unaware it was made. By explicitly including such a `throw` statement, the tests become more literal, making them easier to consume and more transparent in their expectations. Document expectation for all tests for early errors to include an explicit `throw` statement. Extend linting script to verify that contributors are automatically notified of violations and to ensure that future contributions satisfy this expectation. --- CONTRIBUTING.md | 4 ++- INTERPRETING.md | 11 +------- tools/lint/lib/checks/frontmatter.py | 11 -------- tools/lint/lib/checks/negative.py | 27 +++++++++++++++++++ tools/lint/lint.py | 6 ++++- .../negative_early_throw_bad_value.js | 15 +++++++++++ ...lid.js => negative_early_throw_missing.js} | 1 + ...ing_phase.js => negative_missing_phase.js} | 2 +- ...ssing_type.js => negative_missing_type.js} | 2 +- ..._negative_string.js => negative_string.js} | 2 +- .../test/fixtures/negative_valid_early.js | 14 ++++++++++ .../test/fixtures/negative_valid_runtime.js | 13 +++++++++ 12 files changed, 82 insertions(+), 26 deletions(-) create mode 100644 tools/lint/lib/checks/negative.py create mode 100644 tools/lint/test/fixtures/negative_early_throw_bad_value.js rename tools/lint/test/fixtures/{frontmatter_negative_valid.js => negative_early_throw_missing.js} (97%) rename tools/lint/test/fixtures/{frontmatter_negative_missing_phase.js => negative_missing_phase.js} (96%) rename tools/lint/test/fixtures/{frontmatter_negative_missing_type.js => negative_missing_type.js} (95%) rename tools/lint/test/fixtures/{frontmatter_negative_string.js => negative_string.js} (95%) create mode 100644 tools/lint/test/fixtures/negative_valid_early.js create mode 100644 tools/lint/test/fixtures/negative_valid_runtime.js diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fcbd0e85e3..584ddb3ed8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -187,7 +187,7 @@ function $DONE(arg) { ## Handling Errors and Negative Test Cases -Expectations for **parsing errors** should be declared using [the `negative` frontmatter flag](#negative): +Expectations for **parsing errors** should be declared using [the `negative` frontmatter flag](#negative). They must also include the exact `throw` statement specified in this example (in order to guarantee that implementations do not execute the code): ```javascript /*--- @@ -196,6 +196,8 @@ negative: type: SyntaxError ---*/ +throw "Test262: This statement should not be evaluated."; + var var = var; ``` diff --git a/INTERPRETING.md b/INTERPRETING.md index 4f837b78d8..6c78fa67c2 100644 --- a/INTERPRETING.md +++ b/INTERPRETING.md @@ -147,8 +147,7 @@ attribute is a YAML dictonary with two keys: - `phase` - the stage of the test interpretation process that the error is expected to be produced; either "early" (meaning, "prior to evaluation") or - "runtime" (meaning, "during evaluation"); in the case of "early", additional - test transformation may be required--see below + "runtime" (meaning, "during evaluation") - `type` - the name of the constructor of the expected error If a test configured with the `negative` attribute completes without throwing @@ -167,14 +166,6 @@ negative: unresolvable; ``` -Consumers are free to assert the "early" phase as they see fit. - -For example, it is possible to insert a `throw` statement with a unique error -type at the beginning of the test file. In this case, the statement should be -inserted *after* the directive desribed in the section titled "Strict Mode" -(where appropriate), though it must *not* be inserted for tests containing the -"raw" flag. - ### `includes` One or more files whose content must be evaluated in the test realm's global diff --git a/tools/lint/lib/checks/frontmatter.py b/tools/lint/lib/checks/frontmatter.py index a528f746a3..b62f419350 100644 --- a/tools/lint/lib/checks/frontmatter.py +++ b/tools/lint/lib/checks/frontmatter.py @@ -29,14 +29,3 @@ class CheckFrontmatter(Check): unrecognized = fields - _VALID_FIELDS if len(unrecognized) > 0: return 'Unrecognized fields: %s' % ', '.join(list(unrecognized)) - - if 'negative' in meta: - negative = meta['negative'] - if not isinstance(negative, dict): - return '"negative" must be a dictionary with fields "type" and "phase"' - - if not 'type' in negative: - return '"negative" must specify a "type" field' - - if not 'phase' in negative: - return '"negative" must specify a "phase" field' diff --git a/tools/lint/lib/checks/negative.py b/tools/lint/lib/checks/negative.py new file mode 100644 index 0000000000..b1ae3e4fb1 --- /dev/null +++ b/tools/lint/lib/checks/negative.py @@ -0,0 +1,27 @@ +import re +from ..check import Check + +_THROW_STMT = re.compile( + r'^throw "Test262: This statement should not be evaluated\.";$', + re.MULTILINE) + +class CheckNegative(Check): + '''Ensure tests have the expected YAML-formatted metadata.''' + ID = 'NEGATIVE' + + def run(self, name, meta, source): + if meta is None or meta.get('negative') is None: + return + + negative = meta['negative'] + if not isinstance(negative, dict): + return '"negative" must be a dictionary with fields "type" and "phase"' + + if not 'type' in negative: + return '"negative" must specify a "type" field' + + if not 'phase' in negative: + return '"negative" must specify a "phase" field' + + if negative["phase"] == "early" and not _THROW_STMT.search(source): + return 'Negative tests of type "early" must include a `throw` statement' diff --git a/tools/lint/lint.py b/tools/lint/lint.py index 9cdbca474a..09e22507a0 100755 --- a/tools/lint/lint.py +++ b/tools/lint/lint.py @@ -9,6 +9,7 @@ from lib.collect_files import collect_files from lib.checks.features import CheckFeatures from lib.checks.frontmatter import CheckFrontmatter from lib.checks.license import CheckLicense +from lib.checks.negative import CheckNegative from lib.eprint import eprint import lib.frontmatter import lib.whitelist @@ -21,7 +22,10 @@ parser.add_argument('path', nargs='+', help='file name or directory of files to lint') -checks = [CheckFrontmatter(), CheckFeatures('features.txt'), CheckLicense()] +checks = [ + CheckFrontmatter(), CheckFeatures('features.txt'), CheckLicense(), + CheckNegative() + ] def lint(file_names): errors = dict() diff --git a/tools/lint/test/fixtures/negative_early_throw_bad_value.js b/tools/lint/test/fixtures/negative_early_throw_bad_value.js new file mode 100644 index 0000000000..22979c3136 --- /dev/null +++ b/tools/lint/test/fixtures/negative_early_throw_bad_value.js @@ -0,0 +1,15 @@ +NEGATIVE +^ expected errors | v input +// Copyright (C) 2017 Mike Pennisi. All rights reserved. +// This code is governed by the BSD license found in the LICENSE file. +/*--- +esid: sec-assignment-operators-static-semantics-early-errors +description: Minimal test +negative: + type: SyntaxError + phase: early +---*/ + +throw "Test262: This statement should not be evaluated!"; + +!!! diff --git a/tools/lint/test/fixtures/frontmatter_negative_valid.js b/tools/lint/test/fixtures/negative_early_throw_missing.js similarity index 97% rename from tools/lint/test/fixtures/frontmatter_negative_valid.js rename to tools/lint/test/fixtures/negative_early_throw_missing.js index f1b5cc05da..44a9a22dd4 100644 --- a/tools/lint/test/fixtures/frontmatter_negative_valid.js +++ b/tools/lint/test/fixtures/negative_early_throw_missing.js @@ -1,3 +1,4 @@ +NEGATIVE ^ expected errors | v input // Copyright (C) 2017 Mike Pennisi. All rights reserved. // This code is governed by the BSD license found in the LICENSE file. diff --git a/tools/lint/test/fixtures/frontmatter_negative_missing_phase.js b/tools/lint/test/fixtures/negative_missing_phase.js similarity index 96% rename from tools/lint/test/fixtures/frontmatter_negative_missing_phase.js rename to tools/lint/test/fixtures/negative_missing_phase.js index 8315882cac..e830d42385 100644 --- a/tools/lint/test/fixtures/frontmatter_negative_missing_phase.js +++ b/tools/lint/test/fixtures/negative_missing_phase.js @@ -1,4 +1,4 @@ -FRONTMATTER +NEGATIVE ^ expected errors | v input // Copyright (C) 2017 Mike Pennisi. All rights reserved. // This code is governed by the BSD license found in the LICENSE file. diff --git a/tools/lint/test/fixtures/frontmatter_negative_missing_type.js b/tools/lint/test/fixtures/negative_missing_type.js similarity index 95% rename from tools/lint/test/fixtures/frontmatter_negative_missing_type.js rename to tools/lint/test/fixtures/negative_missing_type.js index 6e3ec6551a..8642c9124d 100644 --- a/tools/lint/test/fixtures/frontmatter_negative_missing_type.js +++ b/tools/lint/test/fixtures/negative_missing_type.js @@ -1,4 +1,4 @@ -FRONTMATTER +NEGATIVE ^ expected errors | v input // Copyright (C) 2017 Mike Pennisi. All rights reserved. // This code is governed by the BSD license found in the LICENSE file. diff --git a/tools/lint/test/fixtures/frontmatter_negative_string.js b/tools/lint/test/fixtures/negative_string.js similarity index 95% rename from tools/lint/test/fixtures/frontmatter_negative_string.js rename to tools/lint/test/fixtures/negative_string.js index b11d19a3e6..706f773a5f 100644 --- a/tools/lint/test/fixtures/frontmatter_negative_string.js +++ b/tools/lint/test/fixtures/negative_string.js @@ -1,4 +1,4 @@ -FRONTMATTER +NEGATIVE ^ expected errors | v input // Copyright (C) 2017 Mike Pennisi. All rights reserved. // This code is governed by the BSD license found in the LICENSE file. diff --git a/tools/lint/test/fixtures/negative_valid_early.js b/tools/lint/test/fixtures/negative_valid_early.js new file mode 100644 index 0000000000..5b6c7a261f --- /dev/null +++ b/tools/lint/test/fixtures/negative_valid_early.js @@ -0,0 +1,14 @@ +^ expected errors | v input +// Copyright (C) 2017 Mike Pennisi. All rights reserved. +// This code is governed by the BSD license found in the LICENSE file. +/*--- +esid: sec-assignment-operators-static-semantics-early-errors +description: Minimal test +negative: + type: SyntaxError + phase: early +---*/ + +throw "Test262: This statement should not be evaluated."; + +!!! diff --git a/tools/lint/test/fixtures/negative_valid_runtime.js b/tools/lint/test/fixtures/negative_valid_runtime.js new file mode 100644 index 0000000000..ed4b056fdc --- /dev/null +++ b/tools/lint/test/fixtures/negative_valid_runtime.js @@ -0,0 +1,13 @@ +^ expected errors | v input +// Copyright (C) 2017 Mike Pennisi. All rights reserved. +// This code is governed by the BSD license found in the LICENSE file. +/*--- +esid: sec-assignment-operators-static-semantics-early-errors +description: Minimal test +negative: + type: ReferenceError + phase: runtime +---*/ + +x; +let x;