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

[Android] Return type allowed on separate line #941

Closed
camsteffen opened this issue Oct 9, 2020 · 5 comments
Closed

[Android] Return type allowed on separate line #941

camsteffen opened this issue Oct 9, 2020 · 5 comments

Comments

@camsteffen
Copy link

Ktlint in Android mode allows the return type of a function signature to be on a separate line when the parameters are not on their own lines.

The Android Style Guide says (link):

When a function signature does not fit on a single line, break each parameter declaration onto its own line. Parameters defined in this format should use a single indent (+4). The closing parenthesis ()) and return type are placed on their own line with no additional indent.

It seems like ktlint does not consider the return type to be part of the function signature.

Example / Steps to Reproduce

ktlint --android --stdin <<< '
fun thisismytestmethod(aaaaaaaaaaaaaaaa: String, bbbbbbbbbbbbbb: String, ccccccccccccccc, String):
    String {
        return "test"
    }'

Expected Behavior

Ktlint should detect that the function signature does not fit on one line and give error(s) that the parameters are not on their own line.

Observed Behavior

No error.

Your Environment

  • Version of ktlint used: 0.39.0
  • Operating System and version: macos 10.15.7
@camsteffen
Copy link
Author

camsteffen commented Oct 9, 2020

Actually, I think this applies to non-android usage as well. (style guide)

@paul-dingemans
Copy link
Collaborator

Actually, I think this applies to non-android usage as well. (style guide)

I agree that this is not adroid specific.

With max-line-length set to 50, the generated output is as follows:

fun thisismytestmethod(
    aaaaaaaaaaaaaaaa: String,
    bbbbbbbbbbbbbb: String,
    ccccccccccccccc,
    String
):
    String {
    return "test"
}

In this case the parameter-list-wrapping rule forces the parameters to separate lines. Note however that the return type should be placed on the same line as the closing parentheses and colon. But also IntelliJ does not fix this mistake.

For example with max-line-length set to 100, the generated output is as follows:

fun thisismytestmethod(aaaaaaaaaaaaaaaa: String, bbbbbbbbbbbbbb: String, ccccccccccccccc, String):
    String {
    return "test"
}

Also the IntelliJ default formatting does not reformat this statement.

So the behavior of ktlint is consistent with IntelliJ default formatting. Technically it is not a bug in the current ktlint rules.

I am creating a new rule method-return-type-wrapping to handle this and other edge cases regarding formatting of the method signature to format below:

fun thisismytestmethod(
    aaaaaaaaaaaaaaaa: String,
    bbbbbbbbbbbbbb: String,
    ccccccccccccccc: String
): String {
    return "test"
}

@paul-dingemans
Copy link
Collaborator

OP states that example below should not result in an error:

ktlint --android --stdin <<< '
fun thisismytestmethod(aaaaaaaaaaaaaaaa: String, bbbbbbbbbbbbbb: String, ccccccccccccccc, String):
String {
return "test"
}'

According to examples in Kotlin coding conventions and Android's Kotlin Styleguide the closing parenthesis and return type should be on the same line. This is also the style that is is used by rule experimental:function-signature which has been released in version 046.0.

@camsteffen
Copy link
Author

OP states that example below should not result in an error:

Actually I said that it should result in an error. But yes, it looks like this is properly addressed with experimental:function-signature, thanks! Is this intended to become a default rule eventually?

@paul-dingemans
Copy link
Collaborator

Yes, at some point in time it will become standard. But it depends on the feedback that we receive about the rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants