Repository: eslint/eslint (js)

Issue:
Gensite fails while building formatters page
During release today, release script failed while generating formatters page for the site. It looks like glob-based configs introduced a regression for configs that use `extend` with `baseConfig`. This branch of code is not covered by unit tests, and it wasn't clear how to fix it during the release. So we skipped generation of formatter page and decided to fix it over the weekend.
We traced the error to this line of code: https://github.com/eslint/eslint/blob/7d9e3beeb58c1ee71d53dfcfd3e3b0721dd79b46/lib/config/config-file.js#L567
Also config is passing two properties here: https://github.com/eslint/eslint/blob/7d9e3beeb58c1ee71d53dfcfd3e3b0721dd79b46/lib/config.js#L70 but `loadObject` function only accepts one. Even when I changed the code locally to accept two arguments and pass `configContext` as second parameter into `applyExtends` it failed with the same error, because we only add `configCache` after we call `loadObject`.

Error message is below:

```js
/var/lib/jenkins/workspace/Releases/ESLint Release/eslint/lib/config/config-file.js:431
            throw e;
            ^

TypeError: Cannot read property 'getConfig' of undefined
Referenced from: undefined
    at load (/var/lib/jenkins/workspace/Releases/ESLint Release/eslint/lib/config/config-file.js:582:51)
    at configExtends.reduceRight.e (/var/lib/jenkins/workspace/Releases/ESLint Release/eslint/lib/config/config-file.js:421:36)
    at Array.reduceRight (native)
    at applyExtends (/var/lib/jenkins/workspace/Releases/ESLint Release/eslint/lib/config/config-file.js:405:28)
    at Object.loadObject (/var/lib/jenkins/workspace/Releases/ESLint Release/eslint/lib/config/config-file.js:567:35)
    at new Config (/var/lib/jenkins/workspace/Releases/ESLint Release/eslint/lib/config.js:70:46)
    at new CLIEngine (/var/lib/jenkins/workspace/Releases/ESLint Release/eslint/lib/cli-engine.js:427:23)
    at getFormatterResults (/var/lib/jenkins/workspace/Releases/ESLint Release/eslint/Makefile.js:467:15)
    at Function.target.gensite (/var/lib/jenkins/workspace/Releases/ESLint Release/eslint/Makefile.js:736:31)
    at Object.global.target.(anonymous function) [as gensite] (/var/lib/jenkins/workspace/Releases/ESLint Release/eslint/node_modules/shelljs/make.js:36:40)
```


Pull request:
Fix: avoid crashing when using baseConfig with extends (fixes #8791)
<!--
    ESLint adheres to the [JS Foundation Code of Conduct](https://js.foundation/community/code-of-conduct).
-->

**What is the purpose of this pull request? (put an "X" next to item)**

[x] Bug fix (https://github.com/eslint/eslint/issues/8791)

<!--
    If the item you've checked above has a template, please paste the template questions below and answer them. (If this pull request is addressing an issue, you can just paste a link to the issue here instead.)
-->

<!--
    Please ensure your pull request is ready:

    - Read the pull request guide (http://eslint.org/docs/developer-guide/contributing/pull-requests)
    - Include tests for this change
    - Update documentation for this change (if appropriate)
-->

<!--
    The following is required for all pull requests:
-->

**What changes did you make? (Give an overview)**

Due to a bug, an invalid Config instance was getting used when applying extensions to a `baseConfig` object. This updates the `Config` constructor to use the correct context, and to make sure the config cache exists when the `baseConfig` is evaluated.

**Is there anything you'd like reviewers to focus on?**

Nothing in particular

Patch:
diff --git a/lib/config.js b/lib/config.js
index c69d120ef..2db87d942 100644
--- a/lib/config.js
+++ b/lib/config.js
@@ -66,13 +66,14 @@ class Config {
         this.parser = options.parser;
         this.parserOptions = options.parserOptions || {};
 
+        this.configCache = new ConfigCache();
+
         this.baseConfig = options.baseConfig
             ? ConfigOps.merge({}, ConfigFile.loadObject(options.baseConfig, this))
             : { rules: {} };
         this.baseConfig.filePath = "";
         this.baseConfig.baseDirectory = this.options.cwd;
 
-        this.configCache = new ConfigCache();
         this.configCache.setConfig(this.baseConfig.filePath, this.baseConfig);
         this.configCache.setMergedVectorConfig(this.baseConfig.filePath, this.baseConfig);
 
diff --git a/lib/config/config-cache.js b/lib/config/config-cache.js
index 0ffcae944..b89f33430 100644
--- a/lib/config/config-cache.js
+++ b/lib/config/config-cache.js
@@ -24,9 +24,9 @@ function hash(vector) {
 //------------------------------------------------------------------------------
 
 /**
- * Configuration caching class (not exported)
+ * Configuration caching class
  */
-class ConfigCache {
+module.exports = class ConfigCache {
 
     constructor() {
         this.filePathCache = new Map();
@@ -125,6 +125,4 @@ class ConfigCache {
     setMergedConfig(vector, config) {
         this.mergedCache.set(hash(vector), config);
     }
-}
-
-module.exports = ConfigCache;
+};
diff --git a/lib/config/config-file.js b/lib/config/config-file.js
index 832529952..c9b6a432d 100644
--- a/lib/config/config-file.js
+++ b/lib/config/config-file.js
@@ -560,11 +560,12 @@ function loadFromDisk(resolvedPath, configContext) {
 /**
  * Loads a config object, applying extends if present.
  * @param {Object} configObject a config object to load
+ * @param {Config} configContext Context for the config instance
  * @returns {Object} the config object with extends applied if present, or the passed config if not
  * @private
  */
-function loadObject(configObject) {
-    return configObject.extends ? applyExtends(configObject, "") : configObject;
+function loadObject(configObject, configContext) {
+    return configObject.extends ? applyExtends(configObject, configContext, "") : configObject;
 }
 
 /**
diff --git a/lib/rules/indent.js b/lib/rules/indent.js
index 52d08ff3d..3027e7720 100644
--- a/lib/rules/indent.js
+++ b/lib/rules/indent.js
@@ -220,8 +220,8 @@ class OffsetStorage {
 
     /**
      * Sets the offset column of token B to match the offset column of token A.
-     * This is different from matchIndentOf because it matches a *column*, even if baseToken is not
-     * the first token on its line.
+     * **WARNING**: This is different from matchIndentOf because it matches a *column*, even if baseToken is not
+     * the first token on its line. In most cases, `matchIndentOf` should be used instead.
      * @param {Token} baseToken The first token
      * @param {Token} offsetToken The second token, whose offset should be matched to the first token
      * @returns {void}
@@ -239,12 +239,62 @@ class OffsetStorage {
     }
 
     /**
-    * Sets the desired offset of a token
-    * @param {Token} token The token
-    * @param {Token} offsetFrom The token that this is offset from
-    * @param {number} offset The desired indent level
-    * @returns {void}
-    */
+     * Sets the desired offset of a token.
+     *
+     * This uses a line-based offset collapsing behavior to handle tokens on the same line.
+     * For example, consider the following two cases:
+     *
+     * (
+     *     [
+     *         bar
+     *     ]
+     * )
+     *
+     * ([
+     *     bar
+     * ])
+     *
+     * Based on the first case, it's clear that the `bar` token needs to have an offset of 1 indent level (4 spaces) from
+     * the `[` token, and the `[` token has to have an offset of 1 indent level from the `(` token. Since the `(` token is
+     * the first on its line (with an indent of 0 spaces), the `bar` token needs to be offset by 2 indent levels (8 spaces)
+     * from the start of its line.
+     *
+     * However, in the second case `bar` should only be indented by 4 spaces. This is because the offset of 1 indent level
+     * between the `(` and the `[` tokens gets "collapsed" because the two tokens are on the same line. As a result, the
+     * `(` token is mapped to the `[` token with an offset of 0, and the rule correctly decides that `bar` should be indented
+     * by 1 indent level from the start of the line.
+     *
+     * This is useful because rule listeners can usually just call `setDesiredOffset` for all the tokens in the node,
+     * without needing to check which lines those tokens are on.
+     *
+     * Note that since collapsing only occurs when two tokens are on the same line, there are a few cases where non-intuitive
+     * behavior can occur. For example, consider the following cases:
+     *
+     * foo(
+     * ).
+     *     bar(
+     *         baz
+     *     )
+     *
+     * foo(
+     * ).bar(
+     *     baz
+     * )
+     *
+     * Based on the first example, it would seem that `bar` should be offset by 1 indent level from `foo`, and `baz`
+     * should be offset by 1 indent level from `bar`. However, this is not correct, because it would result in `baz`
+     * being indented by 2 indent levels in the second case (since `foo`, `bar`, and `baz` are all on separate lines, no
+     * collapsing would occur).
+     *
+     * Instead, the correct way would be to offset `baz` by 1 level from `bar`, offset `bar` by 1 level from the `)`, and
+     * offset the `)` by 0 levels from `foo`. This ensures that the offset between `bar` and the `)` are correctly collapsed
+     * in the second case.
+     *
+     * @param {Token} token The token
+     * @param {Token} offsetFrom The token that `token` should be offset from
+     * @param {number} offset The desired indent level
+     * @returns {void}
+     */
     setDesiredOffset(token, offsetFrom, offset) {
         if (offsetFrom && token.loc.start.line === offsetFrom.loc.start.line) {
             this.matchIndentOf(offsetFrom, token);


Test patch:
diff --git a/tests/lib/config.js b/tests/lib/config.js
index 935c210bd..59e1a47d0 100644
--- a/tests/lib/config.js
+++ b/tests/lib/config.js
@@ -158,6 +158,24 @@ describe("Config", () => {
             assert.deepEqual(customBaseConfig, { foo: "bar" });
             assert.equal(configHelper.options.format, "foo");
         });
+
+        it("should create config object when using baseConfig with extends", () => {
+            const customBaseConfig = {
+                extends: path.resolve(__dirname, "..", "fixtures", "config-extends", "array", ".eslintrc")
+            };
+            const configHelper = new Config({ baseConfig: customBaseConfig }, linter);
+
+            assert.deepEqual(configHelper.baseConfig.env, {
+                browser: false,
+                es6: true,
+                node: true
+            });
+            assert.deepEqual(configHelper.baseConfig.rules, {
+                "no-empty": 1,
+                "comma-dangle": 2,
+                "no-console": 2
+            });
+        });
     });
 
     describe("findLocalConfigFiles()", () => {