Repository: eslint/eslint
Pull request #10991 (2018-10-17 20:30:43)

Upgrade: eslint-release@1.0.0 (refs #10631)
<!--
    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] Other, please explain:

<!--
    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 (https://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)**

This upgrades `eslint-release` to 1.0.0. The new version of `eslint-release` has a separate "generate" and "publish" phase, to allow for a 2FA code to be supplied in between. As a side-effect, it's now possible to manually test most aspects of the release process (by running `npm run generate-release`) without actually publishing a release somewhere. In lieu of unit tests, this should make it easier to test changes to the release process, like this one. (I tested running `npm run generate-release` and `npm run generate-alpharelease` locally, and ensured that everything got generated properly.)

(refs https://github.com/eslint/eslint/issues/10631)

**Is there anything you'd like reviewers to focus on?**

Nothing in particular


Patch:
diff --git a/.gitignore b/.gitignore
index eae2b21d1..d2c98d47b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -15,3 +15,4 @@ versions.json
 /packages/**/node_modules
 /.vscode
 .sublimelinterrc
+.eslint-release-info.json
diff --git a/Makefile.js b/Makefile.js
index ee7d37b24..896be6a5b 100644
--- a/Makefile.js
+++ b/Makefile.js
@@ -150,10 +150,11 @@ function execSilent(cmd) {
 /**
  * Generates a release blog post for eslint.org
  * @param {Object} releaseInfo The release metadata.
+ * @param {string} [prereleaseMajorVersion] If this is a prerelease, the next major version after this prerelease
  * @returns {void}
  * @private
  */
-function generateBlogPost(releaseInfo) {
+function generateBlogPost(releaseInfo, prereleaseMajorVersion) {
     const ruleList = ls("lib/rules")
 
         // Strip the .js extension
@@ -166,7 +167,7 @@ function generateBlogPost(releaseInfo) {
          */
         .sort((ruleA, ruleB) => ruleB.length - ruleA.length);
 
-    const renderContext = Object.assign({ prereleaseMajorVersion: null, ruleList }, releaseInfo);
+    const renderContext = Object.assign({ prereleaseMajorVersion, ruleList }, releaseInfo);
 
     const output = ejs.render(cat("./templates/blogpost.md.ejs"), renderContext),
         now = new Date(),
@@ -250,11 +251,12 @@ function generateRuleIndexPage(basedir) {
 }
 
 /**
- * Commits the changes in the site and publishes them to GitHub.
- * @param {string} [tag] The string to tag the commit with.
+ * Creates a git commit and tag in an adjacent `eslint.github.io` repository, without pushing it to
+ * the remote. This assumes that the repository has already been modified somehow (e.g. by adding a blogpost).
+ * @param {string} [tag] The string to tag the commit with
  * @returns {void}
  */
-function publishSite(tag) {
+function commitSiteToGit(tag) {
     const currentDir = pwd();
 
     cd(SITE_DIR);
@@ -266,39 +268,53 @@ function publishSite(tag) {
     }
 
     exec("git fetch origin && git rebase origin/master");
-    exec("git push origin master --tags");
     cd(currentDir);
 }
 
 /**
- * Creates a release version tag and pushes to origin.
- * @param {boolean} [ciRelease] Set to true to indicate this is a CI release.
+ * Publishes the changes in an adjacent `eslint.github.io` repository to the remote. The
+ * site should already have local commits (e.g. from running `commitSiteToGit`).
  * @returns {void}
  */
-function release(ciRelease) {
+function publishSite() {
+    const currentDir = pwd();
+
+    cd(SITE_DIR);
+    exec("git push origin master --tags");
+    cd(currentDir);
+}
 
-    const releaseInfo = ReleaseOps.release(null, ciRelease);
+/**
+ * Updates the changelog, bumps the version number in package.json, creates a local git commit and tag,
+ * and generates the site in an adjacent `eslint.github.io` folder.
+ * @returns {void}
+ */
+function generateRelease() {
+    ReleaseOps.generateRelease();
+    const releaseInfo = JSON.parse(cat(".eslint-release-info.json"));
 
     echo("Generating site");
     target.gensite();
     generateBlogPost(releaseInfo);
-    publishSite(`v${releaseInfo.version}`);
-    echo("Site has been published");
-
-    echo("Publishing to GitHub");
-    ReleaseOps.publishReleaseToGitHub(releaseInfo);
+    commitSiteToGit(`v${releaseInfo.version}`);
 }
 
 /**
- * Creates a prerelease version tag and pushes to origin.
+ * Updates the changelog, bumps the version number in package.json, creates a local git commit and tag,
+ * and generates the site in an adjacent `eslint.github.io` folder.
  * @param {string} prereleaseId The prerelease identifier (alpha, beta, etc.)
  * @returns {void}
  */
-function prerelease(prereleaseId) {
-
-    const releaseInfo = ReleaseOps.release(prereleaseId);
+function generatePrerelease(prereleaseId) {
+    ReleaseOps.generateRelease(prereleaseId);
+    const releaseInfo = JSON.parse(cat(".eslint-release-info.json"));
     const nextMajor = semver.inc(releaseInfo.version, "major");
 
+    echo("Generating site");
+
+    // always write docs into the next major directory (so 2.0.0-alpha.0 writes to 2.0.0)
+    target.gensite(nextMajor);
+
     /*
      * Premajor release should have identical "next major version".
      * Preminor and prepatch release will not.
@@ -313,21 +329,23 @@ function prerelease(prereleaseId) {
          * Blog post generation logic needs to be aware of this (as well as
          * know what the next major version is actually supposed to be).
          */
-        releaseInfo.prereleaseMajorVersion = nextMajor;
+        generateBlogPost(releaseInfo, nextMajor);
+    } else {
+        generateBlogPost(releaseInfo);
     }
 
-    echo("Generating site");
-
-    // always write docs into the next major directory (so 2.0.0-alpha.0 writes to 2.0.0)
-    target.gensite(nextMajor);
-    generateBlogPost(releaseInfo);
-    publishSite(`v${releaseInfo.version}`);
-    echo("Site has been published");
-
-    echo("Publishing to GitHub");
-    ReleaseOps.publishReleaseToGitHub(releaseInfo);
+    commitSiteToGit(`v${releaseInfo.version}`);
 }
 
+/**
+ * Publishes a generated release to npm and GitHub, and pushes changes to the adjacent `eslint.github.io` repo
+ * to remote repo.
+ * @returns {void}
+ */
+function publishRelease() {
+    ReleaseOps.publishRelease();
+    publishSite();
+}
 
 /**
  * Splits a command result to separate lines.
@@ -1129,18 +1147,6 @@ target.perf = function() {
     });
 };
 
-target.release = function() {
-    release();
-};
-
-target.ciRelease = function() {
-    release(true);
-};
-
-target.publishsite = function() {
-    publishSite();
-};
-
-target.prerelease = function(args) {
-    prerelease(args[0]);
-};
+target.generateRelease = generateRelease;
+target.generatePrerelease = ([prereleaseType]) => generatePrerelease(prereleaseType);
+target.publishRelease = publishRelease;
diff --git a/docs/rules/prefer-const.md b/docs/rules/prefer-const.md
index cf86f5b7b..3e5a5d0c3 100644
--- a/docs/rules/prefer-const.md
+++ b/docs/rules/prefer-const.md
@@ -79,6 +79,15 @@ for (let i = 0, end = 10; i < end; ++i) {
     console.log(a);
 }
 
+// `predicate` is only assigned once but cannot be separately declared as `const`
+let predicate;
+[object.type, predicate] = foo();
+
+// `a` is only assigned once but cannot be separately declared as `const`
+let a;
+const b = {};
+({ a, c: b.c } = func());
+
 // suggest to use `no-var` rule.
 var b = 3;
 console.log(b);
diff --git a/lib/rules/prefer-const.js b/lib/rules/prefer-const.js
index 774fcf064..8b3bc5e0e 100644
--- a/lib/rules/prefer-const.js
+++ b/lib/rules/prefer-const.js
@@ -57,6 +57,7 @@ function canBecomeVariableDeclaration(identifier) {
  * @returns {boolean} Indicates if the variable is from outer scope or function parameters.
  */
 function isOuterVariableInDestructing(name, initScope) {
+
     if (initScope.through.find(ref => ref.resolved && ref.resolved.name === name)) {
         return true;
     }
@@ -96,6 +97,54 @@ function getDestructuringHost(reference) {
     return node;
 }
 
+/**
+ * Determines if a destructuring assignment node contains
+ * any MemberExpression nodes. This is used to determine if a
+ * variable that is only written once using destructuring can be
+ * safely converted into a const declaration.
+ * @param {ASTNode} node The ObjectPattern or ArrayPattern node to check.
+ * @returns {boolean} True if the destructuring pattern contains
+ *      a MemberExpression, false if not.
+ */
+function hasMemberExpressionAssignment(node) {
+    switch (node.type) {
+        case "ObjectPattern":
+            return node.properties.some(prop => {
+                if (prop) {
+
+                    /*
+                     * Spread elements have an argument property while
+                     * others have a value property. Because different
+                     * parsers use different node types for spread elements,
+                     * we just check if there is an argument property.
+                     */
+                    return hasMemberExpressionAssignment(prop.argument || prop.value);
+                }
+
+                return false;
+            });
+
+        case "ArrayPattern":
+            return node.elements.some(element => {
+                if (element) {
+                    return hasMemberExpressionAssignment(element);
+                }
+
+                return false;
+            });
+
+        case "AssignmentPattern":
+            return hasMemberExpressionAssignment(node.left);
+
+        case "MemberExpression":
+            return true;
+
+        // no default
+    }
+
+    return false;
+}
+
 /**
  * Gets an identifier node of a given variable.
  *
@@ -148,7 +197,8 @@ function getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign) {
 
             if (destructuringHost !== null && destructuringHost.left !== void 0) {
                 const leftNode = destructuringHost.left;
-                let hasOuterVariables = false;
+                let hasOuterVariables = false,
+                    hasNonIdentifiers = false;
 
                 if (leftNode.type === "ObjectPattern") {
                     const properties = leftNode.properties;
@@ -157,16 +207,23 @@ function getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign) {
                         .filter(prop => prop.value)
                         .map(prop => prop.value.name)
                         .some(name => isOuterVariableInDestructing(name, variable.scope));
+
+                    hasNonIdentifiers = hasMemberExpressionAssignment(leftNode);
+
                 } else if (leftNode.type === "ArrayPattern") {
                     const elements = leftNode.elements;
 
                     hasOuterVariables = elements
                         .map(element => element && element.name)
                         .some(name => isOuterVariableInDestructing(name, variable.scope));
+
+                    hasNonIdentifiers = hasMemberExpressionAssignment(leftNode);
                 }
-                if (hasOuterVariables) {
+
+                if (hasOuterVariables || hasNonIdentifiers) {
                     return null;
                 }
+
             }
 
             writer = reference;
@@ -192,9 +249,11 @@ function getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign) {
     if (!shouldBeConst) {
         return null;
     }
+
     if (isReadBeforeInit) {
         return variable.defs[0].name;
     }
+
     return writer.identifier;
 }
 
@@ -295,7 +354,7 @@ module.exports = {
     create(context) {
         const options = context.options[0] || {};
         const sourceCode = context.getSourceCode();
-        const checkingMixedDestructuring = options.destructuring !== "all";
+        const shouldMatchAnyDestructuredVariable = options.destructuring !== "all";
         const ignoreReadBeforeAssign = options.ignoreReadBeforeAssign === true;
         const variables = [];
 
@@ -316,7 +375,7 @@ module.exports = {
         function checkGroup(nodes) {
             const nodesToReport = nodes.filter(Boolean);
 
-            if (nodes.length && (checkingMixedDestructuring || nodesToReport.length === nodes.length)) {
+            if (nodes.length && (shouldMatchAnyDestructuredVariable || nodesToReport.length === nodes.length)) {
                 const varDeclParent = findUp(nodes[0], "VariableDeclaration", parentNode => parentNode.type.endsWith("Statement"));
                 const shouldFix = varDeclParent &&
 
diff --git a/package.json b/package.json
index dd3fe4615..12c1a3c33 100644
--- a/package.json
+++ b/package.json
@@ -11,11 +11,11 @@
     "test": "node Makefile.js test",
     "lint": "node Makefile.js lint",
     "fuzz": "node Makefile.js fuzz",
-    "release": "node Makefile.js release",
-    "ci-release": "node Makefile.js ciRelease",
-    "alpharelease": "node Makefile.js prerelease -- alpha",
-    "betarelease": "node Makefile.js prerelease -- beta",
-    "rcrelease": "node Makefile.js prerelease -- rc",
+    "generate-release": "node Makefile.js generateRelease",
+    "generate-alpharelease": "node Makefile.js generatePrerelease -- alpha",
+    "generate-betarelease": "node Makefile.js generatePrerelease -- beta",
+    "generate-rcrelease": "node Makefile.js generatePrerelease -- rc",
+    "publish-release": "node Makefile.js publishRelease",
     "docs": "node Makefile.js docs",
     "gensite": "node Makefile.js gensite",
     "browserify": "node Makefile.js browserify",
@@ -90,7 +90,7 @@
     "eslint-plugin-eslint-plugin": "^1.2.0",
     "eslint-plugin-node": "^7.0.1",
     "eslint-plugin-rulesdir": "^0.1.0",
-    "eslint-release": "^0.11.1",
+    "eslint-release": "^1.0.0",
     "eslint-rule-composer": "^0.3.0",
     "eslump": "^1.6.2",
     "esprima": "^4.0.1",


Test patch:
diff --git a/tests/lib/rules/prefer-const.js b/tests/lib/rules/prefer-const.js
index c7b8a93c2..f97320234 100644
--- a/tests/lib/rules/prefer-const.js
+++ b/tests/lib/rules/prefer-const.js
@@ -117,6 +117,54 @@ ruleTester.run("prefer-const", rule, {
             parser: fixtureParser("babel-eslint5/destructuring-object-spread")
         },
 
+        // https://github.com/eslint/eslint/issues/8308
+        {
+            code: "let predicate; [typeNode.returnType, predicate] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [typeNode.returnType, ...predicate] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+
+            // intentionally testing empty slot in destructuring assignment
+            code: "let predicate; [typeNode.returnType,, predicate] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [typeNode.returnType=5, predicate] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [[typeNode.returnType=5], predicate] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [[typeNode.returnType, predicate]] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [typeNode.returnType, [predicate]] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [, [typeNode.returnType, predicate]] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [, {foo:typeNode.returnType, predicate}] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let predicate; [, {foo:typeNode.returnType, ...predicate}] = foo();",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+        {
+            code: "let a; const b = {}; ({ a, c: b.c } = func());",
+            parserOptions: { ecmaVersion: 2018 }
+        },
+
         // ignoreReadBeforeAssign
         {
             code: "let x; function foo() { bar(x); } x = 0;",
@@ -380,6 +428,33 @@ ruleTester.run("prefer-const", rule, {
                 { message: "'y' is never reassigned. Use 'const' instead.", type: "Identifier" },
                 { message: "'z' is never reassigned. Use 'const' instead.", type: "Identifier" }
             ]
+        },
+
+        // https://github.com/eslint/eslint/issues/8308
+        {
+            code: "let predicate; [, {foo:returnType, predicate}] = foo();",
+            output: null,
+            parserOptions: { ecmaVersion: 2018 },
+            errors: [
+                { message: "'predicate' is never reassigned. Use 'const' instead.", type: "Identifier" }
+            ]
+        },
+        {
+            code: "let predicate; [, {foo:returnType, predicate}, ...bar ] = foo();",
+            output: null,
+            parserOptions: { ecmaVersion: 2018 },
+            errors: [
+                { message: "'predicate' is never reassigned. Use 'const' instead.", type: "Identifier" }
+            ]
+        },
+        {
+            code: "let predicate; [, {foo:returnType, ...predicate} ] = foo();",
+            output: null,
+            parserOptions: { ecmaVersion: 2018 },
+            errors: [
+                { message: "'predicate' is never reassigned. Use 'const' instead.", type: "Identifier" }
+            ]
         }
+
     ]
 });