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

Continuation indent is doubled when indentation and function signature are enabled #5366

Closed
eygraber opened this issue Sep 29, 2022 · 9 comments

Comments

@eygraber
Copy link
Contributor

x-posting from pinterest/ktlint#1661 because ktlint itself isn't the issue

Expected Behavior

I expect a single indent to be used for continuation indents.

The Kotlin style guide migration suggests that:

In Kotlin coding conventions, it's recommended to use a single indent in cases where the long continuation indent has been forced before.

Observed Behavior

A double indent is being used.

Steps to Reproduce

repro project

The issue can be seen using the command ./gradlew detektMetadataMain. In between each of these cases, I am running git reset --hard in order to reset the file to original formatting. The rules can be configured in the detekt.yml file.

  1. Indentation and FunctionSignature enabled - two function bodies are moved inline with the signature and indentation is doubled for continuations
  2. Indentation enabled and FunctionSignature disabled - no changes
  3. Indentation disabled and FunctionSignature enabled - two function bodies are moved inline with the signature

Contrast this with running ktlint "repro/src/**/*.kt" --experimental -F which only auto formats the FunctionSignature violation.

Your Environment

Detekt 1.22.0-RC1
Gradle 7.5.1
Kotlin 1.7.10

@eygraber eygraber added the bug label Sep 29, 2022
@3flex
Copy link
Member

3flex commented Sep 30, 2022

Possibly related to #5259, though I haven't confirmed.

@sanggggg
Copy link
Contributor

sanggggg commented Oct 15, 2022

Oh... I found the reason why. This bug is already resolved in #5312

The exact bug spec is if the FunctionSignature is active, indent size is changed to 4(FunctionSignature indentSize config) for later formatting rules

  FunctionSignature:
    active: true
+   indentSize: 6
  FunctionStartOfBodySpacing:
    active: true

You can easily check the above by setting this config in the repro project

When FunctionSignature rule is enabled on 1.22.0-RC1, DefaultEditorConfigProperties.indentSizeProperty is set to FunctionSignature's indent size config. (which is 4 in reproducing project)

DefaultEditorConfigProperties.indentSizeProperty to indentSize.toString(),

DefaultEditorConfigProperties.indentSizeProperty change global configuration KtLint.EDITOR_CONFIG_PROPERTIES_USER_DATA_KEY, so it affects the later formatting of the Indent rule. (indenting with 4, not 2)

editorConfigProperties.forEach { (editorConfigProperty, defaultValue) ->
userData[editorConfigProperty.type.name] = Property.builder()
.name(editorConfigProperty.type.name)
.type(editorConfigProperty.type)
.value(defaultValue)
.build()
}
root.node.putUserData(KtLint.EDITOR_CONFIG_PROPERTIES_USER_DATA_KEY, userData)
}
root.node.putUserData(KtLint.FILE_PATH_USER_DATA_KEY, root.name)

And thanks to #5312 this bug is resolved. (editorConfigProperties is not global anymore)
I think we can close this issue @3flex (this issue is not related to #5259)

@eygraber
Copy link
Contributor Author

I'm still seeing this behavior in RC2 (not sure if #5312 is included in that build or not)

@github-actions
Copy link

This issue is stale because it has been open 90 days with no activity. Please comment or this will be closed in 7 days.

@github-actions github-actions bot added the stale label Jan 22, 2023
@eygraber
Copy link
Contributor Author

Was this released as part of 1.22.0?

@3flex
Copy link
Member

3flex commented Jan 22, 2023

RC2 and the final 1.22.0 release used ktlint 0.47.1, so if you still saw the issue in RC2 it's likely not fixed in the final release. It would be great if you can reconfirm on the snapshot version of detekt (which uses ktlint 0.48.1).

@eygraber
Copy link
Contributor Author

Looks like the original issue is resolved.

Not sure if I should file a separate issue for this, but with the latest snapshot:

      val failure = try {
        client.newCall(request).execute().closeSilently()
        null
      }
      catch(t: Throwable) {
        t
      }

gets corrected to:

      val failure = try {
        client.newCall(request).execute().closeSilently()
        null
      }
        catch(t: Throwable) {
          t
        }

and

typealias MutableSurveyInterviewResponse =
  HashMap<SurveyInterviewResponseId, SurveyInterviewResponseValue?>

gets corrected to:

typealias MutableSurveyInterviewResponse =
HashMap<SurveyInterviewResponseId, SurveyInterviewResponseValue?>

@github-actions github-actions bot removed the stale label Jan 23, 2023
@3flex
Copy link
Member

3flex commented Jan 23, 2023

I think a separate defect if that's ok - and can you please confirm in that issue whether ktlint does this when run directly (and if it does, it should be raised with ktlint project instead of detekt), thanks!

@eygraber
Copy link
Contributor Author

#5720

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

No branches or pull requests

4 participants