Skip to content

Commit 3b76083

Browse files
chrisbobbegnprice
authored andcommitted
eslint: Use new hermes-eslint parser, instead of @babel/eslint-parser
This is "the recommended parser for use for linting with Flow code" ( https://www.npmjs.com/package/hermes-eslint ) and it's the one most recommended in Flow's guide to enabling Flow enums, which we'd like to do soon: https://flow.org/en/docs/enums/enabling-enums/ Since that Flow-enums guide says "Upgrade hermes-parser to at least version 0.4.8", add a `resolutions` line to guarantee that. The hermes-eslint code seems to live here on GitHub: https://github.com/facebook/hermes/tree/main/tools/hermes-parser/js/hermes-eslint With this parser, it will be possible (once we enable Flow enums, coming up) to define a Flow enum without getting an ESLint error like Parsing error: Unexpected token, expected "{" . The new parsing also gives better feedback on existing code. We fix a new warning and a new error in this commit: - A warning "'V' is defined but never used" on the type param V at src/generics.js:175:54. Sure enough, V isn't used; remove it. - The eslint-disable for no-use-before-define, at src/presence/__tests__/heartbeat-test.js:49:5, isn't suppressing any error and can be removed. Yep, the use isn't before the definition; remove the eslint-disable. Finally, repeal a few now-redundant ESLint rules, with comments. For posterity, here's the full error output that I abbreviated in a comment on ft-flow/define-flow-type: Running lint... Oops! Something went wrong! :( ESLint: 8.17.0 TypeError: globalScope.__defineGeneric is not a function Occurred while linting /Users/chrisbobbe/dev/zulip-mobile/src/api/modelTypes.js:53 Rule: "ft-flow/define-flow-type" at makeDefined (/Users/chrisbobbe/dev/zulip-mobile/node_modules/eslint-plugin-ft-flow/dist/rules/defineFlowType.js:21:21) at GenericTypeAnnotation (/Users/chrisbobbe/dev/zulip-mobile/node_modules/eslint-plugin-ft-flow/dist/rules/defineFlowType.js:67:9) at ruleErrorHandler (/Users/chrisbobbe/dev/zulip-mobile/node_modules/eslint/lib/linter/linter.js:1114:28) at /Users/chrisbobbe/dev/zulip-mobile/node_modules/eslint/lib/linter/safe-emitter.js:45:58 at Array.forEach (<anonymous>) at Object.emit (/Users/chrisbobbe/dev/zulip-mobile/node_modules/eslint/lib/linter/safe-emitter.js:45:38) at NodeEventGenerator.applySelector (/Users/chrisbobbe/dev/zulip-mobile/node_modules/eslint/lib/linter/node-event-generator.js:297:26) at NodeEventGenerator.applySelectors (/Users/chrisbobbe/dev/zulip-mobile/node_modules/eslint/lib/linter/node-event-generator.js:326:22) at NodeEventGenerator.enterNode (/Users/chrisbobbe/dev/zulip-mobile/node_modules/eslint/lib/linter/node-event-generator.js:340:14) at CodePathAnalyzer.enterNode (/Users/chrisbobbe/dev/zulip-mobile/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:795:23)
1 parent 16d1182 commit 3b76083

File tree

7 files changed

+52
-11
lines changed

7 files changed

+52
-11
lines changed

.eslintrc.yaml

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
# $ git diff --no-index /tmp/{foo,bar}.json
2121

2222

23-
parser: "@babel/eslint-parser"
23+
parser: "hermes-eslint"
2424

2525
extends:
2626
- airbnb
@@ -75,6 +75,14 @@ rules:
7575
# of Airbnb's settings where we don't want a full repeal.
7676
#
7777

78+
# With hermes-eslint for `parser`, above, this rule can analyze Flow
79+
# types. It would fire on global types like $ReadOnly, for which it
80+
# doesn't see a declaration. We could find out how to teach the rule that
81+
# these types are globally defined…or just turn the rule off, because it's
82+
# redundant with Flow (specifically `Flow(cannot-resolve-name)`, I think):
83+
# https://github.com/flow-typed/eslint-plugin-ft-flow/issues/33#issue-1274050123
84+
no-undef: off # https://eslint.org/docs/latest/rules/no-undef
85+
7886
# Tricky-looking code. These are all sometimes perfectly legitimate.
7987
no-bitwise: off
8088
no-confusing-arrow: off
@@ -284,6 +292,13 @@ rules:
284292
# React; plugin `react`.
285293
#
286294

295+
# These two are workarounds to suppress no-unused-vars when the parser
296+
# doesn't understand JSX. Our parser (hermes-eslint, set above) does, so
297+
# they're unnecessary:
298+
# https://github.com/flow-typed/eslint-plugin-ft-flow/issues/33#issue-1274050123
299+
react/jsx-uses-react: off # https://github.com/jsx-eslint/eslint-plugin-react/blob/v7.30.0/docs/rules/jsx-uses-react.md
300+
react/jsx-uses-vars: off # https://github.com/jsx-eslint/eslint-plugin-react/blob/v7.30.0/docs/rules/jsx-uses-vars.md
301+
287302
react/jsx-filename-extension: off # Like RN upstream, we call the files *.js.
288303

289304
react/destructuring-assignment: off # The opposite is often much cleaner.
@@ -324,8 +339,18 @@ rules:
324339
# Flow; plugin `ft-flow`.
325340
#
326341

342+
343+
# These two are workarounds to suppress no-undef and no-unused-vars,
344+
# respectively, when the parser doesn't understand Flow types. Our parser
345+
# (hermes-eslint, set above) does, so they're unnecessary:
346+
# https://github.com/flow-typed/eslint-plugin-ft-flow/issues/33#issue-1274050123
347+
# In fact, linting breaks if I leave define-flow-type on with this parser:
348+
# TypeError: globalScope.__defineGeneric is not a function
349+
# (See commit message for full error output.)
350+
ft-flow/define-flow-type: off # https://github.com/flow-typed/eslint-plugin-ft-flow/tree/v2.0.1#define-flow-type
351+
ft-flow/use-flow-type: off # https://github.com/flow-typed/eslint-plugin-ft-flow/tree/v2.0.1#use-flow-type
352+
327353
ft-flow/boolean-style: [error, boolean]
328-
ft-flow/define-flow-type: warn
329354
ft-flow/delimiter-dangle: off
330355
ft-flow/no-dupe-keys: error
331356
ft-flow/no-primitive-constructor-types: error
@@ -341,7 +366,6 @@ rules:
341366
# For more formatting rules, see tools/formatting.eslintrc.yaml.
342367
ft-flow/type-id-match: [error, '^([A-Z][a-z0-9]+)+$']
343368
ft-flow/union-intersection-spacing: [error, always]
344-
ft-flow/use-flow-type: warn
345369
ft-flow/valid-syntax: warn
346370

347371
overrides:

package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@
105105
"flow-bin": "^0.162.0",
106106
"flow-coverage-report": "^0.8.0",
107107
"flow-typed": "^3.3.1",
108+
"hermes-eslint": "^0.9.0",
108109
"immutable-devtools": "^0.1.5",
109110
"jest": "^26.6.3",
110111
"jest-cli": "^26.4.1",
@@ -127,6 +128,7 @@
127128
"yarn-deduplicate": "^3.0.0"
128129
},
129130
"resolutions": {
131+
"hermes-parser": ">=0.4.8",
130132
"jest-expo/react-test-renderer": "17.0.2",
131133
"prettier-eslint-cli/prettier-eslint": "^15.0.0",
132134
"react-native/use-subscription": ">=1.0.0 <1.6.0"

src/generics.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ export type ReadWrite<T: $ReadOnly<{ ... }>> = $Diff<T, {||}>;
172172
// $Diff and other "type destructors", which as of 2020-08 the Flow team is
173173
// taking up as a priority:
174174
// https://github.com/facebook/flow/issues/7886#issuecomment-669977952
175-
export type SubsetProperties<T, U> = $ObjMapi<U, <K, V>(K) => T[K]>;
175+
export type SubsetProperties<T, U> = $ObjMapi<U, <K>(K) => T[K]>;
176176

177177
/**
178178
* Assert a contradiction, statically. Do nothing at runtime.

src/presence/__tests__/heartbeat-test.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ describe('Heartbeat', () => {
4646
return this.heartbeat.isActive();
4747
}
4848

49-
// eslint-disable-next-line no-use-before-define
5049
static getExtant(): $ReadOnlyArray<JestHeartbeatHelper> {
5150
return this._currentHeartbeats;
5251
}

tools/flow-todo.eslintrc.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
parser: "@babel/eslint-parser"
1+
parser: hermes-eslint
22
plugins:
33
- ft-flow
44
rules:

tools/formatting.eslintrc.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
# we can follow each Prettier run with a streamlined (and faster) ESLint run
33
# that uses only these rules.
44

5-
parser: "@babel/eslint-parser"
5+
parser: "hermes-eslint"
66

77
plugins:
88
- ft-flow

yarn.lock

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6006,10 +6006,26 @@ hermes-engine@~0.9.0:
60066006
resolved "https://registry.yarnpkg.com/hermes-engine/-/hermes-engine-0.9.0.tgz#84d9cfe84e8f6b1b2020d6e71b350cec84ed982f"
60076007
integrity sha512-r7U+Y4P2Qg/igFVZN+DpT7JFfXUn1MM4dFne8aW+cCrF6RRymof+VqrUHs1kl07j8h8V2CNesU19RKgWbr3qPw==
60086008

6009-
6010-
version "0.4.7"
6011-
resolved "https://registry.yarnpkg.com/hermes-parser/-/hermes-parser-0.4.7.tgz#410f5129d57183784d205a0538e6fbdcf614c9ea"
6012-
integrity sha512-jc+zCtXbtwTiXoMAoXOHepxAaGVFIp89wwE9qcdwnMd/uGVEtPoY8FaFSsx0ThPvyKirdR2EsIIDVrpbSXz1Ag==
6009+
hermes-eslint@^0.9.0:
6010+
version "0.9.0"
6011+
resolved "https://registry.yarnpkg.com/hermes-eslint/-/hermes-eslint-0.9.0.tgz#f1423abe3dfd959257430d61a9bccd4700b59e09"
6012+
integrity sha512-rlkK51UpGwo0ZWg8hu8DVICth7RfGSvaEJzFflos8bDOYm/d842/J3IXi0lB9R9waOp4VGGSc8VDmh+a9p2Q2w==
6013+
dependencies:
6014+
esrecurse "^4.3.0"
6015+
hermes-estree "0.9.0"
6016+
hermes-parser "0.9.0"
6017+
6018+
6019+
version "0.9.0"
6020+
resolved "https://registry.yarnpkg.com/hermes-estree/-/hermes-estree-0.9.0.tgz#026e0abe6db1dcf50a81a79014b779a83db3b814"
6021+
integrity sha512-5DZ7Y0CbHVk8zPqgRCvqp8iw+P05svnQDI1aJFjdqCfXJ/1CZ+8aYpGlhJ29zCG5SE5duGTzSxogAYYI4QqXqw==
6022+
6023+
[email protected], [email protected], hermes-parser@>=0.4.8:
6024+
version "0.9.0"
6025+
resolved "https://registry.yarnpkg.com/hermes-parser/-/hermes-parser-0.9.0.tgz#ede3044d50479c61843cef5bbdcea83933d4e4ec"
6026+
integrity sha512-IcvJIlAn+9tpHkP+HTsxWKrIdQPp0gvGrrQmxlL4XnNS+Oh6R/Fpxbcoflm2kY3zgQjEvxZxLiK/2+k3/5wsrw==
6027+
dependencies:
6028+
hermes-estree "0.9.0"
60136029

60146030
hermes-profile-transformer@^0.0.6:
60156031
version "0.0.6"

0 commit comments

Comments
 (0)