Issue #5148: Add MissingJavadocType to google_checks.xml by nrmancuso · Pull Request #8987 · checkstyle/checkstyle

nrmancuso · 2020-11-13T18:24:15Z

Issue #5148: Add MissingJavadocType to google_checks.xml

ATTENTION quotes from style guide:

From https://google.github.io/styleguide/javaguide.html#s1.1-terminology:
"The term class is used inclusively to mean an "ordinary" class, enum class, interface or annotation type (@interface)."

From https://google.github.io/styleguide/javaguide.html#s7.3-javadoc-where-required:
"At the minimum, Javadoc is present for every public class, and every public or protected member of such a class, with a few exceptions noted below."

This PR is a continuation of #8105.

Diff Regression projects: https://gist.githubusercontent.com/nmancus1/32ddbff9e69986ac2fcbbeff7def2f80/raw/8480445657f1d215e683dbb6b14c1577bbd44e6b/projects-to-test-on.properties

Diff Regression config: https://gist.githubusercontent.com/nmancus1/f0a81b257e7f37f23ea4abc2b17ece6f/raw/a5231cef69f0ae909ae0274c2f189b74899c6be8/base.xml

Diff Regression patch config: https://gist.githubusercontent.com/nmancus1/a84775a4bc42a6a3a60899f355d3cf15/raw/76e1c9c783c7160f58c7475ebdded06ceec81f93/patch.xml

nrmancuso · 2020-11-13T18:25:00Z

Github, generate report

github-actions · 2020-11-13T18:25:59Z

Report generation job failed on phase "make_report", step "Download files".
Link: https://github.com/checkstyle/checkstyle/actions/runs/362119974

nrmancuso · 2020-11-13T18:30:10Z

Github, generate report

github-actions · 2020-11-13T18:30:57Z

Report generation job failed on phase "make_report", step "Download files".
Link: https://github.com/checkstyle/checkstyle/actions/runs/362127265

nrmancuso · 2020-11-13T18:35:18Z

Github, generate report

github-actions · 2020-11-13T18:36:06Z

Report generation job failed on phase "make_report", step "Download files".
Link: https://github.com/checkstyle/checkstyle/actions/runs/362134982

nrmancuso · 2020-11-13T18:43:07Z

@strkkk any ideas what's going on with "generate report"?

➜  temp wget -q "https://gist.githubusercontent.com/nmancus1/3a33a5c2de95b3d04673b1381d103a34/raw/26a5bb9e0af56616091079973a85380b3c477291/projects-to-test-on.properties" -O project.properties

➜  temp ls
project.properties

➜  temp man wget | grep -B 12 -A 9 Network 
EXIT STATUS
       Wget may return one of several error codes if it encounters problems.

       0   No problems occurred.

       1   Generic error code.

       2   Parse error---for instance, when parsing command-line options, the
           .wgetrc or .netrc...

       3   File I/O error.

       4   Network failure.

       5   SSL verification failure.

       6   Username/password authentication failure.

       7   Protocol errors.

       8   Server issued an error response.

Seems to be a network error, what can we do?

strkkk · 2020-11-13T20:58:21Z

@nmancus1 no idea, may be try again

Upd: I restarted it and it failed again.
Today I have generated report for my PR #8984 and it was fine
May be it is because this PR is a draft?

github-actions · 2020-11-13T21:00:14Z

Report generation job failed on phase "make_report", step "Download files".
Link: https://github.com/checkstyle/checkstyle/actions/runs/362127265

nrmancuso · 2020-11-13T21:20:29Z

May be it is because this PR is a draft?

Marked as ready for review, I will try again.

nrmancuso · 2020-11-13T21:20:41Z

Github, generate report

github-actions · 2020-11-13T21:21:23Z

Report generation job failed on phase "make_report", step "Download files".
Link: https://github.com/checkstyle/checkstyle/actions/runs/362342980

romani · 2020-11-15T01:33:26Z

Looks like parsing of description is broken

checkstyle/.github/workflows/diff_report.yml

Line 49 in 1e9dd66

grep "^Diff Regression projects:" text | cat > temp

and we try to download not a link but title/name of link

Update: I made sure that there is space between name of link and link, not any other symbol.

romani · 2020-11-15T01:37:01Z

Github, generate report

github-actions · 2020-11-15T02:21:17Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/googlestyle-rule73_2020022056/reports/diff/index.html

romani · 2020-11-15T02:35:35Z

@nmancus1 , as you do no changes in check logic, diff does not make sense, as nothing is changed.
We need to see new violations from google style config that users will see after upgrade.
So we need two different config, without Check and with this Check. So report will show all violations from this new Check.

And we need to make sure there is no false positives in report.

nrmancuso · 2020-11-15T13:06:49Z

Github, generate report

github-actions · 2020-11-15T13:11:44Z

Report generation job failed on phase "make_report", step "Generate report".
Link: https://github.com/checkstyle/checkstyle/actions/runs/364428770

nrmancuso · 2020-11-15T13:41:37Z

Above failure caused by failure to clone mercurial openjdk repos:

➜  temp curl -Is http://hg.openjdk.java.net | head -1
HTTP/1.1 502 Bad Gateway

I will comment them out for now. I have noticed this failure several times before, should we consider switching to one of the mirrors on github?

nrmancuso · 2020-11-15T13:41:59Z

github, generate report

nrmancuso · 2020-11-16T14:38:23Z

github, generate report

github-actions · 2020-11-16T15:20:37Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/googlestyle-rule73_2020151858/reports/diff/index.html

romani · 2020-11-16T15:44:39Z

@nmancus1 , we forgot about classes inside methods
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/googlestyle-rule73_2020151858/reports/diff/jOOL/index.html#A1 - is it public class (it is in interface method) ? please recheck.
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/googlestyle-rule73_2020151858/reports/diff/Vavr/index.html#A19

please add one more test in our Input to place class in public method of class. It should not be considered as public in classes for sure.

romani · 2020-11-16T15:51:42Z

@nmancus1 , https://travis-ci.org/github/checkstyle/checkstyle/jobs/743835279#L724

nrmancuso · 2020-11-16T15:52:57Z

https://travis-ci.org/github/checkstyle/checkstyle/jobs/743835279#L724

I saw this, but it doesn't make sense, does it? I don't see those violation locally, and indentation level of those should be 4, right?

nrmancuso · 2020-11-16T17:44:01Z

classes inside methods

Since local classes are only visible within the enclosing block, this config should not apply to them. I will exclude nothing in the config to avoid this and run regression report again.

please recheck.
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/googlestyle-rule73_2020151858/reports/diff/Vavr/index.html#A19

This is correct, since all members of public interfaces are implicitly public.

nrmancuso · 2020-11-16T17:57:23Z

github, generate report

github-actions · 2020-11-16T18:35:44Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/googlestyle-rule73_2020183245/reports/diff/index.html

romani

Items:

romani

last item:

#8987 (comment)

I saw this, but it doesn't make sense, does it? I don't see those violation locally, and indentation level of those should be 4, right?

no, should be 2, as you used this class in execution by Google style (see update in assembly-run-all-jar)) and Google style have indentation as +2 in blocks.
https://google.github.io/styleguide/javaguide.html#s4.2-block-indentation
Please update

diff Report is good.
I updated issue with migration notes to suggest filter for users.

nrmancuso · 2020-11-19T13:41:08Z

see update in assembly-run-all-jar

Thanks for explaining. I meant to ask about that, now I understand.

I updated issue with migration notes to suggest filter for users.

It was good to put it in the initial post for visibility, thanks a lot.

romani · 2020-11-19T14:24:02Z

CodeQL is failed with:
Error: 1-19 13:42:24] [autobuild] [ERROR] Failed to execute goal org.codehaus.mojo:antlr-maven-plugin:2.2:generate (default) on project checkstyle: Execution default of goal org.codehaus.mojo:antlr-maven-plugin:2.2:generate failed: Plugin org.codehaus.mojo:antlr-maven-plugin:2.2 or one of its dependencies could not be resolved: Failed to collect dependencies at org.codehaus.mojo:antlr-maven-plugin:jar:2.2 -> org.apache.maven.reporting:maven-reporting-impl:jar:2.0.2 -> org.apache.maven.reporting:maven-reporting-api:jar:2.0.2: Failed to read artifact descriptor for org.apache.maven.reporting:maven-reporting-api:jar:2.0.2: Could not transfer artifact org.apache.maven.reporting:maven-reporting-api:pom:2.0.2 from/to central (https://repo.maven.apache.org/maven2): Connection reset -> [Help 1]
https://github.com/checkstyle/checkstyle/pull/8987/checks?check_run_id=1424448455#step:4:2075

restarted ...

romani

ok to merge

pbludov · 2020-11-21T05:56:08Z

github, generate site

github-actions · 2020-11-21T06:00:08Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/googlestyle-rule73_2020055915/index.html

pbludov

I'm OK to merge if CI passes.

romani · 2020-11-21T20:44:31Z

@nmancus1 , please fix https://travis-ci.org/github/checkstyle/checkstyle/jobs/744655385#L724

nrmancuso · 2020-11-21T21:10:53Z

please fix https://travis-ci.org/github/checkstyle/checkstyle/jobs/744655385#L724

Done, I should have ran that locally, apologies.

nrmancuso marked this pull request as draft November 13, 2020 18:24

nrmancuso mentioned this pull request Nov 13, 2020

Issue #5148: Add MissingJavadocType to google_checks.xml #8105

Closed

nrmancuso marked this pull request as ready for review November 13, 2020 21:19

romani requested review from pbludov, romani and strkkk November 15, 2020 01:41

romani assigned strkkk Nov 15, 2020

strkkk approved these changes Nov 15, 2020

View reviewed changes

strkkk assigned pbludov and unassigned strkkk Nov 15, 2020

nrmancuso force-pushed the googlestyle-rule73 branch 3 times, most recently from 99d388f to 6160698 Compare November 16, 2020 18:31

romani requested changes Nov 17, 2020

View reviewed changes

Comment thread ...est/chapter7javadoc/rule73wherejavadocrequired/InputMissingJavadocTypeCheckNoViolations.java Outdated

Comment thread src/main/resources/google_checks.xml

Comment thread ...checkstyle/test/chapter7javadoc/rule73wherejavadocrequired/InputMissingJavadocTypeCheck.java

romani reviewed Nov 18, 2020

View reviewed changes

Comment thread ...est/chapter7javadoc/rule73wherejavadocrequired/InputMissingJavadocTypeCheckNoViolations.java

Comment thread ...checkstyle/test/chapter7javadoc/rule73wherejavadocrequired/InputMissingJavadocTypeCheck.java

romani requested changes Nov 18, 2020

View reviewed changes

romani assigned romani and unassigned pbludov Nov 18, 2020

nrmancuso force-pushed the googlestyle-rule73 branch from 6160698 to 2e41edb Compare November 19, 2020 13:39

romani approved these changes Nov 19, 2020

View reviewed changes

romani assigned pbludov and unassigned romani Nov 19, 2020

pbludov approved these changes Nov 21, 2020

View reviewed changes

Issue checkstyle#5148: Add MissingJavadocType to google_checks.xml

f2ef1a3

nrmancuso force-pushed the googlestyle-rule73 branch from 2e41edb to f2ef1a3 Compare November 21, 2020 21:09

pbludov merged commit 14005e3 into checkstyle:master Nov 22, 2020

Uh oh!

Conversation

nrmancuso commented Nov 13, 2020 • edited by romani Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nrmancuso commented Nov 13, 2020

Uh oh!

github-actions Bot commented Nov 13, 2020

Uh oh!

nrmancuso commented Nov 13, 2020

Uh oh!

github-actions Bot commented Nov 13, 2020

Uh oh!

nrmancuso commented Nov 13, 2020

Uh oh!

github-actions Bot commented Nov 13, 2020

Uh oh!

nrmancuso commented Nov 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

strkkk commented Nov 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Nov 13, 2020

Uh oh!

nrmancuso commented Nov 13, 2020

Uh oh!

nrmancuso commented Nov 13, 2020

Uh oh!

github-actions Bot commented Nov 13, 2020

Uh oh!

romani commented Nov 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

romani commented Nov 15, 2020

Uh oh!

github-actions Bot commented Nov 15, 2020

Uh oh!

romani commented Nov 15, 2020

Uh oh!

nrmancuso commented Nov 15, 2020

Uh oh!

github-actions Bot commented Nov 15, 2020

Uh oh!

nrmancuso commented Nov 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nrmancuso commented Nov 15, 2020

Uh oh!

nrmancuso commented Nov 16, 2020

Uh oh!

github-actions Bot commented Nov 16, 2020

Uh oh!

romani commented Nov 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

romani commented Nov 16, 2020

Uh oh!

nrmancuso commented Nov 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nrmancuso commented Nov 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nrmancuso commented Nov 16, 2020

Uh oh!

github-actions Bot commented Nov 16, 2020

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

romani left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nrmancuso commented Nov 13, 2020 •

edited by romani

Loading

nrmancuso commented Nov 13, 2020 •

edited

Loading

strkkk commented Nov 13, 2020 •

edited

Loading

romani commented Nov 15, 2020 •

edited

Loading

nrmancuso commented Nov 15, 2020 •

edited

Loading

romani commented Nov 16, 2020 •

edited

Loading

nrmancuso commented Nov 16, 2020 •

edited

Loading

nrmancuso commented Nov 16, 2020 •

edited

Loading

romani left a comment •

edited

Loading

nrmancuso commented Nov 19, 2020 •

edited

Loading