sevntu-checkstyle / sevntu.checkstyle

Additional Checkstyle checks, that could be added as extension to EclipseCS plugin and maven-checkstyle-plugin, Sonar checkstyle plugin, extension for CheckStyle IDEA plugin.
http://sevntu-checkstyle.github.io/sevntu.checkstyle/
190 stars 146 forks source link

minor: update checks(related to checkstyle issue#10100) #851

Closed nrmancuso closed 3 years ago

nrmancuso commented 3 years ago

Related to https://github.com/checkstyle/checkstyle/pull/10104

Original reports from Checkstyle PR showing regression:

https://nmancus1.github.io/issue-10100_check_diff_reports_2021_06_11/diff_sevntu-check-regression_part_1/index.html

NoMainMethodInAbstractClassCheck- new NPE ForbidWildcardAsReturnType - lots of new legit violations, we were missing violations on methods like public Class<?>[] getClasses() {} InnerClass - symmetrical column changes StaticMethodCandidate - new violations, we were missing violations previously on methods with String return type


https://nmancus1.github.io/issue-10100_check_diff_reports_2021_06_11/diff_sevntu-check-regression_part_2/index.html

MoveVariableInsideIf - symmetrical column number changes SimpleAccessorNameNotation - symmetrical column number changes CustomDeclarationOrder - symmetrical column number changes


New reports showing no regression:

https://nmancus1.github.io/issue-10100_check_diff_reports_2021_06_16/diff_sevntu-check-regression_part_1/index.html

Symmetric column changes and new legitimate violations NoMainMethodInAbstractClassCheck- NPE is gone


https://nmancus1.github.io/issue-10100_check_diff_reports_2021_06_16/diff_sevntu-check-regression_part_2/index.html

Symmetric column changes only


All other changes were because of failing existing unit tests (found via mvn clean verify).

I will let the build fail to show that the changes in this PR are dependent on the changes at https://github.com/checkstyle/checkstyle/pull/10104, then I will change the Checkstyle repo to my PR to show that all pass.

Failed build (I had to add mvn clean verify to CI):

Results :

Failed tests: 
  NoMainMethodInAbstractClassCheckTest.testDefault:87->AbstractModuleTestSupport.verify:229->AbstractModuleTestSupport.verify:267->AbstractModuleTestSupport.verify:304 error message 0 expected:<...actClassCheck.java:1[00]:5: Main method insi...> but was:<...actClassCheck.java:1[16]:5: Main method insi...>
  ForbidWildcardAsReturnTypeCheckTest.testNewArrayStructure:461->AbstractModuleTestSupport.verify:229->AbstractModuleTestSupport.verify:267->AbstractModuleTestSupport.verify:304 error message 0 expected:<[/pipeline/source/sevntu-checks/src/test/resources/com/github/sevntu/checkstyle/checks/design/InputForbidWildcardAsReturnTypeCheckNewArrayStructure.java:4:5: Wildcard as return type should be avoided].> but was:<[Audit done].>
  PreferMethodReferenceCheckTest.testObjectCreation:146->AbstractModuleTestSupport.verify:229->AbstractModuleTestSupport.verify:267->AbstractModuleTestSupport.verify:304 error message 2 expected:<...ferenceCheck4.java:1[7]:39: Lambda can be r...> but was:<...ferenceCheck4.java:1[8]:39: Lambda can be r...>
  PreferMethodReferenceCheckTest.testStatementsListsWithExpressions:128->AbstractModuleTestSupport.verify:229->AbstractModuleTestSupport.verify:267->AbstractModuleTestSupport.verify:304 error message 11 expected:<...eferenceCheck3.java:[46]:40: Lambda can be r...> but was:<...eferenceCheck3.java:[50]:40: Lambda can be r...>
  Jsr305AnnotationsCheckTest.testArrays:161->AbstractModuleTestSupport.verify:229->AbstractModuleTestSupport.verify:267->AbstractModuleTestSupport.verify:304 error message 6 expected:<...kWithArrays.java:90:[5]: Return value must ...> but was:<...kWithArrays.java:90:[8]: Return value must ...>
  CustomDeclarationOrderCheckTest.mainMethod:189->AbstractModuleTestSupport.verify:229->AbstractModuleTestSupport.verify:267->AbstractModuleTestSupport.verify:304 error message 0 expected:<...d(private )' then 'M[ainM]ethod(.*)'.> but was:<...d(private )' then 'M[]ethod(.*)'.>

Tests run: 411, Failures: 6, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 32.994 s
[INFO] Finished at: 2021-06-16T11:42:15Z
[INFO] Final Memory: 56M/980M
[INFO] ---------------------------------------------------------------------

Full log at https://app.wercker.com/checkstyle/sevntu.checkstyle/runs/build/60c9e32c237ec900086455fa?step=60c9e3678ea84100087595c9


mvn clean verify is passing with Checkstyle version from https://github.com/checkstyle/checkstyle/pull/10104

rnveach commented 3 years ago

the changes in this PR are dependent on the changes at checkstyle/checkstyle#10104

This means this PR is dependent on 4 things before it can be merged: 1) checkstyle/checkstyle#10104 must be merged (UPDATE: DONE) 2) checkstyle must release those changes (DONE - merged and released as 8.44) 3) eclipse-cs must release with that new version of checkstyle 4) The version of checkstyle in sevntu must update to the new eclipse-cs version (see https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/master/sevntu-checks/pom.xml#L18-L19 )

rnveach commented 3 years ago

Added blocked label because of https://github.com/sevntu-checkstyle/sevntu.checkstyle/pull/851#issuecomment-861974113 to prevent accidental merging. Label can be removed when this PR can be safely merged.

ghost commented 3 years ago

Hi, I am a beginner at this issue can you help me. How to start it?

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-0.0009%) to 98.796% when pulling 850a0913a4620ac211646e15b77994db9202c7ce on nmancus1:checkstyle-10100 into b7c8c7226baa623365b0028c0cb629816de1032c on sevntu-checkstyle:master.