feat(eslint-plugin): add fixer for use-pipe-transform-interface #260

rafaelss95 · 2020-12-22T06:04:53Z

No description provided.

rafaelss95 · 2020-12-22T06:23:52Z

packages/eslint-plugin/src/rules/use-pipe-transform-interface.ts

+  if (!importDeclaration?.specifiers.length) {
+    return fixer.insertTextAfterRange(
+      [0, 0],
+      `import { ${PIPE_TRANSFORM} } from '${ANGULAR_CORE_MODULE_PATH}';\n`,
+    );
+  }


This case should not happen as in a pipe file you may already have the import from @angular/core, however I prefer to let it here to avoid something strange.

JamesHenry

Nice work!

JamesHenry

Nice work! This changes the output of the integration tests, so the snapshots will need to be updated.

You can just run the integration tests with -u set for jest and then review the changes before committing

rafaelss95 · 2020-12-22T16:56:01Z

Hello @JamesHenry, sorry to ask here, but do you still have access to the email address james@nrwl.io? If not, could you give me your new email address? I'm trying to contact you there, thanks.

JamesHenry · 2020-12-22T18:14:16Z

@rafaelss95 No problem, please could you use james@henry.sc instead?

rafaelss95 · 2020-12-22T19:21:16Z

You can just run the integration tests with -u set for jest and then review the changes before committing

I ran npm run integration-tests in the root folder and I got the following log error:

0 verbose cli [
0 verbose cli   '/usr/local/Cellar/node/15.3.0/bin/node',
0 verbose cli   '/usr/local/bin/npm',
0 verbose cli   'run',
0 verbose cli   'integration-tests'
0 verbose cli ]
1 info using npm@7.0.14
2 info using node@v15.3.0
3 timing config:load:defaults Completed in 1ms
4 timing config:load:file:/usr/local/lib/node_modules/npm/npmrc Completed in 1ms
5 timing config:load:builtin Completed in 1ms
6 timing config:load:cli Completed in 2ms
7 timing config:load:env Completed in 0ms
8 timing config:load:file:/Users/PATH/angular-eslint/.npmrc Completed in 1ms
9 timing config:load:project Completed in 2ms
10 timing config:load:file:/Users/*/.npmrc Completed in 1ms
11 timing config:load:user Completed in 1ms
12 timing config:load:file:/usr/local/etc/npmrc Completed in 0ms
13 timing config:load:global Completed in 0ms
14 timing config:load:cafile Completed in 0ms
15 timing config:load:validate Completed in 1ms
16 timing config:load:setUserAgent Completed in 0ms
17 timing config:load:setEnvs Completed in 1ms
18 timing config:load Completed in 10ms
19 verbose npm-session 426629919b90fffd
20 timing npm:load Completed in 21ms
21 timing command:run-script Completed in 84574ms
22 verbose stack Error: command failed
22 verbose stack     at ChildProcess.<anonymous> (/usr/local/lib/node_modules/npm/node_modules/@npmcli/promise-spawn/index.js:64:27)
22 verbose stack     at ChildProcess.emit (node:events:376:20)
22 verbose stack     at maybeClose (node:internal/child_process:1055:16)
22 verbose stack     at Process.ChildProcess._handle.onexit (node:internal/child_process:288:5)
23 verbose cwd /Users/PATH/angular-eslint
24 verbose Darwin 20.2.0
25 verbose argv "/usr/local/Cellar/node/15.3.0/bin/node" "/usr/local/bin/npm" "run" "integration-tests"
26 verbose node v15.3.0
27 verbose npm  v7.0.14
28 error code 1
29 error path /Users/PATH/angular-eslint
30 error command failed
31 error command sh -c lerna run integration-tests
32 verbose exit 1

Could you point out to me what I'm missing?

rafaelss95 · 2020-12-22T19:34:49Z

Sorry for the amended commits, I messed up with the rebase.

JamesHenry · 2020-12-22T19:34:57Z

Hmm I have never used npm 7, possibly that?

Can you try using yarn as that is what the workspace uses in CI

You did a full install and build of the monorepo before running right?

rafaelss95 · 2020-12-22T19:37:31Z

packages/eslint-plugin/src/rules/use-pipe-transform-interface.ts

@@ -1,45 +1,106 @@
 import { TSESTree } from '@typescript-eslint/experimental-utils';
+import { RuleFixer } from '@typescript-eslint/experimental-utils/dist/ts-eslint';


@JamesHenry Could we have RuleFixer exported directly from the main entry point? This deep import just doesn't feels good for me.

Yeah, very true, please could you open an issue on typescript-eslint, and add a TODO comment referencing it so we don't forget in future?

I created the issue typescript-eslint/typescript-eslint#2931 and put a comment here, take a look please.

I've seen Brad's response, shows how rusty I am :) Please can we update this accordingly

rafaelss95 · 2020-12-22T19:52:05Z

You did a full install and build of the monorepo before running right?

That was it 💯! After ran this, I got 36 files changes. Should I commit it all here?

JamesHenry · 2020-12-22T19:58:06Z

@rafaelss95 Ah no, just the snapshot updates - the other parts of the diff are just the results of the integration tests doing their thing and should not be committed

JamesHenry · 2020-12-23T13:29:11Z

packages/integration-tests/tests/__snapshots__/v1014-multi-project-manual-config.test.ts.snap

-Linting \\"robot-websites\\"...
-All files pass linting.
-"
+Linting \\"v1014-multi-project-manual-config\\"..."


This snapshot is clearly broken, you can debug by cd'ing into the relevant workspace in the integration-tests package fixtures and running normal ng lint commands

I did and I discovered that the problem is that the ng lint is not running for this folder (v1014-multi-project-manual-config). When I try to run it, I get the following error:

[error] Error: Failed to load plugin '@angular-eslint' declared in '.eslintrc.json#overrides[0]': Cannot find module '/PATH/angular-eslint/packages/integration-tests/fixtures/v1014-multi-project-manual-config/node_modules/@angular-eslint/eslint-plugin/dist/index.js'. Please verify that the package.json has a valid "main" entry Require stack: - /PATH/angular-eslint/packages/integration-tests/fixtures/v1014-multi-project-manual-config/__placeholder__.js Referenced from: /PATH/angular-eslint/packages/integration-tests/fixtures/v1014-multi-project-manual-config/.eslintrc.json at tryPackage (node:internal/modules/cjs/loader:338:19) at Function.Module._findPath (node:internal/modules/cjs/loader:551:18) at Function.Module._resolveFilename (node:internal/modules/cjs/loader:905:27) at Function.resolve (node:internal/modules/cjs/helpers:98:19) at Object.resolve (/PATH/angular-eslint/packages/integration-tests/fixtures/v1014-multi-project-manual-config/node_modules/@eslint/eslintrc/lib/shared/relative-module-resolver.js:28:50) at ConfigArrayFactory._loadPlugin (/PATH/angular-eslint/packages/integration-tests/fixtures/v1014-multi-project-manual-config/node_modules/@eslint/eslintrc/lib/config-array-factory.js:1011:39) at ConfigArrayFactory._loadExtendedPluginConfig (/PATH/angular-eslint/packages/integration-tests/fixtures/v1014-multi-project-manual-config/node_modules/@eslint/eslintrc/lib/config-array-factory.js:831:29) at ConfigArrayFactory._loadExtends (/PATH/angular-eslint/packages/integration-tests/fixtures/v1014-multi-project-manual-config/node_modules/@eslint/eslintrc/lib/config-array-factory.js:778:29) at ConfigArrayFactory._normalizeObjectConfigDataBody (/PATH/angular-eslint/packages/integration-tests/fixtures/v1014-multi-project-manual-config/node_modules/@eslint/eslintrc/lib/config-array-factory.js:719:25) at _normalizeObjectConfigDataBody.next (<anonymous>)

I also noticed that this happens only for this fixture, the others ran normally.

@JamesHenry I would appreciate your help on this to move forward with this PR, thanks.

JamesHenry · 2020-12-23T13:31:25Z

...ges/integration-tests/tests/__snapshots__/v1020-multi-project-yarn-auto-convert.test.ts.snap

@@ -90,15 +90,15 @@ Object {
        "@angular-eslint/component-selector": Array [
          "error",
          Object {
-            "prefix": "customprefix",
+            "prefix": "app",


These prefixes should not have been changed

Idk why it has changed, but after revert everything and do a fresh build/run, it didn't change anymore.

JamesHenry · 2020-12-23T13:32:04Z

...s/integration-tests/tests/__snapshots__/v1101-strict-multi-project-auto-convert.test.ts.snap

@@ -149,15 +149,15 @@ Object {
        "@angular-eslint/component-selector": Array [
          "error",
          Object {
-            "prefix": "customprefix",
+            "prefix": "app",


These prefixes should not have been changed

JamesHenry · 2021-01-11T10:26:48Z

packages/integration-tests/tests/__snapshots__/v1014-multi-project-manual-config.test.ts.snap

-   6:35  error  Invalid binding syntax. Use [(expr)] instead  @angular-eslint/template/banana-in-box
-  14:48  error  Invalid binding syntax. Use [(expr)] instead  @angular-eslint/template/banana-in-box
+   4:13  error  Strings must use singlequote                    @typescript-eslint/quotes
+   9:14  error  Pipes should implement PipeTransform interface  @angular-eslint/use-pipe-transform-interface


Hmm, please make sure you actually verify all these changes, it is a red flag that adding a fixer would cause so many changes to the integration-tests and I think most of them are regressions.

The source of this file I have commented on (multiple-inline-templates.page.ts) is:

import { Component } from '@angular/core'; @Component({ selector: "app-one-inline-template", template: ` <input type="text" name="foo" ([ngModel])="foo"> `, }) export class OneInlineTemplateComponent {} @Component({ selector: "app-two-inline-template", template: ` <div [oneWay]="oneWay" (emitter)="emitter" ([twoWay])="twoWay"></div> `, }) export class TwoInlineTemplateComponent {}

The use-pipe-transform-interface rule should not be erroring for that source, right?

I believe this is the case for a number of these changes

Ups, it should be fine now.

JamesHenry

Thanks a lot!

rafaelss95 requested a review from JamesHenry December 22, 2020 06:04

rafaelss95 force-pushed the feat/use-pipe-transform-interface branch from bfb2ec7 to c5e5ffd Compare December 22, 2020 06:21

rafaelss95 commented Dec 22, 2020

View reviewed changes

JamesHenry approved these changes Dec 22, 2020

View reviewed changes

JamesHenry requested changes Dec 22, 2020

View reviewed changes

rafaelss95 force-pushed the feat/use-pipe-transform-interface branch 2 times, most recently from 2ba8b6f to 261fcd9 Compare December 22, 2020 19:24

feat(eslint-plugin): add fixer for use-pipe-transform-interface

519e1ef

rafaelss95 force-pushed the feat/use-pipe-transform-interface branch from 261fcd9 to 519e1ef Compare December 22, 2020 19:30

refactor: export functions from utils

01d62e0

rafaelss95 commented Dec 22, 2020

View reviewed changes

JamesHenry requested changes Dec 23, 2020

View reviewed changes

JamesHenry reviewed Dec 23, 2020

View reviewed changes

build: update snapshot files

ef60817

rafaelss95 force-pushed the feat/use-pipe-transform-interface branch from 570c8a5 to ef60817 Compare December 23, 2020 17:04

rafaelss95 mentioned this pull request Jan 4, 2021

fix(eslint-plugin): handle DoBootstrap correctly in lifecycle rules #243

Merged

rafaelss95 added 4 commits January 10, 2021 16:22

test: trying to update snapshot files manually

f45ab7d

test: trying to update snapshot files manually

05a9c51

Merge branch 'master' into feat/use-pipe-transform-interface

c91b3c2

test: trying to update snapshot files manually

15fe940

rafaelss95 requested a review from JamesHenry January 10, 2021 22:06

JamesHenry reviewed Jan 11, 2021

View reviewed changes

rafaelss95 added 2 commits January 11, 2021 08:55

fixup! feat(eslint-plugin): add fixer for use-pipe-transform-interface

a45614a

test: update snapshot files

767df81

rafaelss95 mentioned this pull request Jan 11, 2021

Export RuleFixer from the main entry point typescript-eslint/typescript-eslint#2931

Closed

3 tasks

rafaelss95 added 6 commits January 11, 2021 09:42

docs: add comment about interfaces from experimental-utils

295597f

build: fix conflicts

f0d0581

fixup! feat(eslint-plugin): add fixer for use-pipe-transform-interface

18316c3

fixup! feat(eslint-plugin): add fixer for use-pipe-transform-interface

2153ba1

fixup! feat(eslint-plugin): add fixer for use-pipe-transform-interface

133d040

fixup! feat(eslint-plugin): add fixer for use-pipe-transform-interface

9cfa114

rafaelss95 requested a review from JamesHenry January 11, 2021 14:53

refactor: simplify imports and usage

138c955

JamesHenry approved these changes Jan 16, 2021

View reviewed changes

JamesHenry merged commit e3f4db6 into angular-eslint:master Jan 16, 2021

rafaelss95 deleted the feat/use-pipe-transform-interface branch January 16, 2021 12:05

This was referenced Mar 15, 2021

THRET-D(deps-dev): Bump @angular-eslint/eslint-plugin from 0.8.0-beta.2 to 2.0.1 thretclothing/home#114

Closed

THRET-D(deps-dev): Bump @angular-eslint/eslint-plugin from 0.8.0-beta.2 to 2.0.2 thretclothing/home#118

Closed

rafaelss95 restored the feat/use-pipe-transform-interface branch April 1, 2021 00:19

rafaelss95 deleted the feat/use-pipe-transform-interface branch April 1, 2021 00:19

rafaelss95 mentioned this pull request Oct 12, 2021

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(eslint-plugin): add fixer for use-pipe-transform-interface #260

feat(eslint-plugin): add fixer for use-pipe-transform-interface #260

rafaelss95 commented Dec 22, 2020

rafaelss95 Dec 22, 2020

JamesHenry left a comment

JamesHenry left a comment

rafaelss95 commented Dec 22, 2020 •

edited

Loading

JamesHenry commented Dec 22, 2020

rafaelss95 commented Dec 22, 2020

rafaelss95 commented Dec 22, 2020

JamesHenry commented Dec 22, 2020

rafaelss95 Dec 22, 2020

JamesHenry Jan 11, 2021

rafaelss95 Jan 11, 2021

JamesHenry Jan 11, 2021

rafaelss95 commented Dec 22, 2020

JamesHenry commented Dec 22, 2020

JamesHenry Dec 23, 2020

rafaelss95 Dec 23, 2020 •

edited

Loading

rafaelss95 Jan 3, 2021

JamesHenry Dec 23, 2020

rafaelss95 Dec 23, 2020

JamesHenry Dec 23, 2020

JamesHenry Jan 11, 2021 •

edited

Loading

rafaelss95 Jan 11, 2021

JamesHenry left a comment

		@@ -1,45 +1,106 @@
		import { TSESTree } from '@typescript-eslint/experimental-utils';
		import { RuleFixer } from '@typescript-eslint/experimental-utils/dist/ts-eslint';

feat(eslint-plugin): add fixer for use-pipe-transform-interface #260

feat(eslint-plugin): add fixer for use-pipe-transform-interface #260

Conversation

rafaelss95 commented Dec 22, 2020

Choose a reason for hiding this comment

JamesHenry left a comment

Choose a reason for hiding this comment

JamesHenry left a comment

Choose a reason for hiding this comment

rafaelss95 commented Dec 22, 2020 • edited Loading

JamesHenry commented Dec 22, 2020

rafaelss95 commented Dec 22, 2020

rafaelss95 commented Dec 22, 2020

JamesHenry commented Dec 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rafaelss95 commented Dec 22, 2020

JamesHenry commented Dec 22, 2020

Choose a reason for hiding this comment

rafaelss95 Dec 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JamesHenry Jan 11, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JamesHenry left a comment

Choose a reason for hiding this comment

rafaelss95 commented Dec 22, 2020 •

edited

Loading

rafaelss95 Dec 23, 2020 •

edited

Loading

JamesHenry Jan 11, 2021 •

edited

Loading