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

SummaryJavadoc: configuration to skip check for simple @return javadoc #3925

Open
ljacqu opened this Issue Mar 5, 2017 · 5 comments

Comments

Projects
None yet
3 participants
@ljacqu
Contributor

ljacqu commented Mar 5, 2017

Originally posted in #3908, it would be interesting to have a configuration option to skip the SummaryJavadoc check for certain fragments. Specifically, I'm interested in enabling this check but for trivial getters I want to allow just having a concise @return blabla javadoc:

/**
 * @return the category the article belongs to
 */
 public Category getCategory() {
   return category;
 }

I could imagine that this can be made configurable by having a whitelisted javadoc tags config (if the javadoc just has a @return, it's fine) or maybe a regex pattern may be provided (if javadoc comment is ^@return .*, skip the check).

@ljacqu ljacqu changed the title from SummaryJavadoc: option for simple @return to SummaryJavadoc: configuration to skip check for simple @return javadoc Mar 5, 2017

@ljacqu

This comment has been minimized.

Show comment
Hide comment
@ljacqu

ljacqu Mar 5, 2017

Contributor

Per input by @romani in the other issue, please note I'm not suggesting to check whether a method is a "trivial getter" or not – I mention it because it's my use case where I have different Javadoc.

I would simply like a way to check that there is a valid Javadoc summary unless there is just an @return description.

Contributor

ljacqu commented Mar 5, 2017

Per input by @romani in the other issue, please note I'm not suggesting to check whether a method is a "trivial getter" or not – I mention it because it's my use case where I have different Javadoc.

I would simply like a way to check that there is a valid Javadoc summary unless there is just an @return description.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Mar 5, 2017

Member

@ljacqu , unfortunately it not good idea to skip if some tag is present.
Imagine that you run on real code, and you want to cleanup javadoc for critical part of it.
Javadoc that human is writing is full of problems and missed items. So you will have number of cases where even for big methods there is only @return is written.

It is more reliable to stay with "skip for getters" idea in this case.

Any other ideas ?

Member

romani commented Mar 5, 2017

@ljacqu , unfortunately it not good idea to skip if some tag is present.
Imagine that you run on real code, and you want to cleanup javadoc for critical part of it.
Javadoc that human is writing is full of problems and missed items. So you will have number of cases where even for big methods there is only @return is written.

It is more reliable to stay with "skip for getters" idea in this case.

Any other ideas ?

@ljacqu

This comment has been minimized.

Show comment
Hide comment
@ljacqu

ljacqu Mar 5, 2017

Contributor

For me this would be OK as there is another inspection that checks for the presence of all required tags, right?

But if you want to go down that road I could imagine checking that the method has no args and a non-void return type. Possibly you could also perform a length check on the method ("trivial getter" is 1 line). I fear that if this length check is also configurable things become a little overwhelming.

Do you have any thoughts on what would work?

Contributor

ljacqu commented Mar 5, 2017

For me this would be OK as there is another inspection that checks for the presence of all required tags, right?

But if you want to go down that road I could imagine checking that the method has no args and a non-void return type. Possibly you could also perform a length check on the method ("trivial getter" is 1 line). I fear that if this length check is also configurable things become a little overwhelming.

Do you have any thoughts on what would work?

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Mar 5, 2017

Member

@ljacqu Just curious. You say you want smaller javadoc for getters, but you still want full validation for setters requiring main description and parameter description? Why the difference?

Member

rnveach commented Mar 5, 2017

@ljacqu Just curious. You say you want smaller javadoc for getters, but you still want full validation for setters requiring main description and parameter description? Why the difference?

@ljacqu

This comment has been minimized.

Show comment
Hide comment
@ljacqu

ljacqu Mar 5, 2017

Contributor

@rnveach, I don't like seeing documentation like the following.

/**
 * Returns the category.
 * @return the category
 */
public Category getCategory() {
  return category;
}

Besides not telling me anything I don't see in the code, it also contains the same information twice. So to improve this one can either omit the javadoc or write something like the example in my first post (keeping only the @return whose text has at least some minimal additional context).

Your question is why I don't handle setters similarly. If Javadoc just states the method signature ("Gets the foo", "Sets the bar") I won't add it. I guess I'm left with more situations where I have a getter without a setter counterpart—e.g. on interfaces or when some immutable "data object" is returned—where I'm interested in giving a little context. In all honesty, I've never noticed until now that this is inconsistent.

In light of this, maybe would it be possible and more useful to offer a configurable method line threshold like minLineCount in the JavadocMethod check? I assume this wouldn't be difficult to implement and it would not be tied to a very specific use case. For such an option, the question arises whether the check should be omitted on methods smaller than minLineCount, or if the check should still fail when there is a summary without an ending period.


Edit: combining this with the above idea of singling out getters, another approach could be to skip the check for "trivial accessors." That would be non-void no-args methods (getters) and void one-arg methods (setters). For both you could also check whether the method is 1 line long (not sure if this is a good or bad idea).

I'm really enjoying this conversation. I'm not trying to get a feature for my super specific use case, but I'm certain there is some option that would suit many users. It's probably just a longer process to find out what covers the most cases with the least surprises. :)

Contributor

ljacqu commented Mar 5, 2017

@rnveach, I don't like seeing documentation like the following.

/**
 * Returns the category.
 * @return the category
 */
public Category getCategory() {
  return category;
}

Besides not telling me anything I don't see in the code, it also contains the same information twice. So to improve this one can either omit the javadoc or write something like the example in my first post (keeping only the @return whose text has at least some minimal additional context).

Your question is why I don't handle setters similarly. If Javadoc just states the method signature ("Gets the foo", "Sets the bar") I won't add it. I guess I'm left with more situations where I have a getter without a setter counterpart—e.g. on interfaces or when some immutable "data object" is returned—where I'm interested in giving a little context. In all honesty, I've never noticed until now that this is inconsistent.

In light of this, maybe would it be possible and more useful to offer a configurable method line threshold like minLineCount in the JavadocMethod check? I assume this wouldn't be difficult to implement and it would not be tied to a very specific use case. For such an option, the question arises whether the check should be omitted on methods smaller than minLineCount, or if the check should still fail when there is a summary without an ending period.


Edit: combining this with the above idea of singling out getters, another approach could be to skip the check for "trivial accessors." That would be non-void no-args methods (getters) and void one-arg methods (setters). For both you could also check whether the method is 1 line long (not sure if this is a good or bad idea).

I'm really enjoying this conversation. I'm not trying to get a feature for my super specific use case, but I'm certain there is some option that would suit many users. It's probably just a longer process to find out what covers the most cases with the least surprises. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment