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.

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

feat(eslint-plugin): [use-injectable-provided-in] add suggestion #594

Merged

Conversation

rafaelss95
Copy link
Member

1st. commit: adds suggestions that covers the most common values and also fixes some false negatives like when we use null as value or when we use a computed argument to @Injectable decorator and false positives when we use 'providedIn' as a Literal;
2nd. commit: ignore classes that implements HttpInterceptor (see #236);
3rd. commit: add option to ignore classes given a pattern (see #236).

Closes #236.

{
type: 'object',
properties: {
ignoreClassNamePattern: {
Copy link
Member Author

Choose a reason for hiding this comment

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

Not really sure about this name.

@rafaelss95 rafaelss95 force-pushed the feat/use-injectable-provided-in branch 2 times, most recently from 4b7380f to 27891f9 Compare July 12, 2021 23:57
@codecov
Copy link

codecov bot commented Jul 13, 2021

Codecov Report

Merging #594 (d8ba868) into master (997991c) will decrease coverage by 0.01%.
The diff coverage is 81.81%.

@@            Coverage Diff             @@
##           master     #594      +/-   ##
==========================================
- Coverage   86.05%   86.03%   -0.02%     
==========================================
  Files          89       89              
  Lines        2402     2421      +19     
  Branches      416      421       +5     
==========================================
+ Hits         2067     2083      +16     
- Misses        200      201       +1     
- Partials      135      137       +2     
Impacted Files Coverage Δ
packages/eslint-plugin/src/utils/selectors.ts 100.00% <ø> (ø)
packages/eslint-plugin/src/utils/utils.ts 87.12% <73.68%> (-0.16%) ⬇️
...int-plugin/src/rules/use-injectable-provided-in.ts 94.73% <92.85%> (-5.27%) ⬇️

@rafaelss95 rafaelss95 marked this pull request as ready for review July 13, 2021 00:14
@rafaelss95 rafaelss95 marked this pull request as draft July 13, 2021 11:31
@rafaelss95 rafaelss95 force-pushed the feat/use-injectable-provided-in branch from 27891f9 to 6bbc8a1 Compare July 13, 2021 13:43
@rafaelss95 rafaelss95 marked this pull request as ready for review July 13, 2021 13:45
@rafaelss95 rafaelss95 force-pushed the feat/use-injectable-provided-in branch 4 times, most recently from 7afac2c to 6f14473 Compare July 14, 2021 17:27
Copy link
Member

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

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

Sorry about the conflicts from when the doc generation work went in

@rafaelss95 rafaelss95 force-pushed the feat/use-injectable-provided-in branch from 6f14473 to d421e22 Compare August 28, 2021 17:15
@JamesHenry JamesHenry merged commit bdef8c7 into angular-eslint:master Aug 31, 2021
@JamesHenry
Copy link
Member

Thanks @rafaelss95!

@Niaro
Copy link

Niaro commented Oct 8, 2021

@rafaelss95 Hiii, my I ask a question. why the null value is wrong? Isn't it a default value? Like official comment says
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use-injectable-provided-in should be configurable
3 participants