Extraction-check: generate SARIF file

[byronantak]

Add logic to generate sarif.json file to extraction-check command

Technical

• Every check adds unique rule ids per possible failure (except AI checker, it's generic)
• Update cli checker to include a breakdown of a failure per source text unit
• Update tests
  • Simplify some tests
• Fix GlossaryCaseChecker side-effect and extra new line problems

[mattwilshire]

Suggested change

	String sarifThing = objectMapper.writeValueAsString(sarif);
	String sarifData = objectMapper.writeValueAsString(sarif);

[mattwilshire]

We could simplify this by doing the join on the map directly:

String suggestFixEntry = failure.getValue().stream()
        .map(AICheckResult::getSuggestedFix)
        .collect(Collectors.joining(System.lineSeparator()));

[mattwilshire]

Could we use method chaining here instead for the builder ?
Something like the following:

sb.append(BULLET_POINT)
     .append(QUOTE_MARKER)
     .append(misspeltWord)
     .append(QUOTE_MARKER)
     .append(" is spelt incorrectly. Suggested correct spellings are: ")
     .append(String.join(", ", suggestions))
     .append(System.lineSeparator());

[mattwilshire]

Can we remove / change this URL string ?

[maallen]

+1, this url should be configurable

[mattwilshire]

Suggested change

	logger.info(
	logger.error(

[mattwilshire]

Or logger.warn

[mattwilshire]

Would it be possible to use an enum here for the ruleId ?
We could then convert it to a string when it is needed, something like the following:

public enum RULE_ID {
    AGGREGATE_SPELLING_SUGGESTIONS,
    EQUAL_CONTEXT_AND_COMMENT_STRINGS,
    EMPTY_CONTEXT_AND_COMMENT_STRINGS,
    EMPTY_COMMENT_STRING;
    ...

    @Override
    public String toString() {
        return name();
    }
}

[maallen]

There is a CliCheckerType Enum for each of the CLI checks which could be re-used here as the rule id

[maallen]

Ah I see why you can't re-use the CliCheckerType enum as some checks have multiple rule ids, moving these to their own enum will centralise our rule ids and make them easier to add/track in future

[maallen]

What will happen if we return a CliCheckResult with it's rule id field set to null? I feel that's a bit cleaner

[byronantak]

I prefer a null object instead of an object of all null fields (in this instance) 🤷‍♂️
It also keeps to the same pattern this method was already following

[maallen]

+1, this url should be configurable