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: no violation on absent summary and on javadoc where '.' is used not as end of sentense #3907

Closed
romani opened this Issue Mar 3, 2017 · 15 comments

Comments

Projects
None yet
3 participants
@romani
Member

romani commented Mar 3, 2017

taken from #3014

$ cat config.xml 
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">
<module name="Checker">
    <module name="TreeWalker">
        <module name="SummaryJavadoc">
        </module>
    </module>
</module>

$ cat Test.java 
public class Test {

    /**
     * JAXB 1.0 only default validation event handler
     */
    public static final byte NUL = 0;

    /**
     * @throws Exception if an error occurs.
     */
    public void foo1() throws Exception {

    }

    /**
      * @return 1.
      */
    public int foo2(){
        return 1;
    }

    /**
      * <a href="mailto:vlad@htmlbook.ru"></a>
      */
    public void foo3() {

    }
    /**
     *  A {@code Foo.  Foo}
     */
    public void foo(){

    }

}

$ java -jar checkstyle-7.6-all.jar -c config.xml Test.java 
Starting audit...
Audit done.

Expected: violations for each javadocs

@sagar-shah94

This comment has been minimized.

Show comment
Hide comment
@sagar-shah94

sagar-shah94 Mar 4, 2017

Contributor

I am on it

Contributor

sagar-shah94 commented Mar 4, 2017

I am on it

@sagar-shah94

This comment has been minimized.

Show comment
Hide comment
@sagar-shah94

sagar-shah94 Mar 4, 2017

Contributor

Sir How following java docs violates condition?
/**
* @return 1.
*/

/**
* @throws Exception if an error occurs.
*/

i think no violation message should be shown for these java docs
@romani

Contributor

sagar-shah94 commented Mar 4, 2017

Sir How following java docs violates condition?
/**
* @return 1.
*/

/**
* @throws Exception if an error occurs.
*/

i think no violation message should be shown for these java docs
@romani

@sagar-shah94

This comment has been minimized.

Show comment
Hide comment
@sagar-shah94

sagar-shah94 Mar 4, 2017

Contributor

Do i have to consider this case?is it valid or invalid?
For java summary docs we have to consider only first line ..Am i right?

/**
 * This is test
 * Valid or invalid.
*/
Contributor

sagar-shah94 commented Mar 4, 2017

Do i have to consider this case?is it valid or invalid?
For java summary docs we have to consider only first line ..Am i right?

/**
 * This is test
 * Valid or invalid.
*/
@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Mar 4, 2017

Member

think no violation message should be shown for these java docs

There are first sentence at all, so it is a violation. Javadoc Tagged lines are not a first sentence. They does not belong to description.

Member

romani commented Mar 4, 2017

think no violation message should be shown for these java docs

There are first sentence at all, so it is a violation. Javadoc Tagged lines are not a first sentence. They does not belong to description.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Mar 4, 2017

Member

Do i have to consider this case?is it valid or invalid?

Valid. Just first sentence is wrapped to few lines.

Member

romani commented Mar 4, 2017

Do i have to consider this case?is it valid or invalid?

Valid. Just first sentence is wrapped to few lines.

@sagar-shah94

This comment has been minimized.

Show comment
Hide comment
@sagar-shah94

sagar-shah94 Mar 4, 2017

Contributor

ok sir thanx

Contributor

sagar-shah94 commented Mar 4, 2017

ok sir thanx

@sagar-shah94

This comment has been minimized.

Show comment
Hide comment
@sagar-shah94

sagar-shah94 Mar 16, 2017

Contributor

@romani @rnveach should i create separate class to check period at the end of sentence or i should add method in summaryjavadoccheck.java we can throw only one exception at a time so it is better to create separate class for this.
What do you say?

I tried creating separate class it is working fine and all test cases i have tried it is working properly i have written junit test cases also for the same.

Contributor

sagar-shah94 commented Mar 16, 2017

@romani @rnveach should i create separate class to check period at the end of sentence or i should add method in summaryjavadoccheck.java we can throw only one exception at a time so it is better to create separate class for this.
What do you say?

I tried creating separate class it is working fine and all test cases i have tried it is working properly i have written junit test cases also for the same.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Mar 16, 2017

Member

We are expecting a fix to SummaryJavadocCheck.
Issue will have special label if new check is required called new module.

we can throw only one exception

Why are we throwing exceptions? Exceptions should be last resort when something out of our hands has gone wrong (file not found, etc.).

Member

rnveach commented Mar 16, 2017

We are expecting a fix to SummaryJavadocCheck.
Issue will have special label if new check is required called new module.

we can throw only one exception

Why are we throwing exceptions? Exceptions should be last resort when something out of our hands has gone wrong (file not found, etc.).

@sagar-shah94

This comment has been minimized.

Show comment
Hide comment
@sagar-shah94

sagar-shah94 Mar 17, 2017

Contributor

@rnveach @romani
i think at this line it violates a condition.this javadoc should not be placed in InputCorrectSummaryJavaDoc.java file .
check below link that points to javadoc i am talking about.
https://github.com/checkstyle/checkstyle/blob/master/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/InputCorrectSummaryJavaDoc.java#L19

Contributor

sagar-shah94 commented Mar 17, 2017

@rnveach @romani
i think at this line it violates a condition.this javadoc should not be placed in InputCorrectSummaryJavaDoc.java file .
check below link that points to javadoc i am talking about.
https://github.com/checkstyle/checkstyle/blob/master/src/test/resources/com/puppycrawl/tools/checkstyle/checks/javadoc/InputCorrectSummaryJavaDoc.java#L19

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Mar 17, 2017

Member

this javadoc should not be placed in InputCorrectSummaryJavaDoc.java file .

it is the same as example's foo1, so yes, it shouldn't be in correct file.

Member

rnveach commented Mar 17, 2017

this javadoc should not be placed in InputCorrectSummaryJavaDoc.java file .

it is the same as example's foo1, so yes, it shouldn't be in correct file.

@sagar-shah94

This comment has been minimized.

Show comment
Hide comment
@sagar-shah94

sagar-shah94 Mar 17, 2017

Contributor

Should i create issue to move that code to another file?
@rnveach

Contributor

sagar-shah94 commented Mar 17, 2017

Should i create issue to move that code to another file?
@rnveach

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Mar 17, 2017

Member

@sagar-shah94 no, it can be done in this issue as we are correcting these cases to be violations.

Member

rnveach commented Mar 17, 2017

@sagar-shah94 no, it can be done in this issue as we are correcting these cases to be violations.

sagar-shah94 pushed a commit to sagar-shah94/checkstyle that referenced this issue Mar 18, 2017

sagar-shah94 pushed a commit to sagar-shah94/checkstyle that referenced this issue Mar 19, 2017

sagar-shah94 pushed a commit to sagar-shah94/checkstyle that referenced this issue Mar 19, 2017

@sagar-shah94

This comment has been minimized.

Show comment
Hide comment
@sagar-shah94

sagar-shah94 Mar 20, 2017

Contributor

@rnveach @romani
in following case period should be after A or after A {@code Foo. Foo}?

    /**
     *  A {@code Foo.  Foo}
     */

in following case do we have to put period after closing anchor tag or summary java doc is missing?

 /**
      *` <a href="mailto:vlad@htmlbook.ru"></a>`
      */
Contributor

sagar-shah94 commented Mar 20, 2017

@rnveach @romani
in following case period should be after A or after A {@code Foo. Foo}?

    /**
     *  A {@code Foo.  Foo}
     */

in following case do we have to put period after closing anchor tag or summary java doc is missing?

 /**
      *` <a href="mailto:vlad@htmlbook.ru"></a>`
      */
@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Mar 21, 2017

Member

in following case period should be after A or after A {@code Foo. Foo}?

we do not know fore sure, only user know,
but in case no period is defined at all we we have to demand at the most end - after {@code Foo. Foo}.

in following case do we have to put period after closing anchor tag or summary java doc is missing?

at the most end , after closing anchor tag.

Member

romani commented Mar 21, 2017

in following case period should be after A or after A {@code Foo. Foo}?

we do not know fore sure, only user know,
but in case no period is defined at all we we have to demand at the most end - after {@code Foo. Foo}.

in following case do we have to put period after closing anchor tag or summary java doc is missing?

at the most end , after closing anchor tag.

sagar-shah94 pushed a commit to sagar-shah94/checkstyle that referenced this issue Mar 21, 2017

sagar-shah94 pushed a commit to sagar-shah94/checkstyle that referenced this issue Mar 21, 2017

sagar-shah94 pushed a commit to sagar-shah94/checkstyle that referenced this issue Mar 21, 2017

sagar-shah94 pushed a commit to sagar-shah94/checkstyle that referenced this issue Mar 21, 2017

sagar-shah94 pushed a commit to sagar-shah94/checkstyle that referenced this issue Mar 21, 2017

sagar-shah94 pushed a commit to sagar-shah94/checkstyle that referenced this issue Mar 21, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Apr 15, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Apr 16, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Apr 18, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Apr 19, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Apr 19, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Apr 19, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Apr 19, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Apr 20, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Apr 20, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Apr 20, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Apr 20, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Apr 30, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Apr 30, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Apr 30, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue May 1, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue May 1, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue May 1, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue May 1, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue May 1, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 3, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 10, 2017

romani added a commit that referenced this issue Jun 12, 2017

@romani romani added the bug label Jun 12, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 17, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 24, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 24, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 24, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 24, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 24, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 24, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 24, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 24, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 24, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 24, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 24, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 24, 2017

@romani romani changed the title from SummaryJavadoc: no violation on javadoc where '.' is used not as end of sentense to SummaryJavadoc: no violation on absent summary and on javadoc where '.' is used not as end of sentense Jun 25, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 25, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 25, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jun 25, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jul 4, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jul 4, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jul 4, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jul 9, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jul 9, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jul 9, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jul 16, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jul 16, 2017

sagar-shah94 added a commit to sagar-shah94/checkstyle that referenced this issue Jul 21, 2017

rnveach added a commit that referenced this issue Jul 22, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jul 22, 2017

Member

Fix was merged, new issues will be created on code style

Member

rnveach commented Jul 22, 2017

Fix was merged, new issues will be created on code style

@rnveach rnveach closed this Jul 22, 2017

@rnveach rnveach added this to the 8.1 milestone Jul 22, 2017

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