Skip to content
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.

Sign up for GitHub

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

Jump to bottom

IDEA inspection suppressions resulted from migration to v2022.3.3 #14625

Open
3 of 4 tasks
MANISH-K-07 opened this issue Mar 8, 2024 · 7 comments · May be fixed by #14812
Open
3 of 4 tasks

IDEA inspection suppressions resulted from migration to v2022.3.3 #14625

MANISH-K-07 opened this issue Mar 8, 2024 · 7 comments · May be fixed by #14812
Labels
approved miscellaneous
Milestone
10.16.0

Comments

@MANISH-K-07
Copy link
Contributor

MANISH-K-07 commented Mar 8, 2024 •
edited

Based on findings at #14604 & #14696

IDEA version used : v2022.3.3 (highest true scopes release)
JetBrains Release docs

Version: 2022.3.3
Build: 223.8836.41
Released: 8 March 2023

The version bump resulted in successful build with validation problems which have been suppressed for migration :
https://app.circleci.com/pipelines/github/checkstyle/checkstyle/1/workflows/0968f80e-db8e-401a-ad87-8c409072ba31/jobs/574793/artifacts

As part of this issue, the following validations have suppressions that need to be picked and dealt with :

After resolving the suppression, the @noinspection & @noinspectionreason tags must be removed from respective Javadoc. There is very little probability of cases which might require permanent suppression, report here as issue comments for such.

 * @noinspection TailRecursion
 * @noinspectionreason TailRecursion- until Issue #14625
 */

See diff in PR #14696 to find such suppressions.
This was referenced Mar 8, 2024
Closed
Closed
@romani romani added the approved label Mar 8, 2024
@checkstyle checkstyle deleted a comment from saicharankalakonda Mar 15, 2024
@MANISH-K-07 MANISH-K-07 changed the title IDEA inspection suppressions resulted from migration to v2023.3.4 IDEA inspection suppressions resulted from migration to v2022.3.3 Apr 16, 2024
@MANISH-K-07
Copy link
Contributor Author

MANISH-K-07 commented Apr 16, 2024 •
edited

@romani , issue description has been updated for new suppressions and image.

MANISH-K-07 added a commit to MANISH-K-07/checkstyle that referenced this issue Apr 19, 2024
@MANISH-K-07
Issue checkstyle#14625: fixed inspections violations MismatchedJavado…
a81f362 
…cCode
@MANISH-K-07 MANISH-K-07 mentioned this issue Apr 19, 2024
Merged
MANISH-K-07 added a commit to MANISH-K-07/checkstyle that referenced this issue Apr 19, 2024
@MANISH-K-07
Issue checkstyle#14625: fix inspection violation OptionalGetWithoutIs…
8161d55 
…Present JavaParserTest tAHSLCN
@MANISH-K-07 MANISH-K-07 mentioned this issue Apr 19, 2024
Merged
MANISH-K-07 added a commit to MANISH-K-07/checkstyle that referenced this issue Apr 19, 2024
@MANISH-K-07
Issue checkstyle#14625: fix inspection violation OptionalGetWithoutIs…
3a2bec5 
…Present JavaParserTest tAHSLCN
MANISH-K-07 added a commit to MANISH-K-07/checkstyle that referenced this issue Apr 19, 2024
@MANISH-K-07
Issue checkstyle#14625: fix noinspectionreason for TestMethodWithoutA…
8ca9668 
…ssertion
@MANISH-K-07 MANISH-K-07 linked a pull request Apr 19, 2024 that will close this issue
Open
@MANISH-K-07
Copy link
Contributor Author

MANISH-K-07 commented Apr 19, 2024 •
edited

@romani , @nrmancuso , the TailRecursion inspection, I'm not sure if we would want a fix for this.

One probability would be to replace Tail recursive call by looping but I don't think it would make much difference in terms of speed/optimization for us, also it is highly likely to get prone to unnecessary errors and too much hacking of existing code while applying fix.

So, shall we move it to permanent suppression? Please suggest what you feel about this.

PS: If we are voting for permanent suppression, please give me good wording for noinspectionreason :)

@romani
Copy link
Member

romani commented Apr 19, 2024 •
edited

If it is not obvious fix, I can understand, but in this case we need to move this inspection to separate issue, list all cases and discuss all of them separately.

In general recursion is not good and prone to failing eventually, some times in several decades :).

@MANISH-K-07
Copy link
Contributor Author

we need to move this inspection to separate issue, list all cases and discuss all of them separately.

Sure @romani , I will move this to new issue.

In general recursion is not good and prone to failing eventually, some times in several decades :).

Agree. Tail recursions are better optimised than non-tail but recursion itself is unreliable on long run :)

Just saying, fix would involve good level of hacking of existing code coz I've seen few cases where tail recursive call is being used as conditional clause too..

@nrmancuso
Copy link
Member

Tail recursion

We need to take this on a case by case basis, there is no point in general discussion about this topic.

@MANISH-K-07

This comment was marked as outdated.

Sign in to view
@MANISH-K-07 MANISH-K-07 mentioned this issue Apr 20, 2024
Open
@MANISH-K-07
Copy link
Contributor Author

@romani , @nrmancuso , Moved to #14814

MANISH-K-07 added a commit to MANISH-K-07/checkstyle that referenced this issue Apr 20, 2024
@MANISH-K-07
Issue checkstyle#14625: move TailRecursion inspection problems to sep…
ce0e4ed 
…arate issue
@MANISH-K-07 MANISH-K-07 mentioned this issue Apr 20, 2024
Merged
MANISH-K-07 added a commit to MANISH-K-07/checkstyle that referenced this issue Apr 20, 2024
@MANISH-K-07
Issue checkstyle#14625: fix inspection violation OptionalGetWithoutIs…
d79c161 
…Present JavaParserTest tAHSLCN
MANISH-K-07 added a commit to MANISH-K-07/checkstyle that referenced this issue Apr 23, 2024
@MANISH-K-07
Issue checkstyle#14625: fix inspection violation OptionalGetWithoutIs…
d2d9029 
…Present JavaParserTest tAHSLCN
MANISH-K-07 added a commit to MANISH-K-07/checkstyle that referenced this issue Apr 23, 2024
@MANISH-K-07
Issue checkstyle#14625: fix inspection violation OptionalGetWithoutIs…
9b17ce1 
…Present JavaParserTest tAHSLCN
nrmancuso pushed a commit that referenced this issue Apr 23, 2024
@MANISH-K-07 @nrmancuso
Issue #14625: fix inspection violation OptionalGetWithoutIsPresent Ja…
d6d4a5d 
…vaParserTest tAHSLCN
@github-actions github-actions bot added this to the 10.16.0 milestone Apr 23, 2024
MANISH-K-07 added a commit to MANISH-K-07/checkstyle that referenced this issue Apr 24, 2024
@MANISH-K-07
Issue checkstyle#14625: fix inspection violations OptionalGetWithoutI…
8230832 
…sPresent
@MANISH-K-07 MANISH-K-07 mentioned this issue Apr 24, 2024
Merged
MANISH-K-07 added a commit to MANISH-K-07/checkstyle that referenced this issue Apr 24, 2024
@MANISH-K-07
Issue checkstyle#14625: Enforced new naming convention in IT area 23
e02daf9
nrmancuso pushed a commit that referenced this issue Apr 27, 2024
@MANISH-K-07 @nrmancuso
Issue #14625: fix inspection violations OptionalGetWithoutIsPresent
10908a4
nrmancuso pushed a commit that referenced this issue Apr 27, 2024
@MANISH-K-07 @nrmancuso
Issue #14625: move TailRecursion inspection problems to separate issue
508757d
nrmancuso pushed a commit that referenced this issue Apr 28, 2024
@MANISH-K-07 @nrmancuso
Issue #14625: fixed inspections violations MismatchedJavadocCode
5b05fff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
approved miscellaneous
Projects
None yet
Milestone
10.16.0
Development

Successfully merging a pull request may close this issue.

Issue #14625: fix noinspectionreason for TestMethodWithoutAssertion MANISH-K-07/checkstyle
3 participants
@romani @nrmancuso @MANISH-K-07