Eslint-plugin-react: autofix for no-unescaped-entities

[modosc]

i'm not 100% sure if this makes sense - it seems like this check is more for finding unintentionally closed jsx tags? it would be super useful though to make the recommended replacements automatically, i'd be happy to do the pr if it's in the spirit of the original rule.

[ljharb]

An autofix would absolutely silence the warnings, but the point of this rule is to catch likely bugs - and the likely bug is that the entities are supposed to be in a jsx context. That can't be autofixed; a human needs to determine the fix.

[benjie]

I can see that automatically escaping < and > would be risky (I would never expect that to work!), but perhaps it could be applied for certain other characters? For example ' and " are commonly written out in JSX (though they shouldn't be!) but they don't typically indicate a bug - just someone being too lazy to escape them in my case 😳

render() {
  return (<div>Benjie's bullish on "eslint --fix"</div>);
}

I've never written an auto-fixer for ESLint so not sure if partial auto-fixing is okay.

[benjie]

There's discussion around this in https://github.com/yannickcr/eslint-plugin-react/issues/1263#issuecomment-309327876

[ljharb]

In the case of straight quotes, those are typographically incorrect - the proper fix there is to use curly quotes if it’s prose. Again, i don’t think that’s something that can be safely programmatically fixed.

[benjie]

Of course it's not always prose; but discussions around correct typography aside, could you give an example of where ' being replaced with ' in the child (text) context would not be safe to fix programatically? Even <code> handles it correctly

render() {
  return (<pre><code>echo 'eslint --fix "path/to/file"' >> ./autofix</code></pre>);
}

vs

render() {
  return (<pre><code>echo &apos;eslint --fix &quot;path/to/file&quot;&apos; >> ./autofix</code></pre>);
}

At the moment the rule seems to relate to entities being unescaped; perhaps there should be a different rule about making typographical recommendations (or maybe the error message for this rule should be change to suggest using typographical quotes to make its intent clearer)?

---

My main motivation for this to be auto-fixed is due to GitHub's JS code formatting messing up when only one single quote is present, e.g.

render() {
  return (<pre><code>echo 'eslint --fix</code></pre>);
}

(Workaround is to use the jsx code formatting (e.g. by renaming your files), or to just correctly escape the quotes.)

[modosc]

for me personally i've never had this rule catch what it's supposed to catch - instead it forced me to learn the ampersand codes for single and double quotes. i suppose it makes more sense for me to disable it since i'm spending more time fixing non-buggy code - i guess this can be closed out then?

[ljharb]

@modosc all that's required to fix it is that you take your literal text, and wrap it in explicit jsx curlies, and one of the JS quote styles. If it's catching human-intended prose, then you should be replacing your straight quotes with curly quotes.

This:

render() {
  return (<pre><code>echo 'eslint --fix "path/to/file"' >> ./autofix</code></pre>);
}

should instead be, for example:

render() {
  return (<pre><code>{`echo 'eslint --fix "path/to/file"' >> ./autofix`}</code></pre>);
}

@benjie this is not intended to catch your example of "code in a pre" - it's intended to catch mistakes like in the rule documentation.

[modosc]

ok, i'm good then. thanks

[stevemao]

Can the error message print more info? EG:

- `>` can be replaced with `>`
- `"` can be replaced with `"`, `“`, `"` or `”`
- `'` can be replaced with `'`, `‘`, `'` or `’`
- `}` can be replaced with `}`

So that I don't have to look it up every time.

[ljharb]

@stevemao sure, that sounds like a great improvement - a PR would be most welcome.

[stevemao]

@ljharb I just submitted a PR 😄 #2016

[bluepeter]

Did anyone come up w/ simple ways to auto-fix? Neither eslint nor prettier seem to have options to auto-fix non-smart quotes in prose to smart quotes.

[talentlessguy]

There could be a plugin setting in which way to replace the escape entity for auto-fix :thinking:

e.g.

"settings": {
    "eslint-plugin-react": {
      "version": "detect",
      "no-unescaped-entities": {
        "\"": """
      }
    }
  }

[ljharb]

@talentlessguy it is very atypical and rare to have settings exclusively for autofix; any autofix, with or without an option, must be something that's safe - and this one wouldn't be.