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

The "safe" option of the "strict" rule doesn't work as documented #8953

Closed
EvgenyOrekhov opened this issue Jul 15, 2017 · 6 comments
Closed
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation good first issue Good for people who haven't worked on ESLint before rule Relates to ESLint's core rules

Comments

@EvgenyOrekhov
Copy link

The documentation says:

The "safe" option corresponds to the "global" option if ESLint considers a file to be a Node.js or CommonJS module because the configuration specifies either of the following:

  • node or commonjs environments
  • "globalReturn": true property in the ecmaFeatures object of parser options

It says either, but if I set node to true and globalReturn to false, the "safe" option corresponds to the "function" option.

  • ESLint Version: 4.2.0
  • Node Version: 8.1.4
  • npm Version: 5.0.3

What parser (default, Babel-ESLint, etc.) are you using?
default

Please show your full configuration:

Configuration
{
    "parserOptions": {
        "ecmaFeatures": {
            "globalReturn": false
        }
    },
    "env": {
        "node": true
    },
    "rules": {
        "strict": [
            "error"
        ]
    }
}

What did you do? Please include the actual source code causing the issue.

"use strict";

module.exports = function foo() {

}

What did you expect to happen?
No warnings.

What actually happened? Please include the actual, raw output from ESLint.
1:1 - Use the function form of 'use strict'. (strict)
3:18 - Use the function form of 'use strict'. (strict)

@eslintbot eslintbot added the triage An ESLint team member will look at this issue soon label Jul 15, 2017
@platinumazure
Copy link
Member

I think you're correct. node or commonjs environment will set globalReturn to true in parserOptions, but actually specifying globalReturn in your config will override the environments.

@platinumazure platinumazure added accepted There is consensus among the team that this change meets the criteria for inclusion documentation Relates to ESLint's documentation rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels Jul 15, 2017
@ilyavolodin ilyavolodin added the good first issue Good for people who haven't worked on ESLint before label Jul 16, 2017
@williamolojede
Copy link

i'd like to help on this issue, but i'd need guidance on how to go about fixing it.

@not-an-aardvark
Copy link
Member

Hi @williamolojede, thanks for being willing to help. Are you looking for guidance on submitting a PR in general, or are you unsure about the details of this particular issue? If it's the former, our pull request guide could be useful.

@williamolojede
Copy link

The PR isn't the issue, i understand that to an extent. I'm unsure about the details of the issue but i'd like to help.

@not-an-aardvark
Copy link
Member

Basically, the issue is as follows:

In "regular" JavaScript, it's not possible to use a return statement outside a function:

if (foo) {
    return; // syntax error
}

However, it is possible to do this in Node.js. This is because when Node.js runs your code, it essentially wraps the entire module in a function, so return statements are treated as returning from that function.

For similar reasons, "use strict" behaves differently outside of a function depending on whether Node.js is being used. In Node.js, it enables strict mode for only the current module, but in browsers, it enables strict mode globally (for all the following code).

The strict ESLint rule is intended to enforce a particular location for "use strict" statements. One of the options is called "safe", and is intended to require "use strict" directives to be outside a function only when the Node.js semantics are in effect, and inside a function otherwise.

The Node.js semantics are usually in effect when the user specifies the node environment. However, it's possible for the user to opt out of the Node.js semantics by explicitly adding globalReturn: false as a parser option.

The issue is that the documentation here is currently misleading. It states that the "global" option is in effect if the node environment is configured or the globalReturn option is set to true. However, this is not quite accurate, because if the node environment is configured and the globalReturn option is explicitly set to false, the "global" option will not be in effect.

Does that make sense? The issue is a bit nuanced, so there is probably something I didn't explain clearly.

@brandonmailhiot
Copy link
Contributor

It looks like this issue is still open, so I'll take a shot at it if that's fine.

EvgenyOrekhov added a commit to EvgenyOrekhov/eslint-config-hardcore that referenced this issue Aug 5, 2017
Changes:

- Turn on the "strict" rule

The "globalReturn" property in the "ecmaFeatures" object of parser
options brakes the "strict" rule, so we had to remove it from the
config.

Having the "strict" rule turned on is more valuable than having the
"globalReturn" property set to false.

See the relevant discussion:
eslint/eslint#8953
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation good first issue Good for people who haven't worked on ESLint before rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

7 participants