Issue #19717: Add input files for FinalLocalVariableCheck about initialization in adjacent catches by lithops-zty · Pull Request #19728 · checkstyle/checkstyle

lithops-zty · 2026-04-21T06:03:43Z

fixes: #19717

Fix false negative in FinalLocalVariableCheck for multi-catch blocks.

Problem

When multiple catch blocks each assign the same uninitialized variable, the variable should be reported as a candidate for final because only one catch branch can execute, so the variable is effectively assigned exactly once on every path. Currently, the check silently misses this case.

Root Cause

Two methods in FinalLocalVariableCheck lack awareness of LITERAL_CATCH as a mutually-exclusive branch (analogous to LITERAL_ELSE for if-else and SWITCH_RULE for arrow-switch):

determineAssignmentConditions: its blockTypes array does not include LITERAL_CATCH, so when the second catch block's assignment is processed and candidate.assigned is already true, the check unconditionally sets candidate.alreadyAssigned = true and removes the variable from the final-candidate list.
shouldUpdateUninitializedVariables: when leaving the first catch block's SLIST, the method does not recognize that another catch follows, so updateAllUninitializedVariables() is never called and the variable's uninitialized state is lost before the second catch is visited.

Fix

Add TokenTypes.LITERAL_CATCH to the blockTypes array in determineAssignmentConditions.
Add a new helper isCatchTokenWithAnotherCatchFollowing (symmetric with the existing isCaseTokenWithAnotherCaseFollowing) and call it from shouldUpdateUninitializedVariables:

lithops-zty · 2026-04-21T08:35:04Z

Github, generate report for FinalLocalVariableCheck/all-examples-in-one

github-actions · 2026-04-21T09:32:57Z

Report for SimplifyBooleanExpression/all-examples-in-one:
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/9351b67eca8f12abd25ce0658874945f2e63a9b8_2026093057/reports/diff/index.html

lithops-zty · 2026-04-21T10:40:41Z

    <module name="LambdaBodyLength"/>
    <module name="MethodCount">
-      <property name="maxTotal" value="34"/>
+      <property name="maxTotal" value="35"/>


The value 34 was chosen exactly because this check had 34 methods and was the largest of all checks: (#4539 (comment))
I think it is reasonable to increase it as the updated version now has 35 methods.

you can put permanent suppression on this Check + File to our suppression xml, there are already bunch like this.
We do not know how to design classes to be less big and same time easy to read and keep logic very close.

done. I restored to 34 and added suppression

lithops-zty · 2026-04-21T11:17:32Z

@romani PR ready for review.

romani

thanks a lot, minor:

romani · 2026-05-11T19:48:25Z

 public class InputFinalLocalVariableMultiCatch {
    public void demo() throws Throwable {
+        // violation below, "Variable 'a' should be declared final"
+        int a; // assigned in adjacent catches


assigned in adjacent catches

assigned in nested catches

very hard to read such Inputs, yes, it is very comprehensive and compact, but it will not make it good in long run.

lets make them compact in formatting, but keep single variable to validate and right under correct form:

int u; // violation try { } catch (IndexOutOfBoundsException e) { u = 9; } final int u; try { } catch (IndexOutOfBoundsException e) { u = 9; }

That's true. I've completely separated each case into different try-catch blocks and added the correct version right under each violation.

romani · 2026-05-11T19:51:04Z

Github, generate report for FinalLocalVariableCheck/all-examples-in-one

github-actions · 2026-05-11T20:35:42Z

Failed to download or process the specified configuration(s).
Error details:

Please ensure you've used one of the following commands correctly:

'GitHub, generate report for <config_bundle_path>'
'GitHub, generate report by config from <file_path>'
'GitHub, generate report for configs in PR description'

And check that all specified configs or files exist and are valid.

Link: https://github.com/checkstyle/checkstyle/actions/runs/25693534092

romani · 2026-05-11T20:38:51Z

Github, generate report for FinalLocalVariable/all-examples-in-one

github-actions · 2026-05-11T21:42:36Z

Report for FinalLocalVariable/all-examples-in-one:
https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/7f8526543a877fb877f449f1865801d056198dd2_2026214216/reports/diff/index.html

romani · 2026-05-12T02:43:19Z

@lithops-zty , small update please and we will merge

romani

Not clear why some violations are missed

romani · 2026-05-12T15:37:31Z

+
+        byte b;
+        try {
+            b = -1;


Why no violation ?

In this case b is assigned in both try and catch.
It is very hard to say whether assignment will happen only once in try, as we cannot determine syntactically if exceptions will be thrown.
In fact, java disallow final in this case:

(base) zty@zty:/mnt/d/IntellijProjects/checkstyle-playground/inputs/final-local-variable$ cat Test.java public class Test { public void test() { final byte b; try { b = -1; } catch (final NumberFormatException e) { b = 0; } catch (IllegalStateException | NullPointerException e) { b = 0; } } }(base) zty@zty:/mnt/d/IntellijProjects/checkstyle-playground/inputs/final-local-variable$ javac Test.java Test.java:7: error: variable b might already have been assigned b = 0; ^ Test.java:9: error: variable b might already have been assigned b = 0; ^ 2 errors

ok, but why this violation:

int a; // violation, "Variable 'a' should be declared final" try { } catch (final NumberFormatException e) { a = 0; } catch (IllegalStateException | NullPointerException e) { a = 1; } catch (final IllegalArgumentException e) { a = 2; }

as if no exception happens, a is not initialized.

if this is defect of compiler, we should not put violation on this, as eventually it will be fixed in compiler.
Can you looks in jdk for defect on this? if not present you can create :)

I am not sure if this is a defect, but Java allows uninitialized final variable declaration. However, such variables cannot be read later on and will lead to a compiling error:

(base) zty@zty:/mnt/d/IntellijProjects/checkstyle-playground/inputs/final-local-variable$ cat Test.java public class Test { public void test() { final int a; try { } catch (final NumberFormatException e) { a = 0; } catch (IllegalStateException | NullPointerException e) { a = 1; } catch (final IllegalArgumentException e) { a = 2; } int b = a; } }(base) zty@zty:/mnt/d/IntellijProjects/checkstyle-playground/inputs/final-local-variable$ javac Test.java Test.java:12: error: variable a might not have been initialized int b = a; ^ 1 error

Something tells me that we should not try to validate this at all, to much uncertainty, even compiler is not consistent.

romani · 2026-05-13T16:02:58Z

I more and inclined to not do violations in such code.

We would rather do less violations, to not produce false positives. False positives blocks builds and annoy a lot user. Missed violations are ok. Missed violations on crazy edges are ok.

I recommend to remove functional changes and keep all test extension to make it clear we considered it but decided to not do.

If we in agreement, I will change issue title

lithops-zty · 2026-05-17T07:14:06Z

Sorry for replying late.
I agree this is an edge case that may not warrant strict enforcement.
But do you think there's merit to leave this an option to users, i.e. add a property to trigger it?
That said, I am OK to remove it completely.

romani · 2026-05-17T13:47:54Z

I don't see value.
Users usually lazy to look at docs for property, they simply disable Check if it misbehaving or blocking them.

Let's in this PR remove all changes in check, and simply merge Inputs, and we can add some comment over variables that until compiler becomes strict, we should not place violation. And we can merge PR.

Can you create issue over java compiler? In worse case they close it with explanation of reason, that is also good to know.

lithops-zty · 2026-05-17T15:50:10Z

No problem, I will remove all functional changes.

However, I don't see a problem with the compiler, could you elaborate on the exact bug you mentioned?

To me, the compiler works as I would expect in terms of allowing and disallowing final:

assignments in both try and catch

final byte b;  // compile error
try {
    b = -1;
} catch (final NumberFormatException e) {
    b = 0;
}

final is forbidden because try and catch are not mutually exclusive and both assignments may execute.
2. assignments in catches only

final int a;
try {
} catch (final NumberFormatException e) {
    a = 0;
} catch (IllegalStateException | NullPointerException e) {
    a = 1;
}

final is allowed because catches are mutually exclusive and at most one assignment may execute.

romani · 2026-05-18T04:44:30Z

assignments in both try and catc

Yes, we should not make violation on such code.

assignments in catches only

If exception is not happening, what value will have a variable?
And there might be some method in try that throw RuntimeExceptoon , We can guarantee initialization only by catch Throwable. There might be a lot of nuances, but still checkstyle should not violate this. Nobody writes code like this, so it might be purely academic problem :).

lithops-zty · 2026-05-27T12:33:25Z

@romani Thanks for the insights. Now I see there is no value checking case 2 since such variable cannot be used at all despite successful compilation. I have pushed a new commit with only input files.

…bout initialization in adjacent catches

romani

Thanks a lot

lithops-zty force-pushed the issue-19717-fix-final-local-variable-adjacent-catches branch from a5e4bb5 to 9351b67 Compare April 21, 2026 08:34

lithops-zty force-pushed the issue-19717-fix-final-local-variable-adjacent-catches branch from 9351b67 to 7f85265 Compare April 21, 2026 10:34

lithops-zty commented Apr 21, 2026

View reviewed changes

romani requested changes May 11, 2026

View reviewed changes

lithops-zty force-pushed the issue-19717-fix-final-local-variable-adjacent-catches branch 2 times, most recently from 7d6ce09 to 9eb5120 Compare May 12, 2026 13:57

romani requested changes May 12, 2026

View reviewed changes

lithops-zty force-pushed the issue-19717-fix-final-local-variable-adjacent-catches branch from 9eb5120 to 1b50d59 Compare May 12, 2026 16:40

lithops-zty force-pushed the issue-19717-fix-final-local-variable-adjacent-catches branch from 1b50d59 to d90977d Compare May 27, 2026 12:35

lithops-zty changed the title ~~Issue #19717: Fix FinalLocalVariableCheck for initialization in adjacent catches~~ Issue #19717: Add input files for FinalLocalVariableCheck about initialization in adjacent catches May 27, 2026

Issue checkstyle#19717: Add input files for FinalLocalVariableCheck a…

371009c

…bout initialization in adjacent catches

lithops-zty force-pushed the issue-19717-fix-final-local-variable-adjacent-catches branch from d90977d to 371009c Compare May 27, 2026 15:03

romani added approved miscellaneous labels May 27, 2026

romani approved these changes May 27, 2026

View reviewed changes

romani merged commit cca5189 into checkstyle:master May 27, 2026
127 of 128 checks passed

romani mentioned this pull request May 27, 2026

False Negative: FinalLocalVariableCheck does not report for variables assigned in adjacent catches #19717

Closed

Uh oh!

Conversation

lithops-zty commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Root Cause

Fix

Uh oh!

lithops-zty commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 21, 2026

Uh oh!

lithops-zty Apr 21, 2026 • 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!

romani May 11, 2026

Choose a reason for hiding this comment

Uh oh!

lithops-zty May 12, 2026

Choose a reason for hiding this comment

Uh oh!

lithops-zty commented Apr 21, 2026

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

romani May 11, 2026

Choose a reason for hiding this comment

Uh oh!

lithops-zty May 12, 2026

Choose a reason for hiding this comment

Uh oh!

romani commented May 11, 2026

Uh oh!

github-actions Bot commented May 11, 2026

Uh oh!

romani commented May 11, 2026

Uh oh!

github-actions Bot commented May 11, 2026

Uh oh!

romani commented May 12, 2026

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

romani May 12, 2026

Choose a reason for hiding this comment

Uh oh!

lithops-zty May 12, 2026

Choose a reason for hiding this comment

Uh oh!

romani May 13, 2026 • 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!

lithops-zty May 13, 2026 • 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!

romani May 13, 2026

Choose a reason for hiding this comment

Uh oh!

romani commented May 13, 2026

Uh oh!

lithops-zty commented May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

romani commented May 17, 2026

Uh oh!

lithops-zty commented May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

romani commented May 18, 2026

Uh oh!

lithops-zty commented May 27, 2026

Uh oh!

romani left a comment

Choose a reason for hiding this comment

lithops-zty commented Apr 21, 2026 •

edited

Loading

lithops-zty commented Apr 21, 2026 •

edited

Loading

lithops-zty Apr 21, 2026 •

edited

Loading

romani May 13, 2026 •

edited

Loading

lithops-zty May 13, 2026 •

edited

Loading

lithops-zty commented May 17, 2026 •

edited

Loading

lithops-zty commented May 17, 2026 •

edited

Loading