Repository: eslint/eslint (js)

Issue:
4.1 breaks on extending multiple configs from a plugin
**Tell us about your environment**

* **ESLint Version:** 4.1.0
* **Node Version:** 8.0.3
* **npm Version:** 5.0.3

**What parser (default, Babel-ESLint, etc.) are you using?**
default and typescript

**Please show your full configuration:**

<!-- Paste your configuration below: -->
```
{
  "plugins": [
    "@corbinu/corbinu"
  ],
  "extends": [
    "plugin:@corbinu/corbinu/base",
    "plugin:@corbinu/corbinu/node",
    "plugin:@corbinu/corbinu/lodash",
    "plugin:@corbinu/corbinu/relax"
  ],
  "rules": {
    "node/no-missing-require": "off"
  }
}
```

**What did you do? Please include the actual source code causing the issue.**
I upgraded to 4.1 and began seeing output as if "sourceType" was not set to module.

If I remove all the extends except for "plugin:@corbinu/corbinu/base" then things begin to resolve again or if I paste and merge the configs from each of those then everything reacts normally

Sorry know it makes it more complex to dig into my plugin the source is here: [https://gitlab.com/corbinu/eslint-plugin-corbinu](https://gitlab.com/corbinu/eslint-plugin-corbinu)

It is only a set of configs no rules of my own.


Pull request:
Fix: ensure configs from a plugin are cached separately (fixes #8792)
<!--
    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/8792)

<!--
    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)**

Config files are cached by file path to avoid needing to load the same config file twice. However, configs provided by plugins all have the same file path (the index file of the plugin). This created a bug when loading multiple configs for the same plugin where only the highest-precedence config from that plugin would be loaded. All other configs from that plugin would be considered identical by the cache, so they would all end up getting pulled from the cache as the same config.

This commit updates the caching logic to use the config full name as a cache index. This is still the file path when resolving a config from the filesystem, but it is the unique plugin config identifier (e.g. 'plugin:foo/node-config') when resolving a plugin config.

**Is there anything you'd like reviewers to focus on?**

Are there any other places where we assume that the `filePath` of a resolved config is unique?

Patch:
diff --git a/docs/developer-guide/working-with-plugins.md b/docs/developer-guide/working-with-plugins.md
index b8667d33a..c3bbccc48 100644
--- a/docs/developer-guide/working-with-plugins.md
+++ b/docs/developer-guide/working-with-plugins.md
@@ -218,3 +218,32 @@ Add these keywords into your `package.json` file to make it easy for others to f
 ## Further Reading
 
 * [npm Developer Guide](https://docs.npmjs.com/misc/developers)
+
+### Working with Custom Parsers
+
+If you want to use your own parser and provide additional capabilities for your rules, you can specify your own custom parser. By default, the ESLint parser will use its parse method that takes in the source code as a first parameter and additional optional parameters as a second parameter to create an AST. You can specify a `parse` configuration to use your own custom parser. If a `parseForESLint` method is exposed, this method will be used to parse. Otherwise, the parser will use the parse method. `parseForESLint` behaves like `parse` and takes in the the source code and optional ESLint configurations. When `parseForESLint` is called, the method should return an object that contains the required property `ast` and an optional `services` property. `ast` should contain the AST. The `services` property contains the parser-dependent services. The value of the service property is available to rules as `context.parserServices`
+
+If no parseForESLint function is found, the parser will use the default parse method with the source code and the parser options. You can find a ESLint parser project [here](https://github.com/eslint/typescript-eslint-parser).
+
+    {
+
+        "parser": './path/to/awesome-custom-parser.js'
+    }
+
+```javascript
+var espree = require("espree");
+// awesome-custom-parser.js
+exports.parseForESLint = function(code, options) {
+    return {
+        ast: espree.parse(code, options),
+        services: {
+            foo: function() {
+                console.log("foo");
+            }
+        }
+    };
+};
+
+```
+
+
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..07436a87c 100644
--- a/lib/config/config-cache.js
+++ b/lib/config/config-cache.js
@@ -24,12 +24,12 @@ function hash(vector) {
 //------------------------------------------------------------------------------
 
 /**
- * Configuration caching class (not exported)
+ * Configuration caching class
  */
-class ConfigCache {
+module.exports = class ConfigCache {
 
     constructor() {
-        this.filePathCache = new Map();
+        this.configFullNameCache = new Map();
         this.localHierarchyCache = new Map();
         this.mergedVectorCache = new Map();
         this.mergedCache = new Map();
@@ -37,23 +37,25 @@ class ConfigCache {
 
     /**
      * Gets a config object from the cache for the specified config file path.
-     * @param {string} configFilePath the absolute path to the config file
+     * @param {string} configFullName the name of the configuration as used in the eslint config(e.g. 'plugin:node/recommended'),
+     * or the absolute path to a config file. This should uniquely identify a config.
      * @returns {Object|null} config object, if found in the cache, otherwise null
      * @private
      */
-    getConfig(configFilePath) {
-        return this.filePathCache.get(configFilePath);
+    getConfig(configFullName) {
+        return this.configFullNameCache.get(configFullName);
     }
 
     /**
      * Sets a config object in the cache for the specified config file path.
-     * @param {string} configFilePath the absolute path to the config file
+     * @param {string} configFullName the name of the configuration as used in the eslint config(e.g. 'plugin:node/recommended'),
+     * or the absolute path to a config file. This should uniquely identify a config.
      * @param {Object} config the config object to add to the cache
      * @returns {void}
      * @private
      */
-    setConfig(configFilePath, config) {
-        this.filePathCache.set(configFilePath, config);
+    setConfig(configFullName, config) {
+        this.configFullNameCache.set(configFullName, config);
     }
 
     /**
@@ -125,6 +127,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..3c790cf3b 100644
--- a/lib/config/config-file.js
+++ b/lib/config/config-file.js
@@ -488,12 +488,15 @@ function normalizePackageName(name, prefix) {
  * @returns {Object} An object containing 3 properties:
  * - 'filePath' (required) the resolved path that can be used directly to load the configuration.
  * - 'configName' the name of the configuration inside the plugin.
- * - 'configFullName' the name of the configuration as used in the eslint config (e.g. 'plugin:node/recommended').
+ * - 'configFullName' (required) the name of the configuration as used in the eslint config(e.g. 'plugin:node/recommended'),
+ *     or the absolute path to a config file. This should uniquely identify a config.
  * @private
  */
 function resolve(filePath, relativeTo) {
     if (isFilePath(filePath)) {
-        return { filePath: path.resolve(relativeTo || "", filePath) };
+        const fullPath = path.resolve(relativeTo || "", filePath);
+
+        return { filePath: fullPath, configFullName: fullPath };
     }
     let normalizedPackageName;
 
@@ -510,7 +513,7 @@ function resolve(filePath, relativeTo) {
     normalizedPackageName = normalizePackageName(filePath, "eslint-config");
     debug(`Attempting to resolve ${normalizedPackageName}`);
     filePath = resolver.resolve(normalizedPackageName, getLookupPath(relativeTo));
-    return { filePath };
+    return { filePath, configFullName: filePath };
 
 
 }
@@ -560,11 +563,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;
 }
 
 /**
@@ -579,7 +583,7 @@ function loadObject(configObject) {
 function load(filePath, configContext, relativeTo) {
     const resolvedPath = resolve(filePath, relativeTo);
 
-    const cachedConfig = configContext.configCache.getConfig(resolvedPath.filePath);
+    const cachedConfig = configContext.configCache.getConfig(resolvedPath.configFullName);
 
     if (cachedConfig) {
         return cachedConfig;
@@ -590,7 +594,7 @@ function load(filePath, configContext, relativeTo) {
     if (config) {
         config.filePath = resolvedPath.filePath;
         config.baseDirectory = path.dirname(resolvedPath.filePath);
-        configContext.configCache.setConfig(resolvedPath.filePath, config);
+        configContext.configCache.setConfig(resolvedPath.configFullName, config);
     }
 
     return config;
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/fixtures/config-file/plugins/.eslintrc2.yml b/tests/fixtures/config-file/plugins/.eslintrc2.yml
new file mode 100644
index 000000000..973d77096
--- /dev/null
+++ b/tests/fixtures/config-file/plugins/.eslintrc2.yml
@@ -0,0 +1,3 @@
+extends:
+    - plugin:test/foo
+    - plugin:test/bar
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()", () => {
diff --git a/tests/lib/config/config-file.js b/tests/lib/config/config-file.js
index a9d2baa0d..23a206b05 100644
--- a/tests/lib/config/config-file.js
+++ b/tests/lib/config/config-file.js
@@ -899,6 +899,49 @@ describe("ConfigFile", () => {
                     }
                 });
             });
+
+            it("should load two separate configs from a plugin", () => {
+                const stubConfig = new Config({}, new Linter());
+                const resolvedPath = path.resolve(PROJECT_PATH, "./node_modules/eslint-plugin-test/index.js");
+
+                const configDeps = {
+                    "../util/module-resolver": createStubModuleResolver({
+                        "eslint-plugin-test": resolvedPath
+                    }),
+                    "require-uncached"(filename) {
+                        return configDeps[filename];
+                    }
+                };
+
+                configDeps[resolvedPath] = {
+                    configs: {
+                        foo: { rules: { semi: 2, quotes: 1 } },
+                        bar: { rules: { quotes: 2, yoda: 2 } }
+                    }
+                };
+
+                const StubbedConfigFile = proxyquire("../../../lib/config/config-file", configDeps);
+
+                const configFilePath = getFixturePath("plugins/.eslintrc2.yml");
+                const config = StubbedConfigFile.load(configFilePath, stubConfig);
+
+                assert.deepEqual(config, {
+                    baseDirectory: path.dirname(configFilePath),
+                    filePath: configFilePath,
+                    parserOptions: {},
+                    globals: {},
+                    env: {},
+                    rules: {
+                        semi: 2,
+                        quotes: 2,
+                        yoda: 2
+                    },
+                    extends: [
+                        "plugin:test/foo",
+                        "plugin:test/bar"
+                    ]
+                });
+            });
         });
 
         describe("even if config files have Unicode BOM,", () => {