Issue #15059: New Check UnusedLambdaParameterShouldBeUnnamed by mahfouz72 · Pull Request #15480 · checkstyle/checkstyle

mahfouz72 · 2024-08-09T23:10:02Z

New module config: https://gist.githubusercontent.com/mahfouz72/afd57aa6e51ca161b7927acd959e5211/raw/3e297236fff03643bdcffa607b54b528b3438ece/check.xml

mahfouz72 · 2024-08-09T23:38:17Z

github, generate site

github-actions · 2024-08-10T01:24:16Z

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

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/ca17d06_2024010320/checks.html#

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/ca17d06_2024010320/checks/coding/index.html#

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/ca17d06_2024010320/checks/coding/unusedlambdaparametershouldbeunnamed.html#

romani · 2024-08-10T01:33:50Z

Unnamed lambda parameters will not compile on java versions between 9 and 21

Please share in comment, violations on our code, we should recheck if all of them are valid.

mahfouz72 · 2024-08-10T03:10:23Z

[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\it\java\org\checkstyle\base\AbstractItModuleTestSupport.java:519:67: Unused lambda parameter 'key' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\TreeWalker.java:305:59: Unused lambda parameter 'empty' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\TreeWalker.java:316:60: Unused lambda parameter 'empty' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\api\FileContents.java:155:17: Unused lambda parameter 'empty' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\checks\UniquePropertiesCheck.java:185:59: Unused lambda parameter 'empty' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\checks\blocks\RightCurlyCheck.java:491:29: Unused lambda parameter 'child' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\checks\coding\ConstructorsDeclarationGroupingCheck.java:126:30: Unused lambda parameter 'first' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\checks\coding\EqualsHashCodeCheck.java:174:39: Unused lambda parameter 'key' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\checks\coding\MultipleStringLiteralsCheck.java:210:58: Unused lambda parameter 'key' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\checks\javadoc\AbstractJavadocCheck.java:326:87: Unused lambda parameter 'key' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\checks\sizes\RecordComponentNumberCheck.java:159:13: Unused lambda parameter 'node' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\filters\SuppressWithPlainTextCommentFilter.java:288:22: Unused lambda parameter 'first' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\gui\ListToTreeSelectionModelWrapper.java:49:58: Unused lambda parameter 'event' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\main\java\com\puppycrawl\tools\checkstyle\gui\MainFrame.java:140:41: Unused lambda parameter 'event' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\test\java\com\puppycrawl\tools\checkstyle\AbstractModuleTestSupport.java:606:67: Unused lambda parameter 'key' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\test\java\com\puppycrawl\tools\checkstyle\ConfigurationLoaderTest.java:714:52: Unused lambda parameter 'context' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\test\java\com\puppycrawl\tools\checkstyle\TreeWalkerTest.java:136:72: Unused lambda parameter 'mock' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\test\java\com\puppycrawl\tools\checkstyle\TreeWalkerTest.java:136:78: Unused lambda parameter 'context' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\test\java\com\puppycrawl\tools\checkstyle\checks\header\HeaderCheckTest.java:305:48: Unused lambda parameter 'context' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\test\java\com\puppycrawl\tools\checkstyle\gui\MainFrameTest.java:128:44: Unused lambda parameter 'context' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\test\java\com\puppycrawl\tools\checkstyle\gui\MainFrameTest.java:147:44: Unused lambda parameter 'context' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\test\java\com\puppycrawl\tools\checkstyle\internal\AllChecksTest.java:590:59: Unused lambda parameter 'key' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\test\java\com\puppycrawl\tools\checkstyle\internal\AllTestsTest.java:110:75: Unused lambda parameter 'key' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\test\java\com\puppycrawl\tools\checkstyle\internal\AllTestsTest.java:129:75: Unused lambda parameter 'key' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\test\java\com\puppycrawl\tools\checkstyle\internal\XdocsJavaDocsTest.java:638:74: Unused lambda parameter 'unused' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\test\java\com\puppycrawl\tools\checkstyle\internal\XdocsJavaDocsTest.java:639:63: Unused lambda parameter 'unused' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]
[ERROR] [checkstyle] [ERROR] D:\CS\checkstyle\src\test\java\com\puppycrawl\tools\checkstyle\internal\XdocsJavaDocsTest.java:640:68: Unused lambda parameter 'unused' should be unnamed. [UnusedLambdaParameterShouldBeUnnamed]

I checked most of them, and they are valid

romani · 2024-08-10T06:09:33Z

Unnamed lambda parameters will not compile on java versions between 9 and 21

Can we simply state Unnamed lambda parameters will not compile on java versions before 21 ?

romani

Item

romani · 2024-08-10T06:40:55Z

 unnecessary.semicolon=Лишняя точка с запятой.
 unused.catch.parameter=Неиспользуемый параметр catch ''{0}'' должен быть безымянным.
+unused.lambda.parameter=Неиспользуемый лямбда-параметр ''{0}'' не должен быть назван.
 unused.local.var=Неиспользуемая локальная переменная ''{0}''.


Неиспользуемый лямбда-параметр ''{0}'' должен быть безымянным

nrmancuso · 2024-08-10T12:42:11Z

Unnamed lambda parameters will not compile on java versions between 9 and 21

Can we simply state Unnamed lambda parameters will not compile on java versions before 21 ?

No, this is not accurate, though they were not called “unnamed” until now. You could use an underscore as a lambda parameter name in Java 8.

romani · 2024-08-10T14:04:19Z

we are on 11, so comments in our code should not be about past old jdk.
I see this error:

We even do not have Inputs that are compiled on old jdk.

You could use an underscore as a lambda parameter name in Java 8

lets put this nuance to xdoc of Check, so this Check potentially can be used on sources that still on jdk8.

mahfouz72 · 2024-08-10T15:09:44Z

You can't use this check on java 8 because unnamed variables concept was not as it is now(21). underscore variable in java 8 can be read from or write to so there will be no benefits if we changed the variable name from 'a' to '_' both can be used as normal identifier in java 8 and below. the rationale of the check is to declare the unused parameter as unnamed because if we are java 21+ we are sure that this unnamed variable can't be used and we want to communicate this explicitly to readers

romani · 2024-08-10T15:28:03Z

underscore variable in java 8 can be read from or write to so there will be no benefits if we changed the variable name from 'a' to '_' both can be used as normal identifier in java 8 and below.

please rephase, I do not understand this sentence.

What we have in comment of config and in web site is different and I just try to point that better to align wording. In our config comment, we should simply state that we need to enable it only after migration to 21. No need to mention all other nuances.

nrmancuso

Looks good, few minor things:

nrmancuso

another:

nrmancuso · 2024-08-10T16:23:41Z

+    public void leaveToken(DetailAST ast) {
+        if (ast.getType() == TokenTypes.LAMBDA) {
+            // when leaving a given lambda, check every parameter associated with it
+            while (!lambdaParameters.isEmpty()


Suggested change

while (!lambdaParameters.isEmpty()

while (lambdaParameters.peek() != null

This will make checker happy

but still checker fails I have no idea how is that possible. may be a false positive? are we ok to suppress?

Suggested change

while (!lambdaParameters.isEmpty()

&& ast.equals(lambdaParameters.peek().enclosingLambda())) {

Would this make checker happy?

No. very weird

while (lambdaParameters.peek() != null && ast.equals(lambdaParameters.peek().enclosingLambda)) { ... }

<checkerFrameworkError unstable="false"> <fileName>src/main/java/com/puppycrawl/tools/checkstyle/checks/coding/UnusedLambdaParameterShouldBeUnnamedCheck.java</fileName> <specifier>dereference.of.nullable</specifier> <message>dereference of possibly-null reference lambdaParameters.peek()</message> <lineContent>&& ast.equals(lambdaParameters.peek().enclosingLambda)) {</lineContent> </checkerFrameworkError>

I am good to suppress this, we should also open a bug report with checker

Leaving this unresolved for the next reviewers to see.

typetools/checker-framework#6760

mahfouz72 · 2024-08-10T19:04:06Z

please rephase, I do not understand this sentence.

I just was trying to say that this check can't be used for source code that is below Jdk 8. In Java 8 and earlier versions, the underscore _ can be used as a valid identifier, just like any other variable name. Therefore, changing a variable name from any name to _ doesn't provide any benefit, as both can be used in the same way

we should simply state that we need to enable it only after migration to 21. No need to mention all other nuances.

make sense, done
@nrmancuso I am ok to make it below 21 in the comment on config. I think It is correct wording. "Unnamed" comes on 21 only. before Java 8 it was not called unnamed so yea it makes sense. better to keep it like this to align wording with xdocs as @romani suggested. do you agree with the current wording in docs and config comment?

romani · 2024-08-10T19:13:53Z

i am good for your fixes on my comments.

I will review this PR at the most end. But I generally good with new Check.

mahfouz72 · 2024-08-10T20:30:38Z

github, generate site

github-actions · 2024-08-10T21:33:43Z

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

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/4c00eee_2024213231/checks.html#

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/4c00eee_2024213231/checks/coding/index.html#

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/4c00eee_2024213231/checks/coding/unusedlambdaparametershouldbeunnamed.html#

nrmancuso

Very good, thanks for making this an easy review :)

mahfouz72 · 2024-08-13T18:23:28Z

github, generate report

github-actions · 2024-08-13T20:59:04Z

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

mahfouz72 · 2024-08-21T16:52:34Z

github, generate site

github-actions · 2024-08-21T19:13:24Z

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

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/56be48a_2024182036/checks.html#

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/56be48a_2024182036/checks/coding/index.html#

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/56be48a_2024182036/checks/coding/unusedlambdaparametershouldbeunnamed.html#

romani

Ok to merge

mahfouz72 force-pushed the new-check-unusedlambda branch 3 times, most recently from f2b8456 to 9ee24f0 Compare August 9, 2024 23:31

mahfouz72 force-pushed the new-check-unusedlambda branch from 9ee24f0 to ca17d06 Compare August 9, 2024 23:50

romani requested changes Aug 10, 2024

View reviewed changes

nrmancuso self-assigned this Aug 10, 2024

nrmancuso suggested changes Aug 10, 2024

View reviewed changes

mahfouz72 force-pushed the new-check-unusedlambda branch from ca17d06 to 2b54a90 Compare August 10, 2024 19:01

mahfouz72 force-pushed the new-check-unusedlambda branch from 2b54a90 to 4c00eee Compare August 10, 2024 19:37

romani self-requested a review August 11, 2024 19:10

mahfouz72 force-pushed the new-check-unusedlambda branch from 4c00eee to 1143b5c Compare August 11, 2024 20:28

rnveach self-requested a review August 11, 2024 23:57

nrmancuso approved these changes Aug 12, 2024

View reviewed changes

nrmancuso assigned rnveach and unassigned nrmancuso Aug 12, 2024

mahfouz72 force-pushed the new-check-unusedlambda branch from 1143b5c to 602de17 Compare August 13, 2024 18:22

mahfouz72 mentioned this pull request Aug 13, 2024

Config: New Check UnusedLambdaParameterShouldBeUnnamed checkstyle/contribution#890

Merged

mahfouz72 mentioned this pull request Aug 18, 2024

Issue #13293: Add supported Java Language Level to website #15524

Merged

rnveach suggested changes Aug 19, 2024

View reviewed changes

Comment thread pom.xml Outdated

mahfouz72 force-pushed the new-check-unusedlambda branch 5 times, most recently from 88a6577 to dabbef2 Compare August 20, 2024 00:13

rnveach suggested changes Aug 21, 2024

View reviewed changes

mahfouz72 force-pushed the new-check-unusedlambda branch from dabbef2 to c7fe71a Compare August 21, 2024 16:47

mahfouz72 force-pushed the new-check-unusedlambda branch 2 times, most recently from 56be48a to cf800d1 Compare August 21, 2024 17:45

rnveach approved these changes Aug 23, 2024

View reviewed changes

rnveach assigned romani and unassigned rnveach Aug 23, 2024

rnveach mentioned this pull request Aug 23, 2024

Issue #15159: RedundantModifierCheck should violate final modifier on… #15492

Merged

Issue checkstyle#15059: New Check UnusedLambdaParameterShouldBeUnnamed

428da11

mahfouz72 force-pushed the new-check-unusedlambda branch from cf800d1 to 428da11 Compare August 23, 2024 16:21

romani approved these changes Aug 23, 2024

View reviewed changes

romani merged commit 81b0261 into checkstyle:master Aug 23, 2024

mahfouz72 deleted the new-check-unusedlambda branch May 9, 2025 09:37

	while (!lambdaParameters.isEmpty()
	while (lambdaParameters.peek() != null

	while (!lambdaParameters.isEmpty()
	&& ast.equals(lambdaParameters.peek().enclosingLambda())) {

Uh oh!

Conversation

mahfouz72 commented Aug 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mahfouz72 commented Aug 9, 2024

Uh oh!

github-actions Bot commented Aug 10, 2024

Uh oh!

romani commented Aug 10, 2024

Uh oh!

mahfouz72 commented Aug 10, 2024

Uh oh!

romani commented Aug 10, 2024

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

romani Aug 10, 2024 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nrmancuso commented Aug 10, 2024

Uh oh!

romani commented Aug 10, 2024

Uh oh!

mahfouz72 commented Aug 10, 2024

Uh oh!

romani commented Aug 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nrmancuso left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nrmancuso left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mahfouz72 commented Aug 10, 2024

Uh oh!

romani commented Aug 10, 2024

Uh oh!

mahfouz72 commented Aug 10, 2024

Uh oh!

github-actions Bot commented Aug 10, 2024

Uh oh!

nrmancuso left a comment

Choose a reason for hiding this comment

Uh oh!

mahfouz72 commented Aug 13, 2024

Uh oh!

github-actions Bot commented Aug 13, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mahfouz72 commented Aug 9, 2024 •

edited

Loading

romani Aug 10, 2024 •

edited

Loading

romani commented Aug 10, 2024 •

edited

Loading