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

Tab indentation breaks KDoc comments #850

Closed
bonkf opened this issue Aug 24, 2020 · 9 comments · Fixed by #1263
Closed

Tab indentation breaks KDoc comments #850

bonkf opened this issue Aug 24, 2020 · 9 comments · Fixed by #1263

Comments

@bonkf
Copy link

bonkf commented Aug 24, 2020

Expected Behavior

/**
 * some function
 */
fun someFunction() {
	return Unit
}

Observed Behavior

/**
	* some function
	*/
fun someFunction() {
	return Unit
}

Steps to Reproduce

ktlint -F whatever.kt

I'm fairly certain that this is the reason for this behavior, i.e. it adds one of whatever indentation (space or tab) instead of one space. I think it's probably best to ignore both BLOCK_COMMENT and KDOC. There also doesn't seem to be a test for that; I think adding a KDoc comment to this test should suffice.

I already tried fixing it but didn't really get anywhere; I might open a PR in a few days if I manage to fix it.

Your Environment

  • Version of ktlint used: 0.38.0
  • .editorconfig contains indent_style = tab
@bonkf bonkf changed the title Tab indentation breaks multiline comments Tab indentation breaks KDoc comments Aug 24, 2020
@bonkf
Copy link
Author

bonkf commented Aug 24, 2020

Note: This obviously also triggers wrong warnings when linting, i.e. Unexpected space character(s) and Unexpected indentation (0) (should be 1).

@JakeWharton
Copy link
Contributor

I was planning on changing the indentation tests to require a tab and space variant for all the fixtures which would have caught this.

Thanks for trying out tabs!

@shashachu shashachu added the bug label Aug 24, 2020
@shashachu shashachu added this to the 0.39.0 milestone Aug 24, 2020
@shashachu shashachu modified the milestones: 0.39.0, 0.40.0 Sep 15, 2020
@Tapchicoma Tapchicoma modified the milestones: 0.40.0, 0.41.0 Dec 5, 2020
@paul-dingemans
Copy link
Collaborator

I can not reproduce the problem as the original code is not submitted. Also I would like to know what indent styling should be used. So based on the expected behaviour I have tried a number of different situations but still could not reproduce the misalignment of the KDoc.

I did however encounter a problem which is partly related. In case the code uses TAB indentation which is to be replaced with SPACE indentation, this is currently only applied to lines which are the wrong indentation level. As a result, the test below fails:

    @Test
    fun `format issue 850`() {
        val code =
            """
            class Foo {
            ${'\t'}/**
            ${'\t'} * some function
            ${'\t'} */
            ${'\t'}fun someFunction() {
            ${'\t'}${'\t'}// The next line is wrongly indented (one level too deep). Only that line is indented again
            ${'\t'}${'\t'}// by replacing the tab indents with the correct amount of spaced. But for all other lines
            ${'\t'}${'\t'}// in the KDoc and function, the indent is not replaced as those line are at the expected
            ${'\t'}${'\t'}// indentation level. That is still a bug to be fixed.
            ${'\t'}${'\t'}${'\t'}return Unit
            ${'\t'}}
            }
            """.trimIndent()
        val expectedCode =
            """
            class Foo {
                /**
                 * some function
                 */
                fun someFunction() {
                    // The next line is wrongly indented (one level too deep). Only that line is indented again
                    // by replacing the tab indents with the correct amount of spaced. But for all other lines
                    // in the KDoc and function, the indent is not replaced as those line are at the expected
                    // indentation level. That is still a bug to be fixed.
                    return Unit
                }
            }
            """.trimIndent()
        assertThat(IndentationRule().format(code)).isEqualTo(expectedCode)
    }

Can you please write a failing test like above for the problem with shows the problem with incorrectly formatted KDoc?

@bonkf
Copy link
Author

bonkf commented Dec 27, 2020

I tested everything just now with ktlint 0.38.0, 0.39.0, and 0.40.0 on macOS. All three behave as described in my bug report.

I'm not sure what you mean with the original code not being submitted.

The files I used are the following (both with trailing newlines):

.editorconfig, SHA-1 74d3c64e142dcffdce4855f45ae179f175299af7:

[*.{kt,kts}]
indent_style = tab

whatever.kt, SHA-1 c1ae5b3fbcb028435dafaf55276912b1ef146f28:

/**
 * some function
 */
fun someFunction() {
	return Unit
}

I noticed some odd behavior with comments in .editorconfig. Consider the following:

[*.{kt,kts}]
indent_style = tab # this should hopefully work in a future version of ktlint

IntelliJ highlighting looks like this:
image
GitHub highlighting looks the same, see above. According to editorconfig.com:

Comments should go on their own lines.

I'm not sure if "should" means same-line comments are illegal or just discouraged.

However, It seems that the same-line comment makes ktlint ignore the entire line.

@bonkf
Copy link
Author

bonkf commented Dec 27, 2020

Oh, I think you meant that there was no file that when formatted should produce the expected behavior. The expected file is the input file, ktlint finds an error where there is none.

@paul-dingemans
Copy link
Collaborator

Just reproduced the errors in following test. There seem to be multiple problems when converting space indentation to tabs.

    @Test
    fun `format issue 850 to Tab`() {
        val code =
            """
            class Foo {
                /**
                 * The four spaces in the indentation of the entire KDoc, including start and end marker,
                 * should be indented with a single tab.
                 */
                fun someFunction() {
                    // Those line are indented at the proper level. The space indentation should be replaced
                    // with tabs.
                    return Unit
                }
            }
            """.trimIndent()
        val expectedCode =
            """
            class Foo {
            ${'\t'}/**
            ${'\t'}* The four spaces in the indentation of the entire KDoc, including start and end marker,
            ${'\t'}* should be indented with a single tab.
            ${'\t'}*/
            ${'\t'}fun someFunction() {
            ${'\t'}${'\t'}// Those line are indented at the proper level. The space indentation should be replaced
            ${'\t'}${'\t'}// with tabs.
            ${'\t'}${'\t'}return Unit
            ${'\t'}}
            }
            """.trimIndent()
        assertThat(
            IndentationRule()
                .format(
                    code,
                    mapOf("indent_style" to "tab")
                )
        ).isEqualTo(expectedCode)
    }

@paul-dingemans
Copy link
Collaborator

Comments should go on their own lines.

I'm not sure if "should" means same-line comments are illegal or just discouraged.

However, It seems that the same-line comment makes ktlint ignore the entire line.

I think that the end of line comments are not allowed. Most like the will just be seen as part of the value. So in this case the indent_style is set to tab # this should hopefully work in a future version of ktlint instead of tab.

@bonkf
Copy link
Author

bonkf commented Dec 27, 2020

Take a look at the editorconfig test suite used by ec4j.

Looks like line comments on the same line should be ignored, no?

@paul-dingemans
Copy link
Collaborator

Take a look at the editorconfig test suite used by ec4j.

Looks like line comments on the same line should be ignored, no?
This example indeed suggests that end of line comments are allowed.

In PropertyType class of ec4j, the indent_style property is defined as:

    public static final PropertyType<IndentStyleValue> indent_style = new LowerCasingPropertyType<>( //
            "indent_style", //
            "set to tab or space to use hard tabs or soft tabs respectively.",
            new PropertyValueParser.EnumValueParser<IndentStyleValue>(IndentStyleValue.class), //
            IndentStyleValue.valueSet() //
    );

    public enum IndentStyleValue {

        space("Space", ' '),

        tab("Tab", '\t');
    // ...
   }

My guess is that, if EOL comments are really removed, this is done too late in the process. Probably the property is ignored as soon as the value is not found in the list of allowed values (IndentStyleValue.valueSet() in above example) if defined.

paul-dingemans pushed a commit to paul-dingemans/ktlint that referenced this issue Oct 31, 2021
paul-dingemans pushed a commit to paul-dingemans/ktlint that referenced this issue Oct 31, 2021
romtsn pushed a commit that referenced this issue Nov 2, 2021
* Remove unused variables

* Fix KDOC indentation for tab indentation style

Closes #850

* Add changelog message for fix of issue #850

Co-authored-by: Paul Dingemans <pdingemans@bol.com>
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.

6 participants