Regression in 7.17 through latest: “Cannot read property 'type' of undefined” at getKeyValue

[wchargin]

Repro code:

// @flow
import {Component} from "react";
type LinkProps = {...{}};
class Link extends Component<LinkProps> {}

Stack trace:

TypeError: Cannot read property 'type' of undefined
Occurred while linting /tmp/tmp.HiVXmY0CJv/test.js:4
    at getKeyValue (/tmp/tmp.HiVXmY0CJv/node_modules/eslint-plugin-react/lib/util/ast.js:172:14)
    at iterateProperties (/tmp/tmp.HiVXmY0CJv/node_modules/eslint-plugin-react/lib/util/propTypes.js:271:19)
    at iterateProperties (/tmp/tmp.HiVXmY0CJv/node_modules/eslint-plugin-react/lib/util/propTypes.js:45:9)
    at declarePropTypesForObjectTypeAnnotation (/tmp/tmp.HiVXmY0CJv/node_modules/eslint-plugin-react/lib/util/propTypes.js:258:5)
    at markPropTypesAsDeclared (/tmp/tmp.HiVXmY0CJv/node_modules/eslint-plugin-react/lib/util/propTypes.js:441:33)
    at Object.ClassDeclaration (/tmp/tmp.HiVXmY0CJv/node_modules/eslint-plugin-react/lib/util/propTypes.js:649:9)
    at updatedRuleInstructions.(anonymous function) (/tmp/tmp.HiVXmY0CJv/node_modules/eslint-plugin-react/lib/util/Components.js:900:43)
    at listeners.(anonymous function).forEach.listener (/tmp/tmp.HiVXmY0CJv/node_modules/eslint/lib/linter/safe-emitter.js:45:58)
    at Array.forEach (<anonymous>)
    at Object.emit (/tmp/tmp.HiVXmY0CJv/node_modules/eslint/lib/linter/safe-emitter.js:45:38)

Config:

module.exports = {
  parser: "babel-eslint",
  plugins: ["react"],
  parserOptions: {
    ecmaVersion: 2018,
    sourceType: "module",
    ecmaFeatures: {
      jsx: true,
    },
  },
  extends: [
    // It also suffices to pick a single "react/*" rule (tested "display-name",
    // "require-render-return", "no-deprecated").
    "plugin:react/recommended",
  ],
  settings: {
    react: {
      version: "16.4.1",
    },
  },
};

package.json:

{
  "name": "tmp.HiVXmY0CJv",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "dependencies": {
    "babel-eslint": "^10.0.3",
    "eslint": "^6.8.0",
    "eslint-plugin-react": "^7.18.3"
  }
}

yarn.lock:
https://gist.github.com/wchargin/141116df4ff3d1ef2da77b75b5fad51e#file-yarn-lock

Self-contained repro repository (warning: runs code from the internet):

url='https://gist.github.com/wchargin/141116df4ff3d1ef2da77b75b5fad51e.git' &&
git clone "${url}" repro &&
cd repro &&
yarn &&
yarn eslint test.js

Versions tested:

• 7.16.0: OK
• 7.17.0: FAIL
• 7.18.3: FAIL

Original code from which repro was created for context, and to answer
any “why would anyone do that?” questions:
https://github.com/sourcecred/sourcecred/blob/8c47dd1c14ed8c7117f3707a52c481b753d18138/src/webutil/Link.js#L24-L31

[ljharb]

Thanks for the simple repro!

[wchargin]

Thank you for the quick patch! This fixes my repro, but doesn’t fix a
slightly different case that I actually have in real code (linked at
bottom of original issue). Here is a second small repro:

// @flow
import {Component} from "react";
type LinkProps = $ReadOnly<{
  ...React$ElementConfig<"a">,
  ...{|+to: string|} | {|+href: string|},
  +styles?: $ReadOnlyArray<
    Object | false | null | void
  > /* Aphrodite styles, as passed to `css` */,
}>;
class Link extends Component<LinkProps> {}

This still has the same error message after updating the dependency on
this package. It looks like the following patch suffices as a “fix”:

diff --git a/lib/util/ast.js b/lib/util/ast.js
index 731a2733..867560f3 100644
--- a/lib/util/ast.js
+++ b/lib/util/ast.js
@@ -171,6 +171,9 @@ function getKeyValue(context, node) {
   if (node.type === 'ObjectTypeAnnotation') {
     return;
   }
+  if (node.type === 'UnionTypeAnnotation') {
+    return;
+  }
   const key = node.key || node.argument;
   return key.type === 'Identifier' ? key.name : key.value;
 }

…but this smells like could be more missing cases—intersection type
annotations?—so may or may not be exactly what you’re looking for. (In
case you do want to use this, I release the above patch under the MIT
and Apache-2 licenses, at your discretion.)

Does it make sense to re-open this, or should I file a new issue?

[ljharb]

There's probably lots of missing cases :-) a new issue (or even better, a new PR) covering as many cases as you can think of would be great!

[ljharb]

@wchargin ping :-)

[Beanow]

@ljharb this issue still persists with v7.18.3. Failed build for this file.

I believe #2570 (comment) still applies as detailed enough to be an issue on it's own :]

[ljharb]

@Beanow my hope was that @wchargin would make a PR with that; my time is limited, is that something you could do?