[Core] Look up docstring converter by type as fallback

[mpkorstanje]

Fixes: #2458

[codecov]

Codecov Report

Merging #2459 (d8b4036) into main (0364976) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head d8b4036 differs from pull request most recent head 59ef90c. Consider uploading reports for the commit 59ef90c to get more accurate results

@@            Coverage Diff            @@
##               main    #2459   +/-   ##
=========================================
  Coverage     83.65%   83.66%           
- Complexity     2675     2677    +2     
=========================================
  Files           319      319           
  Lines          9434     9439    +5     
  Branches        916      917    +1     
=========================================
+ Hits           7892     7897    +5     
  Misses         1203     1203           
  Partials        339      339           

Impacted Files	Coverage Δ
...ucumber/core/stepexpression/DocStringArgument.java	85.71% <100.00%> (ø)
...o/cucumber/core/stepexpression/StepExpression.java	100.00% <100.00%> (ø)
...a/io/cucumber/docstring/DocStringTypeRegistry.java	100.00% <100.00%> (ø)
...tring/DocStringTypeRegistryDocStringConverter.java	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0364976...59ef90c. Read the comment docs.

[mpkorstanje]

Pffff. To cover all cases I've got to take the Cartesian product of:

content type: null, text, json
target type: String, JsonNode
registry: 
 - [default]
 - [default, String/text]
 - [default, String/text, String/xml]
 - [default, JsonNode/json]
 - [default, String/text, JsonNode/json]
 - [default, JsonNode/json, JsonNode/xml] 

Which works out to:

content type | target type | registry                              ---> doc string type
             | String      | [String/]                             ---> String/
text         | String      | [String/]                             ---> String/
json         | String      | [String/]                             ---> String/
             | JsonNode    | [String/]                             ---> exception: suggest registering docstring type for JsonNode
text         | JsonNode    | [String/]                             ---> exception: suggest registering docstring type for JsonNode/text
json         | JsonNode    | [String/]                             ---> exception: suggest registering docstring type for JsonNode/json

             | String      | [String/, String/text]                ---> String/
text         | String      | [String/, String/text]                ---> String/text
json         | String      | [String/, String/text]                ---> String/
             | JsonNode    | [String/, String/text]                ---> exception: suggest registering docstring type for JsonNode
text         | JsonNode    | [String/, String/text]                ---> exception: suggest registering docstring type for JsonNode/text
json         | JsonNode    | [String/, String/text]                ---> exception: suggest registering docstring type for JsonNode/json

             | String      | [String/, String/text, String/xml]    ---> String/
text         | String      | [String/, String/text, String/xml]    ---> String/text
json         | String      | [String/, String/text, String/xml]    ---> String/json
text         | JsonNode    | [String/, String/text]                 ---> exception: suggest registering docstring type for JsonNode/text
text         | JsonNode    | [String/, String/text, String/xml]    ---> exception: suggest registering docstring type for JsonNode/text
json         | JsonNode    | [String/, String/text, String/xml]    ---> exception: suggest registering docstring type for JsonNode/json


             | String      | [String/, JsonNode/json]              ---> String/
text         | String      | [String/, JsonNode/json]              ---> String/
json         | String      | [String/, JsonNode/json]              ---> String/
             | JsonNode    | [String/, JsonNode/json]              ---> JsonNode/json
text         | JsonNode    | [String/, JsonNode/json]              ---> exception: suggest registering docstring type for JsonNode/text
json         | JsonNode    | [String/, JsonNode/json]              ---> JsonNode/json

             | String      | [String/, String/text, JsonNode/json] ---> String/
text         | String      | [String/, String/text, JsonNode/json] ---> String/text
json         | String      | [String/, String/text, JsonNode/json]  ---> exception: suggest adding content type to docstring
             | JsonNode    | [String/, String/text, JsonNode/json] ---> JsonNode/json
text         | JsonNode    | [String/, String/text, JsonNode/json] ---> exception: suggest registering docstring type for JsonNode/text
json         | JsonNode    | [String/, String/text, JsonNode/json] ---> JsonNode/json

             | String      | [String/, JsonNode/json, JsonNode/xml] ---> String/
text         | String      | [String/, JsonNode/json, JsonNode/xml] ---> String/ 
json         | String      | [String/, JsonNode/json, JsonNode/xml] ---> String/ 
             | JsonNode    | [String/, JsonNode/json, JsonNode/xml] ---> exception: suggest adding content type
text         | JsonNode    | [String/, JsonNode/json, JsonNode/xml] ---> exception: suggest registering docstring type for JsonNode/text
json         | JsonNode    | [String/, JsonNode/json, JsonNode/xml] ---> JsonNode/json

I think we're missing a few test cases.

[mpkorstanje]

So given:

DocStringType docStringType = lookupByContentTypeAndType(contentType, type);
if (docStringType != null) {
    return Collections.singletonList(docStringType);
}
return lookUpByType(type);

These cases are covered by lookupByContentTypeAndType because there is an exact match for content type and target type:

content type | target type | registry                               ---> doc string type
             | String      | [String/]                              ---> String/
             | String      | [String/, String/text]                 ---> String/
             | String      | [String/, String/text, String/xml]     ---> String/
             | String      | [String/, JsonNode/json]               ---> String/
             | String      | [String/, String/text, JsonNode/json]  ---> String/
             | String      | [String/, JsonNode/json, JsonNode/xml] ---> String/

text         | String      | [String/, String/text]                 ---> String/text
text         | String      | [String/, String/text, String/xml]     ---> String/text
text         | String      | [String/, String/text, JsonNode/json]  ---> String/text
json         | String      | [String/, JsonNode/json, JsonNode/xml] ---> String/json

These then are covered by lookUpByType:

content type | target type | registry                               ---> doc string type
text         | String      | [String/]                              ---> String/
text         | String      | [String/, JsonNode/json]               ---> String/
text         | String      | [String/, JsonNode/json, JsonNode/xml] ---> String/ 
json         | String      | [String/]                              ---> String/
json         | String      | [String/, String/text]                 ---> String/
json         | String      | [String/, String/text, String/xml]     ---> String/
json         | String      | [String/, JsonNode/json]               ---> String/
             | JsonNode    | [String/, JsonNode/json]               ---> JsonNode/json
             | JsonNode    | [String/, String/text, JsonNode/json]  ---> JsonNode/json
json         | JsonNode    | [String/, JsonNode/json]               ---> JsonNode/json
json         | JsonNode    | [String/, String/text, JsonNode/json]  ---> JsonNode/json
json         | JsonNode    | [String/, JsonNode/json, JsonNode/xml] ---> JsonNode/json

Then these exceptional situations exist because no converters could be found:

content type | target type | registry                               ---> doc string type
             | JsonNode    | [String/]                              ---> exception: suggest registering docstring type for JsonNode
             | JsonNode    | [String/, String/text]                 ---> exception: suggest registering docstring type for JsonNode
             | JsonNode    | [String/, String/text, String/xml]     ---> exception: suggest registering docstring type for JsonNode
text         | JsonNode    | [String/]                              ---> exception: suggest registering docstring type for JsonNode/text
text         | JsonNode    | [String/, String/text]                 ---> exception: suggest registering docstring type for JsonNode/text
text         | JsonNode    | [String/, String/text, String/xml]     ---> exception: suggest registering docstring type for JsonNode/text
text         | JsonNode    | [String/, JsonNode/json]               ---> exception: suggest registering docstring type for JsonNode/text
text         | JsonNode    | [String/, String/text, JsonNode/json]  ---> exception: suggest registering docstring type for JsonNode/text
text         | JsonNode    | [String/, JsonNode/json, JsonNode/xml] ---> exception: suggest registering docstring type for JsonNode/text
json         | JsonNode    | [String/]                              ---> exception: suggest registering docstring type for JsonNode/json
json         | JsonNode    | [String/, String/text]                 ---> exception: suggest registering docstring type for JsonNode/json
json         | JsonNode    | [String/, String/text, String/xml]     ---> exception: suggest registering docstring type for JsonNode/json

And these because multiple converters were found:

content type | target type | registry                               ---> doc string type
json         | String      | [String/, String/text, JsonNode/json]  ---> exception: suggest adding content type to docstring
             | JsonNode    | [String/, JsonNode/json, JsonNode/xml] ---> exception: suggest adding content type to docstring
            

But that is not the behaviour we observe. So something with the way the empty docstring is handled is broken.