Repository: eslint/eslint
Pull request #10992 (2018-10-17 23:59:45)

Docs: add instructions for npm 2FA (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] Documentation update

<!--
    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)**

(refs #10631)

This adds a page to the maintainer guide describing how to get access to the npm 2FA secret for the `eslint` account. It also updates the release guidelines to account for the 2FA step.

**Is there anything you'd like reviewers to focus on?**

If you're on the TSC and you haven't set up Keybase yet, it might be useful to use these instructions to get set up, and point out any issues you encounter with the instructions.


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/maintainer-guide/governance.md b/docs/maintainer-guide/governance.md
index 15e5a6340..81d5908ea 100644
--- a/docs/maintainer-guide/governance.md
+++ b/docs/maintainer-guide/governance.md
@@ -93,7 +93,7 @@ A Committer is invited to become a TSC member by existing TSC members. A nominat
 
 1. Add the GitHub user to the "ESLint TSC" team
 1. Set the GitHub user to be have the "Owner" role for the ESLint organization
-1. Send welcome email with link to maintainer guide
+1. Send a welcome email with a link to the [maintainer guide](./) and the [npm 2FA guide](./npm-2fa).
 1. Add the TSC member to the README
 1. Invite to the Gitter TSC chatroom
 1. Make the TSC member an admin on the ESLint team mailing list
diff --git a/docs/maintainer-guide/npm-2fa.md b/docs/maintainer-guide/npm-2fa.md
new file mode 100644
index 000000000..e9a517761
--- /dev/null
+++ b/docs/maintainer-guide/npm-2fa.md
@@ -0,0 +1,16 @@
+# npm two-factor authentication
+
+The `eslint` npm account has two-factor authentication (2FA) enabled. The 2FA secret is distributed using a team on [Keybase](https://keybase.io). Anyone doing a release of a package from the Jenkins server needs to have access to the 2FA secret.
+
+If you're on ESLint's TSC, you should perform the following steps to obtain the 2FA secret:
+
+1. Download the [Keybase app](https://keybase.io/download) on a smartphone.
+1. Open the app and create an account.
+1. From the app, link your Keybase username with your GitHub username. (At the time of writing, the UI for this is to tap the face icon in the bottom-left of the app, then the profile picture in the top-right, then tap "Prove your GitHub" and follow the instructions.)
+1. Mention your Keybase username in the team chatroom, and wait for someone to add you to the Keybase team.
+1. Download an authenticator app like [Google Authenticator](https://support.google.com/accounts/answer/1066447) or [Authy](https://authy.com/), if you don't have one installed already.
+1. In the Keybase app, navigate to the Keybase filesystem (at the time of writing, the UI for this is to tap the hamburger icon in the bottom-right, then tap "Files") and then navigate to `/team/eslint/auth`.
+    * If your authenticator app is downloaded on the same device as your Keybase app (this will usually be the case if you're using the Keybase mobile app), then open `npm_2fa_code.txt` and copy the contents to the clipboard. Open your authenticator app, and paste the contents as a new key (by selecting something like "Enter a provided key" or "Enter key manually").
+    * If your authenticator app is downloaded on a *different* device from your Keybase app (e.g. if you're using a Keybase desktop app), then open `npm_2fa_code.png` and scan it as a QR code from your authenticator app.
+
+You should now be able to generate 6-digit 2FA codes for the `eslint` npm account using your authenticator app.
diff --git a/docs/maintainer-guide/releases.md b/docs/maintainer-guide/releases.md
index e8faab5f6..9d00030f3 100644
--- a/docs/maintainer-guide/releases.md
+++ b/docs/maintainer-guide/releases.md
@@ -16,6 +16,8 @@ A two-person release team is assigned to each scheduled release. This two-person
 
 The two-person team should seek input from the whole team on the Monday following a release to double-check if a patch release is necessary.
 
+At least one member of the release team needs to have access to [eslint's two-factor authentication for npm](./npm-2fa) in order to do a release.
+
 ## Release Communication
 
 Each scheduled release should be associated with a release issue ([example](https://github.com/eslint/eslint/issues/8138)). The release issue is the source of information for the team about the status of a release. Be sure the release issue has the "release" label so that it's easy to find.
@@ -30,7 +32,9 @@ On the day of a scheduled release, the release team should follow these steps:
     * Documentation changes.
     * Small bug fixes written by a team member.
 1. Log into Jenkins and schedule a build for the "ESLint Release" job.
-1. Wait for the "ESLint Release" job to complete.
+1. Watch the console output of the build on Jenkins. At some point, the build will pause and a link will be produced with an input field for a six-digit 2FA code.
+1. Enter the current six-digit 2FA code from your authenticator app. (Also see: [npm-2fa](./npm-2fa))
+1. Continue the build and wait for it to finish.
 1. Update the release blog post with a "Highlights" section, including new rules and anything else that's important.
 1. Make a release announcement in the public chatroom.
 1. Make a release announcement on Twitter.
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" }
+            ]
         }
+
     ]
 });