fix(type-utils): union types always being marked as readonly #4419

RebeccaStevens · 2022-01-09T08:41:46Z

PR Checklist

Addresses an existing issue: fixes [isTypeReadonly] Unions are always marked as readonly #4418
That issue was marked as accepting prs
Steps in CONTRIBUTING.md were taken

Overview

isTypeReadonly now checks unions properly.

Note: This PR builds off #4416

fix typescript-eslint#4410

nx-cloud · 2022-01-09T08:41:48Z

☁️ Nx Cloud Report

CI ran the following commands for commit 727d999. 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 · 2022-01-09T08:41:51Z

Thanks for the PR, @RebeccaStevens!

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 · 2022-01-09T08:41:57Z

❌ Deploy Preview for typescript-eslint failed.

🔨 Explore the source changes: 727d999

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

codecov · 2022-01-09T08:57:43Z

Codecov Report

Merging #4419 (727d999) into main (ef3147c) will decrease coverage by 1.52%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #4419      +/-   ##
==========================================
- Coverage   94.23%   92.70%   -1.53%     
==========================================
  Files         169      161       -8     
  Lines        9448     8144    -1304     
  Branches     2948     2612     -336     
==========================================
- Hits         8903     7550    -1353     
- Misses        321      395      +74     
+ Partials      224      199      -25

Flag	Coverage Δ
unittest	`92.70% <ø> (-1.53%)`	⬇️

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

Impacted Files	Coverage Δ
packages/type-utils/src/isTypeReadonly.ts	`78.57% <ø> (ø)`
...ript-estree/src/create-program/createSourceFile.ts
...ckages/typescript-estree/src/jsx/xhtml-entities.ts
...lugin-internal/src/rules/plugin-test-formatting.ts
...-internal/src/rules/no-typescript-estree-import.ts
packages/typescript-estree/src/convert.ts
...ges/typescript-estree/src/create-program/shared.ts
packages/typescript-estree/src/node-utils.ts
...estree/src/create-program/createIsolatedProgram.ts
...pt-estree/src/create-program/createWatchProgram.ts
... and 27 more

JoshuaKGoldberg

LGTM! I'm up for merging this in as-is, but feel mildly positive about suggested changes. What about you?

If nobody is opinionated after a few days + the next time I take a look, I can just merge :)

JoshuaKGoldberg · 2022-01-09T23:20:23Z

packages/type-utils/tests/isTypeReadonly.test.ts

+      type: ts.Type;
+      checker: ts.TypeChecker;
+    } {
+      const { ast, services } = parseForESLint(code, {


Hmm, this is essentially the same as the code at the beginning of isUnsafeAssignment.

typescript-eslint/packages/type-utils/tests/isUnsafeAssignment.test.ts

Line 16 in a9eb0b9

const { ast, services } = parseForESLint(code, {

Since we're starting to establish a precedent of unit testing these type utilities directly, I think we'll want to make a standard way to generate the AST & node maps for them. What do you think about making a standard util like this?:

const { body, checker, esTreeNodeToTSNodeMap } = parseForUtilTest(code);

packages/type-utils/tests/isTypeReadonly.test.ts

fix typescript-eslint#4418

bradzacher · 2022-01-17T07:34:40Z

I merged your test PR first - just needs the conflict resolved and we can merge this!

bradzacher

thanks for fixing this!

packages/type-utils/tests/isTypeReadonly.test.ts

…ipt-eslint#4419)

fix(type-utils): make isTypeReadonly's options param optional

974858a

fix typescript-eslint#4410

RebeccaStevens force-pushed the issue-4418 branch 3 times, most recently from e6fa110 to 50f6c13 Compare January 9, 2022 08:55

RebeccaStevens mentioned this pull request Jan 9, 2022

test(type-utils): add basic tests for isTypeReadonly #4416

Merged

3 tasks

JoshuaKGoldberg reviewed Jan 9, 2022

View reviewed changes

RebeccaStevens force-pushed the issue-4418 branch 2 times, most recently from 1e1bfdd to c61f18d Compare January 10, 2022 07:37

RebeccaStevens added 3 commits January 10, 2022 20:39

test(type-utils): add basic tests for isTypeReadonly

bb209f4

test: add union tests for isTypeReadonly

468ac17

fix(type-utils): union types always being marked as readonly

dfd5a24

fix typescript-eslint#4418

RebeccaStevens force-pushed the issue-4418 branch from c61f18d to dfd5a24 Compare January 10, 2022 07:41

bradzacher added the bug Something isn't working label Jan 11, 2022

Merge branch 'main' into issue-4418

f2a8cf3

RebeccaStevens mentioned this pull request Jan 12, 2022

New rule: prefer-readonly-return-types eslint-functional/eslint-plugin-functional#317

Closed

bradzacher added the awaiting response Issues waiting for a reply from the OP or another party label Jan 17, 2022

RebeccaStevens added 2 commits January 17, 2022 20:44

Merge branch 'main' into issue-4418

bce11f3

Merge branch 'main' into issue-4418

7b36d90

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

bradzacher previously approved these changes Jan 17, 2022

View reviewed changes

bradzacher dismissed their stale review via 6eedf23 January 17, 2022 09:00

bradzacher previously approved these changes Jan 17, 2022

View reviewed changes

RebeccaStevens commented Jan 17, 2022

View reviewed changes

packages/type-utils/tests/isTypeReadonly.test.ts Outdated Show resolved Hide resolved

Merge branch 'main' into issue-4418

727d999

RebeccaStevens dismissed bradzacher’s stale review via 727d999 January 17, 2022 09:26

RebeccaStevens force-pushed the issue-4418 branch from 6eedf23 to 727d999 Compare January 17, 2022 09:26

bradzacher merged commit 99ab193 into typescript-eslint:main Jan 17, 2022

RebeccaStevens deleted the issue-4418 branch January 17, 2022 10:39

lonyele pushed a commit to lonyele/typescript-eslint that referenced this pull request Feb 12, 2022

fix(type-utils): union types always being marked as readonly (typescr…

a059e8f

…ipt-eslint#4419)

github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(type-utils): union types always being marked as readonly #4419

fix(type-utils): union types always being marked as readonly #4419

RebeccaStevens commented Jan 9, 2022 •

edited

Loading

nx-cloud bot commented Jan 9, 2022 •

edited

Loading

typescript-eslint bot commented Jan 9, 2022

netlify bot commented Jan 9, 2022 •

edited

Loading

codecov bot commented Jan 9, 2022 •

edited

Loading

JoshuaKGoldberg left a comment •

edited

Loading

JoshuaKGoldberg Jan 9, 2022

bradzacher commented Jan 17, 2022

bradzacher left a comment

fix(type-utils): union types always being marked as readonly #4419

fix(type-utils): union types always being marked as readonly #4419

Conversation

RebeccaStevens commented Jan 9, 2022 • edited Loading

PR Checklist

Overview

nx-cloud bot commented Jan 9, 2022 • edited Loading

☁️ Nx Cloud Report

typescript-eslint bot commented Jan 9, 2022

netlify bot commented Jan 9, 2022 • edited Loading

codecov bot commented Jan 9, 2022 • edited Loading

Codecov Report

JoshuaKGoldberg left a comment • edited Loading

Choose a reason for hiding this comment

JoshuaKGoldberg Jan 9, 2022

Choose a reason for hiding this comment

bradzacher commented Jan 17, 2022

bradzacher left a comment

Choose a reason for hiding this comment

RebeccaStevens commented Jan 9, 2022 •

edited

Loading

nx-cloud bot commented Jan 9, 2022 •

edited

Loading

netlify bot commented Jan 9, 2022 •

edited

Loading

codecov bot commented Jan 9, 2022 •

edited

Loading

JoshuaKGoldberg left a comment •

edited

Loading