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

Code indents are according to Super call instead of to the main constructor block #1222

Closed
nokite opened this issue Aug 26, 2021 · 2 comments · Fixed by #1304
Closed

Code indents are according to Super call instead of to the main constructor block #1222

nokite opened this issue Aug 26, 2021 · 2 comments · Fixed by #1304

Comments

@nokite
Copy link

nokite commented Aug 26, 2021

Expected Behavior

This is how Android Studio formats the code:

    constructor(context: Context, attrs: AttributeSet?, defStyleAttr: Int) : super(context, attrs, defStyleAttr) {
        init(attrs, defStyleAttr)
    }

    constructor(context: Context, attrs: AttributeSet?, defStyleAttr: Int, defStyleRes: Int) :
        super(context, attrs, defStyleAttr, defStyleRes) {
        init(attrs, defStyleAttr, defStyleRes)
    }

Notice how beautiful and logical the two constructors look, aligned and indented the same way.

Observed Behavior

This is how I have to format my code in order to satisfy ktlint:

    constructor(context: Context, attrs: AttributeSet?, defStyleAttr: Int) : super(context, attrs, defStyleAttr) {
        init(attrs, defStyleAttr)
    }

    // @formatter:off
    constructor(context: Context, attrs: AttributeSet?, defStyleAttr: Int, defStyleRes: Int) :
        super(context, attrs, defStyleAttr, defStyleRes) {
            init(attrs, defStyleAttr, defStyleRes)
        }
    // @formatter:on

Notice how the second constructor is indented in a weird way, the code is aligned to the "super" call instead of to the actual function. (and I have to use "@Formatter:off - on" to prevent the IDE formatter from making it right and breaking ktlint)

Steps to Reproduce

I've imported the config from ktlint via ktlint --android applyToIDEAProject (on top of the Default Android studio config).

This is reproducible in all these cases:

  • WIth and without my (small) customizations of the config
  • With and without an .editorconfig

Your Environment

ktlint:0.42.1
Android Studio Arctic Fox | 2020.3.1 Patch 1 (Build #AI-203.7717.56.2031.7621141, built on August 7, 2021)
macOS 11.5.2

Same problem when I run it via gradle or command line. (the gradle task is the same as in the ktlint readme)

@paul-dingemans
Copy link
Collaborator

The problem is still reproducible in version 0.43.2.

If I reformat the original code sample in IntelliJ Ultimate (with ktlint disabled), it formats the code like:

class Issue1222 {
    constructor(context: Context, attrs: AttributeSet?, defStyleAttr: Int) : super(context, attrs, defStyleAttr) {
        init(attrs, defStyleAttr)
    }

    constructor(context: Context, attrs: AttributeSet?, defStyleAttr: Int, defStyleRes: Int) :
            super(context, attrs, defStyleAttr, defStyleRes) {
        init(attrs, defStyleAttr, defStyleRes)
    }
}

Small difference is that super(context, attrs, defStyleAttr, defStyleRes) { is indented on level deeper. Can you please double check whether that line is formatted differently in Androiud Studio?

paul-dingemans pushed a commit to paul-dingemans/ktlint that referenced this issue Dec 11, 2021
paul-dingemans pushed a commit to paul-dingemans/ktlint that referenced this issue Dec 13, 2021
This aligns the formatting with the default IntelliJ formatting of secondary
constructors.

Closes pinterest#1222
@paul-dingemans
Copy link
Collaborator

The problem is still reproducible in version 0.43.2.

If I reformat the original code sample in IntelliJ Ultimate (with ktlint disabled), it formats the code like:

class Issue1222 {
    constructor(context: Context, attrs: AttributeSet?, defStyleAttr: Int) : super(context, attrs, defStyleAttr) {
        init(attrs, defStyleAttr)
    }

    constructor(context: Context, attrs: AttributeSet?, defStyleAttr: Int, defStyleRes: Int) :
            super(context, attrs, defStyleAttr, defStyleRes) {
        init(attrs, defStyleAttr, defStyleRes)
    }
}

Small difference is that super(context, attrs, defStyleAttr, defStyleRes) { is indented on level deeper. Can you please double check whether that line is formatted differently in Androiud Studio?

Sorry, false alarm. I probably had a local ".editconfig" in continuation indents were enabled. Default IntelliJ formatting is indeed without the continuation index.

paul-dingemans pushed a commit to paul-dingemans/ktlint that referenced this issue Dec 13, 2021
There is no need for a separate build task to run the experimental rules. The
experimental rules can be executed by default in the "ktlint" task. Also, the
baseline has been fixed so there is no longer a need to use extension "_kt"
for the baseline test files.

Closes pinterest#1222
romtsn pushed a commit that referenced this issue Dec 14, 2021
* Fix indentation of secondary constructor

Closes #1222

* Remove support of continuation index in secondary constructor

This aligns the formatting with the default IntelliJ formatting of secondary
constructors.

Closes #1222

Co-authored-by: Paul Dingemans <pdingemans@bol.com>
romtsn added a commit that referenced this issue Jan 21, 2022
* Add separate verification build step to include experimental rules

Ktlint should apply the dogfooding principal and only provide experimental
rules that at least on the ktlint code base itself gives satisfiable
results.

Initially all experimental rules that cause violations have been disabled,
so that a separate commit can be created to enable each specific rule.

* Enable rule experimental:spacing-between-declarations-with-comments

For BaselineTests it was necessary to rename the files which are used for testing
to have a non standard kotlin file extension. This prevents the files from being
changed when running the ktlint formatting on the ktlint code base itself. Note
that the baseline protection mechanism did work in this case and as of that has
been removed from the command.

* Enable rule experimental:no-empty-first-line-in-method-block

* Enable rule experimental:annotation

* Resolve some violations of rule experimental:argument-list-wrapping

It is not yet possible to enable the rule as some violations are actually false
positives. This will be solved by #1284

* Enable rule experimental:spacing-between-declarations-with-annotations

* Enable rule experimental:trailing-comma

For now, it has been chosen to disallow trailing comma's instead of forcing them to
be added. Reasons for this are two folded. The number of changes is considerably
smaller. More importantly is that the benefit, with respect to avoiding future merge
conflicts, seems not that big when scanning the code change which would result from
forcing the trailing comma to be added.

* Update changelog

* Remove autocorrect mode from build step ktlint_experimental

* Run the experimental rules by default

There is no need for a separate build task to run the experimental rules. The
experimental rules can be executed by default in the "ktlint" task. Also, the
baseline has been fixed so there is no longer a need to use extension "_kt"
for the baseline test files.

Closes #1222

* Update ktlint/src/test/resources/TestBaselineFile.kt

* Update ktlint/src/test/resources/TestBaselineFile.kt

* Update ktlint/src/test/resources/TestBaselineExtraErrorFile.kt

* Update ktlint/src/test/resources/TestBaselineExtraErrorFile.kt

* Revert BaselineTests

* Revert ktlint-test-baseline

* Fix tests

* Exclude all test resources from the ktlint module from the linting task

Those source files all contain linting errors which have to be reported
by unit tests. Therefore they should not be reported during a normal
build because those should not be fixed as that would result in the
tests to fail.

* Revert renaming of file test-baseline.xml

* Fix lint errors due to merge of master in branch

Co-authored-by: Paul Dingemans <pdingemans@bol.com>
Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants