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

WRONG_INDENTATION: running mvn diktat:fix adds whitespace to string literals #1347

Closed
0x6675636b796f75676974687562 opened this issue Jun 3, 2022 · 4 comments · Fixed by #1371, #1376, #1382 or #1387
Assignees
Labels
autofix Issues related to diktat in autofix mode bug Something isn't working
Milestone

Comments

@0x6675636b796f75676974687562
Copy link
Member

Consider this code fragment:

    @Test
    fun `checking that suppression with ignore everything works`() {
        val code =
                """
                    @Suppress("diktat")
                    fun foo() {
                        val a = 1
                    }
                """.trimIndent()
        lintMethod(code)
    }

Whenever the code doesn't match the settings of DiKTat (i.e., in the above example, the body of the code expression uses continuation indent while extendedIndentAfterOperators is false), running mvn diktat:fix is expected to re-format the code.

While doing so (particularly, when WRONG_INDENTATION check is on), it sometimes adds extra whitespace around and within string literals, i. e.

                    @Suppress("diktat")

may become

                    @Suppress(    "    diktat    "    )

Moreover, the operation is not idempotent, so running mvn diktat:fix consecutively yet increases the amount of whitespace.

Sometimes excessively long lines get wrapped, and this may even result in non-compilable code:

                    @Suppress(    "    diktat
    "    )

Screenshot:

Originally from this comment.

@0x6675636b796f75676974687562
Copy link
Member Author

Another example (from #811):

fun checkScript() {
    lintMethod(
            """
                        |val A = "aa"
                    """.trimMargin(),
    )
}

0x6675636b796f75676974687562 added a commit that referenced this issue Jun 15, 2022
### What's done:

 * Added unit tests for mis-indented expression body functions.
 * Comparison failures now recognized by IDEA.
 * Content comparison deltas prevented from being wrapped when presented to the user
   (`columnWidth` increased from 80 to `Int.MAX_VALUE`, so deltas will no longer have any inline `<br/>`).
 * Failing tests (caused by #1347, skipped by default) can be re-enabled by running with `-Dtests.can.be.muted=true`
 * See #1330.
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 16, 2022
### What's done:

 * Running `mvn diktat:fix` with `WRONG_INDENTATION` rule enabled injects
   whitespace into multiline string literals. Now, unit tests were added which
   reproduce this behaviour.
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 16, 2022
### What's done:

 * Running `mvn diktat:fix` with `WRONG_INDENTATION` rule enabled injects
   whitespace into multiline string literals. Now, unit tests were added which
   reproduce this behaviour.
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 16, 2022
### What's done:

 * Running `mvn diktat:fix` with `WRONG_INDENTATION` rule enabled injects
   whitespace into multiline string literals. Now, unit tests were added which
   reproduce this behaviour.
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 17, 2022
### What's done:

 * Running `mvn diktat:fix` with `WRONG_INDENTATION` rule enabled injects
   whitespace into multiline string literals. Now, unit tests were added which
   reproduce this behaviour.
 * See #1347 for more details.
@0x6675636b796f75676974687562
Copy link
Member Author

Originally introduced with the fix for #766.

The problem is caused by the wrong assumption that there can be no more than two LITERAL_STRING_TEMPLATE_ENTRY nodes per a single line in a STRING_TEMPLATE (the 1st one not terminated with a newline and the last one newline-terminated).

Actually, there can be more, e.g., the 1st line in this literal:

        """
            @Suppress("diktat")
            fun foo() {
                val a = 1
            }
        """.trimIndent()

— which is @Suppress("diktat"), consists of LITERAL_STRING_TEMPLATE_ENTRY nodes:

  1. @Suppress(
  2. "
  3. diktat
  4. "
  5. )

All these nodes get indented via IndentationRule#fixFirstTemplateEntries.

psi-kotlin-string-literal

@0x6675636b796f75676974687562 0x6675636b796f75676974687562 added the autofix Issues related to diktat in autofix mode label Jun 17, 2022
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 20, 2022
### What's done:

 * Code cleaned up in preparation to fix #1347.
 * No logic is modified with this change.
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 20, 2022
### What's done:

 * `IndentationError` moved to a separate source file.
 * Top-level extensions moved from `IndentationRule.kt` to `StringUtils.kt`.
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 20, 2022
### What's done:

 * `IndentationError` moved to a separate source file.
 * Top-level extensions moved from `IndentationRule.kt` to `StringUtils.kt`.
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 20, 2022
### What's done:

 * `IndentationError` moved to a separate source file.
 * Top-level extensions moved from `IndentationRule.kt` to `StringUtils.kt`.
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 20, 2022
### What's done:

 * Code cleaned up in preparation to settle #1347.
 * No logic is modified with this change.
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 20, 2022
### What's done:

 * `IndentationError` moved to a separate source file.
 * Top-level extensions moved from `IndentationRule.kt` to `StringUtils.kt`.
@petertrr
Copy link
Member

@0x6675636b796f75676974687562 looks like this issues has been incorrectly closed by Github automation, because commit message contained fix #1347?

@0x6675636b796f75676974687562
Copy link
Member Author

@0x6675636b796f75676974687562 looks like this issues has been incorrectly closed by Github automation, because commit message contained fix #1347?

Indeed, despite I was sure I changed the commit message.
Thanks for the heads-up.
Reopening.

This was linked to pull requests Jun 21, 2022
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 21, 2022
### What's done:

 * `DEFAULT_INDENT_SIZE` is no longer used directly, in favor of `IndentationConfig.indentationSize`.
 * `IndentationConfig.indentationSize` usages encapsulated where possible.
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 21, 2022
### What's done:

 * `DEFAULT_INDENT_SIZE` is no longer used directly, in favor of `IndentationConfig.indentationSize`.
 * `IndentationConfig.indentationSize` usages encapsulated where possible.
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 21, 2022
…wline

### What's done:

 * Now, only those `LITERAL_STRING_TEMPLATE_ENTRY` nodes which immediately
   follow a newline are indented.
 * Additionally, with this change, `trimIndent()` and `trimMargin(...)` calls
   are more reliably detected.
 * This fixes #1347.
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 21, 2022
…wline

### What's done:

 * Now, only those `LITERAL_STRING_TEMPLATE_ENTRY` nodes which immediately
   follow a newline are indented.
 * Additionally, with this change, `trimIndent()` and `trimMargin(...)` calls
   are more reliably detected.
 * This fixes #1347.
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 21, 2022
…wline

### What's done:

 * Minor code clean-up.
0x6675636b796f75676974687562 added a commit that referenced this issue Jun 21, 2022
…wline (#1387)

### What's done:

 * Now, only those `LITERAL_STRING_TEMPLATE_ENTRY` nodes which immediately
   follow a newline are indented.
 * Additionally, with this change, `trimIndent()` and `trimMargin(...)` calls
   are more reliably detected.
 * This fixes #1347.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment