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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rule proposal: `string-content` #327

Open
fregante opened this issue Jun 22, 2019 · 9 comments

Comments

@fregante
Copy link

commented Jun 22, 2019

Issuehunt badges

Good 馃憤

const description = 'Add text to user鈥檚 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 sindresorhus/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)

Become a backer now!

Or submit a pull request to get the deposits!

Tips


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

@MrHen

This comment has been minimized.

Copy link
Collaborator

commented Jun 22, 2019

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

This comment has been minimized.

Copy link
Author

commented Jun 22, 2019

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

A generic 鈥渢ypography鈥 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

This comment has been minimized.

Copy link
Author

commented Jun 22, 2019

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

This comment has been minimized.

Copy link
Owner

commented Jun 23, 2019

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

This comment has been minimized.

Copy link
Owner

commented Jun 23, 2019

I would use the rule for important things:

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

@sindresorhus sindresorhus changed the title Rule proposal: Prefer typographic apostrophe in strings: `\'` 鈫 `鈥檂 Rule proposal: string-content Sep 5, 2019

@sindresorhus sindresorhus changed the title Rule proposal: string-content Rule proposal: `string-content` Sep 5, 2019

@issuehunt-app

This comment has been minimized.

Copy link

commented Sep 6, 2019

@issuehunt has funded $80.00 to this issue.


@otofu-square

This comment has been minimized.

Copy link

commented Sep 11, 2019

I'd like to work on this issue 馃憖

@fregante

This comment has been minimized.

Copy link
Author

commented Sep 11, 2019

It's all yours :)

@sindresorhus

This comment has been minimized.

Copy link
Owner

commented Sep 11, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can鈥檛 perform that action at this time.