Add "backtick" option for quotemark #4029

ericbf · 2018-07-12T00:13:27Z

This lets users enforce only backtick "`" strings for everywhere that it’s permissible.

See #539
The corresponding option in eslint is called "backtick": http://eslint.org/docs/rules/quotes
See https://ponyfoo.com/articles/template-literals-strictly-better-strings

PR checklist

Addresses an existing issue: Add option to quotemark rule to disallow all quotes #539
New feature, bugfix, or enhancement
- Includes tests
Documentation update

Overview of change:

Adds an option "backtick" to "quotemark" rule, which would allow users to enforce "`" only strings where permissible.

CHANGELOG.md entry:

[rule-change], [enhancement]

palantirtech · 2018-07-12T00:13:28Z

Thanks for your interest in palantir/tslint, @ericferreiralife! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

giladgray

@ericferreiralife thanks! this is a sweet feature 👏
a few requests to tighten up the code and we're g2g.

giladgray · 2018-07-12T06:09:06Z

src/rules/quotemarkRule.ts

@@ -37,13 +43,14 @@ export class Rule extends Lint.Rules.AbstractRule {
    /* tslint:disable:object-literal-sort-keys */
    public static metadata: Lint.IRuleMetadata = {
        ruleName: "quotemark",
-        description: "Requires single or double quotes for string literals.",
+        description: "Requires single or double quotes or backticks for string literals.",


"Enforces quote character for string literals."

giladgray · 2018-07-12T06:19:41Z

src/rules/quotemarkRule.ts

+                jsxQuoteFromOption[jsxPref] as JSX_QUOTE_MARK :
+                quoteMark !== "`" ?
+                    quoteMark :
+                    '"',


can all of this logic be moved inside getQuotemarkPreference and getJSXQuotemarkPreference?
may need another argument, but it will be much more legible.

Yup, doing that

giladgray · 2018-07-12T06:19:48Z

src/rules/quotemarkRule.ts

-        const quoteMark = getQuotemarkPreference(args) === OPTION_SINGLE ? "'" : '"';
+        const pref = getQuotemarkPreference(args);
+        const jsxPref = getJSXQuotemarkPreference(args);
+        const quoteMark = quoteFromOption[pref != undefined ? pref : OPTION_DOUBLE] as QUOTE_MARK;


refactor this logic. aiming for simply const quoteMark = getQuotemark(args);

giladgray · 2018-07-12T06:21:30Z

src/rules/quotemarkRule.ts

+        }
+    }
+
+    return undefined;


return the default value instead of nothing

Already doing that as part of the refactor

giladgray · 2018-07-12T06:27:12Z

src/rules/quotemarkRule.ts

+        const jsxQuoteFromOption = {
+            [OPTION_JSX_SINGLE]: "'",
+            [OPTION_JSX_DOUBLE]: '"',
+        };


move these out to static scope

Doing that as part of the refactor.

ericbf · 2018-07-13T17:10:06Z

Any updates here?

ericbf · 2018-07-15T01:22:45Z

Hey @giladgray I pushed those changes you requested. Any follow up?

This lets users enforce only backtick "`" strings for everywhere that it's permissible. This also simplifies some logic in the preferred quote determination, combining some disparate logic into the nice self-packaged functions. > See #539 > The corresponding option in eslint is called "backtick": http://eslint.org/docs/rules/quotes > See https://ponyfoo.com/articles/template-literals-strictly-better-strings

ericbf · 2018-07-17T03:41:14Z

Any new changes requested here?

ericbf · 2018-07-17T22:45:51Z

@giladgray @adidahiya can anyone take a look here?

johnwiseheart

LGTM

Old review

ShayBox · 2018-09-08T03:01:41Z

Any reason setting the option to backtick now would recommend using double quotes? Does it with both of the vscode extensions, seems it doesn't understand the new rule option.

EDIT: Nvm the rule just doesn't work at all, not listed as an option on the docs, seems this merged PR didn't actually stay in effect

ericbf · 2018-11-04T01:34:00Z

@ShayBox let me look into that. This code was committed after the latest release, so it's not included in v5.11.0

JasCodes · 2018-12-05T14:02:10Z

@ericbf Any idea on next release? Its been while.

ericbf · 2018-12-10T21:26:15Z

@Jas99 I'm wondering the same thing... @johnwiseheart?

johnwiseheart · 2018-12-10T21:29:18Z

Likely next week, I’ll try to make a release if I have time.

…

On Mon, 10 Dec 2018 at 21:26, Eric Ferreira ***@***.***> wrote: @Jas99 <https://github.com/jas99> I'm wondering the same thing... @johnwiseheart <https://github.com/johnwiseheart>? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4029 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEEo1LllMJYpEQqDJfbljdpqRwh6FdCgks5u3tF5gaJpZM4VMBoT> .

alexeymrkn · 2018-12-22T02:03:29Z

Still ignores the backtick option and defaults to double in v5.12.0

johnwiseheart · 2018-12-22T09:15:12Z

I tried you believe there's a bug in the backtick option (haven't tried it yet myself), opening a new issue would be your best bet.

johnwiseheart · 2018-12-22T09:19:12Z

In fact, #4402 might be the issue to follow for that one.

adidahiya · 2019-01-09T22:22:07Z

@alexeymrkn I'm not seeing that issue in 5.12.0, backtick works as intended for me. Please file a new issue with more details if your problem persists.

adidahiya · 2019-01-10T20:37:07Z

Actually, I found some issues and confusing behavior with avoid-template and backticks, will try to fix them in #4408

spencersmb · 2019-02-09T18:14:01Z

I feel like something isn't working right. I don't get the option for backticks.
Using webstorm(2018.3.4), ts-lint(5.12.1) & typescript(3.3.3)

Any ideas why it's doing this?
Thanks!

ericbf · 2019-02-09T22:15:06Z

I've been using the option with great success in my projects, not in webstorm. Does just putting the option in work for you? Does it just not show up as an option?

spencersmb · 2019-02-10T18:59:57Z

@ericbf It kinda works - it does this to a tsx file for example:

It changed my imports, but it did not change my props on components, instead it made them double quotes. Now my imports can't be read lol. (the other backticks were already present)

ericbf · 2019-02-10T20:13:56Z

Yes, I've seen the behavior on imports. Apparently ECMA only allows single or double quoted strings in imports, not no template literals. I've been putting // tslint:disable:quotemark and a respective enable above and below my imports to keep using the backtick option as a workaround.

JoshuaKGoldberg · 2019-02-10T22:08:41Z

@spencersmb perhaps your editor's TSLint version isn't up-to-date? The dropdown/autocompletion options are auto-generated from the metadata in the rule.

Re your imports: that looks like a bug in the rule 😢; would you mind filing a new issue as a bug report? I'm curious about your source code, etc.

ericbf · 2019-02-10T22:37:51Z

@JoshuaKGoldberg @spencersmb I'll open another issue to refer to the situations where typescript/ecma doesn't like backticks at all.

ericbf · 2019-02-10T22:48:47Z

Opened #4514

spencersmb · 2019-02-11T20:45:55Z

@JoshuaKGoldberg @ericbf Thanks for starting that ticket, I added on to it my info. I have to tell webstorm the path to Ts-lint module after I install it via npm so it should be running the latest version.
If you want to take a look at my repo here it is ->
https://github.com/spencersmb/etShop-gatsby

giladgray previously requested changes Jul 12, 2018

View reviewed changes

johnwiseheart approved these changes Jul 19, 2018

View reviewed changes

johnwiseheart merged commit 559d880 into palantir:master Jul 19, 2018

ericbf deleted the add-quotemark-backtick branch July 19, 2018 19:11

JoshuaKGoldberg mentioned this pull request Nov 3, 2018

Add option to quotemark rule to disallow all quotes #539

Closed

JasCodes mentioned this pull request Dec 5, 2018

New Release ( waiting for backtick )? #4340

Closed

Add "backtick" option for quotemark #4029

Add "backtick" option for quotemark #4029

Conversation

ericbf commented Jul 12, 2018 • edited Loading

PR checklist

Overview of change:

CHANGELOG.md entry:

palantirtech commented Jul 12, 2018

giladgray left a 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericbf commented Jul 13, 2018

ericbf commented Jul 15, 2018

ericbf commented Jul 17, 2018

ericbf commented Jul 17, 2018

johnwiseheart left a comment

Choose a reason for hiding this comment

ShayBox commented Sep 8, 2018 • edited Loading

ericbf commented Nov 4, 2018 • edited Loading

JasCodes commented Dec 5, 2018

ericbf commented Dec 10, 2018

johnwiseheart commented Dec 10, 2018 via email

alexeymrkn commented Dec 22, 2018

johnwiseheart commented Dec 22, 2018

johnwiseheart commented Dec 22, 2018

adidahiya commented Jan 9, 2019

adidahiya commented Jan 10, 2019

spencersmb commented Feb 9, 2019

ericbf commented Feb 9, 2019

spencersmb commented Feb 10, 2019

ericbf commented Feb 10, 2019

JoshuaKGoldberg commented Feb 10, 2019

ericbf commented Feb 10, 2019

ericbf commented Feb 10, 2019

spencersmb commented Feb 11, 2019 • edited Loading

ericbf commented Jul 12, 2018 •

edited

Loading

ShayBox commented Sep 8, 2018 •

edited

Loading

ericbf commented Nov 4, 2018 •

edited

Loading

spencersmb commented Feb 11, 2019 •

edited

Loading