fix: misspelling should still be recognised as a misspelling when no suggestions are returned

[benjam-es]

…

What:

• Bug Fix

Description:

Fixes #100

When a misspelling occurs, but no suggestions are found, the spellchecker returns as a pass. This is incorrect as the misspelling should still be reported.

results in:

With this PR:

Related:

Issue #100
Also just found #101

[c0nst4ntin]

@benjam-es I reviewed the changes. Generally, I think this is a good contribution. I just added some feedback where I see some problems with your approach.

Please take a look the the feedback and suggestions. I really like that you added good tests for your changes!

[c0nst4ntin]

I would prefer checking directly for "not equals" or am I missing something?

Suggested change

	$suggestionsHelperText = ($suggestions > '') ? 'Did you mean:' : 'No suggestions found.';
	$suggestionsHelperText = ($suggestions !=== '') ? 'Did you mean:' : 'No suggestions found.';

[c0nst4ntin]

See previous comment ^^

Suggested change

	$suggestionsHelperText = ($suggestions > '') ? 'Did you mean:' : 'No suggestions found.';
	$suggestionsHelperText = ($suggestions !=== '') ? 'Did you mean:' : 'No suggestions found.';

[c0nst4ntin]

Can we really drop this conversion at this position?
Then it would be serialized as an iterator and returned as an iterator from the cache.

This would cause the is_array() call to always be false:

peck/src/Services/Spellcheckers/Aspell.php

Lines 48 to 53 in 818c85c

	/** @var array<int, Misspelling>|null $misspellings */
	$misspellings = $this->cache->has($text) ? $this->cache->get($text) : $this->getMisspellings($text);
	if (! is_array($misspellings)) {
	$misspellings = $this->getMisspellings($text);
	}

[c0nst4ntin]

We don't need the variable and can pass this directly:

Suggested change

	$lines = $this->getAspellLinesFromOutput($output);
	return $this->getMisspellingsFromOutputLines($lines);
	return $this->getMisspellingsFromOutputLines(
	$this->getAspellLinesFromOutput($output)
	);

[c0nst4ntin]

If you wanted to, we could turn this if-else block into two match() statements.

This way we do not rely on the variables from inside the if block in the return statement and clearly separate the aggregation of the $word and the $suggestions.

Suggested change

	if (preg_match('/#\\s(\\w*)\\sd*/', $line, $matches)) {
	$word = $matches[1];
	$suggestions = [];
	} else {
	[$wordMetadataAsString, $suggestionsAsString] = explode(':', trim($line));
	$word = explode(' ', $wordMetadataAsString)[1];
	$suggestions = explode(', ', trim($suggestionsAsString));
	}
	$word = match (true) {
	preg_match('/#\\s(\\w*)\\sd*/', $line, $matches) => $matches[1],
	default => explode(' ', explode(':', trim($line))[0])[1],
	};
	$suggestions = match (true) {
	preg_match('/#\\s(\\w*)\\sd*/', $line, $matches) => [],
	default => explode(', ', trim(explode(':', trim($line))[1])),
	};

[c0nst4ntin]

I marked this as a draft for now until we discussed and implemented some of the feedback I gave you.
If you belive this request is ready for another review or to be merged, please open it back up for review