Rule proposal: string-content

[fregante]

Good 👍

const description = 'Add text to user’s comments'

Not good 👎

const description = 'Add text to user\'s comments'

Prior art: https://github.com/jmont/eslint-plugin-smart-quotes

I tried that one but it doesn't have a fixer and it has many false positives. Also strings could be CSS selectors so[alt=""] needs to stay as is.

---

Originally posted by @sindresorhus in refined-github/refined-github#2167:

'use strict';

const create = context => {
	return {
		Literal: node => {
          	const {value} = node;
     
			if (typeof value !== 'string') {
				return;
			}
          
        	const fixedValue = value.replace(/'/, '‘');

			if (fixedValue === value) {
				return;
			}

			context.report({
				node,
				message: 'Use other quote kind',
				fix: fixer => fixer.replaceTextRange([node.range[0] + 1, node.range[1] - 1], fixedValue)
			});
		}
	};
};

module.exports = {
	create,
	meta: {
		type: 'problem',
		fixable: 'code'
	}
};

https://astexplorer.net/#/gist/3a67df351a57608ae38a6b55102376d6/c8425e326ddce75bf7242295c8818134ef2d2e16

---

IssueHunt Summary

Backers (Total: $80.00)

• issuehunt ($80.00)

Submitted pull Requests

• #496 Add string-content rule

---

Become a backer now!

Or submit a pull request to get the deposits!

Tips

• Checkout the Issuehunt explorer to discover more funded issues.
• Need some help from other developers? Add your repositories on IssueHunt to raise funds.

---

IssueHunt has been backed by the following sponsors. Become a sponsor

[MrHen]

I'm not sure how a lint rule would detect when it is appropriate to use one over the other. If you have a string such as Sally said, "Bob said, 'I am hungry'." you cannot flag both single quotes for conversion. The strings themselves could be split up across multiple literals so you couldn't check the content of any particular literal to know which is appropriate.

That being said, there are similar potential rules with other shorthand strings:

• " versus “ and ” (which is locale specific)
• ... versus …
• -> versus → (or a plethora of other arrows)

I think the ellipsis would be the easiest to detect because exactly three periods in a row is pretty indicative of an ellipsis. But I'm wary of trying to challenge the content in strings -- I think linting is more suited for checking code syntax and style than English syntax and style.

---

I suppose we could make this a generic string content scanner that took configured regex search / replace:

"unicorn/string-content": ["warn", {
  "patterns": [
    {
      "match": "'",
      "suggest": "’",
      "fix": false
    },
    {
      "match": "..." ,
      "suggest": "…",
      "fix": true
    }
  ]
}],

Or some such. But I'm fairly skeptical.

[fregante]

My fear is that it might cause some false positives; that’s why it should probably be limited to single apostrophes, exclusively to avoid escaping them.

A generic “typography” rule would be difficult to manage since there are many cases in which punctuation is not part of user-visible text (e.g. selectors, queries)

The replacer would be great too, as a generic rule

[fregante]

To test the applicability of this rule in your projects, try looking for situations where \' needs to stay like that:

grep "\\\'" -R . \
	--exclude-dir node_modules \
	--exclude-dir .git \
	--include "*.js" \
	--include "*.ts" \
	--include "*.tsx" \
	--exclude '*.min.*'

[sindresorhus]

I like the idea of a generic string-content rule. Let's go with that.

I think match could be even more flexible by accepting: string | RegExp | Array<string | RegExp> (?)

[sindresorhus]

I would use the rule for important things:

"unicorn/string-content": ["error", {
  "patterns": [
    {
      "match": "unicorn",
      "suggest": "🦄",
      "fix": true
    }
  ]
}]

[issuehunt-oss]

@issuehunt has funded $80.00 to this issue.

---

• Submit pull request via IssueHunt to receive this reward.
• Want to contribute? Chip in to this issue via IssueHunt.
• Checkout the IssueHunt Issue Explorer to see more funded issues.
• Need help from developers? Add your repository on IssueHunt to raise funds.

[otofu-square]

I'd like to work on this issue 👀

[fregante]

It's all yours :)

[sindresorhus]

Just make sure you read this thoroughly and have an understanding of AST and ESLint rules :) And add a lot of tests. It's better with too many than too few.

[fisker]

@sindresorhus I have a problem on report message, if we treat match as RegExp

{
	match: 'https?:\\\/\\\/www\\\\.github\\\\.com',
    suggest: 'github.com'
}

How do we report?

1. Message: Prefer `github.com`.
2. Add pattern.message, and default to Prefer `github.com`.
3. Report the pattern, Message Prefer `github.com` over `/https\?:\/\/www\\.github\\.com/gu`.
4. Don't support RegExp

Another problem, this rule original idea to fix ' to ’, so default to

[
	{
	  match: '\\'',
	  suggest: '‘',
	}
]

patterns should override or append? or more option?

[sindresorhus]

How do we report?

I would go with both 2 and 3. Default to 3, but allow overriding the message.

patterns should override or append? or more option?

We could make the key be the match:

{
	'\\'': {
		suggest: '‘'
	}
}

And allow setting it to false. Then the user could either override or disable with:

{
	'\\'': false
}

[sindresorhus]

Rewarding this issue is blocked by a dumb IssueHunt bug: https://spectrum.chat/issuehunt/general/error-when-accessing-an-issue-page~edd3be59-9228-472f-ae32-ce888e56bc1c?m=MTU4MzcyNjY3MjIxOQ==

[isghe]

It's a dangerous rule xojs/xo#439 (comment)