indent: outerIIFEBody option not using a multiplier, warning on non-IIFEs?

[ljharb]

What version of ESLint are you using?
v3.0.0

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

Please show your full configuration:

eslint --rule indent:[2,2,{outerIIFEBody:1}] file.js

What did you do? Please include the actual source code causing the issue.
https://github.com/airbnb/javascript/blob/master/packages/eslint-config-airbnb-base/test/test-base.js

import fs from 'fs';
import path from 'path';
import test from 'tape';

import index from '../';

const files = { index };

fs.readdirSync(path.join(__dirname, '../rules')).forEach(name => {
  files[name] = require(`../rules/${name}`); // eslint-disable-line global-require
});

Object.keys(files).forEach(name => {
  const config = files[name];

  test(`${name}: does not reference react`, t => {
    t.plan(2);

    // scan plugins for react and fail if it is found
    const hasReactPlugin = Object.prototype.hasOwnProperty.call(config, 'plugins') &&
      config.plugins.indexOf('react') !== -1;
    t.notOk(hasReactPlugin, 'there is no react plugin');

    // scan rules for react/ and fail if any exist
    const reactRuleIds = Object.keys(config.rules)
      .filter(ruleId => ruleId.indexOf('react/') === 0);
    t.deepEquals(reactRuleIds, [], 'there are no react/ rules');
  });
});

What did you expect to happen?
No warnings.

What actually happened? Please include the actual, raw output from ESLint.

$PWD/packages/eslint-config-airbnb-base/test/test-base.js
  10:3  error  Expected indentation of 1 space character but found 2  indent
  14:3  error  Expected indentation of 1 space character but found 2  indent
  16:3  error  Expected indentation of 1 space character but found 2  indent

There are two problems here:

1. the warned lines are not actually file-level IIFEs - the file doesn't have an IIFE at all.
2. outerIIFEBody is set to 1 - this should be a multiplier of 1 on the indentation config of "2 spaces" - the errors indicate that it wants "1 space" which should never be required when the base indent is "2".

[alberto]

Confirmed

[alberto]

@eslint/eslint-team do you agree on number 2, that the option should be configured as a multiplier of the indentation (in the same way as the other options), and not directly as the indent size?

[platinumazure]

@alberto It's consistent with the current options (so possibly the intended design), but I still think we should abolish multipliers in that rule long-term.

[wojdyr]

false negatives also happen - some top-level IIFEs are not recognized:

(function () {
var i;  // ok
})();

~function () {
var i;  // not
}();

!(function () {
var i;  // not
})();

!function () {
var i;  // not
}();

;(function () {
var i;  // ok
})();

var MyClass = (function () {
var i;  // not
})();

[ljharb]

@platinumazure why would we want to get rid of multipliers? Don't forget that "tab" is a valid option as well, and someone may want 3-space indentation.

Multipliers are the only thing that makes sense to me for this option, since everything should be measured in "indentation levels", and "what is an indentation level" is what's configured by the first option (ie, n spaces, or a hard tab).

[platinumazure]

Because export statements, which technically can contain variable
declarations, render multipliers laughably useless in those cases.
Multipliers are useful in 2-space indentation but fall apart with nearly
anything else (e.g., there is no way to require 4 space indentation
everywhere but 6 spaces for const declarations that take more than one
like). The whole design is clearly wrong and needs to be redesigned from
the ground up.

I'm okay with allowing some things to be expressed in terms of indentation
levels, but we also need an "align" option (at least for space indentation
users). And it would be good to restructure the options so that numbers are
less magical and ambiguous. Speaking from an information architecture
perspective, it is not good that a number sometimes means number of spaces
and sometimes means number of indentation levels.
On Jul 3, 2016 7:49 PM, "Jordan Harband" notifications@github.com wrote:

@platinumazure https://github.com/platinumazure why would we want to get
rid of multipliers? Don't forget that "tab" is a valid option as well, and
someone may want 3-space indentation.

Multipliers are the only thing that makes sense to me for this option,
since everything should be measured in "indentation levels", and "what is
an indentation level" is what's configured by the first option (ie, n
spaces, or a hard tab).

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#6585 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AARWevqo7_tAMjUgw0opa7XqcgTuWCy_ks5qSFiCgaJpZM4JD4oH
.

[ljharb]

If we have an "align" option I hope we have a "never align" option - in my code, I absolutely forbid any indentation solely for alignment.

[nzakas]

@platinumazure let's keep the scope of this issue narrow - there's no need to talk about future long-term possibilities in this context as it's confusing for everyone involved.

This option should definitely act like the others and be a multiplier. I'll take a look and see if it's something that can be fixed easily.

[nzakas]

@ljharb want to try this out and see if it does the trick for you? #6596

[ljharb]

@nzakas confirmed, no errors this time!

[alberto]

@nzakas should we also cover the cases @wojdyr mentioned?

[wojdyr]

I took the examples of IIFE from https://en.wikipedia.org/wiki/Immediately-invoked_function_expression, no idea if anyone is using the ones with unary operator.

I just tried the proposed fix -- it doesn't work on example from google style guide:

goog.scope(function() {
var Button = goog.ui.Button;
});

[ljharb]

@wojdyr in that case, that's not an IIFE - that's just a function passed to another function.

[wojdyr]

@ljharb yes, true. And since eslint-config-google doesn't look concerned with indentation of goog.scope it may not be important.

[ljharb]

Thank you!

[nzakas]

Sorry, I missed @wojdyr's comment before I had made the fix. Please open a new issue if these are still a problem for you.