Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Javadoc for constructors missing "description" on parameters? #1463

Closed
loosebazooka opened this issue Jan 17, 2023 · 9 comments · Fixed by #1481
Closed

Javadoc for constructors missing "description" on parameters? #1463

loosebazooka opened this issue Jan 17, 2023 · 9 comments · Fixed by #1481

Comments

@loosebazooka
Copy link

loosebazooka commented Jan 17, 2023

I'm using the jsonschema2dataclass gradle plugin (issue clone from jsonschema2dataclass/js2d-gradle#604) but I think this issue is coming from the core library here.

for data like:

...
                "hash": {
                    "description": "Specifies the hash algorithm and value for the content",
                    "type": "object",
                    "properties": {
                        "algorithm": {
                            "description": "The hashing function used to compute the hash value",
                            "type": "string",
                            "enum": [ "sha256" ]
                        },
                        "value": {
                            "description": "The hash value for the content",
                            "type": "string"
                        }
                    },
                    "required": [ "algorithm", "value" ]
                }
...

a constructor is created like:

     /**
      *
      * @param value
      * @param algorithm
      */
     public Hash(Hash.Algorithm algorithm, String value) {
         super();
         this.algorithm = algorithm;
         this.value = value;
     }

Would it make sense to inject the description into the generated javadoc here so we don't get javadoc generation warnings (happens with Temurin Java11) like:

/home/.../Hash.java:44: warning: no description for @param
     * @param value
       ^

I was hoping the javadoc could include descriptions from the json spec itself and be generated something like:

     /**
      *
      * @param value      the hash value for the content
      * @param algorithm  the hashing function used to compute the hash value
      */
     public Hash(Hash.Algorithm algorithm, String value) {
         super();
         this.algorithm = algorithm;
         this.value = value;
     }
@unkish
Copy link
Collaborator

unkish commented Feb 4, 2023

It looks like adding descriptions to constructor's javadoc @param could be added fairly easily.
It would probably need to support line splitting similar to:

if(StringUtils.isNotBlank(descriptionText)) {
String[] lines = node.asText().split("/\r?\n/");
for(String line : lines) {
javadoc.append(line);
}
}


The title and description keywords must be strings. A “title” will preferably be short, whereas a “description” will provide a more lengthy explanation about the purpose of the data described by the schema.

  • does current solution (or if requested change would be added) make sense in case no description provided ? Should generator still add @param without any text which would upset compiler ?

@loosebazooka
Copy link
Author

my 2c:

should it only contain value from description, title or both ? From json schema reference

They're probably redundant, so maybe some order? ->

  1. description if available
  2. title if available

does current solution (or if requested change would be added) make sense in case no description provided ? Should generator still add @param without any text which would upset compiler ?

I think if the schema itself doesn't provide a description, then that's a problem with the schema, not this library. Although I could see a solution where it's just the param again to not trigger some javadoc error?

@param value value

@unkish
Copy link
Collaborator

unkish commented Feb 6, 2023

Sounds reasonable.

@joelittlejohn if you would happen to have a spare minute at some point, would be happy to hear your thoughts on given topic.

@eirnym
Copy link

eirnym commented Feb 6, 2023

I'd put following as a javadoc. Presented dot after title and description is placed if a part doesn't have it.

/**
@param title. description. 
*/

@joelittlejohn
Copy link
Owner

My 2c: the javadoc warning is there for a reason, because I think adding param on its own, with no additional string, is completely useless. I can't think of any benefit that it provides to just have @param value. There's also no benefit to having @param value value. The javadoc tool understands what parameters exist, even if they're not added to the javadoc.

I like what @eirnym proposed, where each part is optional of course, so this kind of thing:

/**
 * @param {title. }?{description.}? 
 */

@unkish
Copy link
Collaborator

unkish commented Feb 7, 2023

because I think adding param on its own, with no additional string, is completely useless

Wouldn't that imply that @param itself should be also optional ? Eg.

/**
 * {@param {title. }?{description.}?}?
 */

Let's consider simple "artificial" schema:

{
  "type": "object",
  "properties": {
    "type": {
      "type": "string",
      "title": "a title",
      "description": "Type lorem ipsum"
    },
    "id": {
      "type": "integer"
    }
  }
}

What would be the expected output in case includeConstructors and includeAllPropertiesConstructor are used as configuration options:

    /**
     * 
     * @param id
     *     
     * @param type
     *     a type. Type lorem ipsum
     */
    public Example(String type, Integer id) {
        super();
        this.type = type;
        this.id = id;
    }

or

    /**
     * 
     * @param type
     *     a type. Type lorem ipsum
     */
    public Example(String type, Integer id) {
        super();
        this.type = type;
        this.id = id;
    }

@joelittlejohn
Copy link
Owner

Yes, param itself would be optional.

I think from your example the second output would be better. We only include params where we can provide some useful info. Do you find the omission of id in the second example to be odd?

@unkish
Copy link
Collaborator

unkish commented Feb 7, 2023

Saying that it looks a bit odd indeed, I have to admit that I have no strong opinion on whether it's something that should be avoided at any cost.

@loosebazooka
Copy link
Author

thanks y'all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants