fix(experimental-utils): support immutable members #3844

rafaelss95 · 2021-09-05T22:25:13Z

This PR is basically to provide support for immutable in some other members/functions besides of #3830.

typescript-eslint · 2021-09-05T22:25:18Z

Thanks for the PR, @rafaelss95!

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.

nx-cloud · 2021-09-05T22:36:06Z

Nx Cloud Report

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

📂 See all runs for this branch

Status	Command
	`Node 16.x - nx test @typescript-eslint/eslint-plugin-tslint`
	`Node 16.x - nx test @typescript-eslint/eslint-plugin`
	`Node 14.x - nx test @typescript-eslint/eslint-plugin-tslint`
	`Node 12.x - nx test @typescript-eslint/eslint-plugin-internal`
	`Node 14.x - nx test @typescript-eslint/eslint-plugin`
	`Node 12.x - nx test @typescript-eslint/eslint-plugin-tslint`
	`Node 12.x - nx test @typescript-eslint/eslint-plugin`
	`Node 12.x - nx test @typescript-eslint/eslint-plugin-tslint`
	`Node 12.x - nx test @typescript-eslint/eslint-plugin`
	`Node 12.x - nx run-many --target=typecheck --all --parallel`
	`Node 14.x - nx test @typescript-eslint/parser`
	`Node 16.x - nx test @typescript-eslint/parser`
	`Node 14.x - nx test @typescript-eslint/experimental-utils`
	`Node 12.x - nx run-many --target=build --all --parallel`
	`Node 14.x - nx test @typescript-eslint/scope-manager`
	`Node 12.x - nx test @typescript-eslint/parser`
	`Node 16.x - nx test @typescript-eslint/experimental-utils`
	`Node 12.x - nx run-many --target=build --all --parallel`
	`Node 12.x - nx test @typescript-eslint/experimental-utils`
	`Node 14.x - nx test @typescript-eslint/visitor-keys`

Sent with 💌 from NxCloud.

rafaelss95 · 2021-09-05T22:52:25Z

PS 1: I tried to change Range from:

typescript-eslint/packages/ast-spec/src/base/Range.ts

Line 6 in 8218055

export type Range = [number, number];

to:

export type Range = readonly [number, number];

but it didn't work as it actually needs to be mutated in places like:

typescript-eslint/packages/typescript-estree/src/convert.ts

Line 168 in 9524424

result.range[0] = varToken!.getStart(this.ast);

So in this case I opted out for just typing the necessary functions as Readonly<AST.Range>.

~~PS 2: The lint failure seems to be unrelated to this change.~~
Since Array.isArray doesn't work properly with readonly T[] as reported in microsoft/TypeScript#17002, I used the type annotation in no-magic-numbers to avoid the no-unsafe-assignment report.

codecov · 2021-09-07T18:09:44Z

Codecov Report

Merging #3844 (0606673) into main (475caa4) will increase coverage by 0.15%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #3844      +/-   ##
==========================================
+ Coverage   93.33%   93.49%   +0.15%     
==========================================
  Files         152      192      +40     
  Lines        8014     8672     +658     
  Branches     2568     2683     +115     
==========================================
+ Hits         7480     8108     +628     
- Misses        180      351     +171     
+ Partials      354      213     -141

Flag	Coverage Δ
unittest	`93.49% <100.00%> (+0.15%)`	⬆️

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

Impacted Files	Coverage Δ
...kages/eslint-plugin/src/rules/no-empty-function.ts	`80.00% <ø> (ø)`
...ges/experimental-utils/src/ts-eslint/RuleTester.ts	`100.00% <ø> (ø)`
...ckages/eslint-plugin/src/rules/no-magic-numbers.ts	`84.31% <100.00%> (+1.96%)`	⬆️
packages/eslint-plugin/src/rules/array-type.ts	`97.18% <0.00%> (-1.33%)`	⬇️
packages/eslint-plugin/src/rules/brace-style.ts	`95.34% <0.00%> (ø)`
packages/eslint-plugin/src/rules/no-redeclare.ts	`91.89% <0.00%> (ø)`
...ackages/eslint-plugin/src/rules/no-extra-parens.ts	`90.41% <0.00%> (ø)`
...ackages/eslint-plugin/src/util/getWrappingFixer.ts	`100.00% <0.00%> (ø)`
...kages/eslint-plugin/src/rules/naming-convention.ts	`81.09% <0.00%> (ø)`
... and 101 more

packages/eslint-plugin/src/rules/no-magic-numbers.ts

rafaelss95 · 2021-09-21T16:12:07Z

This PR was accidentally closed earlier and I had forgotten to reopen it.

bradzacher

One change - otherwise LGTM
Thanks for your work!

packages/eslint-plugin/src/rules/no-magic-numbers.ts

bradzacher · 2021-10-03T21:44:15Z

packages/experimental-utils/src/ts-eslint/Rule.ts

@@ -209,13 +209,13 @@ interface RuleContext<
   * the root of the AST and continuing through the direct parent of the current node.
   * This array does not include the currently-traversed node itself.
   */
-  getAncestors(): TSESTree.Node[];
+  getAncestors(): readonly TSESTree.Node[];


This would be a breaking change because it's not uncommon for people to do getAncestors().reverse()

Also - it isn't necessary because getAncestors returns a new array each invocation:
https://github.com/eslint/eslint/blob/d867f8100737bb82742debee2b5dc853c5f07c91/lib/linter/linter.js#L789-L797

packages/experimental-utils/src/ts-eslint/Rule.ts

bradzacher

LGTM - thanks for this!

bradzacher

the lint error is actually legit.
repro

Found this TS issue: microsoft/TypeScript#17002
It's a TS bug :(

Which I see you already found and mentioned above!!

netlify · 2021-12-15T07:19:25Z

❌ Deploy Preview for typescript-eslint failed.

🔨 Explore the source changes: 0606673

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

rafaelss95 force-pushed the master branch from 1c867cd to a4a5084 Compare September 7, 2021 18:09

bradzacher added the bug Something isn't working label Sep 7, 2021

rafaelss95 closed this Sep 17, 2021

rafaelss95 force-pushed the master branch from a4a5084 to b1df817 Compare September 17, 2021 16:02

rafaelss95 reopened this Sep 21, 2021

rafaelss95 force-pushed the master branch 4 times, most recently from 7625ee6 to 5b7c952 Compare September 21, 2021 16:01

rafaelss95 commented Sep 21, 2021

View reviewed changes

packages/eslint-plugin/src/rules/no-magic-numbers.ts Show resolved Hide resolved

bradzacher requested changes Oct 3, 2021

View reviewed changes

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

rafaelss95 force-pushed the master branch from 5b7c952 to 725d507 Compare October 4, 2021 03:21

rafaelss95 added 3 commits October 12, 2021 18:31

fix(experimental-utils): support immutable members

c182269

refactor: drop modifier from getAncestors

ec18ced

build: merge with v5

ee29109

rafaelss95 force-pushed the master branch from 725d507 to 4507fa5 Compare October 12, 2021 21:41

rafaelss95 mentioned this pull request Oct 12, 2021

test: use uvu to run rule tests for better reports timdeschryver/eslint-plugin-ngrx#257

Merged

refactor: accept readonly on suggestions

02b1fbd

rafaelss95 force-pushed the master branch from 4507fa5 to 02b1fbd Compare October 12, 2021 23:08

rafaelss95 mentioned this pull request Oct 13, 2021

fix: accept readonly arrays in fromFixture's parameters cartant/eslint-etc#6

Merged

bradzacher removed the awaiting response Issues waiting for a reply from the OP or another party label Oct 18, 2021

bradzacher previously approved these changes Oct 29, 2021

View reviewed changes

bradzacher added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Oct 29, 2021

Merge branch 'master' into master

f4526a7

armano2 previously approved these changes Dec 15, 2021

View reviewed changes

bradzacher reviewed Dec 15, 2021

View reviewed changes

suppress lint

0606673

bradzacher dismissed stale reviews from armano2 and themself via 0606673 December 15, 2021 07:19

bradzacher approved these changes Dec 15, 2021

View reviewed changes

bradzacher merged commit 3d33a77 into typescript-eslint:main Dec 15, 2021

bradzacher mentioned this pull request Dec 20, 2021

feat(eslint-plugin): add extension rule space-before-blocks (#1606) #4184

Merged

3 tasks

github-actions bot locked as resolved and limited conversation to collaborators Jan 15, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(experimental-utils): support immutable members #3844

fix(experimental-utils): support immutable members #3844

Uh oh!

rafaelss95 commented Sep 5, 2021

Uh oh!

typescript-eslint bot commented Sep 5, 2021

Uh oh!

nx-cloud bot commented Sep 5, 2021 •

edited

Loading

Uh oh!

rafaelss95 commented Sep 5, 2021 •

edited

Loading

Uh oh!

codecov bot commented Sep 7, 2021 •

edited

Loading

Uh oh!

Uh oh!

rafaelss95 commented Sep 21, 2021

Uh oh!

bradzacher left a comment

Uh oh!

Uh oh!

bradzacher Oct 3, 2021

Uh oh!

Uh oh!

bradzacher left a comment

Uh oh!

bradzacher left a comment •

edited

Loading

Uh oh!

netlify bot commented Dec 15, 2021 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

fix(experimental-utils): support immutable members #3844

fix(experimental-utils): support immutable members #3844

Uh oh!

Conversation

rafaelss95 commented Sep 5, 2021

Uh oh!

typescript-eslint bot commented Sep 5, 2021

Uh oh!

nx-cloud bot commented Sep 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Nx Cloud Report

Uh oh!

rafaelss95 commented Sep 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Sep 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

rafaelss95 commented Sep 21, 2021

Uh oh!

bradzacher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bradzacher Oct 3, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bradzacher left a comment

Choose a reason for hiding this comment

Uh oh!

bradzacher 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!

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

Uh oh!

Uh oh!

Uh oh!

nx-cloud bot commented Sep 5, 2021 •

edited

Loading

rafaelss95 commented Sep 5, 2021 •

edited

Loading

codecov bot commented Sep 7, 2021 •

edited

Loading

bradzacher left a comment •

edited

Loading

netlify bot commented Dec 15, 2021 •

edited

Loading