Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(eslint-plugin): [use-lifecycle-interface] add fixer for the rule #1691

Merged
merged 4 commits into from May 22, 2024
Merged

feat(eslint-plugin): [use-lifecycle-interface] add fixer for the rule #1691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fcfde57
feat(eslint-plugin): [use-lifecycle-interface] add fixer for the rule
bulldog98 Jan 31, 2024
9ccafd5
Merge branch 'main' into main
JamesHenry May 21, 2024
2b0b762
Merge branch 'main' into main
JamesHenry May 21, 2024
87c13f5
Merge branch 'main' into main
JamesHenry May 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/eslint-plugin/README.md
View file
Open in desktop
Expand Up @@ -71,7 +71,7 @@
| [`use-component-selector`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/use-component-selector.md) | Component selector must be declared | | | |
| [`use-component-view-encapsulation`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/use-component-view-encapsulation.md) | Disallows using `ViewEncapsulation.None` | | | :bulb: |
| [`use-injectable-provided-in`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/use-injectable-provided-in.md) | Using the `providedIn` property makes `Injectables` tree-shakable | | | :bulb: |
| [`use-lifecycle-interface`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/use-lifecycle-interface.md) | Ensures that classes implement lifecycle interfaces corresponding to the declared lifecycle methods. See more at https://angular.io/styleguide#style-09-01 | | | |
| [`use-lifecycle-interface`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/use-lifecycle-interface.md) | Ensures that classes implement lifecycle interfaces corresponding to the declared lifecycle methods. See more at https://angular.io/styleguide#style-09-01 | | :wrench: | |
| [`use-pipe-transform-interface`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/use-pipe-transform-interface.md) | Ensures that `Pipes` implement `PipeTransform` interface | :white_check_mark: | :wrench: | |
<!-- prettier-ignore-end -->

Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/docs/rules/use-lifecycle-interface.md
View file
Open in desktop
Expand Up @@ -18,6 +18,7 @@
Ensures that classes implement lifecycle interfaces corresponding to the declared lifecycle methods. See more at https://angular.io/styleguide#style-09-01

- Type: suggestion
- 🔧 Supports autofix (`--fix`)

<br>

Expand Down
18 changes: 17 additions & 1 deletion packages/eslint-plugin/src/rules/use-lifecycle-interface.ts
View file
Open in desktop
@@ -1,4 +1,9 @@
import { ASTUtils, toPattern } from '@angular-eslint/utils';
import {
ASTUtils,
toPattern,
RuleFixes,
isNotNullOrUndefined,
} from '@angular-eslint/utils';
import type { TSESTree } from '@typescript-eslint/utils';
import { createESLintRule } from '../utils/create-eslint-rule';

Expand All @@ -18,6 +23,7 @@ export default createESLintRule<Options, MessageIds>({
messages: {
useLifecycleInterface: `Lifecycle interface '{{interfaceName}}' should be implemented for method '{{methodName}}'. (${STYLE_GUIDE_LINK})`,
},
fixable: 'code',
},
defaultOptions: [],
create(context) {
Expand Down Expand Up @@ -50,6 +56,16 @@ export default createESLintRule<Options, MessageIds>({
node: key,
messageId: 'useLifecycleInterface',
data: { interfaceName, methodName },
fix: (fixer) => {
const { implementsNodeReplace, implementsTextReplace } =
RuleFixes.getImplementsSchemaFixer(parent, interfaceName);
return [
fixer.insertTextAfter(
implementsNodeReplace,
implementsTextReplace,
),
].filter(isNotNullOrUndefined);
},
});
},
};
Expand Down
41 changes: 41 additions & 0 deletions packages/eslint-plugin/tests/rules/use-lifecycle-interface/cases.ts
View file
Open in desktop
Expand Up @@ -58,6 +58,14 @@ export const invalid = [
interfaceName: ASTUtils.AngularLifecycleInterfaces.OnInit,
methodName: ASTUtils.AngularLifecycleMethods.ngOnInit,
},
annotatedOutput: `
@Component()
class Test implements OnInit {
ngOnInit() {

}
}
`,
}),
convertAnnotatedSourceToFailureCase({
description:
Expand All @@ -77,6 +85,16 @@ export const invalid = [
interfaceName: ASTUtils.AngularLifecycleInterfaces.OnDestroy,
methodName: ASTUtils.AngularLifecycleMethods.ngOnDestroy,
},
annotatedOutput: `
@Directive()
class Test extends Component implements OnInit, OnDestroy {
ngOnInit() {}

ngOnDestroy() {

}
}
`,
}),
convertAnnotatedSourceToFailureCase({
description:
Expand Down Expand Up @@ -120,6 +138,19 @@ export const invalid = [
},
},
],
annotatedOutput: `
@Injectable()
class Test implements DoBootstrap {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This didn't fix all the issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understood the documentation of eslint correctly the fixes are applied again and again, as long as the lint error is still reported. I'm unsure, how I can use the plugin in a local testing repo to test that out, please advice me on that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So yes I found a way to try it out. (run e2e tests and then try your changes in tmp/e2e-fixtures/ in any of the apps there. Maybe that should be documented) Indeed eslint runs again on the output of fixes until there are no changes, so it indeed results in a fix that adds all the implements clauses.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only thing I noticed is that I do not add an import for the Interface, here I would like guidance @JamesHenry

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bulldog98 Sorry for the delay. We have a utility for it used in a few other rules, it's very straightforward: #1829

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to get things ready for Angular 18, so I went ahead and applied the change

ngDoBootstrap() {}


ngOnInit() {}


ngOnDestroy() {}

}
`,
}),
convertAnnotatedSourceToFailureCase({
description:
Expand All @@ -139,5 +170,15 @@ export const invalid = [
interfaceName: ASTUtils.AngularLifecycleInterfaces.OnDestroy,
methodName: ASTUtils.AngularLifecycleMethods.ngOnDestroy,
},
annotatedOutput: `
@NgModule()
class Test extends Component implements ng.OnInit, OnDestroy {
ngOnInit() {}

ngOnDestroy() {

}
}
`,
}),
];