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.

Sign up for GitHub

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

Prevent line separators to be changed from CRLF to LF #2752

Merged
merged 1 commit into from
Jul 23, 2024
Merged

Prevent line separators to be changed from CRLF to LF #2752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 48 additions & 3 deletions ...nt-api-consumer/src/test/kotlin/com/pinterest/ktlint/api/consumer/KtLintRuleEngineTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ class KtLintRuleEngineTest {
@Nested
inner class `Format with KtLintRuleEngine` {
@Nested
inner class `Given a file that does not contain an error` {
inner class `Given a file that containing some errors` {
@Test
fun `Given defaultAutocorrect is not set`(
@TempDir
Expand Down Expand Up @@ -402,7 +402,7 @@ class KtLintRuleEngineTest {
}

@Nested
inner class `Given a kotlin code snippet that does contain an indentation error` {
inner class `Given a kotlin code snippet containing some errors` {
@Test
fun `Given defaultAutocorrect is not set`() {
val lintErrors = mutableListOf<LintError>()
Expand Down Expand Up @@ -511,7 +511,7 @@ class KtLintRuleEngineTest {
}

@Nested
inner class `Given a kotlin script code snippet that does contain an indentation error` {
inner class `Given a kotlin script code snippet containing some errors` {
@Test
fun `Given defaultAutocorrect is not set`() {
val lintErrors = mutableListOf<LintError>()
Expand Down Expand Up @@ -662,6 +662,51 @@ class KtLintRuleEngineTest {
""".trimIndent(),
)
}

@Test
fun `Issue 2747 - Given some code with crlf separators instead of lfs, but not containing any lint error, then do no reformat the line separators`(
@TempDir
tempDir: Path,
) {
val codeWithCrlfSeparators =
"""
fun bar() {
// FOO
// BAR
}
""".trimIndent().replace("\n", "\r\n")
val filePath = "$tempDir/Code.kt"
FileWriter(filePath).use {
it.write(codeWithCrlfSeparators)
}

val lintErrors = mutableListOf<LintError>()
val actual =
KtLintRuleEngine(
ruleProviders =
setOf(
RuleProvider { IndentationRule() },
RuleProvider { RuleWithAutocorrectApproveHandler() },
RuleProvider { RuleWithoutAutocorrectApproveHandler() },
),
editorConfigOverride =
EditorConfigOverride.from(
// Do not set END_OF_LINE_PROPERTY explicitly!
RULE_WITHOUT_AUTOCORRECT_APPROVE_HANDLER.createRuleExecutionEditorConfigProperty() to RuleExecution.enabled,
RULE_WITH_AUTOCORRECT_APPROVE_HANDLER.createRuleExecutionEditorConfigProperty() to RuleExecution.enabled,
),
fileSystem = ktlintTestFileSystem.fileSystem,
).format(
code = Code.fromFile(File(filePath)),
defaultAutocorrect = true,
) { lintError ->
lintErrors.add(lintError)
ALLOW_AUTOCORRECT
}

assertThat(lintErrors).isEmpty()
assertThat(actual).isEqualTo(codeWithCrlfSeparators)
}
}

@Test
Expand Down
16 changes: 11 additions & 5 deletions ...nt-rule-engine/src/main/kotlin/com/pinterest/ktlint/rule/engine/internal/CodeFormatter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.pinterest.ktlint.rule.engine.core.api.Rule
import com.pinterest.ktlint.rule.engine.core.api.editorconfig.END_OF_LINE_PROPERTY
import io.github.oshai.kotlinlogging.KotlinLogging
import org.ec4j.core.model.PropertyType
import org.jetbrains.kotlin.util.prefixIfNot

private val LOGGER = KotlinLogging.logger {}.initKtLintKLogger()

Expand All @@ -30,7 +31,7 @@ internal class CodeFormatter(
.sortedWith(lintErrorLineAndColumnComparator { it.first })
.forEach { (e, corrected) -> callback(e, corrected) }

return (code.utf8Bom() + formattedCode).also {
return (formattedCode.prefixIfNot(code.utf8Bom())).also {
LOGGER.debug { "Finished with processing file '${code.fileNameOrStdin()}'" }
}
}
Expand Down Expand Up @@ -71,8 +72,13 @@ internal class CodeFormatter(
}
}
}
val lineSeparator = code.determineLineSeparator(editorConfig[END_OF_LINE_PROPERTY])
return Pair(formattedCode(lineSeparator), errors)
return if (mutated || formatRunCount > 1) {
val lineSeparator = code.determineLineSeparator(editorConfig[END_OF_LINE_PROPERTY])
Pair(formattedCode(lineSeparator), errors)
} else {
// None of the format runs has found
Pair(code.content, errors)
}
}
}

Expand Down Expand Up @@ -149,9 +155,9 @@ internal class CodeFormatter(
eolEditorConfigProperty == PropertyType.EndOfLineValue.crlf ||
eolEditorConfigProperty != PropertyType.EndOfLineValue.lf &&
doesNotContain('\r') ->
"\r\n".also { LOGGER.debug { "line separator: ${eolEditorConfigProperty.name} --> CRLF" } }
"\r\n".also { LOGGER.trace { "line separator: ${eolEditorConfigProperty.name} --> CRLF" } }

else -> "\n".also { LOGGER.debug { "line separator: ${eolEditorConfigProperty.name} --> LF" } }
else -> "\n".also { LOGGER.trace { "line separator: ${eolEditorConfigProperty.name} --> LF" } }
}

private fun Code.doesNotContain(char: Char) = content.lastIndexOf(char) != -1
Expand Down