search

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

Issue #882: update LogicConditionNeedOptimizationCheck to avoid false positive #881

Closed nrmancuso closed 1 year ago

nrmancuso commented 2 years ago

From https://github.com/checkstyle/checkstyle/pull/11100

Discovered in report at https://nmancus1.github.io/issue-10848/issue-10848_check_diff_reports_2021_12_31/diff_sevntu-check-regression_part_2/index.html

Builds will fail until 9.3 release.

rnveach commented 2 years ago

https://github.com/sevntu-checkstyle/sevntu.checkstyle/pull/881#issue-1108339836

Builds will fail until 9.3 release.

rnveach commented 2 years ago

@nrmancuso @romani Is this PR still blocked? https://github.com/checkstyle/eclipse-cs/issues/339 is merged.

romani commented 2 years ago

rebased on latest, but there are problems in UT execution

rnveach commented 2 years ago 
[ERROR] Tests run: 8, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 3.701 s <<< FAILURE! - in com.github.sevntu.checkstyle.internal.AllChecksTest

[ERROR] testAllInputsHaveTest  Time elapsed: 0.148 s  <<< FAILURE!
org.opentest4j.AssertionFailedError: Resource must be named after a Test like 'InputMyCheck.java' and be in the same package as the test: com/github/sevntu/checkstyle/checks/coding/InputLogicConditionNeedsOptimizationPatterns.java ==> expected: <true> but was: <false>
    at com.github.sevntu.checkstyle.internal.AllChecksTest.verifyInputFile(AllChecksTest.java:290)
    at com.github.sevntu.checkstyle.internal.AllChecksTest.lambda$testAllInputsHaveTest$4(AllChecksTest.java:185)
    at com.github.sevntu.checkstyle.internal.AllChecksTest.testAllInputsHaveTest(AllChecksTest.java:184)
rnveach commented 2 years ago 
[INFO] --- jacoco-maven-plugin:0.8.7:check (default-check) @ sevntu-checks ---
[WARNING] Rule violated for class com.github.sevntu.checkstyle.checks.coding.LogicConditionNeedOptimizationCheck: branches covered ratio is 0.97, but expected minimum is 1.00
rnveach commented 2 years ago

@nrmancuso Can we finish this?

nrmancuso commented 2 years ago

@nrmancuso Can we finish this?

We should close this as syntax for pattern matching in switch has changed. We (I) made a bad decision to support a preview feature, and we will need to update grammar in main repo as this syntax is not valid any longer. I will leave this open until I create an issue in the main repo to explain all.

nrmancuso commented 1 year ago

I have opened an issue for syntax update at https://github.com/checkstyle/checkstyle/issues/12515, we need to make sure that this check suffers no regressions from changes there. I am closing this PR.