MD053 should support "[//]: # (Comment)" pattern by default

[ssbarnea]

This seems to be a regression introduced by v0.32.1 release which affects people using markdown comments as recommended here, which is hugely popular, with almost 2k upvotes. We used the format below in order to add some placeholder comments which are needed when we compile the documentation, so we know were a specific section starts or ends.

[//]: # DO-NOT-REMOVE-README-TITLE

This worked fine until yesterday, when pre-commit monthly update upgraded markdownlint to v0.32.1.

README.md:3:1 MD053/link-image-reference-definitions Link and image reference definitions should be needed [Unused link or image reference definition: "//"] [Context: "[//]: # DO-NOT-REMOVE-README-T..."]

Is there any workaround for this?

PS. Even a search on https://sourcegraph.com/search?q=context:global+%5B//%5D:+%23&patternType=literal can show that use of this is extremely popular.

[ssbarnea]

I tried adding parentheses around the placeholder but prettier will reformat the line to make it like below, which is still not allowed by markfownlint:

[//]: # "DO-NOT-REMOVE-README-TITLE"

[DavidAnson]

A few clarifications:

• The version v0.32.1 refers to a markdownlint-cli release version.
• You say this is a "regression" which implies that it worked in a previous version. However, the rule was introduced in CLI version 0.32.0 and I expect it behaved the same there. So I think you are actually complaining that the newly introduced rule has a behavior you don't like?
• This rule is correctly identifying your example as an unused link reference definition. This is called out in the Stack Overflow comments which refer to this syntax as an "abuse" of the feature.
• As with all other rules, you can disable this rule for your project or on a file-by-file or line-by-line basis in cases where you disagree with it. That seems appropriate here.
• Or are you suggesting a modification to the rule to ignore patterns like this?

[ssbarnea]

I did not test 0.32.0 in particular and I suspect that the issue was caused by the introduction of this rule. As this rule was created many years after the practice of using that placeholder was standardized, I would say that it might be wise to adapt the rule to bypass that common pattern, which is not really a bug.

The rule itself is benefit, I am not questioning its intentions and it would be sad to have to disable it. As I do not know any valid workaround that can be used and not conflict with other tooling (see comment about prettier reformatting it), I think it would be better to add an exception for this pattern inside the rule. WDYT?

IMHO, A linter aims to find bugs while minimizing the chance of producing false-positives. If there is a legit use-case that cannot be rewritten that produces a false-positive, it makes sense to tune the rule to avoid it.

[DavidAnson]

Agreed. I am thinking I will add a configurable list of "unused" referenced labels to ignore and default it to just the "//" case you highlight as that seems to have gained traction with the community.

[DavidAnson]

@ssbarnea Thank you for the sponsorship!

[ssbarnea]

@DavidAnson Apparently the fix does not work yet, see https://results.pre-commit.ci/run/github/389989116/1660058623.KSC139LtSLeY6AHYF_SEtg -- now this might be related to the fact that the cli repo is maybe behind?

[DavidAnson]

I published version 0.26.2 of the library last night and here is an example of your scenario not reporting an error: https://dlaa.me/markdownlint/#%25m%23%20Issue%20545%0A%0A%5B%2F%2F%5D%3A%20%23%20DO-NOT-REMOVE-README-TITLE%0A

Dependent project like CLI and CLI2 will pick up pick up the change soon.