fix(eslint-plugin): [prefer-readonly-parameter-types] handle class sharp private field and member without throwing error by lonyele · Pull Request #4343 · typescript-eslint/typescript-eslint

lonyele · 2021-12-24T07:55:31Z

PR Checklist

Addresses an existing issue: fixes [prefer-readonly-parameter-types] crashing with error Non-null Assertion Failed: Expected to find a property "#promise" for the type. #4068
That issue was marked as accepting prs
Steps in CONTRIBUTING.md were taken

Overview

It was my first time diving into typescript code and it was a rabbit hole... I feel like it's a bug from typescript that other symbols have the same escapedName and name like escapedName: "__@foo@10", name: "__@foo@10" but this# private identifiers have different escapedName and name like escapedName: "__#1@#foo", name: "#foo". This is why the code goes into checker.getTypeOfPropertyOfType(type, name) and return undefined that at the end throws the error.

I stepped into const type = checker.getTypeAtLocation(tsNode) to find why escapedName and name are different but failed...even going into the end

Aside from handling this error case from typescript-eslint side. I'm curious how this rule should handles the Class type. The code is handling parameter properties but what about some complicated combos of public/private fields and members? I haven't really used to this extend so I'm purely interested in what behavior is expected. I found that making read-only member in class is impossible but private or this # private is possible which is automatically readonly? more precisely just cannot be accessed on compile(private) or on runtime(# private)

nx-cloud · 2021-12-24T07:55:33Z

☁️ Nx Cloud Report

CI ran the following commands for commit 3d8acb4. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 48 targets

Sent with 💌 from NxCloud.

typescript-eslint · 2021-12-24T07:55:35Z

Thanks for the PR, @lonyele!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

netlify · 2021-12-24T07:55:39Z

❌ Deploy Preview for typescript-eslint failed.

🔨 Explore the source changes: 3d8acb4

🔍 Inspect the deploy log: https://app.netlify.com/sites/typescript-eslint/deploys/621cfd42804d350008c96c22

codecov · 2021-12-24T08:03:27Z

Codecov Report

Merging #4343 (3d8acb4) into main (d698f6b) will increase coverage by 0.34%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #4343      +/-   ##
==========================================
+ Coverage   92.49%   92.83%   +0.34%     
==========================================
  Files         346      164     -182     
  Lines       11696     8490    -3206     
  Branches     3340     2725     -615     
==========================================
- Hits        10818     7882    -2936     
+ Misses        608      406     -202     
+ Partials      270      202      -68

Flag	Coverage Δ
unittest	`92.83% <100.00%> (+0.34%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
packages/type-utils/src/isTypeReadonly.ts	`85.14% <100.00%> (+0.45%)`	⬆️
packages/type-utils/src/propertyTypes.ts	`90.90% <100.00%> (+3.40%)`	⬆️
...ges/eslint-plugin/src/rules/no-misused-promises.ts	`98.63% <0.00%> (-1.37%)`	⬇️
...ckages/eslint-plugin/src/util/getESLintCoreRule.ts	`75.00% <0.00%> (ø)`
...plugin/src/rules/explicit-module-boundary-types.ts	`91.04% <0.00%> (ø)`
...ackages/scope-manager/src/variable/VariableBase.ts
packages/typescript-estree/src/version-check.ts
packages/scope-manager/src/lib/esnext.intl.ts
...kages/scope-manager/src/referencer/ClassVisitor.ts
packages/scope-manager/src/scope/FunctionScope.ts
... and 190 more

bradzacher

I might be missing something - but I'm not sure how this changes the logic.
Before it checked for __<string>, now it checks for __@<string> or __#<string>. Aren't the new checks covered by the old check?

The code is handling parameter properties but what about some complicated combos of public/private fields and members?

The code should just iterate over the properties of the object. If they can be seen as readonly, then it's readonly - else it's mutable. It depends entirely on what the TS APIs reports when the type is interrogated.

I'd assume that the TS APIs would not report the private/protected members unless they're accessible at that location.

bradzacher · 2021-12-27T05:51:42Z

+    // PrivateIdentifier is handled without throwing error.
+    {
+      code: `
+        class Foo {
+          readonly #privateField = 'foo';
+          #privateMember() {}
+        }
+        function foo(arg: Foo) {}
+      `,
+      options: [
+        {
+          treatMethodsAsReadonly: true,
+        },
+      ],
+    },


to double check - this case is ignored when #privateField is not readonly as well?
Could you add a test to verify this please? EG this should not error:

{ code: ` class Foo { #privateField = 'foo'; #privateMember() {} } function foo(arg: Foo) {} `, options: [ { treatMethodsAsReadonly: false, }, ], },

For the quick answer, it is not. It fails on test. But not throwing an exception.

{ code: ` class Foo { #privateField = 'foo'; ---> test fails becuase it is not readonly #privateMember() {}. ---> test fails because "treatMethodsAsReadonly" is set to "false" } function foo(arg: Foo) {} `, options: [ { treatMethodsAsReadonly: false, }, ], },

Additional info is that before the fix, #privateMember throws an exception when treatMethodsAsReadonly: true, but no exception and test fails on treatMethodsAsReadonly: false

After the fix, whether private method with treatMethodsAsReadonly is set true/false or private field with readonly or not. There is no exception thrown.

Here are some results of after the fix

{ code: ` class Foo { #privateField1 = 'foo'; ----> test fails becuase it is not readonly readonly #privateField2 = 'foo'; -----> test passed because it's readonly private privateField3 = 'foo'; ----> test fails becuase it's not readonly private readonly privateField4 = 'foo'; ----> test passed because it's readonly #privateMember1() {} ----> test fails because "treatMethodsAsReadonly" is set false private #privateMember2() {} ----> same as above private privateMember3() {} ----> same as above } function foo(arg: Foo) {} `, options: [ { treatMethodsAsReadonly: false, }, ], },

Interesting.
For private fields - we 100% should be treating them as "public" (and thus subject them to readonly checks), however for #private fields, they should be exempt from readonly checks.

The intent behind the rule is that you should not be able to mutate the type at runtime. You can access private fields via computed member access syntax, but because you cannot access #private fields at all outside the class - they shouldn't be included in the checks.
(playground)

Thanks for the playground. I didn't know about that. I just enjoy all your comments on any issues or prs. I think very constructive(very educational to me) :D

please check this commit

lonyele · 2021-12-27T07:52:08Z

I might be missing something - but I'm not sure how this changes the logic. Before it checked for __<string>, now it checks for __@<string> or __#<string>. Aren't the new checks covered by the old check?

The difference is what to check. Changed code use escapedName instead of name. Technically fix can be simple as this.

from previous code 
if (!escapedName || !name.startsWith('__')) { ... }

to fixed code
if (!escapedName || !escapedName.startsWith('__')) { ... }

I just wrote a little bit more explicitly why .startsWith('__') is needed using isKnowSymbol and isPrivateIdentifierSymbol with comments.

Why escapedName should be used instaed of name? because of this case. #private's escapedName and name are different. reference
case for escapedName: "__@foo@10", name: "__@foo@10"
case for escapedName: "__#1@#foo", name: "#foo"

bradzacher · 2021-12-28T02:37:40Z

sounds good!
just a response for my above query about the tests and we're good to go

lonyele · 2021-12-28T03:02:32Z

Not sure why my reply to your request on additional test is not showing on this main page.
But i actually replied it, please go to the Files Changed section

bradzacher · 2022-01-01T00:09:09Z

wierdly I can't see your reply there either.
Could you please try replying again?
You might need to finish "reviewing" the PR to make the comment public.

lonyele · 2021-12-24T07:59:08Z

 ): ts.Type | undefined {
  // Most names are directly usable in the checker and aren't different from escaped names
-  if (!escapedName || !name.startsWith('__')) {
+  if (!escapedName || !isSymbol(escapedName)) {


@JoshuaKGoldberg I also checked your previous pr and this tsutil's handling. I was wondering handling this if statement with using escapedName is still ok
tsutil handling

lonyele · 2021-12-24T08:02:26Z

+
+// Symbolic names need to be specially handled because TS api is not sufficient for these cases.
+function isSymbol(escapedName: string): boolean {
+  return isKnownSymbol(escapedName) || isPrivateIdentifierSymbol(escapedName);
+}
+
+// case for escapedName: "__@foo@10", name: "__@foo@10"
+function isKnownSymbol(escapedName: string): boolean {
+  return escapedName.startsWith('__@');
+}
+
+// case for escapedName: "__#1@#foo", name: "#foo"
+function isPrivateIdentifierSymbol(escapedName: string): boolean {
+  return escapedName.startsWith('__#');
+}


I took it from typescript codebase

export function getPropertyNameForUniqueESSymbol(symbol: Symbol): __String { return `__@${getSymbolId(symbol)}@${symbol.escapedName}` as __String; } export function getSymbolNameForPrivateIdentifier(containingClassSymbol: Symbol, description: __String): __String { return `__#${getSymbolId(containingClassSymbol)}@${description}` as __String; } export function isKnownSymbol(symbol: Symbol): boolean { return startsWith(symbol.escapedName as string, "__@"); } export function isPrivateIdentifierSymbol(symbol: Symbol): boolean { return startsWith(symbol.escapedName as string, "__#"); }

lonyele · 2021-12-27T07:42:16Z

+    // PrivateIdentifier is handled without throwing error.
+    {
+      code: `
+        class Foo {
+          readonly #privateField = 'foo';
+          #privateMember() {}
+        }
+        function foo(arg: Foo) {}
+      `,
+      options: [
+        {
+          treatMethodsAsReadonly: true,
+        },
+      ],
+    },


For the quick answer, it is not. It fails on test. But not throwing an exception.

{ code: ` class Foo { #privateField = 'foo'; ---> test fails becuase it is not readonly #privateMember() {}. ---> test fails because "treatMethodsAsReadonly" is set to "false" } function foo(arg: Foo) {} `, options: [ { treatMethodsAsReadonly: false, }, ], },

Additional info is that before the fix, #privateMember throws an exception when treatMethodsAsReadonly: true, but no exception and test fails on treatMethodsAsReadonly: false

After the fix, whether private method with treatMethodsAsReadonly is set true/false or private field with readonly or not. There is no exception thrown.

Here are some results of after the fix

{ code: ` class Foo { #privateField1 = 'foo'; ----> test fails becuase it is not readonly readonly #privateField2 = 'foo'; -----> test passed because it's readonly private privateField3 = 'foo'; ----> test fails becuase it's not readonly private readonly privateField4 = 'foo'; ----> test passed because it's readonly #privateMember1() {} ----> test fails because "treatMethodsAsReadonly" is set false private #privateMember2() {} ----> same as above private privateMember3() {} ----> same as above } function foo(arg: Foo) {} `, options: [ { treatMethodsAsReadonly: false, }, ], },

lonyele · 2022-01-01T06:26:06Z

wow... how newbie mistake it is. Actually I didn't even know there is this pending system exists in github. holy cow. though kind of weird that I also checked in incognito mode whether it was actually published... anyway...

JoshuaKGoldberg

Agree with bradzacher here. Thanks for the discussion + PR so far! ❤️

Would like to see some tests of this behavior too please :)

…nyele/typescript-eslint into fix/handle-private-identifier

lonyele · 2022-02-12T00:40:24Z

+    // PrivateIdentifier is handled without throwing error.
+    {
+      code: `
+        class Foo {
+          readonly #privateField = 'foo';
+          #privateMember() {}
+        }
+        function foo(arg: Foo) {}
+      `,
+      options: [
+        {
+          treatMethodsAsReadonly: true,
+        },
+      ],
+    },


Thanks for the playground. I didn't know about that. I just enjoy all your comments on any issues or prs. I think very constructive(very educational to me) :D

please check this commit

lonyele · 2022-02-12T00:44:36Z

+      const name = ts.getNameOfDeclaration(property.valueDeclaration);
+      const isPrivateIdentifier = name && ts.isPrivateIdentifier(name);
+      if (isPrivateIdentifier) {
+        continue;
+      }
+


It seems like there is no handy function for checking private identifier from ts or tsutils. I borrowed from a typescript's src/compiler/checker.ts that suits this situation.

function checkIndexConstraintForProperty(type: Type, prop: Symbol, propNameType: Type, propType: Type) { const declaration = prop.valueDeclaration; const name = getNameOfDeclaration(declaration); if (name && isPrivateIdentifier(name)) { return; } ........ }

lonyele · 2022-02-12T01:17:43Z

@JoshuaKGoldberg Thanks! just to double-check, what does it mean by some tests of this behavior? add more tests at prefer-readonly-parameter-types.test.ts or somewhere else?(codecov?)

JoshuaKGoldberg · 2022-02-12T17:46:47Z

some tests of this behavior

What I mean is, Codecov is pointing out that there is at least one line in isTypeReadonly.ts that isn't covered by unit tests. So there should be some unit test(s) somewhere that run that line of code to make sure it works & stays working.

I think there are a couple of options you could go with:

isTypeReadonly.test.ts directly tests the isTypeReadonly function, so I'd probably lean towards getting one in there
- Prior example:
  
  typescript-eslint/packages/type-utils/tests/isTypeReadonly.test.ts
  
  Line 63 in 5046882
  
  ['type Test = { readonly bar: string; };'],
  
  is an example of a unit test in isTypeReadonly.test.ts
Additionally, since the linked issue [prefer-readonly-parameter-types] crashing with error Non-null Assertion Failed: Expected to find a property "#promise" for the type. #4068 shows a crash in the prefer-readonly-parameter-types rule with [prefer-readonly-parameter-types] crashing with error Non-null Assertion Failed: Expected to find a property "#promise" for the type. #4068 (comment), we should probably have that added as a test in prefer-readonly-parameter-types.test.ts

Does that help @lonyele?

lonyele · 2022-02-13T06:56:37Z

@JoshuaKGoldberg Absolutely! thanks for the clear guide!

JoshuaKGoldberg

Source code looks good to me in general (alas, TypeScript API woes...) but missing a crashing test case from the issue.

JoshuaKGoldberg

Super, thanks for all your work on this @lonyele! 🙌

Since @bradzacher also reviewed I'll wait a bit to give them a chance to re-review too.

bradzacher · 2022-03-01T04:08:45Z

LGTM! Thanks!

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.13.0` -> `5.14.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/5.13.0/5.14.0) | | [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.13.0` -> `5.14.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/5.13.0/5.14.0) | --- ### Release Notes <details> <summary>typescript-eslint/typescript-eslint (@typescript-eslint/eslint-plugin)</summary> ### [`v5.14.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#5140-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5130v5140-2022-03-07) [Compare Source](typescript-eslint/typescript-eslint@v5.13.0...v5.14.0) ##### Bug Fixes - **eslint-plugin:** \[naming-convention] cover case that requires quotes ([#4582](typescript-eslint/typescript-eslint#4582)) ([3ea0947](typescript-eslint/typescript-eslint@3ea0947)) - **eslint-plugin:** \[no-misused-promises] factor thenable returning function overload signatures ([#4620](typescript-eslint/typescript-eslint#4620)) ([56a09e9](typescript-eslint/typescript-eslint@56a09e9)) - **eslint-plugin:** \[prefer-readonly-parameter-types] handle class sharp private field and member without throwing error ([#4343](typescript-eslint/typescript-eslint#4343)) ([a65713a](typescript-eslint/typescript-eslint@a65713a)) - **eslint-plugin:** \[return-await] correct autofixer in binary expression ([#4401](typescript-eslint/typescript-eslint#4401)) ([5fa2fad](typescript-eslint/typescript-eslint@5fa2fad)) ##### Features - **eslint-plugin:** \[no-misused-promises] add granular options within `checksVoidReturns` ([#4623](typescript-eslint/typescript-eslint#4623)) ([1085177](typescript-eslint/typescript-eslint@1085177)) </details> <details> <summary>typescript-eslint/typescript-eslint (@typescript-eslint/parser)</summary> ### [`v5.14.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#5140-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5130v5140-2022-03-07) [Compare Source](typescript-eslint/typescript-eslint@v5.13.0...v5.14.0) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) </details> --- ### Configuration 📅 **Schedule**: At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1201 Reviewed-by: 6543 <6543@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

bradzacher added the bug Something isn't working label Dec 27, 2021

bradzacher reviewed Dec 27, 2021

View reviewed changes

bradzacher added the awaiting response Issues waiting for a reply from the OP or another party label Dec 27, 2021

bradzacher added awaiting response Issues waiting for a reply from the OP or another party and removed awaiting response Issues waiting for a reply from the OP or another party labels Dec 28, 2021

lonyele commented Jan 1, 2022

View reviewed changes

fix: handle class sharp private field and member

43e34e4

lonyele force-pushed the fix/handle-private-identifier branch from b63de76 to 43e34e4 Compare January 1, 2022 07:35

bradzacher added awaiting response Issues waiting for a reply from the OP or another party and removed awaiting response Issues waiting for a reply from the OP or another party labels Jan 11, 2022

JoshuaKGoldberg requested changes Jan 23, 2022

View reviewed changes

fix: handle class sharp private field and member

cf9199f

lonyele force-pushed the fix/handle-private-identifier branch from 43e34e4 to cf9199f Compare February 11, 2022 20:50

lonyele added 2 commits February 12, 2022 05:52

Merge branch 'fix/handle-private-identifier' of https://github.com/lo…

915f1b3

…nyele/typescript-eslint into fix/handle-private-identifier

feat: exempt private identifier from the rule

9a8d4f9

lonyele commented Feb 12, 2022

View reviewed changes

chore: shorten unnecessary logic

2754294

bradzacher removed the awaiting response Issues waiting for a reply from the OP or another party label Feb 12, 2022

JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Feb 12, 2022

lonyele added 2 commits February 13, 2022 15:43

test: add unit test

4ba1c7c

test: cover case that crashed before

33e4789

bradzacher removed the awaiting response Issues waiting for a reply from the OP or another party label Feb 14, 2022

JoshuaKGoldberg reviewed Feb 23, 2022

View reviewed changes

Comment thread packages/type-utils/tests/isTypeReadonly.test.ts Outdated

JoshuaKGoldberg requested changes Feb 23, 2022

View reviewed changes

Comment thread packages/type-utils/src/propertyTypes.ts

JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Feb 23, 2022

test: add readonly test case

0dc841a

JoshuaKGoldberg reviewed Feb 28, 2022

View reviewed changes

Comment thread packages/type-utils/src/propertyTypes.ts

Update packages/type-utils/src/propertyTypes.ts

3d8acb4

JoshuaKGoldberg approved these changes Feb 28, 2022

View reviewed changes

bradzacher removed the awaiting response Issues waiting for a reply from the OP or another party label Mar 1, 2022

bradzacher merged commit a65713a into typescript-eslint:main Mar 1, 2022

lonyele deleted the fix/handle-private-identifier branch March 1, 2022 04:45

github-actions Bot locked as resolved and limited conversation to collaborators May 25, 2022

Uh oh!

Conversation

lonyele commented Dec 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

Overview

Uh oh!

nx-cloud Bot commented Dec 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

☁️ Nx Cloud Report

Uh oh!

typescript-eslint Bot commented Dec 24, 2021

Uh oh!

netlify Bot commented Dec 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Dec 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bradzacher left a comment

Choose a reason for hiding this comment

Uh oh!

bradzacher Dec 27, 2021

Choose a reason for hiding this comment

Uh oh!

lonyele Dec 27, 2021

Choose a reason for hiding this comment

Uh oh!

bradzacher Jan 17, 2022

Choose a reason for hiding this comment

Uh oh!

lonyele Feb 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lonyele commented Dec 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bradzacher commented Dec 28, 2021

Uh oh!

lonyele commented Dec 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bradzacher commented Jan 1, 2022

Uh oh!

lonyele Dec 24, 2021

Choose a reason for hiding this comment

Uh oh!

lonyele Dec 24, 2021

Choose a reason for hiding this comment

Uh oh!

lonyele Dec 27, 2021

Choose a reason for hiding this comment

Uh oh!

lonyele commented Jan 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoshuaKGoldberg left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lonyele Feb 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lonyele Feb 12, 2022

Choose a reason for hiding this comment

Uh oh!

lonyele commented Feb 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JoshuaKGoldberg commented Feb 12, 2022

Uh oh!

lonyele commented Feb 13, 2022

Uh oh!

Uh oh!

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

Uh oh!

lonyele commented Dec 24, 2021 •

edited

Loading

nx-cloud Bot commented Dec 24, 2021 •

edited

Loading

netlify Bot commented Dec 24, 2021 •

edited

Loading

codecov Bot commented Dec 24, 2021 •

edited

Loading

lonyele Feb 12, 2022 •

edited

Loading

lonyele commented Dec 27, 2021 •

edited

Loading

lonyele commented Dec 28, 2021 •

edited

Loading

lonyele commented Jan 1, 2022 •

edited

Loading

JoshuaKGoldberg left a comment •

edited

Loading

lonyele Feb 12, 2022 •

edited

Loading

lonyele commented Feb 12, 2022 •

edited

Loading

JoshuaKGoldberg left a comment •

edited

Loading