Spring Framework Version：5.3.1

When I was writing the formatter special column. I need to write a demo about the detailed use of FormatterRegistry. You know Formattingconversionservice can be considered as its unique implementation.

In the process of using Formattingconversionservice, I found a bug in it by accident. Here is my demo source code:

@Data
@NoArgsConstructor
@AllArgsConstructor
public class Person {
    private Integer id;
    private String name;
}

public class Demo {

    // Customize a parser: String -> Integer
    private static class IntegerParser implements Parser<Integer> {
        @Override
        public Integer parse(String text, Locale locale) throws ParseException {
            return NumberUtils.parseNumber(text, Integer.class);
        }
    }


    @Test
    public void test4() {
        FormattingConversionService formattingConversionService = new FormattingConversionService();
        FormatterRegistry formatterRegistry = formattingConversionService;
        ConversionService conversionService = formattingConversionService;

        // 注册格式化器
        formatterRegistry.addFormatterForFieldType(Person.class, null, new IntegerParser());
        formatterRegistry.addConverter(new Converter<Integer, Person>() {
            @Override
            public Person convert(Integer source) {
                return new Person(source, "YourBatman");
            }
        });

        System.out.println(conversionService.canConvert(String.class, Person.class));
        System.out.println(conversionService.convert("1", Person.class));
    }

}

Run the program,throw an NullPointerException exception:

java.lang.NullPointerException
    at org.springframework.format.support.FormattingConversionService$PrinterConverter.resolvePrinterObjectType(FormattingConversionService.java:179)
    at org.springframework.format.support.FormattingConversionService$PrinterConverter.<init>(FormattingConversionService.java:155)
    at org.springframework.format.support.FormattingConversionService.addFormatterForFieldType(FormattingConversionService.java:95)
    at cn.yourbatman.formatter.Demo.test4(Demo.java:86)
        ...

The location of the exception is obvious. In FormattingConversionService:

FormattingConversionService:

    @Override
    public void addFormatterForFieldType(Class<?> fieldType, Printer<?> printer, Parser<?> parser) {
        addConverter(new PrinterConverter(fieldType, printer, this));
        addConverter(new ParserConverter(fieldType, parser, this));
    }

        private static class PrinterConverter implements GenericConverter {
                ...
        public PrinterConverter(Class<?> fieldType, Printer<?> printer, ConversionService conversionService) {
            this.fieldType = fieldType;
                        // Where the NullPointerException exception occurred
            this.printerObjectType = TypeDescriptor.valueOf(resolvePrinterObjectType(printer));
            this.printer = printer;
            this.conversionService = conversionService;
        }
                ...
        }

If I modify my demo just like this:

    @Test
    public void test4() {
        ...

        // 注册格式化器
       // just use new IntegerPrinter() instead of null for this second param
        formatterRegistry.addFormatterForFieldType(Person.class, new IntegerPrinter(), new IntegerParser());
        ...
    }

Run the program again.Normal output:

true
Person(id=1, name=YourBatman)

I think the implementation of FormattingConversionService is not rigorous. I just want to register a Parser for specified type, but Printer doesn't need. Even if I specify the second param null must also work natural.There's no reason to tie them together.

So I think it's a bug.I have the following suggestions to fix it:

FormattingConversionService:

    @Override
    public void addFormatterForFieldType(Class<?> fieldType, Printer<?> printer, Parser<?> parser) {
                if (printer != null) {
                    addConverter(new PrinterConverter(fieldType, printer, this));
                }
        if (parser != null) {
                    addConverter(new ParserConverter(fieldType, parser, this));
                }
    }

If printer/parser is null,There's no point in registering PrinterConverter/ParserConverter in the ConverterRegistry.In FormattingConversionService all registered places need to be judge null.

In addition, at the API level，I suggest adding the following two APIs:

FormatterRegistry:

    void addPrinter(Class<?> fieldType, Printer<?> printer);
    void addParser(Class<?> fieldType, Parser<?> parser);

Thanks for reading my advice.

Comment From: snicoll

The NullPointerException is legit. The printer argument is not marked as @Nullable so it must be provided. Not providing it is a user error so I am not sure why you're reporting a bug for this.