eslint-plugin-immutable icon indicating copy to clipboard operation
eslint-plugin-immutable copied to clipboard

Disallow ExpressionStatement and SequenceExpression.

Open dtinth opened this issue 10 years ago • 5 comments

Alternate title: Disallow function calls whose return values are discarded.

Based on this blog post, when you call a function and don’t use it’s return value, chances are high that it is being called for its side effect. e.g.

array.push(1)
alert('Hello world!')

Thinking it further, I found this to be an expansion of no-unused-expressions rule, but with every ExpressionStatement disallowed, even if it only contains a CallExpression.

Disallowing SequenceExpression may also make sense because there’s no point in evaluating a, b in (a, b, c) except for the side-effects caused by evaluating a and b.

I think this can also potentially fix #2.

As a result, every expression that gets evaluated must either go into a constant or returned. If we also ensure that there are no-unused-vars, we can get a pure-functional-language-like semantic.

dtinth avatar Mar 11 '16 17:03 dtinth

I agree this is a worthwhile addition to the rules.

jhusain avatar Mar 12 '16 21:03 jhusain

I really like this idea for pushing code to functional style. Off the top of my head, we'd need a few key exceptions to make it reasonable:

  • console.log() or any log()-style method call
  • anything with side effects, like process.exit() or any library from fs
  • any kickoff of an async task which calls a provided or closed-upon callback (though we could force a return statement)

I might just have to implement it, start using it, then see what exceptions I'd want to put in place...

scottnonnenberg avatar May 28 '16 22:05 scottnonnenberg

@scottnonnenberg

I agree that there are certain exceptions to this rule, but the whole point of pushing code to functional style is to eliminate side-effects. 😄

In my opinion, the only exception would be using invariant to make sure that the function is properly called, although it might be better enforced using babel-plugin-typecheck:

export const add = (a, b) => (
  invariant(typeof a === 'number', 'a must be a number'),
  invariant(typeof b === 'number', 'b must be a number'),
  a + b
)

But for a program to function, it needs side effects, right?

I’d put all the imperative, side-effecting code outside the core of the application, where this rule is disabled. While the core of my application (with main logic) would all be functional and without side-effects. This idea is called “Functional Core, Imperative Shell.”.

In fact, Haskell enforces this idea at a type level. All side effecting function has type IO<something>. For example, a function that returns an IO<String> takes some side-causes, produces some side-effects, and returns a String. Normally, IO functions can call normal functions, but not the other way around. This helps me make sure that all of my core logic is side-effects free.

For debugging via console, you can create a trace function. (This file must have the rule turned off).

/* eslint-disable no-impure-code */
export function trace (...args) {
  return (value) => {
    console.log(...args, value)
    return value
  }
}

Then you can add trace anywhere to log a value. The code below looks purely functional, but it produces some console.log as side-effects.

export const add = (a, b) => (
  trace('a =')(a) + trace('b =', b)
)

This breaks the purity rule, but when you’re debugging, anything goes. Haskell also contains the same trace function under Debug.Trace.

dtinth avatar May 29 '16 00:05 dtinth

As long as the documentation recommends something like installing it for only part of your application, then it could be reasonable with no exceptions. It's not a standard use of eslint that I've seen, though I'm finding real benefits in having different configs for my src/, test/ and scripts/ directories.

Also, where's the PR? :0)

scottnonnenberg avatar May 29 '16 01:05 scottnonnenberg

No one implemented it yet I guess…

So I just pushed it as a separate plugin in order to experiment more quickly: purely-functional/eslint-plugin-pure.

I’m going to test this rule, along with other rules from this repo, with some of my personal projects and work projects.

dtinth avatar May 29 '16 01:05 dtinth