search

FasterXML / jackson-databind

General data-binding package for Jackson (2.x): works on streaming API (core) implementation(s)
Apache License 2.0
3.49k stars 1.37k forks source link

`nonPublicPartOfAPI` for `MyTokenizer` reported by revapi #4613

Open sbernard31 opened 3 days ago

sbernard31 commented 3 days ago

I'm using revapi, to check I respect semantic versioning in my project.

But it also checks some code smell about API. When building my project, it report something wrong with jackson (which is one of my dependencies)

Report : 

{
  "ignore": true,
  "code": "java.class.nonPublicPartOfAPI",
  "new": "class com.fasterxml.jackson.databind.type.TypeParser.MyTokenizer",
  "justification": "ADD YOUR EXPLANATION FOR THE NECESSITY OF THIS CHANGE"
   "classSimpleName": "MyTokenizer",
  "exampleUseChainInNewApi": "com.fasterxml.jackson.databind.type.TypeParser.MyTokenizer is used as parameter in method java.lang.IllegalArgumentException com.fasterxml.jackson.databind.type.TypeParser::_problem(com.fasterxml.jackson.databind.type.TypeParser.MyTokenizer, java.lang.String) <- field com.fasterxml.jackson.databind.type.TypeFactory._parser has the type com.fasterxml.jackson.databind.type.TypeParser <- com.fasterxml.jackson.databind.type.TypeFactory is used as parameter in method com.fasterxml.jackson.databind.JavaType com.fasterxml.jackson.databind.util.Converter<IN, OUT>::getInputType(com.fasterxml.jackson.databind.type.TypeFactory) <- com.fasterxml.jackson.databind.util.Converter<IN, OUT> is returned from method com.fasterxml.jackson.databind.util.Converter<java.lang.Object, java.lang.Object> com.fasterxml.jackson.databind.DatabindContext::converterInstance(com.fasterxml.jackson.databind.introspect.Annotated, java.lang.Object) throws com.fasterxml.jackson.databind.JsonMappingException @ com.fasterxml.jackson.databind.SerializerProvider <- com.fasterxml.jackson.databind.SerializerProvider is used as parameter in method com.fasterxml.jackson.annotation.JsonFormat.Value com.fasterxml.jackson.databind.ser.std.StdSerializer<T>::findFormatOverrides(com.fasterxml.jackson.databind.SerializerProvider, com.fasterxml.jackson.databind.BeanProperty, java.lang.Class<?>) @ com.fasterxml.jackson.databind.ser.ContainerSerializer<T> <- com.fasterxml.jackson.databind.ser.ContainerSerializer<T> is returned from method com.fasterxml.jackson.databind.ser.ContainerSerializer<?> com.fasterxml.jackson.databind.ser.ContainerSerializer<T>::withValueTypeSerializer(com.fasterxml.jackson.databind.jsontype.TypeSerializer) @ com.fasterxml.jackson.databind.ser.std.MapSerializer <- field com.fasterxml.jackson.databind.ser.AnyGetterWriter._mapSerializer has the type com.fasterxml.jackson.databind.ser.std.MapSerializer <- com.fasterxml.jackson.databind.ser.AnyGetterWriter is used as parameter in method void com.fasterxml.jackson.databind.ser.BeanSerializerBuilder::setAnyGetter(com.fasterxml.jackson.databind.ser.AnyGetterWriter) <- com.fasterxml.jackson.databind.ser.BeanSerializerBuilder is used as parameter in method com.fasterxml.jackson.databind.ser.BeanSerializerBuilder com.fasterxml.jackson.databind.ser.BeanSerializerModifier::updateBuilder(com.fasterxml.jackson.databind.SerializationConfig, com.fasterxml.jackson.databind.BeanDescription, com.fasterxml.jackson.databind.ser.BeanSerializerBuilder) <- com.fasterxml.jackson.databind.ser.BeanSerializerModifier is used as parameter in method com.fasterxml.jackson.databind.ser.SerializerFactory com.fasterxml.jackson.databind.ser.SerializerFactory::withSerializerModifier(com.fasterxml.jackson.databind.ser.BeanSerializerModifier) <- field com.fasterxml.jackson.databind.ObjectWriter._serializerFactory has the type com.fasterxml.jackson.databind.ser.SerializerFactory <- com.fasterxml.jackson.databind.ObjectWriter is returned from method com.fasterxml.jackson.databind.ObjectWriter com.fasterxml.jackson.databind.ObjectMapper::writerWithType(com.fasterxml.jackson.databind.JavaType) <- com.fasterxml.jackson.databind.ObjectMapper is used as parameter in method void org.eclipse.leshan.senml.json.jackson.SenMLJsonJacksonEncoderDecoder::<init>(org.eclipse.leshan.core.util.json.JacksonJsonSerDes<org.eclipse.leshan.senml.SenMLRecord>, com.fasterxml.jackson.databind.ObjectMapper) (method void org.eclipse.leshan.senml.json.jackson.SenMLJsonJacksonEncoderDecoder::<init>(org.eclipse.leshan.core.util.json.JacksonJsonSerDes<org.eclipse.leshan.senml.SenMLRecord>, com.fasterxml.jackson.databind.ObjectMapper) is part of the API)",
  "package": "com.fasterxml.jackson.databind.type",
  "classQualifiedName": "com.fasterxml.jackson.databind.type.TypeParser.MyTokenizer",
  "elementKind": "class",
  "newArchive": "com.fasterxml.jackson.core:jackson-databind:jar:2.17.1",
  "newArchiveRole": "supplementary",
},

nonPublicPartOfAPI is described here.

I copy/paste the exampleUseChainInNewApi here because this is the most important part to understand the issue :

"com.fasterxml.jackson.databind.type.TypeParser.MyTokenizer is used as parameter in method java.lang.IllegalArgumentException com.fasterxml.jackson.databind.type.TypeParser::_problem(com.fasterxml.jackson.databind.type.TypeParser.MyTokenizer, java.lang.String) <- field com.fasterxml.jackson.databind.type.TypeFactory._parser has the type com.fasterxml.jackson.databind.type.TypeParser <- com.fasterxml.jackson.databind.type.TypeFactory is used as parameter in method com.fasterxml.jackson.databind.JavaType com.fasterxml.jackson.databind.util.Converter<IN, OUT>::getInputType(com.fasterxml.jackson.databind.type.TypeFactory) <- com.fasterxml.jackson.databind.util.Converter<IN, OUT> is returned from method com.fasterxml.jackson.databind.util.Converter<java.lang.Object, java.lang.Object> com.fasterxml.jackson.databind.DatabindContext::converterInstance(com.fasterxml.jackson.databind.introspect.Annotated, java.lang.Object) throws com.fasterxml.jackson.databind.JsonMappingException @ com.fasterxml.jackson.databind.SerializerProvider <- com.fasterxml.jackson.databind.SerializerProvider is used as parameter in method com.fasterxml.jackson.annotation.JsonFormat.Value com.fasterxml.jackson.databind.ser.std.StdSerializer::findFormatOverrides(com.fasterxml.jackson.databind.SerializerProvider, com.fasterxml.jackson.databind.BeanProperty, java.lang.Class<?>) @ com.fasterxml.jackson.databind.ser.ContainerSerializer <- com.fasterxml.jackson.databind.ser.ContainerSerializer is returned from method com.fasterxml.jackson.databind.ser.ContainerSerializer<?> com.fasterxml.jackson.databind.ser.ContainerSerializer::withValueTypeSerializer(com.fasterxml.jackson.databind.jsontype.TypeSerializer) @ com.fasterxml.jackson.databind.ser.std.MapSerializer <- field com.fasterxml.jackson.databind.ser.AnyGetterWriter._mapSerializer has the type com.fasterxml.jackson.databind.ser.std.MapSerializer <- com.fasterxml.jackson.databind.ser.AnyGetterWriter is used as parameter in method void com.fasterxml.jackson.databind.ser.BeanSerializerBuilder::setAnyGetter(com.fasterxml.jackson.databind.ser.AnyGetterWriter) <- com.fasterxml.jackson.databind.ser.BeanSerializerBuilder is used as parameter in method com.fasterxml.jackson.databind.ser.BeanSerializerBuilder com.fasterxml.jackson.databind.ser.BeanSerializerModifier::updateBuilder(com.fasterxml.jackson.databind.SerializationConfig, com.fasterxml.jackson.databind.BeanDescription, com.fasterxml.jackson.databind.ser.BeanSerializerBuilder) <- com.fasterxml.jackson.databind.ser.BeanSerializerModifier is used as parameter in method com.fasterxml.jackson.databind.ser.SerializerFactory com.fasterxml.jackson.databind.ser.SerializerFactory::withSerializerModifier(com.fasterxml.jackson.databind.ser.BeanSerializerModifier) <- field com.fasterxml.jackson.databind.ObjectWriter._serializerFactory has the type com.fasterxml.jackson.databind.ser.SerializerFactory <- com.fasterxml.jackson.databind.ObjectWriter is returned from method com.fasterxml.jackson.databind.ObjectWriter com.fasterxml.jackson.databind.ObjectMapper::writerWithType(com.fasterxml.jackson.databind.JavaType) <- com.fasterxml.jackson.databind.ObjectMapper is used as parameter in method void org.eclipse.leshan.senml.json.jackson.SenMLJsonJacksonEncoderDecoder::(org.eclipse.leshan.core.util.json.JacksonJsonSerDes, com.fasterxml.jackson.databind.ObjectMapper) (method void org.eclipse.leshan.senml.json.jackson.SenMLJsonJacksonEncoderDecoder::(org.eclipse.leshan.core.util.json.JacksonJsonSerDes, com.fasterxml.jackson.databind.ObjectMapper) is part of the API)"

Solution ?

I think MyTokenizer should be protected instead of package ?

(issue found with 2.15.3 and also tested with 2.17.1)

pjfanning commented 3 days ago

Makes sense - if protected JavaType parseType(MyTokenizer tokens, int nestingAllowed) is protected and we allow users to subclass com.fasterxml.jackson.databind.type.TypeParser then MyTokenizer needs to be public or protected. There are a number of other protected methods in TypeParser that take a MyTokenizer instance as input too.

I would prefer if a lot more of jackson-databind was private. Making so many methods protected has consequences. It requires that a lot of other code needs to be public or protected.

In this case, it could be viewed as breaking binary compatibility but I think the MyTokenizer should be made private or left as package private and the API methods that take MyTokenizer inputs could be changed to have StringTokenizer as the param type instead. MyTokenizer extends StringTokenizer.

cowtowncoder commented 3 days ago

Ok. Here's my perspective: I think these kinds of items are almost waste of time. I see rather modest benefits from possible changes; moderate chance of regression; and a high chance of bike shedding.

As a result I have no interest in changes for 2.x BUT if others are more interested in making changes, am perfectly fine with changes to upcoming 3.0 (branch master). So PR(s) against master may be accepted.