Syntax Error: add message with the plugin that should be enabled #658

hzoo · 2017-08-03T21:02:58Z

Fixes #169
Closes #178
Rebased/rewrite of #178

Add a new method: this.expectPlugin instead of this.hasPlugin which throws if the plugin isn't enabled.

This also means something better than "unexpected token" when it's a config issue. This also isn't tied to Babel but just babylon's plugins, so other tools can catch the error like Babel will to produce their own specific error messages.

non-block: make sure this covers all the relevant uses of hasPlugin (can be in later PRs)
- bigInt, classPrivateProperties, exportExtensions, numericSeparator
document the need to write a test without a plugin to get test coverage for each plugin added in the future

Jessidhia · 2017-08-07T02:14:27Z

src/parser/statement.js

@@ -491,12 +486,13 @@ export default class StatementParser extends ExpressionParser {
    if (this.match(tt._catch)) {
      const clause = this.startNode();
      this.next();
-      if (this.match(tt.parenL) || !this.hasPlugin("optionalCatchBinding")) {
+      if (this.match(tt.parenL)) {
        this.expect(tt.parenL);


I guess this became redundant now?

bakkot · 2017-08-07T04:54:42Z

src/parser/expression.js

@@ -627,11 +622,11 @@ export default class ExpressionParser extends LValParser {
        return this.finishNode(node, "Super");

      case tt._import:
-        if (this.hasPlugin("importMeta") && this.lookahead().type === tt.dot) {
+        if (this.lookahead().type === tt.dot) {
          return this.parseImportMetaProperty();


Did you drop a this.expectPlugin("importMeta") here?

It's called by the parseImportMetaProperty function.

Yeah this problem was brought up by @motiz88, @danez in https://github.com/babel/babylon/pull/178/files#r87556780

If we make the error here I guess it's a different error if they just have import. vs import.asdf but maybe we shouldn't make a big deal out of it?

danez

besides the if(true) {} looks good

danez · 2017-08-18T23:14:43Z

src/parser/expression.js

@@ -679,7 +674,9 @@ export default class ExpressionParser extends LValParser {
        return id;

      case tt._do:
-        if (this.hasPlugin("doExpressions")) {
+        // TODO
+        if (true) {


What is this ? :D

there's a bug somewhere if I don't have it if you want to look into that 😄

Is this if necessary at all? The _do context can only be legally entered if the plugin is enabled to begin with.

You do need a block here because you have lexical declarations inside the case, but it should just be regular braces.

no it's not yeah I just need a block if that works

danez · 2017-08-18T23:16:06Z

src/parser/expression.js

+        this.expectPlugin("functionSent");
+      } else if (!this.hasPlugin("functionSent")) {
+        // They didn't actually say `function.sent`, just `function.`, so a simple error would be less confusing.
+        this.unexpected();


Uncovered. I add a test.

danez · 2017-08-19T08:43:13Z

I enabled the no-case-declarations. This should prevent this problems.

danez · 2017-08-19T09:35:38Z

Okay I also rebased the branch.

nicolo-ribaudo · 2017-08-20T07:20:31Z

src/parser/statement.js

    }

    this.state.inClassProperty = true;

    if (this.match(tt.eq)) {
-      if (!this.hasPlugin("classProperties"))
-        this.raise(this.state.start, noPluginMsg);
+      this.expectOnePlugin(["classProperties"]);


Here you can use exceptPlugin

Also here

nicolo-ribaudo · 2017-08-20T07:21:26Z

src/parser/statement.js

-    if (!node.typeAnnotation && !this.hasPlugin("classProperties")) {
-      this.raise(node.start, noPluginMsg);
+    if (!node.typeAnnotation) {
+      this.expectOnePlugin(["classProperties"]);


Here you can use exceptPlugin

nicolo-ribaudo · 2017-08-20T07:31:32Z

test/fixtures/experimental/_no-plugin/class-properties-with-initializer/options.json

@@ -0,0 +1,4 @@
+{
+  "throws": "This experimental syntax requires enabling one of the following parser plugin(s): 'classProperties, typescript' (2:6)",


Is this affected by #666?

existentialism

👍

xtuc

Awesome 👍

Just a question about the name expectPlugin sounds like it's part of the lookahead process. What about assertPlugin? shouldHavePlugin? checkPlugin?

hzoo requested review from existentialism, danez, motiz88 and loganfsmyth August 7, 2017 01:56

Jessidhia approved these changes Aug 7, 2017

View reviewed changes

bakkot reviewed Aug 7, 2017

View reviewed changes

This was referenced Aug 7, 2017

Typescript Support babel/babel-eslint#505

Closed

Create aug-09.md babel/notes#31

Merged

hzoo mentioned this pull request Aug 15, 2017

Create aug-16.md babel/notes#32

Merged

danez approved these changes Aug 18, 2017

View reviewed changes

danez force-pushed the expect branch 2 times, most recently from e0d3d59 to 5f58d89 Compare August 18, 2017 23:44

danez force-pushed the expect branch from ee99ff3 to 8b99f27 Compare August 19, 2017 09:35

nicolo-ribaudo reviewed Aug 20, 2017

View reviewed changes

danez force-pushed the expect branch from 8b99f27 to ef1284e Compare August 20, 2017 10:49

existentialism approved these changes Aug 20, 2017

View reviewed changes

xtuc approved these changes Aug 21, 2017

View reviewed changes

motiz88 and others added 7 commits August 28, 2017 17:30

add expectPlugin which throws an error with the missing plugin

cf3ebac

change some things to expectPlugin

2dbba25

rename folders, add more

de47b04

Add test for function.

dfb279f

Enable no-case-declarations to prevent bugs and remove if (true)

aec1bdb

Fix tests

96bd271

Use expectPlugin

19c099d

hzoo force-pushed the expect branch from e24906b to 19c099d Compare August 28, 2017 21:32

hzoo added the Tag: Polish label Aug 28, 2017

hzoo merged commit cde4232 into master Aug 28, 2017

hzoo mentioned this pull request Aug 29, 2017

Bump babylon to 7.0.0-beta.22 prettier/prettier#2687

Merged

hzoo mentioned this pull request Sep 6, 2017

Handle syntax errors from babylon that need a plugin babel/babel#6205

Closed

hzoo deleted the expect branch October 14, 2017 13:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Syntax Error: add message with the plugin that should be enabled #658

Syntax Error: add message with the plugin that should be enabled #658

hzoo commented Aug 3, 2017 •

edited

Loading

Jessidhia Aug 7, 2017

bakkot Aug 7, 2017

nicolo-ribaudo Aug 7, 2017

hzoo Aug 7, 2017

danez left a comment

danez Aug 18, 2017

hzoo Aug 19, 2017

Jessidhia Aug 19, 2017 •

edited

Loading

hzoo Aug 19, 2017

danez Aug 18, 2017

danez commented Aug 19, 2017

danez commented Aug 19, 2017 •

edited

Loading

nicolo-ribaudo Aug 20, 2017 •

edited

Loading

nicolo-ribaudo Aug 20, 2017

nicolo-ribaudo Aug 20, 2017 •

edited

Loading

existentialism left a comment

xtuc left a comment

		@@ -0,0 +1,4 @@
		{
		"throws": "This experimental syntax requires enabling one of the following parser plugin(s): 'classProperties, typescript' (2:6)",

Syntax Error: add message with the plugin that should be enabled #658

Syntax Error: add message with the plugin that should be enabled #658

Conversation

hzoo commented Aug 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jessidhia Aug 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danez commented Aug 19, 2017

danez commented Aug 19, 2017 • edited Loading

nicolo-ribaudo Aug 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolo-ribaudo Aug 20, 2017 • edited Loading

Choose a reason for hiding this comment

existentialism left a comment

Choose a reason for hiding this comment

xtuc left a comment

Choose a reason for hiding this comment

hzoo commented Aug 3, 2017 •

edited

Loading

Jessidhia Aug 19, 2017 •

edited

Loading

danez commented Aug 19, 2017 •

edited

Loading

nicolo-ribaudo Aug 20, 2017 •

edited

Loading

nicolo-ribaudo Aug 20, 2017 •

edited

Loading