feat(select-style): add fixer

[rafaelss95]

Part of #179.

[timdeschryver]

This is great, and I'm sure that it will help some codebases!
I'm going to take a look at the implementation later, but I already have a question.

[rafaelss95]

Something that came to my mind... since we have some rules relying on Store import, wouldn't it be a good idea to have a rule like use-consistent-global-import or something like this? Because if I do import { Store as ngrxStore } from '@ngrx/store' these rules won't work as expected...

[timdeschryver]

Looks good to me!
I'm so excited about this fix 🚀
About your question, I think it's good to keep in mind, but I also think we won't get a lot of benefit from it. Maybe we should make the store lookup mechanism smarter, so it looks for the renamed node, e.g. ngrxStore instead of Store?

[github-actions]

🎉 This PR is included in version 1.30.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[rafaelss95]

About your question, I think it's good to keep in mind, but I also think we won't get a lot of benefit from it. Maybe we should make the store lookup mechanism smarter, so it looks for the renamed node, e.g. ngrxStore instead of Store?

Oh, I just noticed that we already handle these cases properly in rules that rely on findNgRxStoreName here:

eslint-plugin-ngrx/src/utils/helper-functions/utils.ts

Line 44 in c4f31b6

importClause.imported.name === importedName

Using imported.name is sufficient to cover this.

---

The only ones that are missing are those that depend on the typeName:

eslint-plugin-ngrx/src/utils/selectors/index.ts

Lines 19 to 20 in dc1ff4a

	export const injectedStore = `MethodDefinition[kind='constructor'] Identifier[typeAnnotation.typeAnnotation.typeName.name='Store']`
	export const typedStore = `MethodDefinition[kind='constructor'] Identifier>TSTypeAnnotation>TSTypeReference[typeName.name='Store'][typeParameters.params]`

But yes, I don't think it's a valid requirement to the point of having a rule just for these few cases (three rules more specifically).

[timdeschryver]

@rafaelss95 this works like a charm!
One nit, would it be possible to remove the select import if we switch from the operator syntax to the method syntax?

For example:

// before
import { select, Store } from '@ngrx/store';
this.searchQuery$ = store.pipe(select(fromBooks.selectSearchQuery));

// becomes (notice that `select` is still imported).
import { select, Store } from '@ngrx/store';
this.searchQuery$ = store.select(fromBooks.selectSearchQuery);

I would've expected that the unused import also had a fixer, but it seems like it does not have one - thus after this fix we end up with a new warning 😅

[rafaelss95]

Hmm, this is tricky as there is no way to actually know if there are other occurrences of select. Coincidentally, I talked about this with James not long ago here angular-eslint/angular-eslint#606 (comment). In this specific case, which had the intention to remove the imports from lifecycle interfaces, he suggested that we do a string check and then remove if there was only one occurrence (the import) and so we did... however, for this case here I don't know if this approach would be viable, since a search for 'select' would still return several occurrences, as it only changed the position/way it is called and we can't really trust on something like .select or (select) because of formatting 😞.

I just sent a PR hoping to accomplish this 🙏