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.

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

Standard allows certain unnecessary semicolons #786

Open
dcposch opened this issue Feb 14, 2017 · 7 comments

Comments

@dcposch
Copy link

commented Feb 14, 2017

Version: 8.6.0

Example 1: reports the unnecessary semicolon

module.exports = function Foo () {
};

Example 2: no error

module.exports = Foo
function Foo () {
};

Example 3: no error

;
;
module.exports = Foo
function Foo () {
};
;
;
@dcposch

This comment has been minimized.

Copy link
Author

commented Feb 14, 2017

Is this on purpose?

Otherwise, are we missing an eslint rule?

@dcousens dcousens added bug and removed bug labels Feb 14, 2017

@feross

This comment has been minimized.

Copy link
Member

commented Feb 14, 2017

We're missing an eslint rule: no-extra-semi.

We used to enable this, but it was relaxed in 3.1.2. Context here: #70

Options are:

  1. Re-enable the rule, though it means that in some circumstances users will need to break the rule "Don't ever start a line with ( or [".

  2. Keep it disabled

  3. See if ESLint would accept a PR that adds an option to the rule to ignore extra semis when they precede a specified whitelist of characters. We would include the full list of ASI gotcha characters: [, (, +, *, /, -, ,, .

@feross feross added the bug label Feb 14, 2017

@dcposch

This comment has been minimized.

Copy link
Author

commented Feb 14, 2017

i vote #1

though it means that in some circumstances users will need to break the rule

i think it means they have to follow that rule, right?

@feross

This comment has been minimized.

Copy link
Member

commented Feb 14, 2017

@dcposch If you have an immediately executing function like this:

;(function () {
  console.log('hey')
})()

It's common to put a semi in front, in case there's something before it that fails with ASI. Here's an example where the semi is required to get the correct behavior:

console.log('hi')
;(function () { // without the semi, this is interpreted as a function invocation
  console.log('hey')
})()

However, there are certain things that could come before that don't actually require a semi:

if (x) {
  console.log('hi')
}
;(function () { // this semi is technically NOT required
  console.log('hey')
})()

If-blocks don't actually require a semi after them, since there's no way the paren that follows could be a function invocation. That said, it's a good idea to just always put in a semi before ( and [. The problem is that no-extra-semi will warn about this last case, forcing the user to remove it.

if (x) {
  console.log('hi')
}
(function () { // this is perfectly safe, though it doesn't look like it
  console.log('hey')
})()
@tunnckoCore

This comment has been minimized.

Copy link

commented Feb 21, 2017

So we should go back to use semicolons everywhere! 😱 Haha, just a joke.

I'm for 1 too.

The problem is that no-extra-semi will warn about this last case, forcing the user to remove it.

There's autofix and everyone should use it. The autofix will remove it in that last case, but won't remove it in the example with console.log

console.log('hi')
;(function () { // without the semi, this is interpreted as a function invocation
  console.log('hey')
})()

Above is confirmed.

So I don't see any problems.

@feross feross added question and removed bug labels Apr 4, 2017

@indiesquidge

This comment has been minimized.

Copy link

commented May 5, 2017

I don't know if this should count as part of this thread or opened as a new issue (or if this case is even supported at all since it requires the babel-eslint parser), but I have the same issue of Standard allowing semicolons for ES class fields and static properties

export class Bork {
  // Property initializer syntax
  instanceProperty = 'bork';
  boundFunction = () => {
    return this.instanceProperty
  };

  // Static class properties
  static staticProperty = 'babelIsCool';
  static staticFunction = function () {
    return Bork.staticProperty
  };
};

// standard does not complain about these semicolons.
@stale

This comment has been minimized.

Copy link

commented May 10, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.