Skip to content
This repository has been archived by the owner on Aug 4, 2020. It is now read-only.

add no-unused-expressions with do expressions support #131

Merged
merged 1 commit into from
Apr 19, 2018

Conversation

lyleunderwood
Copy link
Contributor

@lyleunderwood lyleunderwood commented Oct 11, 2017

/cc #13
I haven't really done any work with babel's AST before, so I'm sure I'm missing a ton of cases.

@lyleunderwood lyleunderwood changed the title #13 add no-unused-expressions with do expressions support eslint-plugin-babel#13 add no-unused-expressions with do expressions support Oct 11, 2017
@lyleunderwood lyleunderwood changed the title eslint-plugin-babel#13 add no-unused-expressions with do expressions support add no-unused-expressions with do expressions support Oct 11, 2017
@lyleunderwood
Copy link
Contributor Author

Also wasn't really sure whether this should be behind an option or not.

@donavon
Copy link

donavon commented Jan 16, 2018

any progress on this?

@lyleunderwood
Copy link
Contributor Author

@donavon thanks for reminding me about this.

I went ahead and added an option to turn support for do expressions off and updated the README. If anything is blocking this from being merged please let me know.

@daniel08s
Copy link

daniel08s commented Jan 22, 2018

I'm trying to test this PR, but I'm not sure how to set it up.

I've fetched the code and then added it to my project by using:
npm install --save-dev <PATH>/eslint-plugin-babel
which was added to my package.json as:
"eslint-plugin-babel": "file:../../eslint-plugin-babel",

My devDependencies:

  "devDependencies": {
    "babel-cli": "^6.26.0",
    "babel-core": "^6.26.0",
    "babel-eslint": "^8.2.1",
    "babel-preset-es2015-node": "^6.1.1",
    "babel-preset-stage-0": "^6.24.1",
    "eslint": "^4.16.0",
    "eslint-config-airbnb": "^16.1.0",
    "eslint-config-airbnb-base": "^12.1.0",
    "eslint-plugin-babel": "file:../../eslint-plugin-babel",
    "eslint-plugin-import": "^2.8.0",
    "eslint-plugin-jsx-a11y": "^6.0.3",
    "eslint-plugin-react": "^7.5.1",
    "supertest": "^3.0.0",
    "tap-spec": "^4.1.1",
    "tape": "^4.8.0"
  }

I then edited my .eslintrc.js file to add the babel plugin and the no-unused-expressions rule, but when I run eslint I still get the error.

My .eslintrc.js file:

module.exports = {
  "parser": "babel-eslint",
  "extends": "airbnb",
  "plugins": [
    "babel"
  ],
  "rules": {
    "object-curly-spacing": ["warn", "never"],
    "func-names": "off",
    "space-before-function-paren": ["error", "never"],
    "max-len": ["error", 120, 4],
    "no-unused-vars": ["error", {"argsIgnorePattern": "next"}],
    "import/prefer-default-export": "off",
    "comma-dangle": ["error", {
      "arrays": "always-multiline",
      "objects": "always-multiline",
      "functions": "ignore"
    }],
    "import/no-extraneous-dependencies": [
    "error", {
       "devDependencies": true, 
       "optionalDependencies": false, 
       "peerDependencies": false, 
       "packageDir": "./"
    }],
    "babel/no-unused-expressions": 1
  }
};

Any help would be very welcome.

@daniel08s
Copy link

Ok, so after some validations I'm actually already testing this version of the plugin.
That said, I'm still getting the no-unused-expressions error when using do { ... } expressions.

Code:

level: do {
        if (process.env.NODE_ENV === 'testing') {
          'error'; // eslint-disable-line
        } else if (process.env.NODE_ENV === 'production') {
          'info'; // eslint-disable-line
        } else {
          'debug'; // eslint-disable-line
        }
      }

For each of the comments removed eslint will produce an error:

  11:11  error  Expected an assignment or function call and instead saw an expression  no-unused-expressions
  13:11  error  Expected an assignment or function call and instead saw an expression  no-unused-expressions
  15:11  error  Expected an assignment or function call and instead saw an expression  no-unused-expressions

@lyleunderwood
Copy link
Contributor Author

@iSkilled make sure you're using the eslint executable from your node_modules and not a globally installed one (run it directly from ./node_modules/.bin/eslint if unsure). Here's all the relevant stuff from my package.json. Your example seems to be working fine when I paste it into a file in my project.

    "eslint": "^3.8.1",
    "eslint-config-airbnb": "^12.0.0",
    "eslint-loader": "^1.7.1",
    "eslint-plugin-babel": "https://github.com/lyleunderwood/eslint-plugin-babel.git#do-expressions",
    "eslint-plugin-flowtype": "^2.21.0",
    "eslint-plugin-fp": "^2.3.0",
    "eslint-plugin-import": "^2.0.1",
    "eslint-plugin-jsx-a11y": "^2.2.3",
    "eslint-plugin-react": "^6.4.1",
  "eslintConfig": {
    "extends": [
      "airbnb",
      "plugin:flowtype/recommended"
    ],
    "parser": "babel-eslint",
    "plugins": [
      "import",
      "flowtype",
      "babel",
      "fp"
    ],
    "env": {
      "browser": true
    },
    "rules": {
      "new-cap": [
        2,
        {
          "capIsNewExceptions": [
            "Map",
            "List"
          ]
        }
      ],
      "no-unused-expressions": 0,
      "babel/no-unused-expressions": 2,
      "import/no-extraneous-dependencies": [
        "error",
        {
          "devDependencies": [
            "**/*.test.js",
            "**/*.test.jsx",
            "**/*.stories.js",
            "**/*.stories.jsx"
          ]
        }
      ],
      "fp/no-mutation": "error",
      "fp/no-arguments": "error",
      "fp/no-delete": "error",
      "fp/no-let": "error",
      "fp/no-loops": "error",
      "fp/no-mutating-assign": "error",
      "fp/no-mutating-methods": "error"
    }
  },

@daniel08s
Copy link

daniel08s commented Jan 23, 2018

I updated the following in my .eslintrc.js:

"babel/no-unused-expressions": 1

to:

"no-unused-expressions": 0,
"babel/no-unused-expressions": 2

and it is now working properly. Thanks!

@Javran
Copy link

Javran commented Feb 13, 2018

any news from maintainers?

@lyleunderwood
Copy link
Contributor Author

Nothing on my end, just waiting.

@mackenzie-orange
Copy link

mackenzie-orange commented Mar 13, 2018

I've been watching this PR for a month now, and no news. Is there a hold up?

I'm pretty excited about this getting merged.

@lyleunderwood
Copy link
Contributor Author

@mackenzie-orange for want of a better option, you can do what I do. My eslint-plugin-babel record looks like this in my package.json:

"eslint-plugin-babel": "https://github.com/lyleunderwood/eslint-plugin-babel.git#do-expressions",

Of course in this scenario you're sort of beholden to me to not somehow break this repo. If that's a concern then you can just fork it to your own account and reference your own branch.

The other problem with this approach is that the branch won't get updates unless manually rebased.

@existentialism
Copy link
Member

@lyleunderwood sorry for the delay, but if you're interested, can you update this now that we've moved to using eslint-plugin-composer?

@lyleunderwood
Copy link
Contributor Author

@existentialism Done.

I rebased on 5.0.0 and implemented in terms of eslint-rule-composer. I also removed my allowDoExpressions option because I'm not sure how I would even get the option in the context of the composer, and it currently serves essentially the same purpose as just enabling / disabling the plugin-specific rule.

@existentialism
Copy link
Member

existentialism commented Apr 3, 2018

@lyleunderwood yeah, accessing options inside filterReports won't land until not-an-aardvark/eslint-rule-composer#1, but I agree, I don't think we need an option here.

@existentialism existentialism merged commit efd2ebe into babel:master Apr 19, 2018
@existentialism
Copy link
Member

@lyleunderwood this was released in 5.1.0! thanks again!

@lyleunderwood
Copy link
Contributor Author

Cool, thanks.

nicolo-ribaudo pushed a commit to babel/babel that referenced this pull request Nov 14, 2019
…o-expressions

add no-unused-expressions with do expressions support
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants