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

Rule proposal: No useless template string, or prefer String() #12866

Closed
marcospgp opened this issue Feb 2, 2020 · 10 comments · Fixed by #13579
Closed

Rule proposal: No useless template string, or prefer String() #12866

marcospgp opened this issue Feb 2, 2020 · 10 comments · Fixed by #13579
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion auto closed The bot closed this issue enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules

Comments

@marcospgp
Copy link

marcospgp commented Feb 2, 2020

Please describe what the rule should do:
Disallow useless template strings, such as ${x}. The thing is this can be used as a way to cast a variable to string, which means the rule could instead be "prefer String()" or "prefer .toString()".

What category of rule is this? (place an "X" next to just one item)

[ ] Warns about a potential error (problem)
[X] Suggests an alternate way of doing something (suggestion)
[ ] Enforces code style (layout)
[ ] Other (please specify:)

Provide 2-3 code examples that this rule will warn about:

const x = "soomething";
callAFunction(123, "mamamia", `${x}`);
const x = 19
callAFunction(123, "mamamia", `${x}`);

Why should this rule be included in ESLint (instead of a plugin)?

Because eslint already had tons of no-useless-something rules, so this one would fit very well with the rest.

Are you willing to submit a pull request to implement this rule?
No unless you point me to the right resources and ask nicely and I have enough free time this week

@marcospgp marcospgp added feature This change adds a new feature to ESLint rule Relates to ESLint's core rules triage An ESLint team member will look at this issue soon labels Feb 2, 2020
@mdjermanovic mdjermanovic added evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Feb 2, 2020
@mdjermanovic
Copy link
Member

Hi @marcospgp , thanks for the rule proposal!

This might as well be a new option in no-implicit-coercion?

@marcospgp
Copy link
Author

Hey @mdjermanovic , seems like a good fit.

@eslint-deprecated eslint-deprecated bot added the auto closed The bot closed this issue label Mar 6, 2020
@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

remcohaszing added a commit to remcohaszing/eslint that referenced this issue Aug 17, 2020
…nt#12866)

This adds the `templateString` option to `no-implicit-coercion`. This makes the
rule report the following code:

```js
`${foo}`
```

For backwards compatibility, this was added as a separate option instead of as
default behaviour.
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Sep 4, 2020
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Sep 4, 2020
@anikethsaha
Copy link
Member

anikethsaha commented Sep 29, 2020

Reopening as there is an active PR for this #13579

@anikethsaha anikethsaha reopened this Sep 29, 2020
@eslint eslint unlocked this conversation Sep 29, 2020
@anikethsaha anikethsaha added enhancement This change enhances an existing feature of ESLint and removed archived due to age This issue has been archived; please open a new issue for any further discussion auto closed The bot closed this issue feature This change adds a new feature to ESLint labels Sep 29, 2020
@anikethsaha
Copy link
Member

+1 to have it as an option for no-implicit-coercion

@eslint-deprecated eslint-deprecated bot added the auto closed The bot closed this issue label Oct 30, 2020
@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@ljharb
Copy link
Sponsor Contributor

ljharb commented Oct 30, 2020

This should be reopened since there’s an active PR for it.

@yeonjuan
Copy link
Member

Reopened. There is an active PR #13579

@yeonjuan yeonjuan reopened this Oct 30, 2020
@yeonjuan yeonjuan removed the auto closed The bot closed this issue label Oct 30, 2020
@eslint-deprecated eslint-deprecated bot added the auto closed The bot closed this issue label Nov 30, 2020
@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@remcohaszing
Copy link
Contributor

This should be reopened. There's still an active PR that's awaiting feedback from the ESLint team.

remcohaszing added a commit to remcohaszing/eslint that referenced this issue Apr 5, 2021
…nt#12866)

This adds the `templateString` option to `no-implicit-coercion`. This makes the
rule report the following code:

```js
`${foo}`
```

For backwards compatibility, this was added as a separate option instead of as
default behaviour.
mdjermanovic added a commit that referenced this issue Apr 9, 2021
…13579)

* Update: Add templateString option in no-implicit-coercion (fixes #12866)

This adds the `templateString` option to `no-implicit-coercion`. This makes the
rule report the following code:

```js
`${foo}`
```

For backwards compatibility, this was added as a separate option instead of as
default behaviour.

* Fix no-implicit-coercion templateString examples

* Skip tagged template strings in no-implicit-coercion

* Add missing tests for no-implicit-coercion

* Used cooked template string values

* Add missing documentation for no-implicit-coercion

Note that the `templateString` option isn’t affected by the `string` option.

* Add missing tests for no-implicit-coercion templateString

* Rename templateString to disallowTemplateString

* Fix typo in no-implicit-coercion docs

* Add tagged template string example for no-implicit-coercion

* Update docs/rules/no-implicit-coercion.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators May 30, 2021
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label May 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion auto closed The bot closed this issue enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants