guardian / typerighter

Even if you’re the right typer, couldn’t hurt to use Typerighter!
Apache License 2.0
276 stars 12 forks source link

Derive categories from rules, not matchers #83

Closed jonathonherbert closed 4 years ago

jonathonherbert commented 4 years ago

What does this change?

In MatcherPool, the entity which provides an interface to run a check against multiple matcher instances, we've a 1-1 mapping between a category and a matcher.

This is restrictive, but worse – it's incorrect! We assign a 'category' to our languageTool matchers, but these matchers have rules that have their own opinions about categories. So a check against an lt instance supposedly responsible for category A may give a response containing category B.

As a result, there's an inconsistency where matchers will report being responsible for certain categories, and deliver rules that report other categories. This causes confusion in tooling downstream.

This PR refactors the getCategory interface in Matcher to report several categories, and in both regexMatcher and languageTool matchers changes the implementation to derive the category list from the rules they contain.

There's a broader question as to how we map from taxonomic entities from matchers that is still unanswered, but this PR at least ensures we're honest in our reporting!

How to test

The automated tests should pass – I've added one to ensure LanguageTool is reporting correctly.

We can test manually this by verifying that a bug in [prosemirror-typerighter] is fixed. Before, the fact that categories were misreported meant that LanguageTool rules would accumulate in the client, because the client clears matches by category, and the categories reported by the matcherResponse root and the matches themselves never added up:

categories-ko

This should no longer happen:

categories-ok

How can we measure success?

The MatcherResponse always reports the categories it has checked correctly.