@octylFractal @pbludov To me, this is not clear from the original issue. I was under the impression we were suppressing a false violation. But are we expanding Check to catch more things? It seems issue is not clear in definition of what to do and it should closely match the PR. The issue and title is what is used to put what we are changing in the release notes.

To me, check is only checking the summary sentence and that another check should be made to identify tags (inline or not) that are used in the wrong place. We need to keep checks from exploding with too much functionality. We have already seen this in other checks that it becomes a burden and they need to be rewritten.

To me, if Javadoc tool accepts inline return as a summary, why should we have an option to forbid it?

That was my feeling as well, but I figured that @pbludov knew better that I did.

I agree with @rnveach. The commit message should be changed to something like "SummaryJavadoc shouldn't report javadocs with inline return tag"

Please remove new messages, validation should stay be done by same messages.

Please remove new messages, validation should stay be done by same messages.

We need some more examples.
What about if there is text before the {@return}.
What about if there is text after the {@return}.
What about if the content of {@return} spans multiple lines?
What about if a void method uses the {@return}? Does the javadoc flag an error on this? Do we?

I will check the first 3, for the last one javadoc produces a hard error:

SummaryJavadocNewInlineReturn.java:3: error: invalid use of @return
     * {@return a foo that is {@code 0}}
       ^

so I think checking for that in checkstyle is redundant.

What about if there is text before the {@return}.

javadoc produces a warning. Should I add a warning to checkstyle as well? Should I add a test that this is accepted by checkstyle?

SummaryJavadocNewInlineReturn.java:5: warning: {@return} not at beginning of description
     * Starting text, {@return a foo that is {@code 0}} extra text afterwards.
                      ^

What about if there is text after the {@return}.

This is normal and allowed, I will add a test that it does not produce a violation.

What about if the content of {@return} spans multiple lines?

Using a normal newline, this produces output like the following:

Using <br>, this produces output like the following:

I do not know if checkstyle should produce a message for either of these. Please inform me as to what is appropriate.

for the last one javadoc produces a hard error:

Is there any issue adding this? We specifically have another where it also produces an error by the javadoc.
#10772 (comment)

I will add a test

Ok.

javadoc produces a warning. Should I add a warning to checkstyle as well?

@romani please comment on scope of issue.

Please inform me as to what is appropriate.

Please update your post to include the raw code of the javadoc, so I can correlate that to the javadoc printouts you provided.

We should not do what is already done by javadoc

Please update your post to include the raw code of the javadoc, so I can correlate that to the javadoc printouts you provided.

/**
 */
public class SummaryJavadocNewInlineReturn {
    /**
     * {@return a foo that is {@code 0}<br>
     * this takes a lot<br>
     * of lines to explain<br>
     * so it is quite long}
     */
    public int foo() {
    }
}

Remove the <br>s for the first output.

I do not know if checkstyle should produce a message for either of these.

As far as I can tell, the javadoc accepts everything as the summary. Checkstyle should do the same.

Were you asking for advice on something else?

No, thank you.

This is all done now.

Is this violation because the return is completely empty?

Yes, ergo there is no summary.

It might good to add reason to comment
// violation below, because the return is completely empty
Or any other way to not conflict with actual message.

Better to be very clear in tests.

Done.

Sorry if I missed it, but can you point to where we analyzed how the javadoc tool behaves with this or the class one?

I didn't analyze it prior, but just now I tested:

public class SummaryJavadocNewInlineReturn {
    /** 
     * {@return invalid}
     */
    public final int foo = 0;
}

$ javadoc SummaryJavadocNewInlineReturn.java
Loading source file SummaryJavadocNewInlineReturn.java...
Constructing Javadoc information...
Building index for all the packages and classes...
Standard Doclet version 16.0.2+7
Building tree for all the packages and classes...
Generating ./SummaryJavadocNewInlineReturn.html...
SummaryJavadocNewInlineReturn.java:3: error: invalid use of @return
     * {@return invalid}
       ^
[other messages removed]

So javadoc produces a hard error when this is placed on a non-method class member. Therefore, we don't check it in checkstyle.

@octylFractal , please repost your comment to issue.

PR in highly unlikely to be reviewed later on, but issues are good source of our decisions to review through time. Javadoc tool might change behavior and will be puzzleed why we did/didn't do something.

Done.