Skip to content

Fix false positives in balanced-punctuation - #229

Open
kdeldycke wants to merge 2 commits into
sindresorhus:mainfrom
kdeldycke:fix-balanced-punctuation-apostrophes
Open

Fix false positives in balanced-punctuation#229
kdeldycke wants to merge 2 commits into
sindresorhus:mainfrom
kdeldycke:fix-balanced-punctuation-apostrophes

Conversation

@kdeldycke

@kdeldycke kdeldycke commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

This PR fix false positives in balanced-punctuation for apostrophes.

Description

The balanced-punctuation rule treats ' (U+2019) after a word character as an apostrophe and skips it. This works for company's or don't, but breaks when the same line also contains a curly-quoted span like 'steal', in which the closing ' after steal has a word character before it and space after it, so it gets classified as an apostrophe too, leaving the opening ' unmatched.

I stumbled upon this issue in my awesome-billing project, where I had to add <!--lint ignore balanced-punctuation--> to bypass this false-positive:

✖  372:86   Unclosed ' without matching closing '  remark-lint:balanced-punctuation
✖  402:252  Unclosed ' without matching closing '  remark-lint:balanced-punctuation

Line 402 has China's (apostrophe) and 'steal' (quoted span) on the same line: 3 ' characters total, with the closing quote misidentified as an apostrophe.

Root cause

The original match-punctuation plugin from remark-lint-plugins had a similar issue (see: #104 and laysent/remark-lint-plugins#44), which was fixed in 2345263. Then af84612 replaced the plugin with the custom balanced-punctuation implementation and reintroduced the same class of bug.

Fix

When isApostrophe() returns true but the character is at a word boundary (not between two word characters), check whether there's a matching opening ' (U+2018) on the stack.

Context

I contributed the quote/punctuation validation logic in the list-item rule 6 years ago (see: #101 and #127) and ran into this balanced-punctuation regression on my downstream awesome lists.

@sindresorhus

Copy link
Copy Markdown
Owner

I think this introduces a regression.

I manually checked it with a minimal case and the new logic hides a real unmatched opening quote if there is a possessive apostrophe later in the same text. For example, ‘The student books were on the table. still reports an unclosed opening quote, but ‘The students’ books were on the table. no longer reports anything.

The issue seems to be that once there is any earlier opening curly single quote on the stack, a later word-boundary can get treated as the closing quote even when it's just the apostrophe in a possessive. So this looks like it can mask actual punctuation errors in some cases.

Might be worth tightening the heuristic here a bit more becuase right now it seems to accept an unrelated opener as a match.

@kdeldycke

Copy link
Copy Markdown
Contributor Author

I manually checked it with a minimal case and the new logic hides a real unmatched opening quote if there is a possessive apostrophe later in the same text. For example, 'The student books were on the table. still reports an unclosed opening quote, but 'The students' books were on the table. no longer reports anything.

Ok now I get the issue. The tension lies in the distinction between what count as an apostrophe and what count as a quoting mark, and which characters to reserve for each usage.

I made an argument in #104 (comment) that:

You should use the ASCII straight apostrophe, i.e. ' (char code 39, HEX 27) instead of ' , which is classified by linter's rules as a quote and need to be matched.

So at the time my position was to reserve straight ' for apostrophes only, and curly ' for quotes only. It was a clean separation that removes the ambiguity entirely. But it was artificial and @jtojnar pointed out that U+2019 is the Unicode-preferred character for apostrophes.

The current balanced-punctuation.js (committed in af84612) took a middle path: it includes curly ' as a default pair but tries to detect apostrophes with the isApostrophe() heuristic. That heuristic is what's causing this chain of bugs: first too aggressive (skipping real closing quotes), then my fix here which you considered too loose.

I just updated the code to tightened the heuristic. Each opener now records its text index, and a word-boundary ' is only treated as a closing quote if the text between it and the opener contains no whitespace. This fixes your 'The students' books case while still handling single-word quotes like 'steal'. The tradeoff is that multi-word curly single quotes, like 'quoted phrase', would be flagged as unmatched. Even if these cases don't seem to appear in awesome lists, it feels like this heuristic is still a bit too fragile.

If my original strict separation holds (awesome lists should use straight ' for apostrophes), then the whole isApostrophe heuristic is solving a problem that shouldn't exist. But if curly apostrophes are valid, @jtojnar is probably right that no robust algorithm exists. I don't have a strong opinion on which way to go, I just want to fix the bug. @sindresorhus, I need your judgment on that so we can implement an acceptable fix.

Repository owner deleted a comment from jm9133520-crypto Jun 10, 2026
Repository owner deleted a comment from jm9133520-crypto Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants