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.

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

WhitespaceAround reports a violation when switch expression is passed as a method argument #14825

Open
grimsa opened this issue Apr 23, 2024 · 2 comments · May be fixed by #14858
Open

WhitespaceAround reports a violation when switch expression is passed as a method argument #14825

grimsa opened this issue Apr 23, 2024 · 2 comments · May be fixed by #14858
Labels

Comments

@grimsa
Copy link

grimsa commented Apr 23, 2024

I have read check documentation: https://checkstyle.sourceforge.io/checks/whitespace/whitespacearound.html
I have downloaded the latest checkstyle from: https://checkstyle.org/cmdline.html#Download_and_Run
I have executed the cli and showed it below, as cli describes the problem better than 1,000 words

C:\DEV\checkstyle>javac --version
javac 21.0.1

C:\DEV\checkstyle>javac Example.java

C:\DEV\checkstyle>type config.xml
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE module PUBLIC "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN" "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
    <module name="SuppressWarningsFilter"/>
    <property name="charset" value="UTF-8"/>
    <property name="severity" value="warning"/>
    <property name="fileExtensions" value="java, properties, xml"/>

    <module name="TreeWalker">
        <module name="WhitespaceAround"/>
    </module>
</module>

C:\DEV\checkstyle>type Example.java
package com.company.example;

import java.util.Optional;

public class Example {
        Optional<String> someMethod(int arg) {
                return Optional.ofNullable(switch (arg) {
                        case 123 -> "yes";
                        default -> null;
                });
        }
}

C:\DEV\checkstyle>set RUN_LOCALE="-Duser.language=en -Duser.country=US"

C:\DEV\checkstyle>java %RUN_LOCALE% -jar checkstyle-10.15.0-all.jar -c config.xml Example.java
Starting audit...
[WARN] C:\DEV\checkstyle\Example.java:7:44: 'switch' is not preceded with whitespace. [WhitespaceAround]
Audit done.

After the introduction of switch expressions (e.g., return switch (...)) the switch keyword can now be used anywhere where an expression could be used previously.

As whitespace is not required before any other expression, it should also not be required before switch (when it is used as part of a larger expression). Therefore, I'd expect Checkstyle to allow this:

return Optional.ofNullable(switch (arg) {
        case 123 -> "yes";
        default -> null;
});

Now it only allows this:

return Optional.ofNullable(
	switch (arg) {                     // <-- with an extra line break
		case 123 -> "yes";
		default -> null;
	}
);

or this

return Optional.ofNullable( switch (arg) {   // <-- notice the space before `switch`
	case 123 -> "yes";
	default -> null;
});

I think this may have been a case missed in: #8687

@nrmancuso
Copy link
Member

I can confirm that other expressions do not show the same behavior:

➜  src cat config.xml
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE module PUBLIC "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN" "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
    <module name="SuppressWarningsFilter"/>
    <property name="charset" value="UTF-8"/>
    <property name="severity" value="warning"/>
    <property name="fileExtensions" value="java, properties, xml"/>

    <module name="TreeWalker">
        <module name="WhitespaceAround"/>
    </module>
</module>
➜  src cat Test.java 
import java.util.Optional;

public class Test {
    Optional<String> someMethod(int arg) {
        return Optional.ofNullable(switch (arg) {
            case 123 -> "yes";
            default -> null;
        });
    }

    Optional<String> someOtherMethod1(int arg) {
        return Optional.ofNullable("yes");
    }

    Optional<String> someOtherMethod2(int arg) {
        return Optional.ofNullable(1 + "yes");
    }
}
➜  src java -jar checkstyle-10.13.0-all.jar -c config.xml Test.java
Starting audit...
[WARN] /home/nick/IdeaProjects/tester/src/Test.java:5:36: 'switch' is not preceded with whitespace. [WhitespaceAround]
Audit done.

@strkkk
Copy link
Member

strkkk commented May 4, 2024

I am on it

strkkk added a commit to strkkk/checkstyle that referenced this issue May 5, 2024
strkkk added a commit to strkkk/checkstyle that referenced this issue May 10, 2024
nrmancuso pushed a commit to strkkk/checkstyle that referenced this issue May 10, 2024
strkkk added a commit to strkkk/checkstyle that referenced this issue May 10, 2024
strkkk added a commit to strkkk/checkstyle that referenced this issue May 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants